Fix rendering of symbol layers under solid fill layers

[ansis]

It looks like depth testing is disabled for symbol rendering. Is there a good reason?

@scothis

[lucaswoj]

Not as far as I know. Looking back at git history suggests that this existed in your original layer-by-layer branch. https://github.com/mapbox/mapbox-gl-js/blame/aaa7e63bc88bada9b967062826161decf1106c94/js/render/draw_symbol.js#L29

[ansis]

When rendering we draw all solid fills top down. We then render everything else bottom-up. We use the depth buffer to drop all fragments that are behind solid fills. This is faster than rendering everything bottom-up because we can drop fragments.

Depth testing is disabled for symbols because it breaks rotation-alignment: viewport labels in perspective view. Since the extrudes are added after projection the depth values don't match up. I don't think there is a simple way to fix the depth values in this case.

We can enable depth testing for rotation-alignment: map labels and fix rendering order in all those cases. We should definitely do this. This would fix oneway arrows drawn under bridges.

We can enable depth testing for rotation-alignment: viewport labels when pitch is 0. Should we do this? The labels would pop up above the fill layers as soon as you tilt.

If we really want to obey layer order for rotation-alignment: viewport labels in tilted views we need to treat all fill layers above the symbol layers as if they are transparent and draw them in the bottom-up pass. Should we do this? Are there any cases when this would matter? I'm not sure this would make sense conceptually since in perspective view these labels exist in 3d space not the 2d layer space.

[lucaswoj]

The "highlight features under the mouse pointer" example demonstrates this bug well

[scothis]

A hacky workaround is to set the fill layer's opacity to 0.99

[jfirebaugh]

I'm starting to think we should just not bother with a two-pass implementation. Currently, the only layers we draw in the opaque pass are opaque, non-patterned fills. And since we'd eventually like to figure out how to antialias fills without a separate GL_LINES draw call, even opaque fills may eventually need to be drawn in the transparent phase.

[anandthakker]

@jfirebaugh was the original reason for the two-pass approach to allow for the depth buffer optimization @ansis described above? I.e.:

When rendering we draw all solid fills top down. We then render everything else bottom-up. We use the depth buffer to drop all fragments that are behind solid fills. This is faster than rendering everything bottom-up because we can drop fragments

[jfirebaugh]

Yes, precisely, the two pass approach is intended to minimize overdraw.

[ansis]

I think enabling depth testing for symbols and setting nearDepth and farDepth to the exact same value here could fix this:

mapbox-gl-js/src/render/painter.js

Lines 280 to 284 in ab7d06a

	depthModeForSublayer(n: number, mask: DepthMaskType, func: ?DepthFuncType): DepthMode {
	const farDepth = 1 - ((1 + this.currentLayer) * this.numSublayers + n) * this.depthEpsilon;
	const nearDepth = farDepth - 1 + this.depthRange;
	return new DepthMode(func || this.context.gl.LEQUAL, mask, [nearDepth, farDepth]);
	}

That should make all fragments in the layer have the same value which should make the depth clipping work correctly. I can't remember why they were made different but I don't think that is necessary.

If depthRange with identical values doesn't work as expected we could modify the matrix to achieve the same result.