[Webkit-unassigned] [Bug 123716] SVG rectangle with filter will not render

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=123716


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #225411|review?                     |review-
               Flag|                            |




--- Comment #13 from Dirk Schulze <krit at webkit.org>  2014-02-28 11:45:27 PST ---
(From update of attachment 225411)
View in context: https://bugs.webkit.org/attachment.cgi?id=225411&action=review

The patched files look good. See comments to change log and testing.

> Source/WebCore/ChangeLog:10
> +        A filtered SVG element with width or length bigger than 5000
> +        (counting the margin/border) will fail to render. This patch will
> +        instead test against the total element area.

This might not be an actual fix. See later comment. If a filter > 4096x4096 still fails, this change log should change wording. Right now it implies that taking the are will fix the overall problem.

> Source/WebCore/ChangeLog:20
> +        * platform/graphics/filters/FilterEffect.cpp:

Could you do some inline comments here please? Just say what you changed in some of the named methods. This makes it easier to review the change log later.

> LayoutTests/svg/filters/big-height-filter-expected.svg:1
> +<svg xmlns="http://www.w3.org/2000/svg">

You need to create a change log entry for tests as well. The change log is located at LayoutTests/ChangeLog

I wonder how the tests test the changed behavior. We do use buffers differently now. In general all filters should run. Even if the buffer size extends beyond 4096x4096. If the filters don't run anymore, we need a test for a filter that is bigger than this area. I support the change in general. If a filter still fails if it is bigger than 4096x4096, then we should open a new bug report and fix it there. This would not be the proper fix for that and I suggest renaming the bug report to reflect the change you made.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.