Closed
Bug 1162824
Opened 6 years ago
Closed 6 years ago
Change box shadow cache to cache the colored blurred box shadow
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: mchang, Assigned: mchang)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file, 1 obsolete file)
17.56 KB,
patch
|
mchang
:
review+
|
Details | Diff | Splinter Review |
Right now, the box shadow cache tries to cache a full size rect blurred surface. With bug 1155828, we use a small minimal blurred surface and stretch it out to fill the target rect. Change the blur cache to cache the small minimal blurred surface.
Assignee | ||
Comment 1•6 years ago
|
||
Changes the current caching strategy from aRect, to only caching the minimum colored box shadow. Relies on bug 1155828. If we want to fix [2], we can do that in a follow up bug. Initial results look good. Blurring the small min rect for [1] takes ~0.3 - 04 ms. Using the cache takes ~1 us. While scrolling the whole page w/ APZ on a retina mbp + APZ + e10s + 1024x1024 tile sizes, we spend ~12ms blurring with this patch + Bug 1155828. The original implementation of blurring before this work took ~305ms, so ~25x improvement! [1] http://www.reddit.com/r/gameofthrones/comments/33zfzq/s5_postpremiere_discussion_503_high_sparrow/ [2] https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxBlur.cpp?from=gfxBlur.cpp&case=true#300
Attachment #8603633 -
Flags: review?(mstange)
Comment 2•6 years ago
|
||
Comment on attachment 8603633 [details] [diff] [review] Cache min box shadow Review of attachment 8603633 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxBlur.cpp @@ +165,4 @@ > typedef const BlurCacheKey* KeyTypePointer; > enum { ALLOW_MEMMOVE = true }; > > + IntSize mMinimumSize; I'd just call this mSize. @@ +186,5 @@ > + } else { > + mCornerRadii.radii[i] = Size(0, 0); > + } > + } > + } Can you instead construct mCornerRadii in the initializer list, like this: , mCornerRadii(aCornerRadii ? *aCornerRadii : RectCornerRadii()) If this doesn't work, can you add a copy constructor to RectCornerRadii so that it does? @@ +196,5 @@ > , mBackend(aOther->mBackend) > + { > + for (size_t i = 0; i < 4; i++) { > + mCornerRadii.radii[i] = aOther->mCornerRadii.radii[i]; > + } and also use that here
Attachment #8603633 -
Flags: review?(mstange) → review+
Assignee | ||
Comment 3•6 years ago
|
||
Carrying r+, updated with feedback from comment 2.
Attachment #8603633 -
Attachment is obsolete: true
Attachment #8605519 -
Flags: review+
Comment 5•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d8a584dddc0c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 6•6 years ago
|
||
Backed out to backout bug 1155828. https://hg.mozilla.org/integration/mozilla-inbound/rev/1e6654f7fdea
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 7•6 years ago
|
||
Merge of backout: https://hg.mozilla.org/mozilla-central/rev/1e6654f7fdea
Target Milestone: mozilla41 → ---
Comment 9•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/869aca2db34d
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•