WIP: Add PolygonClipping support #8915

Samulus · 2020-06-04T22:44:19Z

Still a W.I.P, added some explanations:

Overview

This PR introduces a new faster alternative to clipping planes, the clipping polygon. The clipping polygon has the same visual effect as a clipping plane (selectively disabling / enabling rendering), but without the performance overhead of checking every pixel being rendered against every clipping plane.

The performance speed up is the result of using an acceleration data structure generated against the triangulated representation of the region to clip. This data structure, called the Polygon Clipping Acceleration Grid, is a 2D array that represents the bounding box around the triangulated clipping region partitioned into N cells. Cells are partially occluded by at least one triangle, totally occluded by at least one triangle, or not occluded by any triangles at all. This data structure allows the fragment shader to be less computationally expensive as it now has to perform dramatically fewer comparisons compared to clipping planes:

If the pixel being rendered is not in the 2D bounding box we can exit early.
If the pixel being rendered is in a totally occluded or not occluded cell we can exit early.
If the pixel being rendered is partially occluded, we only need to check the triangles that are partially overlapping the cell, not the entire mesh.

The number of cells to generate is a trade off: Higher cell counts increase the chances that the fragment shader can exit early at the cost of taking longer to generate the initial acceleration grid on the CPU side. Not enough cells results has the opposite effect, where initial generation is fast but GPU performance is slow as many triangles will now have to be checked every frame. In my experience 34-40 splits (1156 - 1681) cells is
a good sweet spot between being extremely fast to generate CPU side and maintaining 60FPS GPU side.

Algorithm Pipeline

At a high level, the algorithm:

Accepts a list of PolygonHierarchy[] (∞ nested holes are supported!).
(Optionally) simplifies each inputted polygon to reduce the total number of triangles to be checked in the algorithm at the cost of accuracy.
Triangulates each PolygonHierarchy into an individual mesh using earcut.
Amalgamates all the individual meshes into a single positions and indices buffer
Generates a PolygonClippingAcceleration data structure using the amalgamated positions and indices buffers
Uploads the clippingAccelerationGrid, meshPositions, and overlappingTriangleIndices to the GPU.
Uses the aforementioned cell logic in the fragment shader to determine if it can use the fast path to test if the pixel being rendered should be clipped or if it has to start checking each triangle that overlaps.

Known Issues / Future Optimizations

Does not work in 2D view right now.
The algorithm does all comparisons in the axis-aligned ENU coordinate system. This works on a large scale (I can clip all of
North America without accuracy issues), but will fall apart for clipping regions that extend around the entire globe. An update to this algorithm could change all the calculations to work with polar coordinates to fix this.
The data textures are all floats right now, to avoid the need for bit shifting. We could store everything as a RGBA unsigned_byte texture but it would reduce the maximum size of a clipping polygon and might reduce performance if we have to emulate bitshifting heavily in the fragment shader.
The fragment shader converts between 1D and 2D coordinates heavily using mod and division. We might see a performance gain by storing 2D indices directly in the data texture instead of their 1D representation.
If the user oversimplifies their inputted mesh (i.e 3 vertices 2 meters apart from each other and they specify a simplify tolerance of 1000km) it's possible for a degenerate triangle to be generated. The ClippingPolygon class should throw an exception explaining they oversimplified if that occured, but we could do a binary search approach to find the maximum simplification that doesn't break their mesh too.

cesium-concierge · 2020-06-04T22:44:23Z

Thanks for the pull request @Samulus!

✔️ Signed CLA found.
❔ CHANGES.md was not updated.
- If this change updates the public API in any way, please add a bullet point to CHANGES.md.
❔ Changes to third party files were made.
- Looks like a file in one of our ThirdParty folders (ThirdParty/, Source/ThirdParty/) has been added or modified. Please verify that it has a section in LICENSE.md and that its license information is up to date with this new version.

Reviewers, don't forget to make sure that:

Cesium Viewer works.
Works in 2D/CV.
Works (or fails gracefully) in IE11.

mramato · 2020-06-05T19:18:19Z

Source/DataSources/ModelGraphics.js

