Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(315)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 376953003: Clean up SkImageFilter constructors. (Closed)

Created:
6 years, 5 months ago by Stephen White
Modified:
6 years, 5 months ago
Reviewers:
reed1
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

Clean up SkImageFilter constructors. Now that all creation of SkImageFilters goes through factory Create() methods, there's no real reason for the convenience constructors. Some SkImageFilter subclasses which actually have zero DAG-able inputs were passing NULL to the superclass constructor. This actually means 1 input, with a NULL value, not zero inputs. This becomes more relevant for the upcoming cache infrastructure, where this indicates that the filter will use its src input, where in fact some of these filters do not (they are image generators only). Limiting SkImageFilter to a single constructor resolves this ambiguity. Along the way, I removed all of the default parameters to the constructors, since the Create methods always call them with the full argument list. BUG=skia: Committed: https://skia.googlesource.com/skia/+/9ea3d57fde28a5fe4487a111dc3dd49418235e5e

Patch Set 1 #

Patch Set 2 : More cleanup #

Patch Set 3 : Fix deserializing constructor for SkRectShaderImageFilter #

Total comments: 8

Patch Set 4 : Fix FailImageFilter deserializing constructor #

Patch Set 5 : Fixed 100-col issues #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -100 lines) Patch
M gm/imagefiltersbase.cpp View 1 2 3 2 chunks +5 lines, -5 lines 0 comments Download
M gm/imagefiltersgraph.cpp View 1 chunk +1 line, -1 line 0 comments Download
M include/core/SkImageFilter.h View 1 chunk +0 lines, -6 lines 0 comments Download
M include/effects/SkAlphaThresholdFilter.h View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M include/effects/SkColorFilterImageFilter.h View 1 1 chunk +1 line, -1 line 0 comments Download
M include/effects/SkComposeImageFilter.h View 1 chunk +3 lines, -2 lines 0 comments Download
M include/effects/SkDisplacementMapEffect.h View 1 2 chunks +4 lines, -4 lines 0 comments Download
M include/effects/SkLightingImageFilter.h View 1 1 chunk +1 line, -1 line 0 comments Download
M include/effects/SkMagnifierImageFilter.h View 1 2 3 4 1 chunk +4 lines, -3 lines 0 comments Download
M include/effects/SkMatrixImageFilter.h View 1 1 chunk +1 line, -1 line 0 comments Download
M include/effects/SkMergeImageFilter.h View 1 2 chunks +5 lines, -6 lines 0 comments Download
M include/effects/SkMorphologyImageFilter.h View 1 2 chunks +2 lines, -6 lines 0 comments Download
M include/effects/SkTestImageFilters.h View 1 2 3 4 1 chunk +4 lines, -3 lines 0 comments Download
M include/effects/SkTileImageFilter.h View 1 chunk +1 line, -1 line 0 comments Download
M include/effects/SkXfermodeImageFilter.h View 2 chunks +4 lines, -3 lines 0 comments Download
M src/core/SkImageFilter.cpp View 1 chunk +0 lines, -17 lines 0 comments Download
M src/effects/SkAlphaThresholdFilter.cpp View 1 2 3 4 3 chunks +8 lines, -5 lines 0 comments Download
M src/effects/SkBlurImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkColorFilterImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkDisplacementMapEffect.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
M src/effects/SkDropShadowImageFilter.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/effects/SkLightingImageFilter.cpp View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M src/effects/SkMagnifierImageFilter.cpp View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M src/effects/SkMatrixConvolutionImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkMatrixImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkMergeImageFilter.cpp View 1 chunk +0 lines, -11 lines 0 comments Download
M src/effects/SkMorphologyImageFilter.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/effects/SkOffsetImageFilter.cpp View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M src/effects/SkRectShaderImageFilter.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M src/effects/SkXfermodeImageFilter.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
M tests/ImageFilterTest.cpp View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Stephen White
reed@: PTAL. Thanks!
6 years, 5 months ago (2014-07-08 14:47:53 UTC) #1
reed1
nice change lgtm w/ nits https://codereview.chromium.org/376953003/diff/30001/include/effects/SkMagnifierImageFilter.h File include/effects/SkMagnifierImageFilter.h (right): https://codereview.chromium.org/376953003/diff/30001/include/effects/SkMagnifierImageFilter.h#newcode17 include/effects/SkMagnifierImageFilter.h:17: static SkMagnifierImageFilter* Create(const SkRect& ...
6 years, 5 months ago (2014-07-08 15:15:24 UTC) #2
Stephen White
Fixed a couple other 100-col issues, and made SkAlphaThresholdFilter DAG-able. https://codereview.chromium.org/376953003/diff/30001/include/effects/SkMagnifierImageFilter.h File include/effects/SkMagnifierImageFilter.h (right): https://codereview.chromium.org/376953003/diff/30001/include/effects/SkMagnifierImageFilter.h#newcode17 ...
6 years, 5 months ago (2014-07-08 15:33:31 UTC) #3
Stephen White
The CQ bit was checked by senorblanco@chromium.org
6 years, 5 months ago (2014-07-08 15:40:57 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/senorblanco@chromium.org/376953003/70001
6 years, 5 months ago (2014-07-08 15:41:56 UTC) #5
commit-bot: I haz the power
6 years, 5 months ago (2014-07-08 16:16:27 UTC) #6
Message was sent while issue was closed. 
Change committed as 9ea3d57fde28a5fe4487a111dc3dd49418235e5e

Powered by Google App Engine
This is Rietveld 408576698