Bug 35320 – SVGResourceFilter needs to be moved to under Renderers

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[David Yonge-Mallo]

Continuation of the refactoring started in https://bugs.webkit.org/show_bug.cgi?id=35020

Cause of Chromium bug http://crbug.com/30939

[Dirk Schulze]

I'm sorry, that I assign this bug to me. I would like to combine this move with little design changes and clean-ups. I'll upload a patch soon. We have to go forward on killing the old resources, also because of speed reasons.

[David Yonge-Mallo]

Ok.  I'm sorry that I wasn't able to get this done.

[Dirk Schulze]

(In reply to comment #2)
> Ok.  I'm sorry that I wasn't able to get this done.

Oh, no problem. But it is a very big patch for the beginning.

[Dirk Schulze]

Created attachment 53698 [details]
patch

patch

[WebKit Review Bot]

Attachment 53698 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1671439

[Dirk Schulze]

Created attachment 53717 [details]
patch

Fogot some witches for filters. On Chromium filters are disabled.

[WebKit Review Bot]

Attachment 53717 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WARNING: Skipping non-existent file: WebCore/svg/graphics/SVGResourceFilter.h
WebCore/rendering/RenderSVGResourceFilter.cpp:291:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
WARNING: Skipping non-existent file: WebCore/svg/graphics/SVGResourceFilter.cpp
Total errors found: 1 in 118 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Attachment 53717 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1676429

[WebKit Review Bot]

Attachment 53717 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1608611

[Dirk Schulze]

Created attachment 53776 [details]
patch

more filter switches.

[Nikolas Zimmermann]

Comment on attachment 53776 [details]
patch

Discussed lots of things during IRC review. Dirk will upload a new version.

[Dirk Schulze]

Created attachment 53785 [details]
patch

Updated patch with some notes of Nikolas.

[WebKit Review Bot]

Attachment 53785 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/rendering/RenderSVGResourceFilter.cpp:169:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 115 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dirk Schulze]

Created attachment 53787 [details]
patch

fixed more style issues.

[Nikolas Zimmermann]

Comment on attachment 53787 [details]
patch

Fantastic patch, moving in the right direction :-)
Still needs a follow-up patch to rework filter effect dumping, but we had nothing before, so this is way better than trunk.

r=me, though a last little nitpick before landing:
> +    if (!filterData->boundaries.width() || !filterData->boundaries.height())
> +        return false;
Please use if (filterData->boundaries.isEmpty()) here :-)

Jeez, what a patch, even larger then those I write ;-)

[WebKit Review Bot]

http://trac.webkit.org/changeset/57880 might have broken Qt Linux Release

[Dirk Schulze]

Created attachment 53797 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 53797 [details]
Patch

Ok, let's try our luck again, r=me.

[Dirk Schulze]

Created attachment 53801 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 53801 [details]
Patch

r=me. Please remember to update ChangeLog to include the win/ results before landing.

[Dirk Schulze]

Created attachment 53803 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 53803 [details]
Patch

r=me, again. I would have trusted you to update the ChangeLog though :-)

[Dirk Schulze]

Comment on attachment 53803 [details]
Patch

Clearing flags on attachment: 53803

Committed r57886: <http://trac.webkit.org/changeset/57886>

[Dirk Schulze]

All reviewed patches have been landed.  Closing bug.