+  /**
+   * A property specifying the {@link ClippingPolygon} used to selectively disable rendering the model.
+   * @memberof ModelGraphics.prototype
+   * @type {Property}


FYI, should be @type {Property|undefined} basically any property that can be undefined has to have the |undefined for strict mode TypeScript to work.

mramato · 2020-06-05T19:23:41Z

Specs/Scene/PolygonClippingAccelerationGridSpec.js

@@ -3,6 +3,7 @@ import PolygonClippingAccelerationGrid from "../../Source/Scene/PolygonClippingA
 describe("Scene/PolygonClippingAcceleration", function () {
  it("developer error if invalid number of positions or indicies provided", function () {
    expect(function () {
+      // eslint-disable-next-line no-unused-vars
      var empty_arrays = new PolygonClippingAccelerationGrid({


This is fine, but as a tip, you can also just do return instead of var empty_arrays = and the test will do what you want without worrying about eslint or local vars.

Also, empty_arrays should probably be emptyArrays. We use camel case for variable names.

…nerator

lilleyse · 2020-06-11T15:44:47Z

We talked about precision problems for large scale clipping polygons offline already, but to demonstrate it I modified the sandcastle to draw the polygons and set the camera to look at the coast of Hawaii. The polygon and clipped globe don't perfectly match up (ignoring the height difference which is expected) and there's some jittering on the edges.

Local sandcastle

lilleyse

Generally looks pretty good. I'm starting to lose steam but I'll review the algorithm and shader in more detail later, and the unit tests.

lilleyse · 2020-06-11T17:13:56Z

Apps/Sandcastle/gallery/Clipping Polygon.html

+        Cesium.when(entity.readyPromise).then(function () {
+          entity.model.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(
+            {
+              polygonHierarchies: [clipLeftWing, clipRightWing],
+              splits: 30,
+              union: false,
+            }
+          );
+        });


Entities don't have a ready promise. It should be fine to set the clippingPolygon and not wrap in a promise.

lilleyse · 2020-06-11T17:15:39Z

Apps/Sandcastle/gallery/Clipping Polygon.html

+        agiHQ.readyPromise.then(function () {
+          agiHQ.clippingPolygon = Cesium.ClippingPolygon.fromPolygonHierarchies(
+            {
+              polygonHierarchies: [buildingA],
+              union: true,
+              splits: 127,
+            }
+          );
+        });


Unlike the previous comment, tilesets do have ready promises, but I don't think you need to wait on the ready promise before setting the clipping polygon

lilleyse · 2020-06-11T17:17:34Z

Apps/Sandcastle/gallery/Clipping Polygon.html

+        });
+
+        Cesium.when(entity.readyPromise).then(function () {
+          entity.model.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(


Suggested change

entity.model.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(

entity.model.clippingPolygon = Cesium.ClippingPolygon.fromPolygonHierarchies(

lilleyse · 2020-06-11T17:17:43Z

Apps/Sandcastle/gallery/Clipping Polygon.html

+            }
+          }
+
+          globe.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(


Suggested change

globe.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(

globe.clippingPolygon = Cesium.ClippingPolygon.fromPolygonHierarchies(

lilleyse · 2020-06-11T17:54:41Z

CHANGES.md

@@ -1,5 +1,9 @@
 # Change Log

+### 1.71.1
+
+- Add `ClippingPolygon` support.


Add this to 1.72.0 instead.

lilleyse · 2020-06-12T03:18:21Z

Source/Scene/PolygonClippingAccelerationGrid.js

+    }
+  }
+
+  this.overlappingTriangleIndices = Float32Array.from(allOverlappingTriangles);


Same comment about Float32Array.from not supported in IE11

lilleyse · 2020-06-12T03:18:49Z

Source/Scene/PolygonClippingAccelerationGrid.js

+  py
+) {
+  if (!pointInsideRect(this.boundingBox, px, py)) {
+    return null;


Same comment about preferring undefined

lilleyse · 2020-06-12T03:20:36Z

Source/Scene/PolygonClippingAccelerationGrid.js

+  this.overlappingTriangleIndices = Float32Array.from(allOverlappingTriangles);
+}
+
+PolygonClippingAccelerationGrid.prototype.getCellFromWorldPosition = function (


Does this only exist for testing purposes? If so, just mark is as such. Usually we do that by adding an underscore to the function name and add a comment. PolygonClippingAccelerationGrid.prototype._getCellFromWorldPosition

lilleyse · 2020-06-12T03:24:05Z

Source/Scene/getPolygonClippingFunction.js

+    "  uniform vec3 u_clippingPolygonBoundingBox[4];\n" +
+    "  uniform vec2 u_clippingPolygonCellDimensions;\n" +
+    "  uniform sampler2D u_clippingPolygonAccelerationGrid;\n" +
+    "  uniform sampler2D u_clippingPolygonMeshPositions;\n" +
+    "  uniform sampler2D u_clippingPolygonOverlappingTriangleIndices;\n" +
+    "  uniform vec2 u_clippingPolygonAccelerationGridPixelDimensions;\n" +
+    "  uniform vec2 u_clippingPolygonOverlappingTrianglePixelIndicesDimensions;\n" +
+    "  uniform vec2 u_clippingPolygonMeshPositionPixelDimensions;\n" +
+    "  uniform mat4 u_clippingPolygonEyeToWorldToENU;\n" +
+    "  uniform float u_clippingPolygonMinimumZ;\n" +


Try to pack uniforms wherever possible since gl.uniform related functions happen to be a huge expense if you look at the profiler for any CeisumJS app. Looks like some of the vec2's can be packed at least.

lilleyse · 2020-06-12T03:24:31Z

Source/Scene/getPolygonClippingFunction.js

+
+function getClippingPolygonFunction(union) {
+  return (
+    "  uniform vec3 u_clippingPolygonBoundingBox[4];\n" +


Can these be vec2 instead? And can they be packed into two vec4?

Samulus · 2020-06-15T14:56:14Z

Follow Up Improvements

Two of the biggest issues with this PR right now is that it it uses a single ENU transform for all geometry, and cannot handle antipodeal rendering (it's ambiguous to the fragment shader which triangle should be used to clip if you have two triangles on opposite sides of the globe). One strategy discussed was to use a secondary global grid to partition the earth into different global cells. Each global cell would have its own ENU matrix for a significantly smaller region (e.g. each cell represents clipping triangles within a 1.1km region). Triangles in the amalgamated clipping mesh could then be placed into each of these global cells (triangles that span across multiple cells would have to be split into smaller triangles). We would also have to record if the clipping triangle belongs to the nothern / southern hemisphere and encode that in the data texture the fragment shader reads directly too, so it can disambiguate which triangle to test. Finally, the data should be encoded in a way that the initial texture lookup allows the fragment shader to be able to look up with a O(1) texture fetch:

Which global cell it should use for the current fragment position
If that global cell has any ENU associated with it in the southern / nothern hemisphere (or both)
Which ENU matrix to use / triangles to check

cesium-concierge · 2020-07-16T02:00:06Z

Thanks again for your contribution @Samulus!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

lilleyse · 2020-07-16T16:42:49Z

@cesium-concierge stop

likangning93 · 2020-10-26T21:10:54Z

Source/Scene/Model.js

+  var newMain =
+    "void main() {\n" +
+    "vec4 positionEC = czm_windowToEyeCoordinates(gl_FragCoord);\n" +
+    "positionEC /= positionEC.w;\n" +


Completely by chance, I found that removing this line removes the fuzziness that @lilleyse and myself were seeing when using logarithmic depth buffers with tilesets at certain distances.
I'm not entirely sure why, but perhaps it's related to the things that get done to the Z in the vertex shader before log depth is written here in the fragment shader.
This may make the TODO item above of avoiding clip space unnecessary.

Huh, and no side effects removing this line? Does it still work if log depth is disabled?

So there weren't problems with the fuzziness issue with and without log depth, but it did break the Sandcastle example in this PR... the mysteries deepen. I'll keep investigating.

Apologies for butting in, but could you briefly explain the reason for dividing the position by w in the first place? I'm not new to shaders, but this isn't clicking.

@midnight-dev I'm not actually sure, it looks like other places where we use this particular overload of czm_windowToEyeCoordinates (like for the earlier clipping planes shader code) we don't do this divide-by-w. the math in that code precedes my involvement in Cesium's clipping planes, although the history is a little murky. There's a post on Stackoverflow explaining that in gl_FragCoord the w can help you linearize depth, and is computed from w on gl_Position in the vertex shader, so I suspect positionEC.w serves a similar purpose. It's just we're already in the fragment shader, so there's maybe no need to actually divide by it?

but it did break the Sandcastle example in this PR... the mysteries deepen

@lilleyse I'm seeing for some reason that the proposed change here no longer breaks the shipping Sandcastle, which was kind of alarming.
I think I can chalk it up to some kind of browser cache thing, since I'm seeing that everything works fine in Firefox now as well.
Combined with the detail that we don't actually do this divide-by-w elsewhere in clipping code, I think it's safe to go ahead and apply this change, so imma do that.

… and for consistency with other clipping code

…t clipping planes do

ebogo1 · 2021-09-21T21:58:29Z

Talked offline with @Samulus - closing this for now but I'll leave the branch around since this needs a good amount of work before it's ready.

Samulus marked this pull request as draft June 5, 2020 14:53

Samulus force-pushed the 3DTILES_extent branch from 5802e2b to e055371 Compare June 5, 2020 18:26

mramato reviewed Jun 5, 2020

View reviewed changes

Samuel Vargas added 24 commits June 9, 2020 12:32

Add PolygonClipping support

2922bfe

Fix accidental semi-colon in bit shift section

0346b8d

Fix wrapping, and convert & back into &&

40c6c21

Remove unused parameters in PolygonClippingAccelerationGrid invocation

bcfedbd

Add basic tests for ClippingPolygon

24587be

Add debug check to ensure polygonHierarchies is array

a1dbfc4

Update CHANGES.md

227f67f

Convert getClippingPolygonFunction from ES6 to ES5

662525e

Move Clipping Polygon.jpg to the correct directory

2057794

Clean up documentation

8e316df

Add |undefined for strict mode TypeScript

35e6607

Silence es-lint on PolygonClippingAccelerationGridSpec.js

5d47753

Swap eslint-disable-next-line with return

622eace

Remove random extraneous whitespaces

4b8490c

Only regenerateShader if clippingPolygon is dirty

34d7ae7

Add Simplify.js to LICENSE.md

1c8acb7

Wrap uniform map callbacks in closure to fix eslint

88ebf17

Add more granular tests for textures / cell sizing

b47afe9

Fix indices typo

128e5c1

Avoid specs failure if floating point textures are unavailable

49886a8

Remove extraneous private documentation to appease typescript type ge…

33a332b

…nerator

Check width and height of generated textures

8208c90

Add explicit @returns for ClippingPolygon, s/let/var

6e4048c

Add WebGL tests to verify global clipping works as expected

92bf7a3

Samulus force-pushed the 3DTILES_extent branch from 99d4476 to 61363cd Compare June 9, 2020 16:32

Add destruction tests / ownership

76a0954

Samulus force-pushed the 3DTILES_extent branch from 61363cd to 76a0954 Compare June 9, 2020 16:33

Samuel Vargas added 2 commits June 9, 2020 14:53

Smooth out clipping camera motion

e1610d6

Fix isWorldPositionInsideAny… so 1xMAX and MAXx1 are indexed correctly

da48563

lilleyse reviewed Jun 12, 2020

View reviewed changes

likangning93 reviewed Oct 26, 2020

View reviewed changes

likangning93 added 4 commits October 27, 2020 13:56

remove divide-by-w to eliminate fuzziness at some views with logdepth…

37da789

… and for consistency with other clipping code

make clipping polygon toggleable for models/3D Tilesets, modeling wha…

594cc76

…t clipping planes do

Merge branch 'master' into 3DTILES_extent

a810903

run up-to-date prettier

3e93589

likangning93 force-pushed the 3DTILES_extent branch from fdd328a to 3e93589 Compare October 28, 2020 22:45

likangning93 added 2 commits October 28, 2020 20:21

fix broken ClippingPolygon spec

b0e0358

add defaults for clipping polygon uniforms

cea8f4b

ebogo1 added the category - graphics label Sep 21, 2021

ebogo1 closed this Sep 21, 2021

ggetz mentioned this pull request Jul 15, 2022

Add support for concave /convex clipping planes + holes in CesiumJS #8751

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add PolygonClipping support #8915

WIP: Add PolygonClipping support #8915

Samulus commented Jun 4, 2020 •

edited

Loading

cesium-concierge commented Jun 4, 2020

mramato Jun 5, 2020

Samulus Jun 5, 2020

mramato Jun 5, 2020

hpinkos Jun 5, 2020

lilleyse commented Jun 11, 2020

lilleyse left a comment

lilleyse Jun 11, 2020

lilleyse Jun 11, 2020

lilleyse Jun 11, 2020

lilleyse Jun 11, 2020

lilleyse Jun 11, 2020

lilleyse Jun 12, 2020

lilleyse Jun 12, 2020

lilleyse Jun 12, 2020

lilleyse Jun 12, 2020 •

edited

Loading

lilleyse Jun 12, 2020

Samulus commented Jun 15, 2020

cesium-concierge commented Jul 16, 2020

lilleyse commented Jul 16, 2020

likangning93 Oct 26, 2020 •

edited

Loading

lilleyse Oct 26, 2020

likangning93 Oct 27, 2020

midnight-dev Oct 27, 2020

likangning93 Oct 27, 2020

ebogo1 commented Sep 21, 2021

	entity.model.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(
	entity.model.clippingPolygon = Cesium.ClippingPolygon.fromPolygonHierarchies(

	globe.clippingPolygon = new Cesium.ClippingPolygon.fromPolygonHierarchies(
	globe.clippingPolygon = Cesium.ClippingPolygon.fromPolygonHierarchies(

WIP: Add PolygonClipping support #8915

WIP: Add PolygonClipping support #8915

Conversation

Samulus commented Jun 4, 2020 • edited Loading

Overview

Algorithm Pipeline

Known Issues / Future Optimizations

cesium-concierge commented Jun 4, 2020

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

lilleyse commented Jun 11, 2020

lilleyse left a 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

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

lilleyse Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Samulus commented Jun 15, 2020

Follow Up Improvements

cesium-concierge commented Jul 16, 2020

lilleyse commented Jul 16, 2020

likangning93 Oct 26, 2020 • edited Loading

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

ebogo1 commented Sep 21, 2021

Samulus commented Jun 4, 2020 •

edited

Loading

lilleyse Jun 12, 2020 •

edited

Loading

likangning93 Oct 26, 2020 •

edited

Loading