[webkit-reviews] review granted: [Bug 78073] [Chromium] Fix opaque tracking for box shadows and non-composited child elements : [Attachment 126092] Patch

[bugzilla-daemon at webkit.org]

Stephen White <senorblanco at chromium.org> has granted Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 78073: [Chromium] Fix opaque tracking for box shadows and non-composited
child elements
https://bugs.webkit.org/show_bug.cgi?id=78073

Attachment 126092: Patch
https://bugs.webkit.org/attachment.cgi?id=126092&action=review

------- Additional Comments from Stephen White <senorblanco at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=126092&action=review


Looks good.  r=me

> Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:146
> +    if (paint.getLooper())
> +	   return false;
> +    if (paint.getImageFilter())
> +	   return false;
> +    if (paint.getMaskFilter())
> +	   return false;
> +    SkColorFilter* colorFilter = paint.getColorFilter();
> +    if (colorFilter && !(colorFilter->getFlags() &
SkColorFilter::kAlphaUnchanged_Flag))
> +	   return false;

You might want to sync with Justin Novosad.  He's been implementing a similar
function for the deferred canvas case.	It's in isPaintOpaque() in
SkDeferredCanvas.cpp.  His version errs on the side of transparent, that is, it
only returns true if he's certain that the paint is opaque.  If those semantics
work for you, it would be cool to unify these two functions as a public API in
skia in the future.