[webkit-reviews] review granted: [Bug 80323] [CSS Filters] Drop Shadow is not repainting correctly when repaint area is smaller than the filtered element : [Attachment 134865] Patch V5

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Chiculita Alexandru
<achicu at adobe.com>'s request for review:
Bug 80323: [CSS Filters] Drop Shadow is not repainting correctly when repaint
area is smaller than the filtered element
https://bugs.webkit.org/show_bug.cgi?id=80323

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

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=134865&action=review


BTW - instead of "document.getElementsByClassName('before')[0]" you could just
do "document.querySelector('.before')" as it always returns the first match.

> LayoutTests/platform/chromium/test_expectations.txt:3210
> +BUGWK80323 SKIP : css3/filters/filter-repaint-blur.html = FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-child-layers.html = FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-composited-fallback-crash.html
= FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-composited-fallback.html =
FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-shadow-clipped.html = FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-shadow-rotated.html = FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint-shadow.html = FAIL
> +BUGWK80323 SKIP : css3/filters/filter-repaint.html = FAIL

I'm not sure what the convention for chromium is. You say they need
rebaselining, but you're linking to this bug not to a bug that should create
the rebaseline?

> Source/WebCore/ChangeLog:84
> +	   * rendering/RenderLayer.h:
> +	   (RenderLayer):

Maybe describe m_filterRepaintRect and how it gets accumulated and reset?

> Source/WebCore/rendering/RenderLayer.cpp:955
> +{
> +    if (includeSelf && requiresFullLayerImageForFilters())
> +	   return const_cast<RenderLayer*>(this);
> +    
> +    for (const RenderLayer* curr = parent(); curr; curr = curr->parent()) {
> +	   if (curr->requiresFullLayerImageForFilters())
> +	       return const_cast<RenderLayer*>(curr);
> +    }

I guess we could have combined this into a const RenderLayer* curr =
includeSelf ? this : parent(); then for (; curr; curr = curr->parent()) right?