D23538: [dolphin] Animate gifs on preview

Méven Car noreply at phabricator.kde.org
Sun Sep 1 17:26:21 BST 2019


meven added a comment.


  Looks good, just two small things.

INLINE COMMENTS

> informationpanelcontent.cpp:244
> +            const bool isAnimatedImage = m_preview->animatedMimeTypes().contains(mimeType);
> +            m_isVideo = mimeType.startsWith(QLatin1String("video/")) && !isAnimatedImage;
>              usePhonon = m_isVideo || mimeType.startsWith(QLatin1String("audio/"));

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.

> informationpanelcontent.cpp:249
> +                m_preview->setAnimatedImageFileName(itemUrl.toLocalFile());
> +            }
> +

You can add an `else if`, animatedMimeTypes and usePhonon can't be true at the same time.
Let's make it more explicit.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D23538

To: iasensio, #dolphin, #vdg, ngraham, elvisangelaccio
Cc: pino, fuksitter, meven, broulik, kfm-devel, fprice, MrPepe, fbampaloukas, alexde, Codezela, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190901/f05c54d5/attachment.htm>


More information about the kfm-devel mailing list