1150973 - Automatically & losslessy compress png assets

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We use `pngcrush` or, preferably, `trimage` (to my knowledge, it's pngcrush and several other compression algorithms working together) to reduce our png assets and thus the size of our shipped APK. It would be great if this process was done automatically.

My preference is to either:
  * Do this on the version control server: when a commit gets pushed, any png files added/modified by the commit within the mobile/ directory would get `trimage` run on it, a new commit would be added with the compressed assests, and the changeset would be added server-side. Downside: used server resources, how can I modify the VC server, and users pushing don't have the most update-to-date changeset history after pushing 
  * Do this as a pre-commit hook: compress any added/modified png assets before completing the commit. Downside: new deps (and I don't think `trimage` is available on OS X). nalexander also mentioned pre-commit hooks commited into the repo are generally frowned upon (why is that?)

nalexander also talked about doing this as an automatic part of the build (specifically pngcrush as part of gradle) but I disagree with this approach because it increases build times on every build.

Please add your thoughts!

Note: trimage is faster when being run on a directory (`-d`) than a file (`-f`), though I'm not sure if it recurs.

Comment 1

[Gregory Szorc [:gps]]

It is unacceptable to perform computationally expensive operations on push.

Applying pngcrush (or whatever post-processing) should either:

a) be enforced by a test that verifies needed operations were performed on checked-in files
b) as part of packaging

We already minimize JS files as part of packaging. We could also automatically run png files through pngcrush as part of packaging if it is desired.

But if we do things as part of packaging, packaging becomes slower and output may change in unexpected ways (e.g. new pngcrush binary on build machines changes output in a weird way). Having pre-pngcrushed files checked in ensures the conversion only occurs once and behavior is deterministic.

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Gregory Szorc [:gps] from comment #1)
> It is unacceptable to perform computationally expensive operations on push.
> 
> Applying pngcrush (or whatever post-processing) should either:
> 
> a) be enforced by a test that verifies needed operations were performed on
> checked-in files

This means that we pay for verification forever on assets that change only intermittently.  That's a bad trade-off.  Unless there's some less-frequent test job this can be part of?

> b) as part of packaging
> 
> We already minimize JS files as part of packaging. We could also
> automatically run png files through pngcrush as part of packaging if it is
> desired.
> 
> But if we do things as part of packaging, packaging becomes slower and
> output may change in unexpected ways (e.g. new pngcrush binary on build
> machines changes output in a weird way). Having pre-pngcrushed files checked
> in ensures the conversion only occurs once and behavior is deterministic.

Packaging on Android is already abysmally slow; I'm loathe to do this for developer builds.  I'm happy to do it for automation builds.

gps: is there some way to run a "reporting job" that could give us suggested pngcrush commands or whatever once a week?  That is, the monitoring needs to be automatic but the fix doesn't need to be automated, (and it doesn't need to turn the build red).

Comment 3

[Gregory Szorc [:gps]]

An individual could set up a CRON, Jenkins, or Task Cluster job that periodically ran a one-off test for pngcrush sanity and alerted people, filed bugs, etc if pngcrush needs to run. Not all tests have to be run on every build and be reported to TreeHerder :)

Comment 4

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.