Sort point features by y-axis

[tmcw]

For overlays and possibly for built-in layer data, we need to sort by y-axis before displaying overlapping features that look like 'markers' - otherwise things look weird. This will need to be recalculated based on rotation if we want to really nail it.

[incanus]

This needs to happen in native, too. Rules, borrowing from iOS SDK current behavior (modeled on Apple's MapKit):

• Markers above shapes #
• Markers ascending in z as relates to ascending in y (top of screen down) #
• Ability to bring a selected marker forward #

This all needs to happen each frame, especially for rotation.

[tmcw]

Markers above shapes

I'd assume this happens at the style level, right? Like, for a low-level interface, all that we're implementing is sorting in a single layer.

[incanus]

Ah, yes, correct. Lines and points would presumably belong to separate layers, which are then ordered in the style.

[incanus]

So in thinking about approaches to this:

• We don't have access to gl_FragDepth in WebGL/OpenGL ES.
• We can't (?) modify gl_FragCoord in the pixel shader.
• We need a viewport-normalized a value based on a_pos.y for something like gl_Position = u_matrix * vec4(a_pos, mix(-0.2, -0.1, a, 1); in the vertex shader. FWIW values in the range of -0.2 to -0.1 seem to work alright with our other shaders' non-zero depth values (e.g. text and lines).

That's where I'm at so far... still trying to figure out a good approach.

[ansis]

Yep, depth buffer values can't be set from the fragment shader, so solutions using depth aren't going to work. Even if we could set the depth value per fragment I think we'd still have problems in semi-transparent areas.

Features are layered in the order they appear in the buffers, so sorting the points before adding them should work. They would need to be resorted after every rotation though.

[incanus]

Why does depth have to be set in the fragment shader? Why can't we adjust the a_pos in the vertex shader?

They would need to be resorted after every rotation though.

Isn't this going to be have to be on the CPU though?

[incanus]

mapbox/mapbox-gl-native#333

[ansis]

Why does depth have to be set in the fragment shader? Why can't we adjust the a_pos in the vertex shader?

Setting the depth doesn't automatically arrange the features in the right z-order. It just allows you to write to the depth buffer and compare against the depth buffer. Setting the depth on the vertices will set the interpolated depth for each fragment. You can then later test fragments against the previously written depth value, and choose to drop the fragments if they fail some condition. You could use this to prevent drawing over higher z-order markers, but it will prevent drawing for the marker's entire box including transparent pixels.

I might be missing something big though

Isn't this going to be have to be on the CPU though?

yes

[incanus]

Got it. Yeah we don't want to drop, not just because of boxing but also alpha values in marker imagery themselves -- these should interleave smoothly.

[tmcw]

@ansis what's the easiest way to get a testcase for this? should I hack debug to add some points to the geojson?

[incanus]

Now that #477 is done, one way would be to do what I've been doing over on native. Here's my diff using a vector point source:

https://gist.github.com/incanus/485f6e63528124c98f2e

[ansis]

@incanus's way seems good, but geojson points would also be fine and easier to make into a testcase.

Also, I think this should be opt-in.

[incanus]

Also, I think this should be opt-in.

Agreed. Likely defaulting to on for iOS, but we should open up the API to hit both POI (non-collide) and overlay (probably want to collide) use cases.

[ansis]

This is easy to implement with the new labeling: 5bf094f

How should this be specified in the style? spec issue here: mapbox/mapbox-gl-style-spec#274

[incanus]

This is mostly designed for annotations, so the style is hardcoded such as in https://github.com/mapbox/mapbox-gl-native/blob/0a2ee766dbe65e3d23d0e5cba79776560ff29275/src/mbgl/style/style_parser.cpp#L40-L69, then would have the ability to turn a feature like this on or off via API.

[ChrisLoer]

#5150 partially reverts this behavior (by allowing sort order inconsistencies at tile boundaries) in order to fix #2706 along with other issues. See discussion at #5150 (comment).