Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2296703003:
test dont-clip-layer (Closed)
Descriptiontest dont-clip-layer
depends on fix from https://codereview.chromium.org/2309623002/
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003
Committed: https://skia.googlesource.com/skia/+/02f9ed74ead11fcb01a24faa7d93f853e869f1ab
Patch Set 1 #Patch Set 2 : fix for gpu? #Patch Set 3 : allow feature to be tested all the time #
Total comments: 6
Patch Set 4 : add dox for new GM #Patch Set 5 : update gm bounds #Messages
Total messages: 27 (17 generated)
Description was changed from ========== test dont-clip-layer BUG=skia: ========== to ========== test dont-clip-layer BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003 ==========
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
reed@google.com changed reviewers: + bsalomon@google.com, djsollen@google.com
diff should simplify after the remove-setmatrixclip CL lands
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on master.client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
Description was changed from ========== test dont-clip-layer BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003 ========== to ========== test dont-clip-layer depends on fix from https://codereview.chromium.org/2309623002/ BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003 ==========
reed@google.com changed reviewers: + mtklein@google.com
On 2016/09/06 13:29:52, reed1 wrote: lgtm other than the picture playback issues caught by dm
The CQ bit was checked by reed@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp File gm/aaclip.cpp (right): https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode19 gm/aaclip.cpp:19: DEF_SIMPLE_GM(dont_clip_to_layer, canvas, 350, 120) { were there going to be 2 more horizontally? https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode28 gm/aaclip.cpp:28: canvas->saveLayer(rec); Can you add a comment about the effect that we're testing here? (I think if you put it above the DEF_SIMPLE_GM, Joe's comments harvester will pick it up.) I don't understand why the two halves of the square end up with the colors they do.
The CQ bit was unchecked by reed@google.com
The CQ bit was checked by reed@google.com to run a CQ dry run
https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp File gm/aaclip.cpp (right): https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode19 gm/aaclip.cpp:19: DEF_SIMPLE_GM(dont_clip_to_layer, canvas, 350, 120) { On 2016/09/06 14:05:19, mtklein wrote: > were there going to be 2 more horizontally? The scrolling code in android sometimes does that: it creates two layers with a "hole" in the middle. However that was harder to diagnose than this simpler example, which exercises all of the needed code paths (and showed the bug on GPU before the fix). https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode28 gm/aaclip.cpp:28: canvas->saveLayer(rec); On 2016/09/06 14:05:19, mtklein wrote: > Can you add a comment about the effect that we're testing here? (I think if you > put it above the DEF_SIMPLE_GM, Joe's comments harvester will pick it up.) I > don't understand why the two halves of the square end up with the colors they > do. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp File gm/aaclip.cpp (right): https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode19 gm/aaclip.cpp:19: DEF_SIMPLE_GM(dont_clip_to_layer, canvas, 350, 120) { On 2016/09/06 14:36:32, reed1 wrote: > On 2016/09/06 14:05:19, mtklein wrote: > > were there going to be 2 more horizontally? > > The scrolling code in android sometimes does that: it creates two layers with a > "hole" in the middle. However that was harder to diagnose than this simpler > example, which exercises all of the needed code paths (and showed the bug on GPU > before the fix). Sorry, was just wondering about why this was so wide. It can probably be 120x120 right?
The CQ bit was unchecked by reed@google.com
https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp File gm/aaclip.cpp (right): https://codereview.chromium.org/2296703003/diff/40001/gm/aaclip.cpp#newcode19 gm/aaclip.cpp:19: DEF_SIMPLE_GM(dont_clip_to_layer, canvas, 350, 120) { On 2016/09/06 14:51:21, mtklein wrote: > On 2016/09/06 14:36:32, reed1 wrote: > > On 2016/09/06 14:05:19, mtklein wrote: > > > were there going to be 2 more horizontally? > > > > The scrolling code in android sometimes does that: it creates two layers with > a > > "hole" in the middle. However that was harder to diagnose than this simpler > > example, which exercises all of the needed code paths (and showed the bug on > GPU > > before the fix). > > Sorry, was just wondering about why this was so wide. It can probably be > 120x120 right? Ah, gotcha. Will fix.
The CQ bit was checked by reed@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com, mtklein@google.com Link to the patchset: https://codereview.chromium.org/2296703003/#ps80001 (title: "update gm bounds")
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 ========== test dont-clip-layer depends on fix from https://codereview.chromium.org/2309623002/ BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003 ========== to ========== test dont-clip-layer depends on fix from https://codereview.chromium.org/2309623002/ BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2296703003 Committed: https://skia.googlesource.com/skia/+/02f9ed74ead11fcb01a24faa7d93f853e869f1ab ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/02f9ed74ead11fcb01a24faa7d93f853e869f1ab |
