Transition all pipelines to new draw_flags field for fill rule
#399
Conversation
* Pipelines downstream of flatten (draw_leaf, coarse) now extract the fill rule using a "draw flags" field. Flatten still writes this to the path bounding-box structure, which gets propagated to the draw info list, and eventually translates to the fill rule written to the PTCL. * draw_leaf no longer deals with transforming the linewidth for strokes. This was a leftover from the previous architecture and the logic is no longer needed. * bbox_clear used to contain a duplicate PathBbox data structure. It now uses the one from shader/shared/bbox.wgsl This continues the work outlined in #303
Wired up the even-odd fill-rule to the CPU version of coarse.
armansito
force-pushed
the
gpu-stroke-rework-contd
branch
from
71037f1 to
fada357
Compare
The CPU version of the coarse stage was missing logic to correctly handle the even-odd fill rule. This change copies over the existing logic from the GPU version with some added documentation.
Added more draws to the fill_types test scene where different fills overlap and blend with each other. This is mostly to validate the even-odd fill logic in the `coarse` stage.
armansito
force-pushed
the
gpu-stroke-rework-contd
branch
from
fada357 to
161a31c
Compare
Moved the per-drawobject tile discard logic near the existing tile inclusion code in the main loop. This avoids the branch conditionals that were present in each draw tag case branch.
armansito
force-pushed
the
gpu-stroke-rework-contd
branch
from
b2a232e to
7a84914
Compare
There was a problem hiding this comment.
This all looks good except for a possible tweak to the include_tile logic, see inline. I especially appreciate the code simplification and reduced divergence in coarse. I think divergence could be further improved by grouping the write_paths for the different draw object types, but I'm not sure how much of a real performance improvement that would be.
shader/coarse.wgsl
Outdated
| // backdrop (i.e. the winding number of its top-left corner) is even. | ||
| // | ||
| // NOTE: A clip should never get encoded with an even-odd fill. | ||
| let even_odd_discard = n_segs == 0u && even_odd && (abs(tile.backdrop) & 1) == 0; |
There was a problem hiding this comment.
In my first pass, I was worried that this would be incorrect for even-odd clips. But I now see this can't be encoded, so my concern is more one of style than correctness.
If we ever do support even-odd clips (which is eoclip in PostScript and Core Graphics, W* in PDF), then we'd want to make the change. I think a version that gets this correct is just as concise, and potentially clearer, so suggest something along these lines:
let backdrop_clear = select(tile.backdrop, tile.backdrop & 1, even_odd) == 0;
let include_tile = n_segs != 0u || (backdrop_clear == is_clip) || is_blend;But I wouldn't block the PR on this, it is correct as written.
There was a problem hiding this comment.
I like your formulation better than what I have as it is concise and doesn't rule out even-odd clips (which I wasn't aware of until now).
| let n_segs = tile.segment_count_or_ix; | ||
| let include_tile = n_segs != 0 || (tile.backdrop == 0) == is_clip || is_blend; | ||
|
|
||
| // If this draw object represents an even-odd fill and we know that no line segment |
There was a problem hiding this comment.
See above comment; if the change is made there, it should be consistent here.
This migrates all pipelines downstream of
flattenaway from the oldlinewidthencoding which was only being used for the fill rule, which now gets stored in adraw_flagsfield. I also cleaned up some leftover linewidth transform logic which was no longer used and wired up the new fill rule flag to the CPU version of thecoarsestage.This continues the work outlined in #303. For more details please refer to the individual commit messages.