Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a pass for 3D Tiles #5168

Merged
merged 4 commits into from
Apr 11, 2017
Merged

Add a pass for 3D Tiles #5168

merged 4 commits into from
Apr 11, 2017

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Mar 29, 2017

Adds a CESIUM_3D_TILE pass for rendering 3D Tiles (name suggestions are welcome). I occurs after the ENVIRONMENT and GLOBE passes but before the GROUND pass. Because it happens before the GROUND pass, I had to disable the depth clear for terrain. If we aren't concerned about GroundPrimitives on 3D Tiles yet, I can move the pass after the depth clear.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 29, 2017

There's a JSHint warning: https://travis-ci.org/AnalyticalGraphicsInc/cesium/builds/216500183

Not sure if the test failure is a CI issue.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 29, 2017

What all requires clearing the terrain's depth buffer? Just polylines on the ellipsoid, right? If we went with a corridor polyline-on-terrain solution for the near/mid-term, could we remove the need to clear the depth buffer completely?

To facilitate this, perhaps labels/billboards/points need a good default for for disable depth distance.

@pjcozzi pjcozzi mentioned this pull request Mar 29, 2017
53 tasks
@bagnell
Copy link
Contributor Author

bagnell commented Mar 29, 2017

What all requires clearing the terrain's depth buffer? Just polylines on the ellipsoid, right? If we went with a corridor polyline-on-terrain solution for the near/mid-term, could we remove the need to clear the depth buffer completely?

If we used corridors for polylines on terrain and have a good default for disable depth distance on labels/billboards/points, then, yes, we could remove clearing the terrains depth.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 29, 2017

What all requires clearing the terrain's depth buffer? Just polylines on the ellipsoid, right? If we went with a corridor polyline-on-terrain solution for the near/mid-term, could we remove the need to clear the depth buffer completely?

If we used corridors for polylines on terrain and have a good default for disable depth distance on labels/billboards/points, then, yes, we could remove clearing the terrains depth.

What do you think? It will require changing a bunch of Sandcastle examples and some of the entity/datasource layer. Could you scope it out? If we think we could do it in a few days, I say we go for it. Maybe also add the auto corridor wide resize to help the workaround.

@bagnell
Copy link
Contributor Author

bagnell commented Apr 7, 2017

@pjcozzi This is ready. I'm going to push always depth testing against terrain for now like we discussed offline.

@lilleyse
Copy link
Contributor

lilleyse commented Apr 7, 2017

Update the pass in PointCloud3DTileContent as well.

@lilleyse
Copy link
Contributor

lilleyse commented Apr 7, 2017

var pass = command instanceof ClearCommand ? Pass.OPAQUE : command.pass;

Does this line need to be modified? To handle stencil clear commands in 3d tiles soon.

@pjcozzi
Copy link
Contributor

pjcozzi commented Apr 9, 2017

LGTM.

@lilleyse please merge when ready.

@bagnell
Copy link
Contributor Author

bagnell commented Apr 11, 2017

Update the pass in PointCloud3DTileContent as well.

Are we sure we want to do this? Will we ever want to have a disable depth test distance like for billboards and labels? If so, then we should still render this after b3dms and i3dms.

@bagnell
Copy link
Contributor Author

bagnell commented Apr 11, 2017

var pass = command instanceof ClearCommand ? Pass.OPAQUE : command.pass;

Does this line need to be modified? To handle stencil clear commands in 3d tiles soon.

I just searched through the code for any clear commands that are added to the current frame's draw commands and there aren't any. Each clear command that we create is explicitly executed at some point. I propose we remove this line. If the new skipping LODs method needs explicit control over when to clear the stencil, we can add a pass property to ClearCommand.

@lilleyse
Copy link
Contributor

lilleyse commented Apr 11, 2017

Are we sure we want to do this? Will we ever want to have a disable depth test distance like for billboards and labels? If so, then we should still render this after b3dms and i3dms.

I don't see the same abilities being added to point clouds. And I think it would good for point clouds to trigger the z-fail material for polylines.

If the new skipping LODs method needs explicit control over when to clear the stencil, we can add a pass property to ClearCommand.

Yeah that's what it needs. Handle there instead?

@bagnell
Copy link
Contributor Author

bagnell commented Apr 11, 2017

I don't see the same abilities being added to point clouds. And I think it would good for point clouds to trigger the z-fail material for polylines.

OK. Done.

Yeah that's what it needs. Handle there instead?

I removed the conditional since it would never evaluate to true. Yeah, I would handle it there. You still shouldn't have to modify any of the renderer code. Give ClearCommands a pass property and it will execute in the right order. ClearCommands don't have a bounding volume so it will execute in every frustum. It may be an optimization to give it a bounding volume of the whole tileset as well.

@lilleyse
Copy link
Contributor

Ok that will be a separate PR. Looks good here.

@lilleyse lilleyse merged commit 77d5f14 into 3d-tiles Apr 11, 2017
@lilleyse lilleyse deleted the 3d-tiles-pass branch April 11, 2017 21:41
@lilleyse lilleyse mentioned this pull request Apr 11, 2017
7 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants