Support for "dashed" polylines

[jasonbeverage]

This PR adds support for screen space "dashed" polylines. This attempts to get something similar to OpenGL line stippling to work with Cesium's neat wide polyline algorithm. I tried various techniques using the texture coordinates of the line but none ended up looking as nice as this one.

There is a new PolylineDashMaterialProperty that controls the PolylineDash material.

I've modified the PolylineMaterialAppearanceVS.glsl shader to compute a varying, v_angle, that is fed to the fragment shader. This angle is the screen space angle between the line and the x axis quantized to 45 degree increments. I've quantized rather than using the actual value to try to avoid artifacts as the polyline expands.

The fragment shader takes the incoming pixel coordinate, rotates it by the incoming angle and then does a simple stippling along the x-axis on the rotated pixel coordinate. The fragment location is rotated to create a striped pattern in the direction of the line so it actually appears like it's being stippled along the line.

I've updated the Polyline and Polyline CZML sandcastle examples to have a dashed line so you can see it in action.

[hpinkos]

This is awesome @jasonbeverage, thanks! This has been a frequently requested feature and will make the Cesium community very happy 😄

[pjcozzi]

Fixes #2584

[pjcozzi]

Thanks @jasonbeverage. This is a pleasant surprise and will be a very popular feature!

I would like to review this before the final merge. @bagnell should also take a look at this first since he wrote the original polyline shader, but he's busy for probably all of this week. In the meantime, if anyone has cycles to review or even just test this, please jump in!

@jasonbeverage I'm not sure if you've already read it, but you might like this paper: Shader-Based Antialiased, Dashed, Stroked Polylines

[mramato]

Awesome work @jasonbeverage. You keep cranking out some great stuff. I'm not qualified to review the shaders here, but the Entity and CZML pieces look spot on (will look closer as soon as I have time if no one else beats me to it).

One of the things me and @bagnell have talked about for stippled lines is the ability to specify a custom pattern (perhaps through a string like -- - ---). Would that be an easy extension to this implementation? (I'm just asking out of curiosity). I know @bagnell also had some ideas (such as using images as part of line styles but that's also outside the scope of this PR).

[emackey]

This is crazy awesome.

@mramato You probably know shaders can't directly take a string, but we could pass a float parameter to indicate a percentage of dash vs. gap, or I could even imagine passing a one-dimensional alpha map that supplies a dashing pattern to use. That would be for a future version of course, this seems cool enough as-is that it should probably go in without a lot of feature creep. Also I was thinking it could be useful to supply a second color for use in the gaps, instead of just discarding them (for example, use the same color with less alpha, or alternate the brightness, etc). But again, that's feature creep, and this is plenty good enough already.

Dashed lines (as they are now) will be very useful in combination with the depthFail feature going on in #5160. Underground lines are crying out for dashes.

[jasonbeverage]

Don't merge this just yet, I'm going to try to push a couple more things and see what you think.

[emackey]

@jasonbeverage great. If you're making changes can you switch Color.BLUE to Color.CYAN to make the line more visible? Likewise in the CZML crank the green channel to 255. Thanks!

[jasonbeverage]

Sure thing, just pushed that change.

[emackey]

I got asked by @GatorScott to test with a more curved path, and I had an inclined GEO handy. Here's a gist. It works pretty well, even for this tough case.

[jasonbeverage]

I just pushed a few changes.

There is a new dashPattern property for the PolylineDashMaterial that defines a 16 bit stipple pattern.

I changed some of the defaults so that dashLength is 16 pixels so it lines up per pixel (approximately) with the dash pattern. The default dash pattern is 255 (0000 0000 1111 1111).

I've also added a new Polyline Dash sandcastle example.

