Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1719773002:
clean-up dead code (Closed)
Descriptionclean-up dead code
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1719773002
Committed: https://skia.googlesource.com/skia/+/8781123a177fbb9f933254da4cee0a983522b33b
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #
Messages
Total messages: 20 (8 generated)
Description was changed from ========== clean-up dead code BUG=skia: ========== to ========== clean-up dead code BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
reed@google.com changed reviewers: + fmalita@chromium.org, senorblanco@chromium.org
ptal
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/patch-status/1719773002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1719773002/1
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/patch-status/1719773002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1719773002/20001
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/patch-status/1719773002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1719773002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Do we still need the clamp-to-premul flag? Do we really want to have a xfermode that doesn't return valid pixels?
LGTM On 2016/02/21 20:15:18, reed1 wrote: > Do we still need the clamp-to-premul flag? Do we really want to have a xfermode > that doesn't return valid pixels? IIUC that flag is an optimization - a hint that the function is guaranteed to preserve the PM invariant => then it's safe to skip clamping. senorblanco may know more about how useful this optimization is. It may be worth adding asserts that the values are PM-valid on the !fEnforcePMColor branch.
Oh, I really thought it was meant to allow the xfermode to output illegal values, since we call the nocheck version of the byte-packer...
The CQ bit was checked by reed@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1719773002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1719773002/40001
Message was sent while issue was closed.
Description was changed from ========== clean-up dead code BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== clean-up dead code 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/+/8781123a177fbb9f933254da4cee0a983522b33b ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/8781123a177fbb9f933254da4cee0a983522b33b
Message was sent while issue was closed.
On 2016/02/22 14:39:17, reed1 wrote: > Oh, I really thought it was meant to allow the xfermode to output illegal > values, since we call the nocheck version of the byte-packer... Yes, I think if the downstream node in the DAG can handle illegal values (usually another arithmetic filter), it will tell the upstream filter to skip validation. See FEComposite::createImageFilterWithoutValidation() and https://chromium.googlesource.com/chromium/src/+/446459722cf93f9dc4f3cce3c768... I can't find a referenced bug, but I think we decided this was actually correct spec behaviour.
Message was sent while issue was closed.
LGTM BTW, it looks like there's a similar flag in SkArithmeticMode_gpu.cpp. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
