Switch FEComponentTransfer to use skia's SkColorFilter when compiling for Skia.
Created attachment 161049 [details] Patch
Comment on attachment 161049 [details] Patch Attachment 161049 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13652573 New failing tests: css3/filters/effect-reference-ordering.html css3/filters/effect-combined.html css3/filters/effect-opacity.html
Created attachment 161074 [details] Archive of layout-test-results from gce-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 161223 [details] Patch
I would like to have Philip or Stephen take a look at this first.
schenney, pdr: ping?
Comment on attachment 161223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161223&action=review > Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:83 > + virtual bool platformApplySkia(); Where is this actually called? Did the patch fail to pick up a changed file? If this is a preparatory patch then the test expectations shouldn't be updated. > Source/WebCore/platform/graphics/filters/skia/FEComponentTransferSkia.cpp:39 > + if (!in) This is only invoked if there is an inputEffect, so no need to check this.
(In reply to comment #7) > > Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:83 > > + virtual bool platformApplySkia(); > > Where is this actually called? FilterEffect::apply()
(In reply to comment #8) > (In reply to comment #7) > > > Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:83 > > > + virtual bool platformApplySkia(); > > > > Where is this actually called? > > FilterEffect::apply() Not in this patch. :-)
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > > Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:83 > > > > + virtual bool platformApplySkia(); > > > > > > Where is this actually called? > > > > FilterEffect::apply() > > Not in this patch. :-) Florin is correct. It's virtual, and the base class version returns false. This patch picks up the override for FEComponentTransfer.
Comment on attachment 161223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161223&action=review >> Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:83 >> + virtual bool platformApplySkia(); > > Where is this actually called? Did the patch fail to pick up a changed file? If this is a preparatory patch then the test expectations shouldn't be updated. Ah, now I remember how this stupid thing works. Everything defines platformApplySkia but the default returns false. Sorry. >> Source/WebCore/platform/graphics/filters/skia/FEComponentTransferSkia.cpp:39 >> + if (!in) > > This is only invoked if there is an inputEffect, so no need to check this. This is a nit. If a reviewer doesn't care then I don't care, but if I were a reviewer I would care having gotten grief about this kind of thing in the past.
Created attachment 163412 [details] Patch
Created attachment 163414 [details] Patch
Comment on attachment 161223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161223&action=review Thanks for the review. >>> Source/WebCore/platform/graphics/filters/skia/FEComponentTransferSkia.cpp:39 >>> + if (!in) >> >> This is only invoked if there is an inputEffect, so no need to check this. > > This is a nit. If a reviewer doesn't care then I don't care, but if I were a reviewer I would care having gotten grief about this kind of thing in the past. Fixed.
Comment on attachment 163414 [details] Patch I would certainly R+ this. Dirk, over to you, 'cause you have the power.
Comment on attachment 163414 [details] Patch r=me
Comment on attachment 163414 [details] Patch Clearing flags on attachment: 163414 Committed r128223: <http://trac.webkit.org/changeset/128223>
All reviewed patches have been landed. Closing bug.