Also, just wanted to say thanks to @timoore for doing the initial work on this effort, including the dashPattern logic. Our original approach just used the distance along the line to drop pixels, but that caused too many artifacts with your wide polyine algorithm (similar to what you'd see with the StipeMaterial with the direction set to vertical). I came up with the screen space fragment shader based stippling just recently and the artifacts aren't nearly as bad.

[emackey]

I don't think result._dashPattern gets an underscore here.

[emackey]

Typo

[emackey]

I wonder if CZML (and maybe Entity) can take a 16-character string instead of numeric dashPattern. For example:

var dashPattern = '        --------';

Could this be converted to 255 by testing each char position and setting a bit? This would make the pattern more user-friendly.

[emackey]

Here's some code to turn an arbitrary-length string of spaces and dashes into a closest-approximation 16-bit field. This just iterates the 16 bits, picks the closest char based on length of input string, and figures out if it will set that bit or not.

    function dashStringToBits(dashPatternString) {
        var dashPatternLength = dashPatternString.length;
        var dashPatternNumber = 0;
        var currentBitValue = 65536;
        for (var i = 0; i < 16; ++i) {
            currentBitValue = currentBitValue >> 1;
            var charPos = Math.floor((i / 16) * dashPatternLength);
            var char = dashPatternString.charAt(charPos);
            if (char !== ' ') {
                dashPatternNumber |= currentBitValue;
            }
        }
        return dashPatternNumber;
    }

dashStringToBits(' -')
255
dashStringToBits('- ')
65280
dashStringToBits('    ----')
255
dashStringToBits('        --------')
255
dashStringToBits('  -  ---')
3135
dashStringToBits('               -')
1
dashStringToBits('              --')
3
dashStringToBits('             ---')
7

[jasonbeverage]

Just pushed a fix for my fat finger typos :)

[jasonbeverage]

I like the dash string idea. Would PolylineDashMaterialProperty.getValue just need to call that to get the appropriate float value to feed the uniform?

[emackey]

Would PolylineDashMaterialProperty.getValue just need to call that

Off the top of my head I'm not sure where it goes. I'd have to research a bit. Curious to hear @mramato's thoughts before we go too far on it.

[jasonbeverage]

I've just been mashing 0 and 1 into the Windows programmer calculator and copying the resulting decimal value :)

I also just tried your gist and it looks awesome!

[pjcozzi]

Also, just wanted to say thanks to @timoore for doing the initial work on this effort, including the dashPattern logic...

Thanks again to both of you!

[bagnell]

I only have that one comment. Looks good! Thanks @jasonbeverage!

I believe @lilleyse also wants to review this.

[bagnell]

@jasonbeverage I spent some time playing with your branch and the PolylineDashMaterial shader today. I created a branch (dashedlines-2) that has your changes plus some of my own. It adds a length in meters from the first point on the line as a new vertex attribute and I tried to compute dashPosition from that. This way you won't see the abrupt changes from v_angle or much of a change in the dash pattern when panning from using gl_FragCoord. It looks pretty good when panning or rotating as long as the camera is mostly looking top-down, but as soon as you start rotating, certain camera views will cause the dashes to shrink or stretch. Any ideas you have about this would be greatly appreciated!

I still want to see this PR merged into master. Computing the dashes based on line length can sit in a branch until someone has time to work on it.

[pjcozzi]

Is the trailing // here and below required?

[jasonbeverage]

Hey @pjcozzi it's not necessary and I've just pushed a change removing it.

[pjcozzi]

atan can be a performance killer.

• Can you precompute atan(1.0, 0.0)? I would not trust all GLSL compilers to optimize that.
• What level of precision is required for this? Would it be possible to replace the other atan call with a taylor series expansion of a few terms? Seems like yes since it is being quantized.

[jasonbeverage]

I pushed a change precomputing the atan(1.0,0.0). We could use a taylor series or a lookup table for the other atan, but I'd save that for later unless performance is a killer.

[pjcozzi]

Last time I tested - which was awhile ago - atan is pretty much the worse thing we can do in a shader. @bagnell said there was already inverse trig in the polyline shader, but I don't see it at quick glance.

If you're not up for it now, we'll try to make this change soon.

[jasonbeverage]

