Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upUse Pathfinder's more accurate area approximation for distance AA. #2445
Conversation
|
Here's an image showing the difference. "New" is with this patch, while "old" is the master branch.
Notice that the pixels inside the red box have a smoother look with this patch, especially the top row. |
|
Looks like the same two Linux-specific tests as the ellipse PR need updating again. I'll update those and do a try run later today. |
|
|
|
Fixed the merge conflict and rebased. |
|
the reftest images are certainly improved with the PR |
| float distance_aa(float aa_range, float signed_distance) { | ||
| return 1.0 - smoothstep(-aa_range, aa_range, signed_distance); | ||
| float dist = 0.5 * signed_distance / aa_range; | ||
| if (dist <= -0.5 + EPSILON) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pcwalton
Feb 21, 2018
Author
Collaborator
If the EPSILON fudge factor isn't taken into account, reftests/clip/custom-clip-chains.yaml has some minor 1 pixel differences on the sides due to what I presume to be floating point error.
|
I think this is fine, but I'm going to wait until #2454 lands to fix the intermittents, and then do another try run with this to be safe. |
Smoothstep is not a very good approximation because it has zero derivatives at 0.0 and 1.0. It makes edges too sharp (i.e. more aliased). From the comments in `prim_shared.glsl`: This cubic polynomial approximates the area of a 1x1 pixel square under a line, given the signed Euclidean distance from the center of the square to that line. Calculating the *exact* area would require taking into account not only this distance but also the angle of the line. However, in practice, this complexity is not required, as the area is roughly the same regardless of the angle. The coefficients of this polynomial were determined through least-squares regression and are accurate to within 2.16% of the total area of the pixel square 95% of the time, with a maximum error of 3.53%.
|
Rebased on the intermittents fix in master. Pending try: https://hg.mozilla.org/try/rev/cf4d827c6ae079c708e188b96e8d6ae2296c3293 |
|
I checked each of the failures in the reftest analyzer, and they are all subpixel AA differences. So we can land this and fuzz those, since this is clearly an improvement in quality. @bors-servo r+ |
|
|
Use Pathfinder's more accurate area approximation for distance AA. Smoothstep is not a very good approximation because it has zero derivatives at 0.0 and 1.0. It makes edges too sharp (i.e. more aliased). From the comments in `prim_shared.glsl`: This cubic polynomial approximates the area of a 1x1 pixel square under a line, given the signed Euclidean distance from the center of the square to that line. Calculating the *exact* area would require taking into account not only this distance but also the angle of the line. However, in practice, this complexity is not required, as the area is roughly the same regardless of the angle. The coefficients of this polynomial were determined through least-squares regression and are accurate to within 2.16% of the total area of the pixel square 95% of the time, with a maximum error of 3.53%. Happy to answer any questions you might have. r? @glennw <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2445) <!-- Reviewable:end -->
|
|
|
@pcwalton @glennw Any theories as to why the box-decoration-break-with-inset-box-shadow-1.html reftest (in R4) is behaving nondeterministically? In opt it seems to always have 8924 differing pixels but on debug occasionally there's only 8772 differing pixels. This is the case in both the try push from #2445 (comment) and the one at https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2c0d91c2fef35bea888505733b98c824c92110c (I did some retriggers on both). |
|
@staktrace Very strange. This is a GPU-code-only change, which shouldn't be affected by debug vs. release… I would guess the issue is likely some sort of race in the CPU-side code that debug vs. release triggers. |
|
It's likely not specific to debug vs release, but just a race somewhere that happens to get hit on debug more frequently. |
|
@staktrace I think this is probably yet another instance of #1943 occurring, since box shadows are involved. I've assigned that bug to me, since it seems to be the main cause of non-determinism we have. I'll see if I can get it fixed next week. |
pcwalton commentedFeb 20, 2018
•
edited by larsbergstrom
Smoothstep is not a very good approximation because it has zero
derivatives at 0.0 and 1.0. It makes edges too sharp (i.e. more
aliased).
From the comments in
prim_shared.glsl:This cubic polynomial approximates the area of a 1x1 pixel square under a
line, given the signed Euclidean distance from the center of the square to
that line. Calculating the exact area would require taking into account
not only this distance but also the angle of the line. However, in
practice, this complexity is not required, as the area is roughly the same
regardless of the angle.
The coefficients of this polynomial were determined through least-squares
regression and are accurate to within 2.16% of the total area of the pixel
square 95% of the time, with a maximum error of 3.53%.
Happy to answer any questions you might have.
r? @glennw
This change is