CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.98k stars 3.5k forks source link

Support for polygon outlines on terrain #6694

Open ediebold opened 6 years ago

ediebold commented 6 years ago

I saw the exciting development in https://github.com/AnalyticalGraphicsInc/cesium/pull/6615 that will allow polylines on terrain!

I was wondering if this new method could also be used for polygon outlines? I assume those are drawn in a very similar way. If we can get polygon outlines on terrain, I believe Cesium will fully support clamping all the classic vector data types to terrain!

hpinkos commented 6 years ago

Hi @ediebold! While that polylines on terrain PR gets us closer to supporting this, there's a little bit more work we'd have to do to make sure the outlines match up with the fill for all of the ground geometry types (corridor, ellipse, polygon and rectangle). I'm just not sure how soon we'll have a chance to look into it, but I don't think it will take very much time to do. Thanks for opening this issue! We'll give you an update when we have one =)

mramato commented 6 years ago

We've talked in the past about replacing the standard WebGL outlines we currently use with polyline primitive lines. The benefit to this is that they would be stylable and as of this month, would support clamping on terrain. @bagnell said the change itself is not very hard but there was concern about outline quality (awkward corners of a box for example).

Of course we haven't had this conversation in years and I definitely think it's time to revisit it. I don't know how we would get to "Terrain on by default" without it.

bagnell commented 6 years ago

Polylines on terrain would have an issue where the lines would overlap the polygon because of the constant pixel with. This might only be a problem for translucent polylines though. The other issue is with extruded geometry where the lines from the top surface to the extruded bottom wouldn't connect. Here is an example: image

mramato commented 6 years ago

I'm curious as to what that looks like with a much narrower outlines, (1-2 pixels).

With the number of posts we get about "outline width not working" and the necessity of outlines on terrain in order to support terrain on by default, my vote would be to make this change. I think it's a net positive win and people would be more than willing to accept the artifacts if they can have terrain and line width. We can always look at improving these lines down the line when we have more time.

Anyone else want to provide any thoughts here?

bagnell commented 6 years ago

Same issues but not as noticeable at 2 pixels. image

ediebold commented 6 years ago

Speaking as a random user, I think the number of use cases this would add support for will outweigh the ones it would won't. I think most people would still be happy with this update even if it came with some caveats such as transparent or thick lines looking weird. (I know I would!)

mramato commented 6 years ago

@bagnell I think it's important to note that there are already existing artifacts with outlines on polygons, so this might actually improve things in that department. Here's the GeoJSON example with outlines turned on, notice how the lines are broken up even though the extruded polygon is translucent:

image

Modified Sandcastle demo

mramato commented 6 years ago

@lilleyse @likangning93 @ggetz anyone have any thoughts here, either technical or on the desires of the community?

likangning93 commented 6 years ago

Polylines on terrain would have an issue where the lines would overlap the polygon because of the constant pixel with

Should be pretty easy to add a flag that lets polylines on terrain only "extend" in one direction for the simulated screen space width thing. Might not even have to be a batch breaker.

For funny corners with regular polylines, I wonder how much more acceptable it'll look if we create polyline loops for the extrusion caps. Maybe want to construct the loops such that the start/end aren't at a corner.

mramato commented 6 years ago

@bagnell @likangning93 I think we'll need to think about making this switch for our Oct 1 release as part of our efforts to have terrain on by default. Since you both seem to have ideas for making this switch, maybe an early PR just to get the discussion going for real?

hpinkos commented 4 years ago

Also reported in https://github.com/CesiumGS/cesium/issues/8364

pyshx commented 1 year ago

Any changes on this?

ethanchristensen01 commented 1 year ago

I'd really like to see this get done. Is there anything specific I can do to contribute?

ggetz commented 1 year ago

Also requested in https://github.com/CesiumGS/cesium/issues/11438

ggetz commented 1 year ago

@ethanchristensen01 If you have the bandwidth to contribution a potential fix, we would be happy to discuss the approach.

Otherwise, use case descriptions and test examples or test data also help as well!

Thanks!

ethanchristensen01 commented 1 year ago

@ggetz Yes, I can work on this, and I would like to hear the approach you recommend. For now, I'll get started with building Cesium on my machine.

ethanchristensen01 commented 1 year ago

I found a way to work around this issue temporarily in my own project, but it's not ideal. I would still prefer to contribute a fix here.

Let me know, when you have time, what approach I can start with!

Specifically, it would be good to know: