Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2477393002:
Relax the extra span's alpha (Closed)
DescriptionRelax the extra span's alpha
BUG=chromium:662862
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002
Committed: https://skia.googlesource.com/skia/+/1d3ab12b82e1fa16a0d6b6b9cc1c0290b45cbca9
Patch Set 1 #
Total comments: 2
Messages
Total messages: 14 (6 generated)
Description was changed from ========== Relax the extra span's alpha BUG=chrome:662862 ========== to ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 ==========
Description was changed from ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 ========== to ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 ==========
liyuqian@google.com changed reviewers: + caryclark@google.com, fmalita@chromium.org, reed@google.com
Please check if this small bug fix looks good.
https://codereview.chromium.org/2477393002/diff/1/src/core/SkAAClip.cpp File src/core/SkAAClip.cpp (right): https://codereview.chromium.org/2477393002/diff/1/src/core/SkAAClip.cpp#newco... src/core/SkAAClip.cpp:1339: // Therefore, instead of always asserting alpha==0, we assert alpha < 0x10. The assert below disagrees with the comment; it asserts that alpha <= 0x10 I understand that it is OK if alpha > 0, but does it get to be as large as 0x0F or 0x10 in the case this is trying to avoid? Or is it only 0x01 or 0x02?
https://codereview.chromium.org/2477393002/diff/1/src/core/SkAAClip.cpp File src/core/SkAAClip.cpp (right): https://codereview.chromium.org/2477393002/diff/1/src/core/SkAAClip.cpp#newco... src/core/SkAAClip.cpp:1339: // Therefore, instead of always asserting alpha==0, we assert alpha < 0x10. On 2016/11/07 16:50:51, caryclark wrote: > The assert below disagrees with the comment; it asserts that alpha <= 0x10 > > I understand that it is OK if alpha > 0, but does it get to be as large as 0x0F > or 0x10 in the case this is trying to avoid? Or is it only 0x01 or 0x02? Is SkASSERT(0x10 > *alpha) equivalent to SkASSERT(*alpha < 0x10)? It's hard to tell how small the alpha is since this only happens in mac trybot and I couldn't reproduce it on my Goobuntu. The reason that I chose 0x10 is because that supersampling could only produce alpha 0x10, 0x20, ... so the threshold of 0x10 should have a quality no worse than that. Theoretically, since SkFixed has a precision of 1/65536, we could have an accumulative error up to 1000/65536 for 1000x1000 resolution, which is about 0x04. (The larger the resolution is, the larger this error becomes.) And as discussed in https://docs.google.com/a/google.com/document/d/1gudGmF9HUSzq12YNUibaYuCcjvtd... , we don't have to worry about this error in supersampling algorithm unless we have 8k resolution screen :)
lgtm
The CQ bit was checked by liyuqian@google.com
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 ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 ========== to ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 Committed: https://skia.googlesource.com/skia/+/1d3ab12b82e1fa16a0d6b6b9cc1c0290b45cbca9 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://skia.googlesource.com/skia/+/1d3ab12b82e1fa16a0d6b6b9cc1c0290b45cbca9
Message was sent while issue was closed.
Description was changed from ========== Relax the extra span's alpha BUG=chrome:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 Committed: https://skia.googlesource.com/skia/+/1d3ab12b82e1fa16a0d6b6b9cc1c0290b45cbca9 ========== to ========== Relax the extra span's alpha BUG=chromium:662862 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2477393002 Committed: https://skia.googlesource.com/skia/+/1d3ab12b82e1fa16a0d6b6b9cc1c0290b45cbca9 ==========
Message was sent while issue was closed.
I still don't understand how this is a fix, and not a work-around for an underlying bug in the scan-converters....?
Message was sent while issue was closed.
On 2016/11/09 16:13:34, reed1 wrote: > I still don't understand how this is a fix, and not a work-around for an > underlying bug in the scan-converters....? Because the AA scan-converters use floor/ceiling instead of round: https://docs.google.com/a/google.com/document/d/1gudGmF9HUSzq12YNUibaYuCcjvtd... ? |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
