Changeset View
Standalone View
src/panels/information/informationpanelcontent.cpp
Show First 20 Lines • Show All 93 Lines • ▼ Show 20 Line(s) | 73 | { | |||
---|---|---|---|---|---|
94 | m_phononWidget = new PhononWidget(parent); | 94 | m_phononWidget = new PhononWidget(parent); | ||
95 | m_phononWidget->hide(); | 95 | m_phononWidget->hide(); | ||
96 | m_phononWidget->setMinimumWidth(minPreviewWidth); | 96 | m_phononWidget->setMinimumWidth(minPreviewWidth); | ||
97 | m_phononWidget->setAutoPlay(InformationPanelSettings::previewsAutoPlay()); | 97 | m_phononWidget->setAutoPlay(InformationPanelSettings::previewsAutoPlay()); | ||
98 | connect(m_phononWidget, &PhononWidget::hasVideoChanged, | 98 | connect(m_phononWidget, &PhononWidget::hasVideoChanged, | ||
99 | this, &InformationPanelContent::slotHasVideoChanged); | 99 | this, &InformationPanelContent::slotHasVideoChanged); | ||
100 | 100 | | |||
101 | // name | 101 | // name | ||
102 | m_nameLabel = new QLabel(parent); | 102 | m_nameLabel = new QLabel(parent); | ||
broulik: Is there a way to get the mime types instead? Format doesn't neccessarily match the part after… | |||||
Not necessarily, but it is usual. Currently, the supported formats are just gif, mng and webp, which match their mimetypes. Since there are only 3 of them, maybe we can also simply harcode them, but this would automatically adapt if a new format gets popular and it's included by Qt. iasensio: Not necessarily, but it is usual. Currently, the supported formats are just `gif`, `mng` and… | |||||
I think the current approach is sane. Using std::transform gets messy since there's a type change involved (QByteArray to QString), and it would be less readable than the current approach too. ngraham: I think the current approach is sane. Using `std::transform` gets messy since there's a type… | |||||
The mimetype for MNG images is video/x-mng. not image/mng. pino: The mimetype for MNG images is video/x-mng. not image/mng.
In general, there's nothing that… | |||||
103 | QFont font = m_nameLabel->font(); | 103 | QFont font = m_nameLabel->font(); | ||
104 | font.setBold(true); | 104 | font.setBold(true); | ||
105 | m_nameLabel->setFont(font); | 105 | m_nameLabel->setFont(font); | ||
106 | m_nameLabel->setTextFormat(Qt::PlainText); | 106 | m_nameLabel->setTextFormat(Qt::PlainText); | ||
107 | m_nameLabel->setAlignment(Qt::AlignHCenter); | 107 | m_nameLabel->setAlignment(Qt::AlignHCenter); | ||
108 | m_nameLabel->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Fixed); | 108 | m_nameLabel->setSizePolicy(QSizePolicy::Ignored, QSizePolicy::Fixed); | ||
109 | 109 | | |||
110 | const bool previewsShown = InformationPanelSettings::previewsShown(); | 110 | const bool previewsShown = InformationPanelSettings::previewsShown(); | ||
▲ Show 20 Lines • Show All 53 Lines • ▼ Show 20 Line(s) | 163 | { | |||
164 | InformationPanelSettings::self()->save(); | 164 | InformationPanelSettings::self()->save(); | ||
165 | } | 165 | } | ||
166 | 166 | | |||
167 | void InformationPanelContent::showItem(const KFileItem& item) | 167 | void InformationPanelContent::showItem(const KFileItem& item) | ||
168 | { | 168 | { | ||
169 | if (item != m_item) { | 169 | if (item != m_item) { | ||
170 | m_item = item; | 170 | m_item = item; | ||
171 | 171 | | |||
172 | m_preview->clearAnimatedImage(); | ||||
172 | refreshMetaData(); | 173 | refreshMetaData(); | ||
173 | } | 174 | } | ||
This change will result in the preview being refreshed a lot more, even if it's not an animated image. Can you find a way to do what you're trying to do without changing this? ngraham: This change will result in the preview being refreshed a lot more, even if it's not an animated… | |||||
175 | // Refresh even on same item to handle resizing events | ||||
Please remove this comment, as D23668 will make it redundant. elvisangelaccio: Please remove this comment, as D23668 will make it redundant. | |||||
174 | refreshPreview(); | 176 | refreshPreview(); | ||
175 | } | 177 | } | ||
176 | 178 | | |||
177 | void InformationPanelContent::refreshPixmapView() | 179 | void InformationPanelContent::refreshPixmapView() | ||
178 | { | 180 | { | ||
179 | // If there is a preview job, kill it to prevent that we have jobs for | 181 | // If there is a preview job, kill it to prevent that we have jobs for | ||
180 | // multiple items running, and thus a race condition (bug 250787). | 182 | // multiple items running, and thus a race condition (bug 250787). | ||
181 | if (m_previewJob) { | 183 | if (m_previewJob) { | ||
Show All 32 Lines | 215 | { | |||
214 | // If there is a preview job, kill it to prevent that we have jobs for | 216 | // If there is a preview job, kill it to prevent that we have jobs for | ||
215 | // multiple items running, and thus a race condition (bug 250787). | 217 | // multiple items running, and thus a race condition (bug 250787). | ||
216 | if (m_previewJob) { | 218 | if (m_previewJob) { | ||
217 | m_previewJob->kill(); | 219 | m_previewJob->kill(); | ||
218 | } | 220 | } | ||
219 | 221 | | |||
220 | m_preview->setCursor(Qt::ArrowCursor); | 222 | m_preview->setCursor(Qt::ArrowCursor); | ||
221 | bool usePhonon = false; | 223 | bool usePhonon = false; | ||
224 | | ||||
elvisangelaccio: Unrelated whitespace change | |||||
222 | setNameLabelText(m_item.text()); | 225 | setNameLabelText(m_item.text()); | ||
223 | if (InformationPanelSettings::previewsShown()) { | 226 | if (InformationPanelSettings::previewsShown()) { | ||
224 | 227 | | |||
225 | const QUrl itemUrl = m_item.url(); | 228 | const QUrl itemUrl = m_item.url(); | ||
226 | const bool isSearchUrl = itemUrl.scheme().contains(QLatin1String("search")) && m_item.localPath().isEmpty(); | 229 | const bool isSearchUrl = itemUrl.scheme().contains(QLatin1String("search")) && m_item.localPath().isEmpty(); | ||
227 | if (isSearchUrl) { | 230 | if (isSearchUrl) { | ||
228 | m_preview->show(); | 231 | m_preview->show(); | ||
229 | 232 | | |||
230 | // in the case of a search-URL the URL is not readable for humans | 233 | // in the case of a search-URL the URL is not readable for humans | ||
231 | // (at least not useful to show in the Information Panel) | 234 | // (at least not useful to show in the Information Panel) | ||
232 | m_preview->setPixmap( | 235 | m_preview->setPixmap( | ||
233 | QIcon::fromTheme(QStringLiteral("nepomuk")).pixmap(KIconLoader::SizeEnormous, KIconLoader::SizeEnormous) | 236 | QIcon::fromTheme(QStringLiteral("nepomuk")).pixmap(KIconLoader::SizeEnormous, KIconLoader::SizeEnormous) | ||
234 | ); | 237 | ); | ||
235 | } else { | 238 | } else { | ||
236 | 239 | | |||
237 | refreshPixmapView(); | 240 | refreshPixmapView(); | ||
238 | 241 | | |||
239 | const QString mimeType = m_item.mimetype(); | 242 | const QString mimeType = m_item.mimetype(); | ||
240 | m_isVideo = mimeType.startsWith(QLatin1String("video/")); | 243 | const bool isAnimatedImage = m_preview->animatedMimeTypes().contains(mimeType); | ||
244 | m_isVideo = !isAnimatedImage && mimeType.startsWith(QLatin1String("video/")); | ||||
A small optimization : invert the two condition, isAnimatedImage has already been computed and if false, mimeType.startsWith(QLatin1String("video/")) won't be run at all. meven: A small optimization : invert the two condition, isAnimatedImage has already been computed and… | |||||
241 | usePhonon = m_isVideo || mimeType.startsWith(QLatin1String("audio/")); | 245 | usePhonon = m_isVideo || mimeType.startsWith(QLatin1String("audio/")); | ||
242 | 246 | | |||
243 | if (usePhonon) { | 247 | if (usePhonon) { | ||
244 | // change the cursor of the preview | 248 | // change the cursor of the preview | ||
245 | m_preview->setCursor(Qt::PointingHandCursor); | 249 | m_preview->setCursor(Qt::PointingHandCursor); | ||
You can add an else if, animatedMimeTypes and usePhonon can't be true at the same time. meven: You can add an `else if`, animatedMimeTypes and usePhonon can't be true at the same time.
Let's… | |||||
It is mutually exclusive with usePhonon but not with the else clause afterwards. I could move this block to there, if that makes it more clear. iasensio: It is mutually exclusive with `usePhonon` but not with the `else` clause afterwards. I could… | |||||
No the next else after if (usePhonon) { if for if (InformationPanelSettings::previewsShown()) {. meven: No the next else after `if (usePhonon) {` if for `if (InformationPanelSettings::previewsShown… | |||||
Sorry, but I don't get it. I mean the else in line 278 (270 before the patch). I hope to have rebased correctly. iasensio: Sorry, but I don't get it. I mean the `else` in line 278 (270 before the patch). I hope to have… | |||||
Sorry my previous comment was not very clear. Currently you have : if (isAnimatedImaget) {} if (isPhonon){} I suggest to have if (isAnimatedImaget) {} else if (isPhonon){} If you look at the code carefully you will see if (isPhonon){} has no else block. meven: Sorry my previous comment was not very clear.
Currently you have :
```
if (isAnimatedImaget)… | |||||
246 | m_preview->installEventFilter(m_phononWidget); | 250 | m_preview->installEventFilter(m_phononWidget); | ||
247 | 251 | | |||
248 | // if the video is playing, has been paused or stopped | 252 | // if the video is playing, has been paused or stopped | ||
249 | // we don't need to update the preview/phonon widget states | 253 | // we don't need to update the preview/phonon widget states | ||
250 | // unless the previewed file has changed, | 254 | // unless the previewed file has changed, | ||
251 | // or the setting previewshown has changed | 255 | // or the setting previewshown has changed | ||
252 | if ((m_phononWidget->state() != Phonon::State::PlayingState && | 256 | if ((m_phononWidget->state() != Phonon::State::PlayingState && | ||
253 | m_phononWidget->state() != Phonon::State::PausedState && | 257 | m_phononWidget->state() != Phonon::State::PausedState && | ||
Show All 9 Lines | 265 | } else { | |||
263 | m_preview->show(); | 267 | m_preview->show(); | ||
264 | } | 268 | } | ||
265 | 269 | | |||
266 | m_phononWidget->show(); | 270 | m_phononWidget->show(); | ||
267 | m_phononWidget->setUrl(m_item.targetUrl(), m_isVideo ? PhononWidget::MediaKind::Video : PhononWidget::MediaKind::Audio); | 271 | m_phononWidget->setUrl(m_item.targetUrl(), m_isVideo ? PhononWidget::MediaKind::Video : PhononWidget::MediaKind::Audio); | ||
268 | adjustWidgetSizes(parentWidget()->width()); | 272 | adjustWidgetSizes(parentWidget()->width()); | ||
269 | } | 273 | } | ||
270 | } else { | 274 | } else { | ||
275 | if (isAnimatedImage) { | ||||
276 | m_preview->setAnimatedImageFileName(itemUrl.toLocalFile()); | ||||
277 | } | ||||
271 | // When we don't need it, hide the phonon widget first to avoid flickering | 278 | // When we don't need it, hide the phonon widget first to avoid flickering | ||
272 | m_phononWidget->hide(); | 279 | m_phononWidget->hide(); | ||
273 | m_preview->show(); | 280 | m_preview->show(); | ||
274 | m_preview->removeEventFilter(m_phononWidget); | 281 | m_preview->removeEventFilter(m_phononWidget); | ||
275 | m_phononWidget->clearUrl(); | 282 | m_phononWidget->clearUrl(); | ||
276 | } | 283 | } | ||
277 | } | 284 | } | ||
278 | } else { | 285 | } else { | ||
286 | m_preview->clearAnimatedImage(); | ||||
279 | m_preview->hide(); | 287 | m_preview->hide(); | ||
280 | m_phononWidget->hide(); | 288 | m_phononWidget->hide(); | ||
281 | } | 289 | } | ||
282 | } | 290 | } | ||
283 | 291 | | |||
284 | void InformationPanelContent::configureShownProperties() | 292 | void InformationPanelContent::configureShownProperties() | ||
285 | { | 293 | { | ||
286 | m_configureLabel->setVisible(true); | 294 | m_configureLabel->setVisible(true); | ||
Show All 11 Lines | |||||
298 | void InformationPanelContent::showItems(const KFileItemList& items) | 306 | void InformationPanelContent::showItems(const KFileItemList& items) | ||
299 | { | 307 | { | ||
300 | // If there is a preview job, kill it to prevent that we have jobs for | 308 | // If there is a preview job, kill it to prevent that we have jobs for | ||
301 | // multiple items running, and thus a race condition (bug 250787). | 309 | // multiple items running, and thus a race condition (bug 250787). | ||
302 | if (m_previewJob) { | 310 | if (m_previewJob) { | ||
303 | m_previewJob->kill(); | 311 | m_previewJob->kill(); | ||
304 | } | 312 | } | ||
305 | 313 | | |||
314 | m_preview->clearAnimatedImage(); | ||||
315 | | ||||
306 | m_preview->setPixmap( | 316 | m_preview->setPixmap( | ||
307 | QIcon::fromTheme(QStringLiteral("dialog-information")).pixmap(KIconLoader::SizeEnormous, KIconLoader::SizeEnormous) | 317 | QIcon::fromTheme(QStringLiteral("dialog-information")).pixmap(KIconLoader::SizeEnormous, KIconLoader::SizeEnormous) | ||
308 | ); | 318 | ); | ||
309 | setNameLabelText(i18ncp("@label", "%1 item selected", "%1 items selected", items.count())); | 319 | setNameLabelText(i18ncp("@label", "%1 item selected", "%1 items selected", items.count())); | ||
310 | 320 | | |||
311 | m_metaDataWidget->setItems(items); | 321 | m_metaDataWidget->setItems(items); | ||
312 | 322 | | |||
313 | m_phononWidget->hide(); | 323 | m_phononWidget->hide(); | ||
Show All 38 Lines | 359 | { | |||
352 | KIconLoader::global()->drawOverlays(item.overlays(), pixmap, KIconLoader::Desktop); | 362 | KIconLoader::global()->drawOverlays(item.overlays(), pixmap, KIconLoader::Desktop); | ||
353 | m_preview->setPixmap(pixmap); | 363 | m_preview->setPixmap(pixmap); | ||
354 | } | 364 | } | ||
355 | 365 | | |||
356 | void InformationPanelContent::showPreview(const KFileItem& item, | 366 | void InformationPanelContent::showPreview(const KFileItem& item, | ||
357 | const QPixmap& pixmap) | 367 | const QPixmap& pixmap) | ||
358 | { | 368 | { | ||
359 | m_outdatedPreviewTimer->stop(); | 369 | m_outdatedPreviewTimer->stop(); | ||
360 | Q_UNUSED(item) | | |||
meven: Please re-add this, it saves compile warning | |||||
Sorry if I'm wrong, but it shouldn't give any warning, since item is used below in the function. iasensio: Sorry if I'm wrong, but it shouldn't give any warning, since `item` is used below in the… | |||||
meven: Indeed my bad | |||||
elvisangelaccio: Then this is an unrelated change, I'll fix in on master. | |||||
361 | 370 | | |||
362 | QPixmap p = pixmap; | 371 | QPixmap p = pixmap; | ||
363 | KIconLoader::global()->drawOverlays(item.overlays(), p, KIconLoader::Desktop); | 372 | KIconLoader::global()->drawOverlays(item.overlays(), p, KIconLoader::Desktop); | ||
364 | 373 | | |||
365 | if (m_isVideo) { | 374 | if (m_isVideo) { | ||
366 | // adds a play arrow | 375 | // adds a play arrow | ||
367 | 376 | | |||
368 | // compute relative pixel positions | 377 | // compute relative pixel positions | ||
▲ Show 20 Lines • Show All 116 Lines • Show Last 20 Lines |
Is there a way to get the mime types instead? Format doesn't neccessarily match the part after "image/", does it?
You can also probably use a std::transform to turn that one list into the other one