Closed Bug 633369 Opened 9 years ago Closed 9 years ago
Alpha Box Blur::Premultiply Alpha should use GFX _PREMULTIPLY rather than float division
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.)
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.
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.
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.
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).
Actually... it looks like PremultiplyAlpha is unused. So we should probably just remove it.
Assignee: ben → dbaron
Status: NEW → ASSIGNED
Attachment #511827 - Flags: review?(roc)
Attachment #511827 - Flags: review?(roc) → review+
Bug 633369 has been filed for paint.
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)
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.