633369 - gfxAlphaBoxBlur::PremultiplyAlpha should use GFX_PREMULTIPLY rather than float division

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Ben Stover was running a profile of scrolling a web page on an ARM device using a new profiling tool pcwalton wrote, with stuart, azakai, and me looking over his shoulder.

There was a good bit of time spent inside gfxAlphaBoxBlur methods doing simulated floating point; it appears likely the problem is the floating point math inside gfxAlphaBoxBlur::PremultiplyAlpha, which is easily avoidable using the GFX_PREMULTIPLY macro (simply multiply the input alpha by 255 and then do conversion to integer *once*).  (This does have slightly different rounding properties, although maybe only if the input alpha wasn't converted from a PRUint8 already, but I don't think it should matter.)

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds good!

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We also have fast premultiplication in http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxUtils.cpp -- right now it just goes from un-premultiplied image surface to premultiplied, but it shouldn't be hard to create another function that takes a passed-in alpha value.  No math, just table lookups.

Comment 3

[Benjamin Stover (:stechz)]

I'm currently worried the profiler is acting up. I've added an early return to PremultiplyAlpha but it still stubbornly shows up as a heavy contributor in the profile report.

Comment 4

[Benjamin Stover (:stechz)]

So. I think the profiler is busted, but it did give me an idea to early return in Paint. This makes checkerboarding *much* less of a problem.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I still think this is worth doing; it may well substantially speed up the blur, but still be hard to notice visually without something actually measuring the paint performance (e.g., an fps counter).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually... it looks like PremultiplyAlpha is unused.  So we should probably just remove it.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: remove it

Comment 8

[Benjamin Stover (:stechz)]

Bug 633369 has been filed for paint.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/projects/birch/rev/fa751027e9b8

(oops, maybe I shouldn't have landed it on birch since I meant it for layout/-only, but it's probably ok)

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/mozilla-central/rev/fa751027e9b8