Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1829913002:
Make a generic list of filters for use in ImageFilterTests. (Closed)
|
Created:
4 years, 9 months ago by Stephen White Modified:
4 years, 9 months ago Reviewers:
robertphillips CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionMake a generic list of filters for use in ImageFilterTests.
Factor list of image filters out of one test for re-use elsewhere.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1829913002
Committed: https://skia.googlesource.com/skia/+/297f7ce2bb6184849adf0e5479e92f76764ec76a
Patch Set 1 #
Total comments: 6
Patch Set 2 : Refactor another test; add input & cropRect; style fixes #Patch Set 3 : Remove useless variable #Patch Set 4 : Use sk_sp instead of SkAutoTUnref #Messages
Total messages: 26 (13 generated)
Description was changed from ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. ========== to ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
senorblanco@chromium.org changed reviewers: + robertphillips@google.com
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829913002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829913002/1
Rob: PTAL. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp File tests/ImageFilterTest.cpp (right): https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:151: all these become "this->addFilter" ? https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:195: const char* fName; Could we make these SkAutoTUnrefs and remove the above destructor ? https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:197: }; addFilter ?
Note that I added an input & cropRect param to the FilterList constructor, which allowed me to refactor another test. Also, 100-col fixes. https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp File tests/ImageFilterTest.cpp (right): https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:151: On 2016/03/23 19:48:48, robertphillips wrote: > all these become "this->addFilter" ? Done. https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:195: const char* fName; On 2016/03/23 19:48:48, robertphillips wrote: > Could we make these SkAutoTUnrefs and remove the above destructor ? Done. https://codereview.chromium.org/1829913002/diff/1/tests/ImageFilterTest.cpp#n... tests/ImageFilterTest.cpp:197: }; On 2016/03/23 19:48:48, robertphillips wrote: > addFilter ? Done.
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829913002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829913002/20001
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829913002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829913002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)
lgtm
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829913002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829913002/60001
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 senorblanco@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robertphillips@google.com Link to the patchset: https://codereview.chromium.org/1829913002/#ps60001 (title: "Use sk_sp instead of SkAutoTUnref")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1829913002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1829913002/60001
Message was sent while issue was closed.
Description was changed from ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Make a generic list of filters for use in ImageFilterTests. Factor list of image filters out of one test for re-use elsewhere. GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/297f7ce2bb6184849adf0e5479e92f76764ec76a ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/297f7ce2bb6184849adf0e5479e92f76764ec76a |
