tangrams / tangram

WebGL map rendering engine for creative cartography
https://tangram.city
MIT License
2.22k stars 290 forks source link

Fix line dash color handling #742

Closed bcamper closed 4 years ago

bcamper commented 4 years ago

This PR revamps the logic for applying line dash patterns and textures. The prior logic had some holes and inconsistencies, including #741. It implements the following logic in the fragment shader for lines (and also replaces some conditional if/else shader branching with nested mix functions):

bcamper commented 4 years ago

@matteblair It looks like the same issue described in #741 also exists in Tangram ES (because it looks like it more or less copies the previously faulty Tangram JS logic... 😅). I took a couple stabs at fixing it, and found some other edge cases (the alpha cutout test was too early, causing inconsistent alpha handling depending on texture + blend mode combinations), and found it easier to just revamp to the logic in this PR, which hopefully is clearer (but using nested mix which isn't always intuitive either). Please give it a look and let me know if it makes sense!

We should open a corresponding issue to #741 for ES as well.

bcamper commented 4 years ago

Actually, I don't think Tangram ES suffers from the same specific behavior reported in #741, current logic here:

https://github.com/tangrams/tangram-es/blob/master/core/shaders/polyline.fs#L67-L82

But, I still think it has related issues, particularly with the order of the alpha cutout test vs. the tinting (vertex color multiplied by line texture color), and would benefit from a review in conjunction with this PR.

bcamper commented 4 years ago

Hey @matteblair, any further thoughts here? I'd like to cut a minor release with @meetar's geojson-vt upgrade. I tried to be complete with the logic description in the PR, but the actual shader code is pretty concise ;)