Fix polyline rendering when using the logarithmic depth buffer

[kring]

Following up on #8703, this fixes most of the rest of the polyline / log depth problems.

Before this PR, (non-ground) polylines were pretty much unusable with the logarithmic depth buffer enabled. They would frequently be missing chunks or appear to be at a different depth than they should be.

For example:
Fixes #6741 (comment)
Fixes #7852
Fixes #7072
Fixes #7374
Fixes #7852

The problem was that the fragment depth values produced for polylines were straight-up wrong, but it took me forever to understand why. Polyline rendering works in screen space, so the vertex shader always produced window coordinates directly, and set gl_Position.w to 1.0.

Now I guess I never fully appreciated just what that w component is good for, and maybe the author of the polyline rendering code didn't either, but it's super important. The fixed function portion of the rendering pipeline in between the vertex and fragment shaders uses that component to do perspective correct interpolation of varyings coming from the vertex shader.

Without perspective-correct interpolation, varyings are interpolated based on how close on the screen the fragment is to each of the vertices that make up the triangle being rendered. Interpolation happens as if the triangle is a real 2D triangle on the screen. With perspective-correct interpolation, varyings are interpolated based on how close in actual 3D space the fragment is to each of the vertices. Interpolation happens as if the triangle is a real 3D triangle with a projection onto a 2D screen.

For polyline rendering, most of the varyings - things like texture coordinates, width, and color - should arguably be interpolated in screen space anyway, so non-perspective correct interpolation is acceptable. However, when log depth is enabled, we pass the vertex's depth from the camera to the fragment shader in a varying as well, in v_depthFromNearPlusOne. If this varying is not interpolated in a perspective-correct manner, the interpolated depth in the middle of the triangle will be wrong. And this was the cause of all these polyline / log depth rendering problems.

As far as I know, there is no way to do perspective-correct interpolation of any varying when the vertex shader sets gl_Position.w to 1.0, as the polyline vertex shader was doing. So step 1 in fixing this was to set that w component correctly. That caused all the varyings to be perspective-correct, which I had hoped would be fine. But unfortunately it caused major artifacts.

Looks ok when looking straight down:

Totally broken at an oblique angle:

It took me ages to understand why perspective-correct interpolation was such a disaster for the texture coordinates of this arrow material. This page eventually helped me understand:
https://web.cs.ucdavis.edu/~amenta/s12/perspectiveCorrect.pdf

The problem is very similar to when you texture map a normal quad at an oblique angle without accounting for perspective. Except that in this case, it's not a normal quad. Because it maintains a constant size in screen space, it's actually becoming much wider (in world space) the deeper you go into the screen. And just like in the image at the top of that PDF, that "growth" is not evenly shared between the two triangles that make up the quad. At steep angles, that creates major distortion.

Ok, so in order to avoid these artifacts, we need to interpolate the varyings (at least the texture coordinates) in screen space, even though we have an accurate w component. In modern OpenGL, including WebGL2, that's easily done: just slap a nonperspective qualifier on the varying and . But sadly we can't use WebGL2 everywhere yet (thanks Apple), so we're stuck with plan B. If we multiply the varying value by gl_Position.w in the vertex shader and then divide by it in the fragment shader (actually we can multiply by gl_FragCoord.w, which is equivalent), that effectively cancels out the math the fixed function does to account for perspective.

Well that all sounds great, except that "cancels out" is pretty suspect in the wild world of 32-bit floating point arithmetic. And indeed that causes some new artifacts when zoomed in close to a long line:

That's a pretty cool visualization of floating point noise, but not a great visualization of a polyline.

At this point I thought I was sunk. My best idea was I could try clipping the line to all 6 sides of the frustum in the vertex shader, helping to keep the magnitude of that w in check in order to minimize the error introduced by multiplying and then dividing by it. But aside from the hassle and performance cost, I wasn't sure it would really work, considering that with log depth the frustum can still fully contain a very long line.

But then I realized that the original artifacts with the perspective-correct interpolation almost entirely occurred over the width of the line, not over its length. And line widths are usually pretty small and mapped to a small number of pixels, so we can get away with more floating point imprecision.

So in this PR:

• The texture t coordinate is interpolated in a non-perspective fashion by multiplying/dividing by w.
• All other varyings - including the s texture coordinate - are interpolated perspective-correct.

