We should pass all the css3/filters tests when using the SkImageFilter DAG path. Currently we run it only when there's a reference filter in the chain.
Created attachment 173673 [details] Patch
Comment on attachment 173673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173673&action=review > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.cpp:62 > +bool DropShadowImageFilter::onFilterImage(Proxy*proxy, const SkBitmap& source, const SkMatrix& matrix, SkBitmap* result, SkIPoint* loc) SK_OVERRIDE { It's unusual to see any sort of OVERRIDE on the implementation - do you really need this? space between 'Proxy*' and 'proxy' > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:38 > + DropShadowImageFilter(SkFlattenableReadBuffer&); we'd normally use explicit here > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:39 > + virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; Is SK_OVERRIDE the same as OVERRIDE? Can we just use the WebKit macros? > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:45 > + typedef SkSingleInputImageFilter INHERITED; What's this? We would normally reserve ALLCAPS for macros in WebKit code
(In reply to comment #2) > (From update of attachment 173673 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=173673&action=review > > > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.cpp:62 > > +bool DropShadowImageFilter::onFilterImage(Proxy*proxy, const SkBitmap& source, const SkMatrix& matrix, SkBitmap* result, SkIPoint* loc) SK_OVERRIDE { > > It's unusual to see any sort of OVERRIDE on the implementation - do you really need this? Whoops, thanks, fixed. > space between 'Proxy*' and 'proxy' Done. > > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:38 > > + DropShadowImageFilter(SkFlattenableReadBuffer&); > > we'd normally use explicit here Done. > > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:39 > > + virtual void flatten(SkFlattenableWriteBuffer&) const SK_OVERRIDE; > > Is SK_OVERRIDE the same as OVERRIDE? Can we just use the WebKit macros? Done. > > Source/WebCore/platform/graphics/filters/skia/DropShadowImageFilter.h:45 > > + typedef SkSingleInputImageFilter INHERITED; > > What's this? We would normally reserve ALLCAPS for macros in WebKit code Removed. (This file started life as a Skia patch, in case it isn't obvious).
Created attachment 173765 [details] Patch
Comment on attachment 173765 [details] Patch Clearing flags on attachment: 173765 Committed r134467: <http://trac.webkit.org/changeset/134467>
All reviewed patches have been landed. Closing bug.