Skia reftest fuzzing for Skia content on OS X

RESOLVED FIXED in Firefox 47

Status

()

Core
Graphics
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mchang, Assigned: mchang)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Changes required to reftest.lists to pass try reftests with skia content on OS X
(Assignee)

Comment 1

2 years ago
Created attachment 8716384 [details] [diff] [review]
Giant reftest fuzz list

One giant reftest fuzz list which passes try with OS X skia content:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=633377ef066c

I wasn't sure I wanted to annotate every test, so I figured I 'd put the explanations in this bug.

One reason for fuzzing reftests with text due to subpixel text positioning. Skia chops the floating point value of glyph offsets into one of four buckets: 0, 0.25, 0.5, 0.75. The floating point value is rounded up by 0.125. E.g, if the offset given to us by layout [1] is 13.15, Skia will see this position at 13.275, and round it down into the 0.125 bucket. Skia renders glyphs at around (0,0) when getting the glyph from CG. Thus Skia will ask CG to render the glyph at ~0.125, which CG does the actual subpixel text positioning. Skia then caches this glyph, copies the image glyph data, and blits the glyph to a target position that is rounded out to device pixels [2]. This glyph is stored in the cache.

Thus, any glyph that falls into the 0.25 bucket that is the same character will use this cached image glyph. e.g. a glyph at offset 17.05 and 17.10 will fall into the 17 glyph and be rendered the same. What's happening with the reftests is that layout is giving us slightly different offsets for some text. e.g. layout/reftests/bidi/unicode-bidi-plaintext gives us two characters: one at 16.11 offset, which falls into the 0 bucket and one at 16.13, which falls into the 0.25 bucket. This generates two slightly different glyphs, causing the reftest failures.

[1] http://hg.mozilla.org/mozilla-central/file/e9875981edf2/gfx/2d/DrawTargetSkia.cpp#l624
[2] http://hg.mozilla.org/mozilla-central/file/e9875981edf2/gfx/skia/skia/src/ports/SkFontHost_mac.cpp#l1133
Attachment #8716384 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

2 years ago
Lee, can you please detail explanations for the alpha blending issue and the linear gradient issue here? This way we have a record in the blame log for why we fuzzed the tests. Thanks!
Flags: needinfo?(lsalzman)
(In reply to Mason Chang [:mchang] from comment #2)
> Lee, can you please detail explanations for the alpha blending issue and the
> linear gradient issue here? This way we have a record in the blame log for
> why we fuzzed the tests. Thanks!

Blending: Skia does a lot of fractional pixel math by using an 8 bit shift to divide by 256, since this is much cheaper than trying to divide by 255. But since pixel values are on a scale of 0..255, Skia first converts the alpha value to a scale of 0..256 before it can do such math on it. For alpha values, it unfortunately does this by simply adding 1 to the alpha value, converting 0..255 to 1..256. This means alpha value can be off by around 1 unit at times, requiring our fuzz to be increased by 1 in many cases.

Gradients: Skia uses a table-based approach to encoding the gradients in a lookup texture. Due to the way texture coordinates map to the texture, since it goes from 0..1, rather than the center of the first pixel to the center of the last pixel, you can get some slight offsetting that depends on the size of the lookup table. Skia also uses a combination of nearest filtering and dithering on the software renderer that can further cause artifacts.

Miscellaneous: Skia also does some fixed point math conversions in various places on vertexes. So if you take slightly different paths through the code (like using an SVG reference vs. normal html content test case) and depending on matrixes supplied, can cause slightly different enough rounding such that boundary pixels on some primitives like beziers may be a bit different between the test case and its reference.
Flags: needinfo?(lsalzman)
Comment on attachment 8716384 [details] [diff] [review]
Giant reftest fuzz list

Review of attachment 8716384 [details] [diff] [review]:
-----------------------------------------------------------------

ok...
Attachment #8716384 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 5

2 years ago
Created attachment 8718995 [details] [diff] [review]
Giant reftest fuzz

Carrying r+, rebased on master.
Attachment #8716384 - Attachment is obsolete: true
Attachment #8718995 - Flags: review+
(Assignee)

Comment 6

2 years ago
Successful try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=f38b491b7ab2

And the tree is closed still :/
Keywords: checkin-needed
(In reply to Mason Chang [:mchang] from comment #1)
> Skia chops the floating point value of glyph offsets into one of four
> buckets: 0, 0.25, 0.5, 0.75.

At one point in the past I tested what CG does, and found out that it uses three buckets: 0, 0.33, 0.67. It might be worth trying the same in Skia and seeing whether it allows us to fuzz less.
failed to apply:

patching file layout/reftests/box-shadow/reftest.list
Hunk #1 succeeded at 10 with fuzz 1 (offset 0 lines).
patching file layout/reftests/bugs/reftest.list
Hunk #30 FAILED at 1784
1 out of 31 hunks FAILED -- saving rejects to file layout/reftests/bugs/reftest.list.rej
patching file layout/reftests/invalidation/reftest.list
Hunk #1 FAILED at 59
1 out of 1 hunks FAILED -- saving rejects to file layout/reftests/invalidation/reftest.list.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh reftestFuzz.patch
Flags: needinfo?(mchang)
Keywords: checkin-needed

Comment 9

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b85eae6c9a05
(Assignee)

Comment 10

2 years ago
Thanks, rebased on master and pushed to inbound.
Flags: needinfo?(mchang)
(Assignee)

Comment 11

2 years ago
(In reply to Markus Stange [:mstange] from comment #7)
> (In reply to Mason Chang [:mchang] from comment #1)
> > Skia chops the floating point value of glyph offsets into one of four
> > buckets: 0, 0.25, 0.5, 0.75.
> 
> At one point in the past I tested what CG does, and found out that it uses
> three buckets: 0, 0.33, 0.67. It might be worth trying the same in Skia and
> seeing whether it allows us to fuzz less.

I suspect that it would. Skia uses 2 bits for both the X and Y axis positioning, so we may be able to hack it a bit to use three buckets. However, we're going to use Skia cross platform, which means other platforms may use more than 3 buckets. I'd also not tweak that many Skia internals, otherwise the benefits of using Skia might be outweighed from the cost of customization.
We could always try to upstream our modifications, but sure, what you say makes sense.

Comment 13

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/387463b5c093

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b85eae6c9a05
https://hg.mozilla.org/mozilla-central/rev/387463b5c093
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.