836377 - [Gaia::Dialer] Ensure images are optimized

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Kevin Grandon :kgrandon]

Verify that all images loaded for dialer are optimized, and don't have unneeded metadata. I've used tools like smushit (http://www.smushit.com/ysmush.it/) in the past, but there may be better tools now.

This is also likely something that we could do during the build process if we desired.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

A patch for this issue is available at

  https://github.com/tdz/gaia/commit/ffd6a95acf08383341219a392d148980cad6b689

I used 'optipng', a nice little tool for the Linux command line. It can strip all meta data and re-encode PNG files with minimal bit depth, maximum compression, etc. The resulting files are only 30-50% of the original one's.

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: File sizes of dialer PNGs, before vs. after

The file sizes of the dialer app's PNG files after running optipng. The .png.bak files are backups of the original files.

Comment 3

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: File-size differences for all apps' PNGs

Just for fun, I re-encoded all app's PNG files. The file-size differences are in the attachment. I don't have any hard data, but the interface sometimes seems to load slightly faster now, for example the image in the FTU's language selection or the wallpapers in the wallpaper selector.

The patch is available at

  https://github.com/tdz/gaia/commit/c300798c7a32129978365cf84370e8d36c42ef27

if you're interested.

BTW: How do I attach a pointer to a pull request on github?

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Optipng is available from

  http://optipng.sourceforge.net/

Comment 5

[Alex Keybl [:akeybl]]

Let's get a patch ready and landed to v1.x before consider for v1.0.0. Please nominate for uplift once landed to master and ready.

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Kevin,

A pull request to gaia/master is pending at

  https://github.com/mozilla-b2g/gaia/pull/7970

Please review and merge, if appropriate.

Comment 7

[Kevin Grandon :kgrandon]

Ok, I will take a look Thomas. We generally make an attachment on this bug that points to the github pull request, but as this is only images it should be fine.

I also have a patch ready that replaces nearly every image on disk and shaves off around 2.5mb of image data. But I'd much rather use something in the build system. We do have a tool that handles it for us in tools/png_recompress.sh, but it's rather manual right now.

Comment 8

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/1b9cd0c3e71a904e086c19f2d5e1becb36154a51

Comment 9

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks for merging the patch.

We should be careful with automating PNG optimizations during builds. Bug 817112 indicates that not all PNG files are support by all components. And it also takes a very long time. When I ran the optimizer for all PNG files it took 15 minutes or more. I wouldn't want to invest this time during builds.

Another idea for automating the optimization is the use of git hooks on Gaia's server-side repository. Whenever someone pushes a PNG file into the repository, the committed file could automatically be optimized by the git-hook script.

Comment 10

[Kevin Grandon :kgrandon]

This is landed so marking as fixed. It's a tiny perf improvement, which I'm assuming we want. Do I change tef to leo then to get it into 1.x?

Comment 11

[Gabriele Svelto [:gsvelto]]

This bug flew under my radar: we already have a script to recompress PNG files which uses a combination of optipng and advpng with brute-force default options, it can be found in gaia under tools/png_recompress.sh. Running it recursively on the apps/communication/dialer directory further shrunk the PNG files by ~6%.

It is obviously very slow so I'm not sure if it would be a good idea to invoke it at build time. It would be nice however to invoke it over all our PNG files just before release (perhaps before creating a new tag?). Last time I tried it shaved ~2MiB from all gaia apps.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: Bugs still affected on b2g18 after 2/13 merge to v1.0.1 branch are affected on v1.0.1 branch.

Comment 13

[Lucas Adamski [:ladamski]]

I think this should be TEF+ if we want to have an impact in this release.  Doesn't seem like a significant regression risk.

Comment 14

[Lucas Adamski [:ladamski]]

Actual patch is in comments 8 / 6.

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train@3c010fc87726453a8677868f4f965ced33454a2a
v1.0.1@ca9439062e07fe32c6272714faee8aae72f6a1cb

Comment 16

[amiller]

No Test case creation is needed in moztrap for this issue.

Comment 17

[Darren]

Cannot verify, need steps to blackbox test the issue.