662130 - Wrong rounding used for alpha (un)premultiply operations in Canvas 2D and WebGL

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Siarhei Siamashka]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) Gecko/20110519 Firefox/4.0.1
Build Identifier: 

This comes from bug 659725. Mozilla is currently using the following math:
    premultiply : "newcolor = (color * alpha + 254) / 255"
    unmultiply  : "newcolor = (255 * color) / alpha"
While the same operations using more correct rounding to nearest would be:
    premultiply : "newcolor = (color * alpha + 127) / 255"
    unmultiply  : "newcolor = (255 * color + (alpha / 2)) / alpha"

The whole point is that integer color components having [0, 255] range in 32-bit ARGB color formats are supposed to represent floating point values in the range [0.0, 1.0]. The classic Porter-Duff compositing operations are defined for floating point color representation. Integer representation is just an optimization, intended to reduce storage space and improve performance, but it still should attempt to preserve as much of precision as possible. The main source of precision loss is happening when storing final or intermediate results as [0, 255] integer values. And doing rounding in a correct way is important for not getting errors accumulate too much when multiple consequent operations are performed one after another (storing results to [0, 255] after each step). Rounding to nearest is the best, just because it provides minimal difference between the real calculated result of compositing operation and the same result rounded and stored as [0, 255]. The absolute error would be 0.5 at most, while rounding away from zero or towards zero introduces more significant error up to 1.0.

Now getting back to the formulas for (un)premultiply. The rationale for current behaviour is explained in bug 659725 comment 26 and bug 659725 comment 44. It just historically happened that in order to provide exact non-lossy round-trip "x == premultiply(unmultiply(x))", wrong rounding has been selected for both premultiply and unmultiply steps (and these bugs happen cancel each other).

What makes the current situation even worse, apparently Chrome also uses wrong rounding (see bug 659725 comment 45). I wonder whether they came up with that idea independently, or did that intentionally to be more compatible with Mozilla (or the other way around). Anyway, fixing this issue in all the browsers would make a lot of sense.

Reproducible: Always

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Reduce error in (un)premultiply alpha tables

Comment 2

[Justin Lebar (not reading bugmail)]

I'd like a more explicit comment.  Exactly what reduces which rounding errors?

Also note that the code in question is duplicated in nsCanvasRenderingContext2D::Ensure{Unp,P}remultiplyTable.

Comment 3

[Kelsey Gilbert [:jgilbert]]

I can't think of a succinct way to explain why adding the fudge values proposed reduces rounding errors due to truncation.

Also, if this code is duplicated, it's probably best to pull it out into a separate header file for reuse.

Comment 4

[Timothy B. Terriberry (:derf)]

(In reply to Jeff Gilbert [:jgilbert] from comment #3)
> Also, if this code is duplicated, it's probably best to pull it out into a
> separate header file for reuse.

That's bug 633467.

Comment 5

[Justin Lebar (not reading bugmail)]

I'd love it if you could un-duplicate the code.

I'm not sure how succinct you're going for, but I might say something like:

  "We use ((c * 255 + a / 2) / a) instead of the more obvious ((c * 255) / a) because the latter rounds improperly.  For example, when c=X and a=Y, the latter gives Z, but we actually want Z'."

Maybe I'd add

  "The integer formula we use is intended to approximate the floating-point formula F, and when we convert X and Y to floating point numbers, F(x, y) = z, which is closer to Z' than Z."

Comment 6

[Justin Lebar (not reading bugmail)]

If you really don't want to put a long-ish comment in there, you could at least link to this bug.  My goal is to get us to avoid making this same mistake in the future...

Comment 7

[Kelsey Gilbert [:jgilbert]]

I think this was fixed long ago.