Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2131643002:
Add HardStopGradientBench.cpp (Closed)
DescriptionAdd HardStopGradientBench.cpp
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002
Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45
Committed: https://skia.googlesource.com/skia/+/d6767aa2d52d2612f2b7f9b27b1c3e72ac1389cd
Patch Set 1 #
Total comments: 3
Patch Set 2 : Brian's suggestions #Patch Set 3 : Make paint field, add 100-color benchmark #Patch Set 4 : Fix segfault #
Total comments: 1
Messages
Total messages: 22 (9 generated)
Description was changed from ========== Add HardStopGradientBench.cpp BUG=skia: ========== to ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 ==========
fmenozzi@google.com changed reviewers: + bsalomon@google.com
Added benchmark for hard stop gradients, ptal
https://codereview.chromium.org/2131643002/diff/1/bench/HardStopGradientBench... File bench/HardStopGradientBench.cpp (right): https://codereview.chromium.org/2131643002/diff/1/bench/HardStopGradientBench... bench/HardStopGradientBench.cpp:17: struct GradData { Is this used? https://codereview.chromium.org/2131643002/diff/1/bench/HardStopGradientBench... bench/HardStopGradientBench.cpp:43: count = std::max(0, std::min(count, kMaxCount)); Not sure I understand the purpose of kMaxCount. https://codereview.chromium.org/2131643002/diff/1/bench/HardStopGradientBench... bench/HardStopGradientBench.cpp:50: // "Evenly spaced" colors Could from here onward and createShaderPaint be done in onPreDraw? nanobench creates instances of Benchmarks just to get their names and filter them. Also doesn't seem like you really need to store fPositions and fColors permanently, just while creating the paint.
- Moved setup to onPreDraw() - Removed unused struct - Removed kMaxCount - Removed fColors, fPositions ptal
- Make paint field - Add 100-color benchmark ptal
lgtm
The CQ bit was checked by fmenozzi@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 ========== to ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2140603003/ by fmenozzi@google.com. The reason for reverting is: Segfaulting on HardStopGradientBench; didn't allocate enough stack memory for color/position arrays..
Message was sent while issue was closed.
fmenozzi@google.com changed reviewers: + tomhudson@google.com
Message was sent while issue was closed.
Fixed segfault, ptal
Message was sent while issue was closed.
LGTM, although I wouldn't have minded leaving in fMaxCount.
Message was sent while issue was closed.
Description was changed from ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45 ========== to ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45 ==========
The CQ bit was checked by fmenozzi@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com Link to the patchset: https://codereview.chromium.org/2131643002/#ps60001 (title: "Fix segfault")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45 ========== to ========== Add HardStopGradientBench.cpp BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2131643002 Committed: https://skia.googlesource.com/skia/+/d0d652e0e0cb4bcecb7ccac9f4f4831fcaccdc45 Committed: https://skia.googlesource.com/skia/+/d6767aa2d52d2612f2b7f9b27b1c3e72ac1389cd ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/d6767aa2d52d2612f2b7f9b27b1c3e72ac1389cd
Message was sent while issue was closed.
https://codereview.chromium.org/2131643002/diff/60001/bench/HardStopGradientB... File bench/HardStopGradientBench.cpp (right): https://codereview.chromium.org/2131643002/diff/60001/bench/HardStopGradientB... bench/HardStopGradientBench.cpp:56: SkColor colors[100]; Why not use a dynamic array? |
