Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1723383002:
Ensure the Gaussian tail of circle blurs goes to zero. (Closed)
|
Created:
4 years, 9 months ago by dogben Modified:
4 years, 9 months ago Reviewers:
robertphillips CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionEnsure the Gaussian tail of circle blurs goes to zero.
Compare blurcircle GM masked so that values <255 are white and =255 are black:
https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-lt255-before.png
https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-lt255-after.png
This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1723383002
Committed: https://skia.googlesource.com/skia/+/9d24023c6664a1469ea4d31ab1c9b5bd764beca5
Patch Set 1 #
Total comments: 2
Patch Set 2 : Force-zero the tail instead of increasing the size. #Messages
Total messages: 20 (11 generated)
Description was changed from ========== Add 1 to texture size to ensure the gaussian tail goes to zero. BUG=skia: ========== to ========== Add 1 to texture size to ensure the gaussian tail goes to zero. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Add 1 to texture size to ensure the gaussian tail goes to zero. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Add 1 to texture size to ensure the gaussian tail goes to zero. Compare blurcircle GM masked so that values <255 are white and =255 are black: https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by benjaminwagner@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/1723383002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1723383002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
benjaminwagner@google.com changed reviewers: + robertphillips@google.com
lgtm as is but I wonder if we should just brute force it. https://codereview.chromium.org/1723383002/diff/1/src/effects/GrCircleBlurFra... File src/effects/GrCircleBlurFragmentProcessor.cpp (right): https://codereview.chromium.org/1723383002/diff/1/src/effects/GrCircleBlurFra... src/effects/GrCircleBlurFragmentProcessor.cpp:215: Are we guaranteed that weights[numSteps-1] will always be zero? Should we just have: weights[numSteps-1] = 0; here?
The CQ bit was checked by benjaminwagner@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/1723383002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1723383002/20001
Description was changed from ========== Add 1 to texture size to ensure the gaussian tail goes to zero. Compare blurcircle GM masked so that values <255 are white and =255 are black: https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Ensure the Gaussian tail of circle blurs goes to zero. Compare blurcircle GM masked so that values <255 are white and =255 are black: https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
https://codereview.chromium.org/1723383002/diff/1/src/effects/GrCircleBlurFra... File src/effects/GrCircleBlurFragmentProcessor.cpp (right): https://codereview.chromium.org/1723383002/diff/1/src/effects/GrCircleBlurFra... src/effects/GrCircleBlurFragmentProcessor.cpp:215: On 2016/02/24 at 12:52:56, robertphillips wrote: > Are we guaranteed that weights[numSteps-1] will always be zero? > Should we just have: > weights[numSteps-1] = 0; > here? sgtm
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 benjaminwagner@google.com
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/1723383002/#ps20001 (title: "Force-zero the tail instead of increasing the size.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1723383002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1723383002/20001
Message was sent while issue was closed.
Description was changed from ========== Ensure the Gaussian tail of circle blurs goes to zero. Compare blurcircle GM masked so that values <255 are white and =255 are black: https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Ensure the Gaussian tail of circle blurs goes to zero. Compare blurcircle GM masked so that values <255 are white and =255 are black: https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... https://x20web.corp.google.com/~benjaminwagner/review/1723383002/blurcircles-... This is a spinoff of https://codereview.chromium.org/1691403002, which was previously causing more "rectangles" in this GM. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/9d24023c6664a1469ea4d31ab1c9b5bd764beca5 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/9d24023c6664a1469ea4d31ab1c9b5bd764beca5 |
