Bug 109831

Summary: [skia] FEOffset should have a Skia implementation.
Product: WebKit Reporter: Stephen White <senorblanco>
Component: SVGAssignee: Stephen White <senorblanco>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, enne, jamesr, junov, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Stephen White 2013-02-14 07:39:09 PST
The FEOffset filter does not yet have a skia implementation (for use in the SkImageFilter DAG).
Comment 1 Stephen White 2013-02-14 10:46:02 PST
Created attachment 188383 [details]
Patch
Comment 2 Justin Novosad 2013-02-14 11:11:42 PST
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.
Comment 3 Stephen White 2013-02-14 12:05:44 PST
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.
Comment 4 Stephen White 2013-02-15 11:55:05 PST
Enne, could you take a look?
Comment 5 Stephen White 2013-02-15 13:09:51 PST
Created attachment 188628 [details]
Patch
Comment 6 WebKit Review Bot 2013-02-15 15:21:13 PST
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
Comment 7 Stephen White 2013-02-16 03:56:19 PST
Created attachment 188707 [details]
Patch for landing
Comment 8 WebKit Review Bot 2013-02-16 04:47:38 PST
Comment on attachment 188707 [details]
Patch for landing

Clearing flags on attachment: 188707

Committed r143101: <http://trac.webkit.org/changeset/143101>
Comment 9 WebKit Review Bot 2013-02-16 04:47:42 PST
All reviewed patches have been landed.  Closing bug.