Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1813793002:
images with offset bitmap don't share genid (Closed)
|
Created:
4 years, 9 months ago by hal.canary Modified:
4 years, 9 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
Descriptionimages with offset bitmap don't share genid
BUG=skia:5096
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1813793002
Committed: https://skia.googlesource.com/skia/+/8d3ad5a1223551927a34f72b35f9dbf5c1f5bc93
Patch Set 1 : 2016-03-17 (Thursday) 12:13:54 EDT #
Total comments: 4
Patch Set 2 : 2016-03-17 (Thursday) 13:56:40 EDT #Patch Set 3 : 2016-03-17 (Thursday) 15:47:30 EDT #Patch Set 4 : 2016-03-17 (Thursday) 15:52:56 EDT #Patch Set 5 : 2016-03-17 (Thursday) 17:29:58 EDT #Messages
Total messages: 39 (21 generated)
Description was changed from ========== images offset bitmap don't share genid ========== to ========== images offset bitmap don't share genid GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== images offset bitmap don't share genid GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== images with offset bitmap don't share genid GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by halcanary@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/1813793002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/1
halcanary@google.com changed reviewers: + reed@google.com
PTAL
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 client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
The CQ bit was checked by halcanary@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/1813793002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/80001
Patchset #4 (id:60001) has been deleted
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp File tests/image-bitmap.cpp (right): https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp#... tests/image-bitmap.cpp:40: // Test that when we make an image from a subset of a bitmap, that it has a diff ID from an image made from the entire bitmap (see bug#...) https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp#... tests/image-bitmap.cpp:44: if (img) { Not sure these runtime checks are needed, unless its defined that these (MakeFromBitmap, extractSubset) are allowed to fail sometimes. Definitely makes reading the test harder
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp File tests/image-bitmap.cpp (right): https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp#... tests/image-bitmap.cpp:40: On 2016/03/17 17:34:15, reed1 wrote: > // Test that when we make an image from a subset of a bitmap, that it has a diff > ID from an image made from the entire bitmap (see bug#...) Done. https://codereview.chromium.org/1813793002/diff/80001/tests/image-bitmap.cpp#... tests/image-bitmap.cpp:44: if (img) { On 2016/03/17 17:34:15, reed1 wrote: > Not sure these runtime checks are needed, unless its defined that these > (MakeFromBitmap, extractSubset) are allowed to fail sometimes. Definitely makes > reading the test harder Done.
The CQ bit was checked by halcanary@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/1813793002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== images with offset bitmap don't share genid GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== images with offset bitmap don't share genid BUG=skia:5096 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Please take another look.
The CQ bit was checked by halcanary@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/1813793002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...) Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
The CQ bit was checked by halcanary@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/1813793002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
halcanary@google.com changed reviewers: + bsalomon@google.com
ping
lgtm. I don't think we lose any perf relative to how SkBitmap used to interact with the GPU backend as the subset rect was part of the cached texture's key.
The CQ bit was checked by halcanary@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1813793002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1813793002/160001
Message was sent while issue was closed.
Description was changed from ========== images with offset bitmap don't share genid BUG=skia:5096 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== images with offset bitmap don't share genid BUG=skia:5096 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/8d3ad5a1223551927a34f72b35f9dbf5c1f5bc93 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:160001) as https://skia.googlesource.com/skia/+/8d3ad5a1223551927a34f72b35f9dbf5c1f5bc93 |
