Improve numerical robustness in path tiling #401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This test scene contains a number of subpaths that contain line segments aligned to the grid, or that cross grid corners. These trigger one-pixel artifacts with the multisampled rendering modes, and are now correct with area antialiasing.
WIP. Currently works but no cleanup.
Change representation to tile-relative coordinates for segment endpoints.
This commit fixes a number of problems with multisampled AA. There are three main changes: * This fixes a cut'n'paste typo where the third word of four in resolution was computed incorrectly. This degraded antialiasing quality. * There is a bit more logic in `is_bump` to handle crossings at the top of the tile better. This is not done in the even-odd variant because it is seemingly not needed. (I don't have a careful analysis why, but suspect it has something to do with the fact that double counting or getting the sign wrong is less critical in even-odd than nonzero). * There is a tweak in tiling to cause vertical lines sent to fine to be not pixel-aligned. That's a difficult case for msaa (area doesn't care). The latter two might be considered hacky, but examination of the test cases should improve confidence. There were artifacts in glyph rendering that this patch also improves, so in any case it's better than the previous state.
CI is broken because the WebGPU stuff doesn't have stable semver guarantees. This pins the web-sys version to protect against that.
armansito
approved these changes
Nov 1, 2023
To say that the msaa logic is dense and hard to understand is an understatement. I've added some comments which will I hope will help the reader navigate the code, but ultimately it requires a real explanation. While reviewing the code, I realized that horizontal pixel-aligned lines are already being discarded by separate robustness logic, so that case doesn't need to be handled in is_bump, so I took it out.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a "blocky artifact" bug in area antialiasing, makes changes in the way path segments are represented, and introduces a test scene crafted to expose numerical robustness artifacts.
The artifact was due to a discrepancy in the way numerical robustness was handled between msaa and area antialiasing. This patch moves that logic into tiling, which should help performance as well as consistency. In so doing, it changes the representation from a viewport-relative coordinate of point0 and (point1 - point0) to one where both points are relative to the top left of the tile. That in turn is preparation for a change to f16 coordinates (not done in this PR).
Here is an informal description of the numerical robustness logic for path tiling.
A grid-aligned vertical line (x0 = x1 = 0 in tile coordinates) is handled specially. If y0 = 0, then the crossing is counted for the entire tile. Otherwise, the line is discarded. The "entire tile" case is modeled in the code as drawing a vertical line offset slightly from the left edge of the tile, which has the same effect, and fine code never has to deal with the case where x0 = x1 = 0.
Non-vertical lines touching the left edge of the tile (x0 = 0 xor x1 = 0) are potentially a y-edge crossing. If the line touches the top left of the tile, it is not a crossing, otherwise it is. The former case is modeled in the code by displacing the point slightly to the right, so that x0 = 0 or x1 = 0 in fine is a reliable indicator of y-edge crossing. Currently a y-edge value is computed and stored in
Segment, but it is redundant with zero comparisons of x values, and is intended to go away when those values move to f16.There are two good ways to understand the numerical robustness logic. One is formally, by counting intersections between the path segments (represented as floating point) against "reference points" which are determined with tiny epsilon values. The reference point corresponding to backdrop is -ε², ε). The reference point for the tile is (ε², ε), which is why a vertical line from the tile origin counts as a crossing for the tile. The reference point for the pixel row y is (ε², ε + y), which is why lines touching the top left corner are not counted as a crossing, while otherwise lines touching the left edge are.
The other way to understand the logic is by cases, and a good way to present those are by illustrations. Unfortunately that's beyond the scope of this PR description, but we will write this up properly.
The added test is very good at triggering numerical robustness issues, and in fact surface errors in msaa that haven't been detected before. Addressing those will be a followup PR. It is believed that area aa is now completely correct.