Extend GroundPrimitive shadow volume based on screen space error #4751

hpinkos · 2016-12-13T22:26:28Z

This extends the bottom of the shadow volume below the terrain based on the screen space error. The shadow volume is updated in the ShadowVolume vertex shader.

Ground primitives geometry types compute a extrudeDirection attribute. The extudeDirection is a normalized vector pointing in the direction in which the position should be moved in the vertex shader.
- Positions at the top of the shadow volume will have extrudeDirection === Cartesian3.ZERO because only the bottom positions need to be moved
Added czm_geometricToleranceOverDistance to AutomaticUniforms
Added pixelSizePerMeter and maximumScreenSpaceError to UniformState
- scene.globe.maximumScreenSpaceError get piped through to UniformState via the frameState. Not sure if that's the best way to do this.
Added u_globeMinimumAltitude to GroundPrimitive.uniforms.
- I set this to 33000.0, which is a magic number I found with testing. I'm not sure what this value should actually be.
- I made this a positive number so I don't have to abs it later when i find the min between this value and the value computed using the screen space error.

TODO

Fix debugShowShadowVolume so it displays the shadow volume after the positions have been updated in the shader
octencode extrudeDirection attribute
Update CHANGES.md

pjcozzi · 2016-12-14T18:44:50Z

Ground primitives geometry types compute a extrudeDirection attribute.

Let's octencode this. See AttributeCompression.js.

pjcozzi · 2016-12-14T18:47:13Z

I set this to 33000.0, which is a magic number I found with testing. I'm not sure what this value should actually be.

This should come from the terrain provider, but we can hard code it to the right min for Earth for now. @bagnell do you have it handy from STK?

pjcozzi · 2016-12-14T18:48:25Z

Positions at the top of the shadow volume will have extrudeDirection === Cartesian3.ZERO because only the bottom positions need to be moved

@lilleyse @bagnell do you guys have any ideas on how to avoid this wasted memory without introducing another draw call per volume?

pjcozzi · 2016-12-14T18:48:54Z

Update CHANGES.md.

pjcozzi · 2016-12-14T18:57:05Z

Perhaps there is a winding order issue:

Click "Create large polygons"

http://cesium-dev.s3-website-us-east-1.amazonaws.com/cesium/ground-primitive-sse/Apps/Sandcastle/index.html?src=development/Ground%20Primitive.html&label=Showcases

pjcozzi · 2016-12-14T18:59:20Z

Positions at the top of the shadow volume will have extrudeDirection === Cartesian3.ZERO because only the bottom positions need to be moved

@lilleyse @bagnell do you guys have any ideas on how to avoid this wasted memory without introducing another draw call per volume?

Related idea: at one point, I just tried to normalize the WGS84 position, to avoid storing an extra attribute, and it was very noticeably wrong at places like Mount Saint Helens. However, perhaps there is another way to compute a better approximation, but still not perfect.

pjcozzi · 2016-12-14T19:02:21Z

Also with that same example, the corridor and circle disappear when zoomed out.

pjcozzi · 2016-12-14T19:03:46Z

@hpinkos did you try using GroundPrimitive.debugShowShadowVolume to verify that the volumes are as low as they need to be (so it renders correctly), but not much lower (to avoid fillrate)?

pjcozzi · 2016-12-14T19:06:16Z

Source/Renderer/AutomaticUniforms.js

