Bug 109831 – [skia] FEOffset should have a Skia implementation.

• Component: SVG
• Status: RESOLVED FIXED

[Stephen White]

The FEOffset filter does not yet have a skia implementation (for use in the SkImageFilter DAG).

[Stephen White]

Created attachment 188383 [details]
Patch

[Justin Novosad]

Comment on attachment 188383 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188383&action=review

> Source/WebCore/platform/graphics/filters/FEOffset.h:48
> +#if USE(SKIA)

Could make this entry-point usable by other ports: Instead of this #if USE(SKIA) in the header, the common .cpp could have an empty implementation inside #if !USE(SKIA)
And don't forget: typedef SkImageFilter PlatformImageFilter;

> LayoutTests/platform/chromium/TestExpectations:4374
> +# New test cases added to these tests will require rebaselines.

It would be feasible to test this particular filter using a ref test.

[Stephen White]

Comment on attachment 188383 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188383&action=review

>> Source/WebCore/platform/graphics/filters/FEOffset.h:48
>> +#if USE(SKIA)
> 
> Could make this entry-point usable by other ports: Instead of this #if USE(SKIA) in the header, the common .cpp could have an empty implementation inside #if !USE(SKIA)
> And don't forget: typedef SkImageFilter PlatformImageFilter;

We could do that, but it's all pretty skia-specific (e.g. SkiaImageFilterBuilder would have to be typedefed or renamed as well).  It's also the way the other filters are implemented, at the request of the SVG guys.

>> LayoutTests/platform/chromium/TestExpectations:4374
>> +# New test cases added to these tests will require rebaselines.
> 
> It would be feasible to test this particular filter using a ref test.

It might be, but since the other filters have actual differing pixels, I'd prefer to keep them all in this format.

[Stephen White]

Enne, could you take a look?

[Stephen White]

Created attachment 188628 [details]
Patch

[WebKit Review Bot]

Comment on attachment 188628 [details]
Patch

Rejecting attachment 188628 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 188628, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
WebKit/chromium/v8 --revision 13634 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
53>At revision 13634.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...
Total errors found: 0 in 1 files

Full output: http://queues.webkit.org/results/16598269

[Stephen White]

Created attachment 188707 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 188707 [details]
Patch for landing

Clearing flags on attachment: 188707

Committed r143101: <http://trac.webkit.org/changeset/143101>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.