Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2263513003:
Fix bug, always keep fIsScaleTranslate in correct state (Closed)
DescriptionFix bug, always keep fIsScaleTranslate in correct state
BUG:639179
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003
Committed: https://skia.googlesource.com/skia/+/9da5a5a198e5dc9148f7f30a6089377590eee55b
Patch Set 1 #
Total comments: 8
Patch Set 2 : Improve comments #
Depends on Patchset: Messages
Total messages: 22 (14 generated)
Description was changed from ========== Fix bug, always keep fIsScaleTranslate in correct state BUG=skia: ========== to ========== Fix bug, always keep fIsScaleTranslate in correct state BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 ==========
Patchset #1 (id:1) has been deleted
The CQ bit was checked by msarett@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...
msarett@google.com changed reviewers: + mtklein@google.com, reed@google.com, robertphillips@google.com
tomhudson@google.com changed reviewers: + tomhudson@google.com
Driveby LGTM
lgtm
https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.cpp File tests/QuickRejectTest.cpp (right): https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:150: // Regression test to make sure that we keep fIsScaleTranslate up to date on the canvas. Presumably the point is that sometimes we set a matrix without going through setMatrix(), and this tests that path? (one of those paths?) https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:154: // Set rotation matrix. Nit: this is not a useful comment? https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:159: // Set image filter. Nit: nor is this. https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:169: // Call quickReject(). It will assert if the matrix is out of sync. // quickReject() will assert if the matrix is out of sync.
https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.cpp File tests/QuickRejectTest.cpp (right): https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:150: // Regression test to make sure that we keep fIsScaleTranslate up to date on the canvas. On 2016/08/19 15:09:03, tomhudson wrote: > Presumably the point is that sometimes we set a matrix without going through > setMatrix(), and this tests that path? (one of those paths?) Yes, updating comment. https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:154: // Set rotation matrix. On 2016/08/19 15:09:03, tomhudson wrote: > Nit: this is not a useful comment? Removed. https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:159: // Set image filter. On 2016/08/19 15:09:03, tomhudson wrote: > Nit: nor is this. Removed. https://codereview.chromium.org/2263513003/diff/20001/tests/QuickRejectTest.c... tests/QuickRejectTest.cpp:169: // Call quickReject(). It will assert if the matrix is out of sync. On 2016/08/19 15:09:03, tomhudson wrote: > // quickReject() will assert if the matrix is out of sync. Done.
The CQ bit was checked by msarett@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...
lgtm
Description was changed from ========== Fix bug, always keep fIsScaleTranslate in correct state BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 ========== to ========== Fix bug, always keep fIsScaleTranslate in correct state BUG:639179 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 ==========
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 msarett@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com, tomhudson@google.com Link to the patchset: https://codereview.chromium.org/2263513003/#ps40001 (title: "Improve comments")
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 ========== Fix bug, always keep fIsScaleTranslate in correct state BUG:639179 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 ========== to ========== Fix bug, always keep fIsScaleTranslate in correct state BUG:639179 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 Committed: https://skia.googlesource.com/skia/+/9da5a5a198e5dc9148f7f30a6089377590eee55b ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://skia.googlesource.com/skia/+/9da5a5a198e5dc9148f7f30a6089377590eee55b |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
