gfxAlphaBoxBlur::PremultiplyAlpha should use GFX_PREMULTIPLY rather than float division

RESOLVED FIXED in mozilla5

Status

()

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

({perf})

Trunk
mozilla5
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 511827 [details] [diff] [review]
remove it
Assignee: ben → dbaron
Status: NEW → ASSIGNED
Attachment #511827 - Flags: review?(roc)
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)
Whiteboard: fixed-in-birch
https://hg.mozilla.org/mozilla-central/rev/fa751027e9b8
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-birch
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.