-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate Appearance on z-fail #5160
Conversation
I could see this being useful for points/labels/billboards (think waypoints), but can you submit a new issue to consider these later? |
At quick glance, the code looks good to me. @lilleyse can you please test and review this? Once ready, we'll also merge this into the |
Source/Scene/Primitive.js
Outdated
modifiedFS += 'varying float v_WindowZ;\n' + | ||
'void main() {\n' + | ||
' czm_non_depth_clamp_main();\n' + | ||
' gl_FragDepthEXT = min(v_WindowZ * gl_FragCoord.w, 1.0);\n' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is depth clamping used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is depth clamping used?
To prevent multi-frustum artifacts. If you turn it off and zoom in close, the line will be clipped.
Still some overlap, but it seems to work ok without clamping.
Maybe I didn't subsample the line enough or the lower res terrain LOD is rendered when zooming out. This isn't the best example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you get a screenshot of that happening?
It may be worth supporting a fallback shader when the extension isn't supported, since it still mostly works without.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this._closedColorBatches = new Array(numberOfShadowModes * 3); | ||
this._closedMaterialBatches = new Array(numberOfShadowModes * 3); | ||
this._openColorBatches = new Array(numberOfShadowModes * 3); | ||
this._openMaterialBatches = new Array(numberOfShadowModes * 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this batching is starting to get out of hand (even when it was just shadows), but probably worth addressing later.
Are there situations where the depth-fail won't work due the ordering of entities? Like I could imagine a large sphere occluding a polyline but the depth material not showing because of the order in which things are rendered. Is this generally ok because the depth-fail batch is last? |
Agreed, later.
Yes. The main use case is depth fail vs. terrain or 3D Tiles. We are also adding an environment pass so that 3D Tiles can be rendered before these. |
In overlapping situations, could something like polygon offset help? Or stenciling? |
On my integrated Intel MacBook Pro, the z-fail looks right, but z-pass does not: Note the frustum boundary between the cyan and blue on the left.
|
+1 for billboards/points. Sometimes I've got cases where I'll need to render a line's vertices along with the line itself. |
Interpolating depth across a triangle that long was causing the issue. If you add more samples to the line, it's fine. Here is the updated code: var viewer = new Cesium.Viewer('cesiumContainer');
viewer.scene.globe.depthTestAgainstTerrain = true;
var cartesianSamples = [];
var step = 1000.0;
var end = (Cesium.Ellipsoid.WGS84.radii.x + 400000.0) / step;
for (var i = 0; i < end; ++i) {
cartesianSamples.push(new Cesium.Cartesian3(i * step, 0.0, 0.0));
}
viewer.entities.add({
polyline : {
positions : cartesianSamples,
followSurface : false,
width : 5,
material : new Cesium.PolylineOutlineMaterialProperty({
color : Cesium.Color.ORANGE,
outlineWidth : 2,
outlineColor : Cesium.Color.BLACK
}),
depthFailMaterial : new Cesium.PolylineOutlineMaterialProperty({
color : Cesium.Color.RED,
outlineWidth : 2,
outlineColor : Cesium.Color.BLACK
})
}
}); |
@denverpierce Is that first screen shot when the example first loads? In the code example we set, You can ensure it is always viewer.scene.terrainProviderChanged.addEventListener(function() {
viewer.scene.globe.depthTestAgainstTerrain = true;
}); |
@lilleyse I can't reproduce that. Maybe it's the same issue here #5160 (comment) Can you try it with EXT_frag_depth disabled? Maybe that's the issue. |
Yeah it works without - this screenshot from before disabled the extension: #5160 (comment) |
The stock Sandcastle example on my MacBook Pro is the same as #5160 (comment). It's not clear to me what the solution is based on the discussion above: does the example need to be updated or there is a bug when |
@bagnell can you also update the |
That isn't the best example. I subsampled the line at the highest terrain resolution. It'll fail depth for lower resolution LODs. Also, I didn't subsample at too fine a granularity. @pjcozzi I cant reproduce #5160 (comment) or #5160 (comment) . Perhaps its a problem with EXT_frag_depth on Macs? Do you also have issues when the camera enters the shadow volume of |
@lilleyse I swapped the draw order like we discussed offline. Can you see if that fixed the issue? |
Awesome that fixed it for me. @pjcozzi can you also confirm? |
Works for me now! Please merge to |
Unfortunately I noticed this right after merging after testing with 3d-tiles. If either
|
@bagnell can you please look at this one soon? |
@lilleyse Can you try the |
What is the plan here? To roll back to a previous version that @bagnell mentioned offline? |
Bump @bagnell - #5160 (comment) |
Adds support for a separate appearance when the primitive fails the depth test. Only polylines are supported at the moment. Example: