-
Notifications
You must be signed in to change notification settings - Fork 200
Add tile intersection checking #1293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I ran this through my PDF test suite and there only seem to be small unnoticeable single-pixel differences, so no regressions. 👍 However, I won't be able to take a closer look at this before Monday. But just one thing I noticed, to me, it does seem like this will add some processing time for a single tile. Perhaps we could add a const generic to the method indicating whether tile intersection data should be computed, and when false it just sets it to 0? This way, we can skip all of the calculations for vello_cpu, where it's not needed. Also, this PR would make #1211 irrelevant, right? |
No worries! Take your time.
Yes there is a special case for a single tile.
Yes, I was thinking the same thing. Not super familiar rust, so not sure what's the rustiest way to do this, but I think even two different
I think so. I was debating whether it would be worth it to case out more fast paths, but as mentioned above, I do have a "line completely enclosed in tile" case. |
tomcur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool stuff. The packing is clever and looks good.
Your explanation of the packing is great, I think it would be worthwhile to add that as documentation to the packed_winding_line_idx.
Cases of 3 bit ambiguity can be resolved by always calculating intersections on the opposing edges of the tile.
Perhaps if you add the documentation to the code, also mention the 4-bit case (where, if my understanding is correct, you just calculate for any two opposing sides).
To be able to properly review the core idea itself I probably need a bit more background on the needs of MSAA, would you have some pointers?
This way, we can skip all of the calculations for vello_cpu
Yes, I was thinking the same thing.
Conditionally running this seems like a good idea. Here's an example of the const-generic pattern: https://github.com/linebender/parley/blob/4f887570bac98c6693e4a3c6937eebb3423dde72/parley/src/layout/alignment.rs#L84-L97, but two versions also makes sense, especially if the two versions can be optimized differently.
|
I think by default I would prefer a single version, and only if it really turns out that having two separate methods is faster do that. Otherwise, when changing stuff in the logic we need to always remember to do it in both methods. |
|
Added changes as discussed, sans the additional comments. I deferred adding comments since the top-left intersection logic needs to be reworked to address the issue mentioned above. Previously, I had planned on doing all top-left corner tie-breaking logic downstream. However, I need to reconsider this decision because of the bug... |
|
Sorry for the delay, I hope that I will be able to take a look in the next few days. |
|
I'm currently working on an overhaul to the intersections which will change the intersection data mask. Laurenz is working on a patch to the blender2d benchmarking suite to better integrate it with vello. We would like to get both done, and then benchmark the performance before moving forward, so for now there is no need to review. |



This PR adds generation of intersection data to
make_tiles.For our MSAA calculations, we need the points where a line intersects with a tile. Since the end goal is to perform the MSAA in parallel, we need these intersections to be watertight between different threads. By performing these calculations in
make_tiles, we can create a source of ground truth, ensuring watertightness.Although it is feasible to calculate the exact intersection points in
make_tiles, we defer that work to the gpu. Instead we create an intersection bitmask, which unambiguously defines which edges the line intersects. We say a line intersects the edge of a tile if it touches that edge AND continues into another tile. A endpoint that exactly touches an edge does NOT count as intersecting, though it may still contribute to winding (in the case of a top edge touch).The bitmask is 5 bits, and will be consumed downstream by the rasterization stage:
P(erfect) | R(ight) | L(eft) | B(ottom) | T(op)The lower 4 bits correspond to the intersection edges. The "Perfect" bit is necessary to resolve ambiguity in cases where a line perfectly intersects with a corner of a tile.
Cases of 3 bit ambiguity can be resolved by always calculating intersections on the opposing edges of the tile.
Consider a tile with intersections
T | L | B:Calculate the intersections on
TandB!Cases of 2 bit ambiguity require the "Perfect" bit, which is set when there is exactly one unique edge intersection.
Consider a tile with intersections
T | L:This is valid:
But so is this:
With the perfect bit, the first case would be
P | T | Land the second case would beT | L