Hey @bagnell , we tried almost the exact same thing initially, and I think it would work fine if you were just drawing regular old lines. But I couldn't figure out a way to get it to play nice with the wide polyline aglo. I think the problem is that the length in meters actually changes along the line when you move the verts around, so you end up with funky artifacts depending on how the line rotates. Your algo also does something along the near plane where it looks like vert is pushed really far towards the near plane so you end up stretching the distance in meters artificially resulting in really wide dashes.

I tried and tried but couldn't come up with something that worked so I ended up scrapping it in favor of this purely fragment based stippling.

[jasonbeverage]

@bagnell I tried wrapping the block in polylinecommon with the ifdef, but it looks like just adding #define POLYLINE_DASH to the material didn't work. It behaves like it's not actually set. Is there somewhere else the define would need to go?

[denverpierce]

@mramato for CZML, I'd think it'd be easier and more intuitive to write a bit mask instead of a 16bit number, especially for users more familiar with the data side of things.

IE:

{
dashLength:16,
dashPatthern: '0000000011111111'
}

is more human read/writeable than:

dashPattern: 255

[mramato]

@bagnell and @pjcozzi do you have any additional comments here? I want to take another pass on the Entity updates (just because I haven't looked at this in a few weeks) but I don't expect any surprises.

Barring any issues, I would like to get this merged ASAP so that it has time to stew before 1.32 on May 1st.

Thanks.

[pjcozzi]

@mramato review and merge when ready.

[bagnell]

@bagnell I tried wrapping the block in polylinecommon with the ifdef, but it looks like just adding #define POLYLINE_DASH to the material didn't work. It behaves like it's not actually set. Is there somewhere else the define would need to go?

Sorry, it needs to be in the vertex shader. Can you modify the PolylineMaterialAppearance.vertexShaderSource getter to search the material shaderSource for varying float v_angle and then append the #define?

[mramato]

@mramato for CZML, I'd think it'd be easier and more intuitive to write a bit mask instead of a 16bit number, especially for users more familiar with the data side of things.

We can probably add this in as a CZML-only feature in a small PR after this gets merged. We'll also need to update the spec and writer too. (Of course we need that for the new dash stuff as well). CC @shunter

[pjcozzi]

@jasonbeverage we'd like to merge this in the next few days so it has some time in master before it ships in Cesium 1.33 on May 1. Can you make the change @bagnell suggested, #5159 (comment), or do you want us to?

Does anyone else want to test or review this?

[jasonbeverage]

If you can do it that would help me out, I'm knee deep in other stuff right now and you'll be faster at it anyway :)

…

On Thu, Apr 20, 2017 at 10:55 AM Patrick Cozzi ***@***.***> wrote: @jasonbeverage <https://github.com/jasonbeverage> we'd like to merge this in the next few days so it has some time in master before it ships in Cesium 1.33 on May 1. Can you make the change @bagnell <https://github.com/bagnell> suggested, #5159 (comment) <#5159 (comment)>, or do you want us to? Does anyone else want to test or review this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5159 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAT74gT0RL6pFeyMJwLjfO6K7fdZy8K0ks5rx3HSgaJpZM4MqyVr> .

[bagnell]

@mramato Can you merge when ready?

[denverpierce]

I threw some fairly complex linework at it and it performed admirably.

[mramato]

Thanks again @jasonbeverage!

[jasonbeverage]

Thanks for merging this! Glad to be able to contribute.

[pjcozzi]

Fantastic! This will quickly become a popular feature. Thanks again @jasonbeverage!

[initialed85]

@jasonbeverage I had a thirst for dashed lines that you have now quenched, thankyou.

[Tezraine]

I know I'm a bit late to the party, but I just wanted to mention that with ECMA6, the 0b format for numbers is supported. Also 0b1111111100000000 (255) if reversed and overlapped is effectively a bitwise OR that leaves you with all 1s. 0b1111000000001111 (61455) or 0b0000111111110000 (4080) is the same pattern, but is the same forwards and backwards.