+         * @alias czm_geometricToleranceOverDistance
+         * @glslUniform
+         */
+        czm_geometricToleranceOverDistance : new AutomaticUniform({


Distance -> Meter

pjcozzi · 2016-12-14T19:09:28Z

Source/Renderer/UniformState.js

+
+        var fov = camera.frustum.fov;
+        var viewport = this._viewport;
+        if (viewport.width > viewport.height * camera.frustum.aspectRatio) {


How does this check work?

This was just a straight port. Did I misinterpreted something? Here's the original source:

if ((double)mainViewport[3] > ((double)mainViewport[2] * p3dScene->pixelAspect)) { pixelSizePerDistance = tan(0.5 * p3dScene->view.m_pCamera->FieldOfView()) * 2.0 / (double)mainViewport[3]; } else { pixelSizePerDistance = tan(0.5 * p3dScene->view.m_pCamera->FieldOfView()) * 2.0 / (double)mainViewport[2]; }

Are you sure mainViewport[3] is width? In Cesium, the array would be [x, y, width, height].

Are you sure pixelAspect ratio is width / height like aspectRatio?

Perhaps ask Greg to double check these.

Right now, I think the if statements reads like "if (width > (height * (width / height))", which is "if (width > width)"

pjcozzi · 2016-12-14T19:11:04Z

Source/Shaders/ShadowVolumeVS.glsl

+    float delta = min(u_globeMinimumAltitude, czm_geometricToleranceOverDistance * length(position.xyz));
+
+    //extrudeDirection is zero for the top layer
+    position = position + vec4(extrudeDirection.xyz * delta, 0);


Use 0.0, mixing integers and floats didn't use to compile on some WebGL implementations.

extrudeDirection is a vec3 so we can remove the .xyz .

pjcozzi · 2016-12-14T19:17:36Z

What do you think about cleaning up the geometry types by adding adding a temporary BOTTOM (or whatever) attribute then running the Geometry through a generic function that removes that attribute and adds the extruded normal? It would come at a potential small performance and GC cost but could be worth it.

@bagnell what do you think about this for vector tiles? Seems like we don't need to store the extruded direction in the payload. We could even separate out the bottom cap vertices so the BOTTOM attribute is implicit over the wire.

I feel like we should do some fill rate tests before getting into all this. This should make a huge difference for horizon and 45 degree views depending on how well early-z does (even with early-z the rasterization load is very high).

pjcozzi · 2016-12-14T19:18:05Z

Just those comments for now.

@hpinkos this will be a much appreciated fix!

updates from review

hpinkos · 2016-12-19T20:46:43Z

Here is a visualization of the shadow volume. The blue is the old volume and the red is the new one. It just extends a little bit below what we were doing previously

large polygon:

small rectangle:

hpinkos · 2016-12-19T21:27:36Z

@pjcozzi I believe I addressed your comments and fixed the problems you found with some of the geometries.

I haven't done the octencodeing yet because I'm waiting to see if we want to change the extrudeDirection attribute

pjcozzi · 2017-01-04T13:27:56Z

Fix debugShowShadowVolume so it displays the shadow volume after the positions have been updated in the shader

Added in 2d50df1.

This restriction was removed:

Must be true on creation for the volumes to be created before the geometry is released or releaseGeometryInstances must be false

But the polygon is no longer drawn when the volume is drawn.

pjcozzi · 2017-01-04T13:28:27Z

I haven't done the octencodeing yet because I'm waiting to see if we want to change the extrudeDirection attribute

Let's just move forward so we don't stall this. We can optimize later if we come up with a good idea.

pjcozzi · 2017-01-04T13:54:58Z

Source/Renderer/AutomaticUniforms.js

+            size: 1,
+            datatype: WebGLConstants.FLOAT,
+            getValue: function(uniformState) {
+                return uniformState.pixelSizePerMeter * uniformState.maximumScreenSpaceError;


This is constant per frame, right? Precompute and store in UniformState.

pjcozzi · 2017-01-04T13:55:30Z

Source/Renderer/UniformState.js

+         * @type {Number}
+         * @default 2
+         */
+        maximumScreenSpaceError: {


These two getters may not be needed after the about change.

pjcozzi · 2017-01-04T13:57:58Z

Just those comments.

hpinkos · 2017-01-04T21:16:33Z

@pjcozzi this is ready for another look. I fix the issues you pointed out and I added octencode for the new attribute. And I updated CHANGES.md

bagnell · 2017-01-04T21:49:33Z

Oct-encoding is fine for lighting because the error isn't noticeable but it may be noticeable for geometry extruded with these normals. Have you seen this at all?

hpinkos · 2017-01-04T21:51:42Z

I didn't see any noticeable errors, but I'll look more closely to make sure

pjcozzi · 2017-01-04T23:09:15Z

Oct-encoding is fine for lighting because the error isn't noticeable but it may be noticeable for geometry extruded with these normals

How many bits are we using? It should be OK if we use enough....more than lighting. For example, normals for rotation of instances in i3dm use 32: https://github.com/AnalyticalGraphicsInc/3d-tiles/tree/master/TileFormats/Instanced3DModel#oct-encoded-normal-vectors

The paper has exact error bounds.

pjcozzi · 2017-01-05T19:51:31Z

Bump:

Oct-encoding is fine for lighting because the error isn't noticeable but it may be noticeable for geometry extruded with these normals
How many bits are we using? It should be OK if we use enough....more than lighting. For example, normals for rotation of instances in i3dm use 32: https://github.com/AnalyticalGraphicsInc/3d-tiles/tree/master/TileFormats/Instanced3DModel#oct-encoded-normal-vectors

The paper has exact error bounds.

bagnell · 2017-01-05T20:00:56Z

It uses AttributeCompression.octEncodeFloat which is oct16P in the paper (the same we use for lighting). If you want to use higher precision, take a look at octEncodeInRange.

hpinkos · 2017-01-05T20:02:00Z

Thanks @bagnell, @pjcozzi, I'll update it to use 32

hpinkos · 2017-01-05T20:48:56Z

@pjcozzi @bagnell ready

pjcozzi · 2017-01-05T23:52:32Z

Source/Shaders/Builtin/Functions/octDecode.glsl

+  * @param {float} range The maximum value of the SNORM range. The encoded vector is stored in log2(rangeMax+1) bits.
+  * @returns {vec3} The decoded and normalized vector
+  */
+  vec3 czm_octDecode(vec2 encoded, float range)


I'll be pleasantly surprised if JSDoc handles function overloads, but it's not an issue now.

pjcozzi · 2017-01-05T23:57:31Z

Do you get these artifacts? If so, do you also get them without normal compression?

If these are due to normal compression, is there anything we can do about it? Are we sure the code is right? Is there a more precise (48 bits?) version from the paper we can try?

hpinkos · 2017-01-06T22:15:38Z

@pjcozzi no I don't see that on my machine. That's happening after the 32P update?

pjcozzi · 2017-01-06T22:29:50Z

I don't get this on Windows. Will test Mac/Intel now, hopefully the artifact is in master.

pjcozzi · 2017-01-06T22:41:26Z

Good news, it is in master. See #4820

pjcozzi · 2017-01-06T22:44:03Z

OK, this is good!

Please reply to all the forum threads. People will be thrilled!

hpinkos added 8 commits December 8, 2016 12:13

add isBottom attribute

2f6d88d

pixelSizePerMeter, fix rectangle normals

73dfdd4

update other ground geometry for extrude normals

77fd93c

Merge branch 'master' into ground-primitive-sse

19798f3

fix specs

a952cdc

Merge branch 'master' into ground-primitive-sse

11bb102

jshint

f14a19e

cleanup

db2e9ca

Merge remote-tracking branch 'origin/master' into ground-primitive-sse

f85431b

pjcozzi reviewed Dec 14, 2016

View reviewed changes

fix rectangle, circle and corridor

a9d93e5

updates from review

adjust u_globeMinimumAltitude

a83ad44

This was referenced Dec 20, 2016

GroundPrimitive bounding volume doesn't take curvature of the earth into consideration #4483

Closed

Terrain 'pokes through' large corridors #4285

Open

pjcozzi added the priority - high label Jan 2, 2017

Merge branch 'master' into ground-primitive-sse

4aeed0d

Fixed running just GroundPrimitive tests

258eb00

bagnell mentioned this pull request Jan 4, 2017

Vector Tiles #4665

Merged

2 tasks

Make debugShowShadowVolume show the actual volume rendered

2d50df1

pjcozzi reviewed Jan 4, 2017

View reviewed changes

Style

0f5b872

hpinkos added 3 commits January 4, 2017 12:46

Merge branch 'master' into ground-primitive-sse

f0dcff4

cleanup, octencode extrudeDirection

fdcb9ae

CHANGES.md

56eec83

octencode 32P

2d672d7

pjcozzi reviewed Jan 5, 2017

View reviewed changes

pjcozzi mentioned this pull request Jan 6, 2017

Ground primitive artifact on Mac/Intel #4820

Closed

pjcozzi merged commit 7253588 into master Jan 6, 2017

pjcozzi deleted the ground-primitive-sse branch January 6, 2017 22:44

Extend GroundPrimitive shadow volume based on screen space error #4751

Extend GroundPrimitive shadow volume based on screen space error #4751

Conversation

hpinkos commented Dec 13, 2016 • edited Loading

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Dec 14, 2016

pjcozzi commented Dec 14, 2016

hpinkos commented Dec 19, 2016

hpinkos commented Dec 19, 2016

pjcozzi commented Jan 4, 2017

pjcozzi commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Jan 4, 2017

hpinkos commented Jan 4, 2017

bagnell commented Jan 4, 2017

hpinkos commented Jan 4, 2017

pjcozzi commented Jan 4, 2017

pjcozzi commented Jan 5, 2017

bagnell commented Jan 5, 2017

hpinkos commented Jan 5, 2017

hpinkos commented Jan 5, 2017

Choose a reason for hiding this comment

pjcozzi commented Jan 5, 2017

hpinkos commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

pjcozzi commented Jan 6, 2017

hpinkos commented Dec 13, 2016 •

edited

Loading