Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1887103002:
Switch SkLightingImageFilter over to new onFilterImage interface (Closed)
|
Created:
4 years, 8 months ago by robertphillips Modified:
4 years, 8 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionSwitch SkLightingImageFilter over to new onFilterImage interface
TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1887103002
Committed: https://skia.googlesource.com/skia/+/ad3dc0da218cc43c9917a256d4afb8cf0d7c5899
Patch Set 1 #Patch Set 2 : clean up #Patch Set 3 : clean up #
Total comments: 3
Patch Set 4 : rename onFilterImageGPU to filterImageGPU #Patch Set 5 : switch false -> nullptr #Messages
Total messages: 20 (10 generated)
Description was changed from ========== Switch SkLightingImageFilter over to new onFilterImage interface ========== to ========== Switch SkLightingImageFilter over to new onFilterImage interface GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Switch SkLightingImageFilter over to new onFilterImage interface GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Switch SkLightingImageFilter over to new onFilterImage interface TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
robertphillips@google.com changed reviewers: + reed@google.com, senorblanco@chromium.org, senorblanco@google.com
LGTM; naming nit is at your discretion https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... File src/effects/SkLightingImageFilter.cpp (right): https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... src/effects/SkLightingImageFilter.cpp:358: sk_sp<SkSpecialImage> onFilterImageGPU(SkSpecialImage* source, microNit: onFoo() is usually reserved for things that are virtual and things that already have a non-virtual foo()?
https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... File src/effects/SkLightingImageFilter.cpp (right): https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... src/effects/SkLightingImageFilter.cpp:358: sk_sp<SkSpecialImage> onFilterImageGPU(SkSpecialImage* source, On 2016/04/14 18:58:25, Stephen White wrote: > microNit: onFoo() is usually reserved for things that are virtual and things > that already have a non-virtual foo()? Agreed. onFoo almost always means: protected virtual onFoo
https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... File src/effects/SkLightingImageFilter.cpp (right): https://codereview.chromium.org/1887103002/diff/40001/src/effects/SkLightingI... src/effects/SkLightingImageFilter.cpp:358: sk_sp<SkSpecialImage> onFilterImageGPU(SkSpecialImage* source, On 2016/04/14 19:58:16, reed1 wrote: > On 2016/04/14 18:58:25, Stephen White wrote: > > microNit: onFoo() is usually reserved for things that are virtual and things > > that already have a non-virtual foo()? > > Agreed. onFoo almost always means: protected virtual onFoo Done.
The CQ bit was checked by robertphillips@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1887103002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1887103002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
The CQ bit was checked by robertphillips@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1887103002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1887103002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by robertphillips@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from senorblanco@chromium.org Link to the patchset: https://codereview.chromium.org/1887103002/#ps80001 (title: "switch false -> nullptr")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1887103002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1887103002/80001
Message was sent while issue was closed.
Description was changed from ========== Switch SkLightingImageFilter over to new onFilterImage interface TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Switch SkLightingImageFilter over to new onFilterImage interface TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/ad3dc0da218cc43c9917a256d4afb8cf0d7c5899 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/ad3dc0da218cc43c9917a256d4afb8cf0d7c5899 |