And I think it looks pretty good! Only a couple of small artifacts I know about (out of of scope of this PR IMO):

• Textures mapped to lines will look different (but maybe not wrong?) at an angle, as in @emackey 's demo from Polyline texture coordinate chunkiness #7689.
• When looking at the tail of a line with an arrow material up close at a steep angle, as in the CZML Polylines example, the arrow head can appear. I spent awhile trying to figure this out and seriously the arrow rendering is black magic, I have no idea.
• Texture coordinates are not adjusted when polylines are clipped to the near plane, which means they're totally wrong in some cases. This is not a new problem in this PR, but it's really hard to fix. To fix this, vertices would need to know the next and previous texture coordinates (e.g. they'd have to be passed in as a vertex attribute). Here's one manifestation of this (mulitple arrow heads), and an explanation of why it happens: Two arrow heads for polyline arrow material #628 (comment)
• Maybe other things I haven't found? Definitely try some things please!

[cesium-concierge]

Thanks for the pull request @kring!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[kring]

I seem to have broken a bunch of polyline tests, but I don't know how. For example, this test fails:

Scene/PolylineCollection renders polylines. one polyline with no positions
Expected not to render [0,0,0,255], but actually rendered [0,0,0,255].
Error: Expected not to render [0,0,0,255], but actually rendered [0,0,0,255].
    at stack (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1475:17)
    at buildExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1445:14)
    at Spec.expectationResultFactory (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:584:18)
    at Spec.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:332:34)
    at Expectation.addExpectationResult (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:528:21)
    at Expectation.notToRender (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1399:12)
    at http://localhost:8080/Specs/Scene/PolylineCollectionSpec.js:404:23
    at Object.<anonymous> (http://localhost:8080/Specs/spec-main.js:89:30)
    at attemptAsync (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1793:24)
    at QueueRunner.run (http://localhost:8080/ThirdParty/jasmine-2.2.0/jasmine.js:1746:26)

But if I turn that test into a Sandcastle example, it renders identically in master and in this branch.

• Master
• This branch

I can only assume the tests are rendering the polylines subtly differently and that's triggered a bug, but I'm having a hard time spotting it. I'll keep looking in the morning, but if anything jumps out at anyone please let me know. :)

[mramato]

Awesome work here @kring! Thanks!

Any concerns from you or @lilleyse to getting this into the release tomorrow? I don't think we have much of a choice, given that it fixes a regression, but also want to make sure we don't make things worse obviously.

We could probably ask some of the team to hammer on this branch a little today if you think that would help at all.

[kring]

This doesn't fix a regression - my other PR (already merged) did that - but it fixes some major longstanding bugs, so I do think it's important and would love to see it go into the release today. But considering how late it is I totally understand if that's not reasonable.

[OmarShehata]

I don't think we have much of a choice, given that it fixes a regression

I marked this as priority next release believing it also fixes regressions, but looking at all the referenced issues at the top, these are all polyline issues users have brought up and disabled log depth in their apps as a workaround.

I can help with testing and will leave it up to Sean to decide on whether this can go into the release.

[mramato]

EDIT: Nevermind, sorry for the confusion. The regression I thought we had is already fixed in master.

@kring @lilleyse completely up to you if this should go into tomorrow' release or not. If not, please remove the priority next release label.

[lilleyse]

Personally I'd feel more comfortable merging it after the release. (Also I'm pretty booked today, so someone else would need to do the review)

[kring]

The test failures are fixed, this is ready now.

[lilleyse]

I also added #7852 to the fixed list. Which means everything in #8600 (comment) has been fixed! Well, except one that we were waiting on confirmation.

[lilleyse]

@kring your writeup was extremely helpful. I also appreciated all the code comments. I've tried all the polyline sandcastle examples I could find and barely noticed any issues.

The only major artifact I noticed was in @emackey's demo, you already mentioned that thicker polylines distort the texture differently, but I see worse artifacts when zooming out. Admittedly master has its own set of artifacts too. Don't want to hold this PR up but do you have any thoughts?

log-depth-polyline

master

[kring]

I see worse artifacts when zooming out

Whoops I broke everything with a typo last night. Ed's example looks good now and all the tests still pass.

[lilleyse]

Cool, I checked out that example and others again and everything seems to be working as expected. Thanks @kring!