Page MenuHomePhabricator

Bug 1609805 - Support a new reftest kind, for verifying rasterizer accuracy.
ClosedPublic

Authored by gw on Jan 16 2020, 10:18 PM.

Details

Summary

This patch introduces a new reftest (syntax ** or !* in reftest files).

This type of test renders a single input file multiple times, at a range
of different picture cache tile sizes. It then verifies that each of the
images matches (or doesn't).

This can be used to verify rasterizer accuracy when drawing primitives
with different tile sizes and/or dirty rect update strategies.

One of the included tests in this patch fails the accuracy test - the
intent is to fix this inaccuracy in a follow up patch and then be able to
mark it pixel exact.

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gw created this revision.Jan 16 2020, 10:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2020, 10:18 PM
phab-bot requested review of this revision.Jan 16 2020, 10:18 PM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: Restricted Project.
gw added a reviewer: jimb.Jan 16 2020, 10:27 PM

Added Kris and Jim - don't feel obliged to review this patch - but I'm trying to get in the habit of adding new gfx people to review lists, to get a bit more coverage / introduction to parts of WR :)

gw added a reviewer: Bert.Jan 17 2020, 1:44 AM
nical accepted this revision.Jan 17 2020, 9:43 AM

To what degree are these inaccuracies common to different hardware (and the software emulation we use in CI)?

This revision is now accepted and ready to land.Jan 17 2020, 9:43 AM
Bert accepted this revision.Jan 17 2020, 6:51 PM
kvark added a comment.Jan 17 2020, 7:37 PM

This can be used to verify rasterizer accuracy when drawing primitives

with different tile sizes and/or dirty rect update strategies.

I'm not entirely clear on what failure we are testing here for. If the vertices are placed on perfect pixel boundaries (and this is possible, given that tile size is power of 2, we have enough bits of precision in f32 to make this perfect), then we should be having the result invariant over the tile size. Only if we don't place the vertices properly, we'd expect the tile size affect whether some pixel centers are erroneously included/excluded. In this case, there shouldn't be a new kind of expectation, we should always expect it to be perfect and fix the shaders otherwise.
Am I missing anything?

gfx/wr/webrender/src/render_backend.rs
1147

Should this be SetPictureTileSize for clarity? I.e. we tile different things, picture caching being one of them.

gfx/wr/wrench/reftests/border/reftest.list
31

how was this working before? is == implicit?

gw added inline comments.Jan 19 2020, 7:44 PM
gfx/wr/webrender/src/render_backend.rs
1147

Yep, done.

gfx/wr/wrench/reftests/border/reftest.list
31

Yea, it was (I think accidentally) assumed as == if not present previously.

gw updated this revision to Diff 219601.Jan 19 2020, 7:44 PM