Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Fix outline rendering #8760

Closed
wants to merge 2 commits into from
Closed

Fix outline rendering #8760

wants to merge 2 commits into from

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Apr 18, 2017

@jfirebaugh discovered in #8751 that we are using undefined behavior in the fill_outline shader: https://github.com/mapbox/mapbox-gl-js/blob/c0dcee76ed5c7d0559bf271ed85692b1b81c654c/src/shaders/fill_outline.fragment.glsl#L11

It seems that some implementations correctly interpolate between 1.0 and 0.0 based on the t value, but per the spec, it's undefined behavior.

It also fixes outline rendering, which has regressed at some point.

Current behavior when drawing an outline with a color different from the fill color:

actual

New (and correct) behavior:

expected

We are drawing outlines as antialiased GL_LINES around the polygon. When the color is identical to the fill color, we are clipping the outline at the polygon bounds (see first image), but we can't do that when the color is different. Therefore, we either render the fill below or above the line. At some point, this regressed into rendering it always below (and in this case, it's clipped/covered by the fill).

@friedbunny friedbunny added the Core The cross-platform C++ core, aka mbgl label Apr 18, 2017
@friedbunny
Copy link
Contributor

This fix looks significant enough to warrant changelog entries (at least, for iOS). 🙇

@jfirebaugh
Copy link
Contributor

Working on the failing query test in mapbox/mapbox-gl-js#4604.

@jfirebaugh jfirebaugh force-pushed the 8751-fill_outline-rendering branch from 9902294 to 6e12e5e Compare April 18, 2017 21:21
@jfirebaugh
Copy link
Contributor

jfirebaugh commented Apr 18, 2017

@kkaefer Even with these changes, macosapp still isn't rendering correctly for me locally.

image

image

@kkaefer
Copy link
Member Author

kkaefer commented Apr 20, 2017

@jfirebaugh What GPU are you using locally? What's the maximum line width in your OpenGL implementation?

@jfirebaugh
Copy link
Contributor

Intel Iris Graphics 6100, GL_ALIASED_LINE_WIDTH_RANGE = [1, 7], GL_SMOOTH_LINE_WIDTH_RANGE = [0, 7]

@kkaefer
Copy link
Member Author

kkaefer commented Aug 7, 2017

Fixed in #9699

@kkaefer kkaefer closed this Aug 7, 2017
@jfirebaugh jfirebaugh deleted the 8751-fill_outline-rendering branch August 7, 2017 23:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants