1162824 - Change box shadow cache to cache the colored blurred box shadow

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

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.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Cache min box shadow

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

Comment 2

[Markus Stange [:mstange] [on PTO until Sept 30]]

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

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attachment: Cache min box shadow

Carrying r+, updated with feedback from comment 2.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8a584dddc0c

Comment 5

[Phil Ringnalda (:philor)]

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

Comment 6

[Mason Chang [Inactive] [:mchang]]

Backed out to backout bug 1155828.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e6654f7fdea

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/1e6654f7fdea

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/869aca2db34d

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/869aca2db34d