Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1704433003:
Make kInvalidConversion a fatal failure in DMSrcSink (Closed)
DescriptionMake kInvalidConversion a fatal failure in DMSrcSink
We just need to abort early on tests that we know
are invalid conversions.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1704433003
Committed: https://skia.googlesource.com/skia/+/55f7bdd956bf73dd2501a35a646870c1aebb34b7
Patch Set 1 #Patch Set 2 : Try again #Patch Set 3 : Third time is the charm #
Total comments: 2
Messages
Total messages: 28 (14 generated)
Description was changed from ========== Test BUG=skia: ========== to ========== Test 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 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/patch-status/1704433003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704433003/1
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 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/patch-status/1704433003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704433003/20001
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 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/patch-status/1704433003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704433003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Mac-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-Arm7...) Build-Mac-Clang-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
Patchset #3 (id:40001) 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/patch-status/1704433003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704433003/60001
Description was changed from ========== Test BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Make kInvalidConversion a fatal failure in DMSrcSink We just need to abort early on tests that we know are invalid conversions. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
msarett@google.com changed reviewers: + scroggo@google.com
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1704433003/diff/60001/dm/DMSrcSink.cpp File dm/DMSrcSink.cpp (right): https://codereview.chromium.org/1704433003/diff/60001/dm/DMSrcSink.cpp#newcod... dm/DMSrcSink.cpp:325: kOpaque_SkAlphaType != decodeInfo->alphaType()) { We only attempt gray if the codec reports it as the default, right? Does this case ever get hit? It seems like this would imply that the image reported gray + alpha?
https://codereview.chromium.org/1704433003/diff/60001/dm/DMSrcSink.cpp File dm/DMSrcSink.cpp (right): https://codereview.chromium.org/1704433003/diff/60001/dm/DMSrcSink.cpp#newcod... dm/DMSrcSink.cpp:325: kOpaque_SkAlphaType != decodeInfo->alphaType()) { On 2016/02/16 21:13:25, scroggo wrote: > We only attempt gray if the codec reports it as the default, right? Does this > case ever get hit? It seems like this would imply that the image reported gray + > alpha? It gets hit because we now test kOpaque images to kOpaque, kPremul, and kUnpremul.
lgtm
The CQ bit was checked by msarett@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704433003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704433003/60001
Message was sent while issue was closed.
Description was changed from ========== Make kInvalidConversion a fatal failure in DMSrcSink We just need to abort early on tests that we know are invalid conversions. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Make kInvalidConversion a fatal failure in DMSrcSink We just need to abort early on tests that we know are invalid conversions. 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/+/55f7bdd956bf73dd2501a35a646870c1aebb34b7 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://skia.googlesource.com/skia/+/55f7bdd956bf73dd2501a35a646870c1aebb34b7 |
