fix anti-aliasing for fill-patterns #2372
Conversation
|
looks like the anti-aliasing is fixed, but I still need to refactor and clean the code up, as well as adjust sensitivity for the test-suite (because as you can see they're still passing with this fix) and run a few more test cases. fill pattern test on master branch fill pattern test on this branch |
| @@ -21,5 +21,13 @@ void main() { | |||
| vec2 pos2 = mix(u_pattern_tl_b, u_pattern_br_b, imagecoord_b); | |||
| vec4 color2 = texture2D(u_image, pos2); | |||
|
|
|||
| gl_FragColor = mix(color1, color2, u_mix) * u_opacity; | |||
| // find distance to outline for alpha interpolation | |||
| float dist_a = length(v_pos_a - gl_FragCoord.xy); | |||
There was a problem hiding this comment.
In order to fade the pixel appropriately we need the distance of the pixel from the center of the line. gl_FragCoord.xy is the position of the pixel which we need to subtract from the position of the closest point on the line.
v_pos_a is the position of texture pixel we need. I think we need another varying v_pos that has the position of the nearest point on the line like in the outline shader.
mollymerp
force-pushed
the
anti-alias-fill-pattern
branch
4 times, most recently
from
afbf454 to
7daa739
Compare
|
@ansis pointed out that I was using incorrect variables to calculate the alpha value before. I have updated the shaders accordingly. the new changes cause 2 of the tests to fail, so I will have to update the test-suite once we confirm the output is what we expect. this branch |
|
The rendering in 7daa739 looks right to me. The render test rendering is a bit off but that is a completely separate issue. The headless renderer we use only supports a gl.lineWidth of 1: #2080 We should still update render tests with the new rendering. I guess the remaining things are:
|
| @@ -49,6 +50,39 @@ function draw(painter, source, layer, coords) { | |||
| for (var j = 0; j < coords.length; j++) { | |||
| drawStroke(painter, source, layer, coords[j]); | |||
| } | |||
| } else if (layer.paint['fill-pattern']) { | |||
There was a problem hiding this comment.
is this the correct conditional to have here? I was thinking else if (layer.paint['fill-pattern'] && !strokeColor) might make sense? do we even need another conditional at all or would simply else suffice? I'm not super familiar with the possible permutations of the condition checked in L27.
There was a problem hiding this comment.
drawFill is called twice for each layer each frame: once for the opaque pass and once for the translucent pass. The opaque pass is only used to draw completely opaque geometry. All antialiasing strokes are translucent so they need to be drawn when painter.isOpaquePass is false.
No antialiasing should be drawn if layer.paint['fill-antialias'] is false.
if (!painter.isOpaquePass && layer.paint['fill-antialias']) {
if (strokeColor || !layer.paint['fill-pattern']) {
// draw outline with either strokeColor or the fill color
} else {
// drawn the new patterned outline
}
}
7daa739 to
f12493a
Compare
mollymerp
changed the title
|
PR made in mapbox-gl-test-suite to update render tests |
mollymerp
force-pushed
the
anti-alias-fill-pattern
branch
from
f12493a to
a088102
Compare
| @@ -221,3 +201,55 @@ function drawStroke(painter, source, layer, coord) { | |||
| gl.drawElements(gl.LINES, count, gl.UNSIGNED_SHORT, elementOffset); | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| function imageOffset(image, opacity, tile, coord, painter, program) { | |||
There was a problem hiding this comment.
can this be renamed to something with a verb like usePattern or setPattern?
|
This looks great to me! the remaining things I see are
|
| @@ -1,7 +1,7 @@ | |||
|
|
|||
| mapboxgl.accessToken = getAccessToken(); | |||
|
|
|||
| var map = new mapboxgl.Map({ | |||
| window.map = new mapboxgl.Map({ | |||
mollymerp
force-pushed
the
anti-alias-fill-pattern
branch
3 times, most recently
from
2b10f31 to
6a985a6
Compare
mollymerp
force-pushed
the
anti-alias-fill-pattern
branch
from
6a985a6 to
c5f154c
Compare
update package.json with new test-suite sha, rename imageOffset -> setPattern include distance interpolation for opacity fix linting tests refactor image offset calculations use distance from line to calculate alpha, change to multiplication for combining u_opacity and alpha add u_world to use_program for outline pattern, refactor using new imageOffset function refactor draw_fill, fix conditionals
|
👏 |
a quick & dirty first attempt at resolving the problem with anti-aliasing when there is a fill-pattern – this code doesn't throw errors but also doesn't solve the problem. 😐