Refactor(core): unify SSE computation methods #674

peppsac · 2018-03-01T13:34:29Z

Until this PR each geometry type had its own SSE computation methods.
The goal of this PR is to provide a single method based on:

bounding-box (BB)
geometricError (in meters)
distance

The computation is simple:

transform camera in BB's basis
compute distance camera - BB
project geometricError on screen at distance

The last step produce a screen-space-error in pixels.

Geometric error values depend on the data type:

3dtiles: the tileset defines a geometricError per node so we can use this value
pointcloud: PotreeConverter defines a spacing, which is the average distance
between points in each node. We use: geometricError = spacing / 2^depth
tiles: this PR adapted the existing formula to use consistent units

The second commit adds a SSE inspector debug tool:

red: computed SSE
yellow: texture resolution (only used for tile sse)
green: sse treshold

peppsac · 2018-03-01T13:36:14Z

Also note : this PR provides almost everything needed to compute the camera far / near plane correctly in realtime (near would be minimum distance from all updated elements, far would be the max)

gchoqueux

review on first commit: refactor(core): unify SSE computation methods

gchoqueux · 2018-03-02T10:11:03Z

src/Core/Scheduler/Providers/PointCloudProvider.js

+                        name: childname,
+                        baseurl: url,
+                        bbox: bounds,
+                        geometricError: layer.metadata.spacing / Math.pow(2, childname.length),


Evoked in the commit message, there is a reference that explains the calculation?

This is by construction: see https://github.com/peppsac/PotreeConverter/blob/master/PotreeConverter/src/PotreeWriter.cpp#L73

gchoqueux · 2018-03-02T10:15:11Z

src/Core/ScreenSpaceError.js

+const m2 = new THREE.Matrix4();
+const m3 = new THREE.Matrix4();
+
+function compute(vector, matrix, camera, distance, _3d) {


compute what?

gchoqueux · 2018-03-02T10:17:06Z

src/Core/ScreenSpaceError.js

+
+function compute(vector, matrix, camera, distance, _3d) {
+    const basis = [
+        new THREE.Vector3(vector.x, 0, 0),


THREE.Vector3 to declare outside the function

gchoqueux · 2018-03-02T10:22:18Z

src/Core/ScreenSpaceError.js

+
+function findBox3Distance(camera, box3, matrix) {
+    // TODO: can be cached
+    m.getInverse(matrix);


matrix it's OBB worldMatrix? you would cache in OBB.worldMatrixInv?

I'd prefer not to cache anything first (hence the TODO node).

I believe we need to rework the notifyChange() mechanism to allow more precise updates / cache invalidation.

gchoqueux · 2018-03-02T10:38:07Z

src/Core/ScreenSpaceError.js

+
+    MODE_3D: 2,
+
+    computeFromBox3(camera, box3, matrix, geometricError, mode) {


there's no method to compute with sphere?

Not yet. As your other comment suggests I could move the sphere -> sse code from 3dtiles to here.

gchoqueux · 2018-03-02T10:42:56Z

src/Core/ScreenSpaceError.js

+
+        const size = computeSizeFromGeometricError(box3, geometricError);
+
+        const sse = compute(size, matrix, camera, distance, mode == this.MODE_3D);


compute calculates a pixel projection size?

Yes, I'll change the function name.

gchoqueux · 2018-03-02T10:44:23Z

src/Core/ScreenSpaceError.js

+}
+
+export default {
+    MODE_2D: 1,


could you explain the modes?

Yes I forgot to add a comment.
The idea here is: do we want to use the 2d bounding-box (x, y) or the 3d one.
For tiles we want to use the 2d one, while for other objects (pointcloud, 3dtiles) the 3d one makes more sense.

gchoqueux · 2018-03-02T10:45:45Z

src/Process/3dTilesProcessing.js

        worldPosition.setFromMatrixPosition(node.matrixWorld);
        cameraLocalPosition.copy(camera.camera3D.position).sub(worldPosition);
-        node.distance = Math.max(0.0, node.boundingVolume.sphere.distanceToPoint(cameraLocalPosition));
+        const distance = Math.max(0.0, node.boundingVolume.sphere.distanceToPoint(cameraLocalPosition));
+        return camera.preSSE * (node.geometricError / distance);


why not place it in ScreenSpaceError.js

gchoqueux · 2018-03-02T10:50:57Z

src/Process/GlobeTileProcessing.js

-        const sse = computeNodeSSE(context.camera, node);
-
-        return SSE_SUBDIVISION_THRESHOLD < sse;
+        node.sse = ScreenSpaceError.computeFromBox3(


why you replace the bounding sphere with OBBs?

Because the OBB is closer to the real volume of the node then a sphere.

gchoqueux · 2018-03-02T10:52:05Z

src/Process/GlobeTileProcessing.js

@@ -164,6 +132,7 @@ export function globeSchemeTileWMTS(type) {
 }

 export function computeTileZoomFromDistanceCamera(distance, view) {
+    // TODO fixme


do you have an idea?

I didn't even try to fix this for now.

I'd rather make sure that the sse computation introduced in this PR is ok.

peppsac · 2018-03-06T16:19:20Z

PR updaded.
Note: the computation in ScreenSpaceError.js can probably be simplified.
But it's easier to review the non-optimized form and then as a last step simplify where possible.

peppsac · 2018-03-06T16:44:09Z

Screenshots showing subdivision differences between master/PR :

Master:

PR :

Or for pointclouds:
Master:

PR:

gchoqueux · 2018-03-06T18:30:30Z

The tiles count between camera's tilt equals 0 and 85 is significant
But, qualitatively, it's too important for 0 and insufficient for 85.
can you balance the subdivision?

tilt == 0

tilt == 85

peppsac · 2018-03-06T19:04:16Z

@gchoqueux : thanks for the feedback.

The current logic is: we project both the bounding box x and y axis on the screen.
The SSE is then the min of these sizes.

Subdivision happens if SSE is > 256 (texture size) + layer.sseThreshold (5 by default).

The reason here for using the min is to prevent subdivision when the camera view vector is ~tangent to the surface but I agree that it's problematic in other case, such as your 2nd screenshot.

An alternative would be to keep SSE in x and y directions separately.
Then:

subdivide if min(sse.x, sse.y) > 256 or
if max(sse.x, sse.y) > 256 and min(sse.x, sse.y) > 128 (or another value that works)

peppsac · 2018-03-07T11:48:33Z

@gchoqueux I pushed an example implementation of #674 (comment)

It breaks everything but the globe (because I only adapted GlobeTileProcessing). Can you give it a try and let me know what you think?

gchoqueux · 2018-03-09T12:16:05Z

in computeVectorSizeAtDistance :

The box's diagonal may also be the largest projection? This approach doesn't take it into account?
Could you use Vector3.Project(camera)?
the scale transformation isn't in the calculation?

peppsac · 2018-03-09T12:47:32Z

The box's diagonal may also be the largest projection? This approach doesn't take it into account?

This approach computes the sse for each axis.
For tiles, texture are mapped on the x/y axis, so I don't think it make sense to use the x/y diag.
For points, we could use the diagonal instead because the points are distributed in all directions in the bbox, but having the 3 sse values is enough it seems.

Could you use Vector3.Project(camera)?

Nope. The Vector3.Project projects a world space vector, while I'm computing a camera space vector.
This vector is then moved at distance from the camera and then projected on screen.

the scale transformation isn't in the calculation?

It's used to compute the distance to camera.
For the size to projectd: we only the ratio of the box so the scale isn't relevant (because we scale the bbox to have its largest side being geometricError)

gchoqueux · 2018-03-13T10:39:34Z

so I don't think it make sense to use the x/y diag

I'm talking about the box's diagonal (in x, y and z)

For tiles, texture are mapped on the x/y axis

DEM is mapped on z axis, not only x and y

while I'm computing a camera space vector.

yet you go through the world space when calculating (with m3)
the projection would be different with Vector3.Project?

peppsac · 2018-03-13T10:55:28Z

DEM is mapped on z axis, not only x and y

Can you compute a correct estimation of the DEM error in meters along the z axis for every tiles?
If you have a formula then yes we can use it as well.
Note that the current formula doesn't use the tile vertices count: it could make sense to use too but again, this needs a correct formula that works on all tiles.

yet you go through the world space when calculating (with m3)

No I'm not. You misread the code: m3 is only the rotation part, not the full world matrix of the camera.

gchoqueux · 2018-03-13T11:11:02Z

What is your formula to estimate error on the x and y axes?

You don't answer this question : the projection would be different with Vector3.Project?

peppsac · 2018-03-13T11:28:59Z

What is your formula to estimate error on the x and y axes?

Compare size in pixels of the tile on the screen to the texture size.

You don't answer this question : the projection would be different with Vector3.Project?

Yes it would be diffrent. I detailled in my earlier comment #674 (comment) the differences between Vector3.Project and my computations.

gchoqueux · 2018-03-13T13:24:43Z

The texels are also on the box's diagonal (x, y, z), it can more revelant by example with box'size (100, 100, 1000)

Yes it would be diffrent

I can't understand why there is a difference between the two, I'm going to look

Until this commit each geometry type had its own SSE computation methods. The goal of this commit is to provide a single method based on: - bounding-box (BB) - geometricError (in meters) - distance The computation is simple: - transform camera in BB's basis - compute distance camera - BB - project geometricError on screen at distance The last step produce a screen-space-error in pixels. Geometric error values depend on the data type: - 3dtiles: the tileset defines a geometricError per node so we can use this value - pointcloud: PotreeConverter defines a spacing, which is the average distance between points in each node. We use: geometricError = spacing / 2^depth - tiles: this commit adapted the existing formula to use consistent units

peppsac · 2018-04-03T10:09:44Z

Rebased. I also removed the MODE_2D / MODE_3D thing since it's not used anymore.

As far as I can tell this PR improves the SSE handling in most situations.

Regarding the diagonal: I still believe it isn't an issue. It's an corner case so I don't think it's worth complexifying the code to cover it. Btw, current master version SSE handling is quite bizarre anyway. E.g: in this screenshot the nearest tile in the middle of the screen is almost 600px wide.

You can try with:

globeView.camera.camera3D.position.copy({x: 4490626.907379815, y: 506551.71291476686, z: 4500105.469154227})
globeView.camera.camera3D.rotation.copy({_x: -1.3092800928909716, _y: 0.4734022844137177, _z: -2.4859644421245957, _order: "XYZ"})
globeView.notifyChange(true);

Project average point spacing on screen and compare to sse threshold. It's a simplification of the approach proposed in #674. This allows to drop a dependency (convexhull) and brings the SSE compuation for pointcloud closer to what is done for other geometries.

mbredif

some comments. could you define mathematically the sse?

mbredif · 2018-06-29T17:27:06Z

src/Core/ScreenSpaceError.js

+    // Move camera position in box3 basis
+    // (we don't transform box3 to camera basis because box3 are AABB)
+    const pt = camera.camera3D.position
+        .clone(v).applyMatrix4(m);


this is wrong if m as some scaling (eigenvalues != 1)

mbredif · 2018-06-29T17:31:12Z

src/Core/ScreenSpaceError.js

+        b.y = b.y * camera.height * 0.5;
+    }
+
+    return basis.map(b => b.length());


this looks like an approximation of some distance, which computation is optimized by considering axes independently. could you phrase mathematically the exact distance you are approximating ?

peppsac · 2018-07-02T07:17:32Z

some comments. could you define mathematically the sse?

Thanks for the comments.

I'll come back to this PR soon, but for now I'm focused on the more recently opened PRs.

peppsac force-pushed the refactor_sse branch from 2f051c7 to dbd2cd9 Compare March 1, 2018 13:51

gchoqueux requested changes Mar 2, 2018

View reviewed changes

peppsac mentioned this pull request Mar 26, 2018

Fix wrong preSSE computation #633

Closed

peppsac added 9 commits April 3, 2018 11:18

feat(core) add debug SSE inspector tool

6e22d5b

Use tile size instead of half-size

182f8a8

review

d0731c0

Experiment globe/sse

cb1376a

adapt sse

36e3344

update for z axis

476501e

fix rebase

0b06f27

Remove MODE_2D/3D option

362fb69

peppsac force-pushed the refactor_sse branch from 5b0780d to 362fb69 Compare April 3, 2018 09:58

peppsac mentioned this pull request Apr 16, 2018

Pointcloud improvement #732

Merged

peppsac mentioned this pull request Apr 24, 2018

example: simplify orthographic controls #734

Merged

peppsac mentioned this pull request Apr 27, 2018

Pointcloud improvement #746

Merged

peppsac mentioned this pull request Jun 29, 2018

Refacto limit dependency on layeredmaterial #792

Merged

mbredif reviewed Jun 29, 2018

View reviewed changes

gchoqueux added the wip 🚧 Still being worked on label Jul 2, 2018

peppsac mentioned this pull request Jul 10, 2018

feat(core): allow layer to specify which elements should be updatable #789

Merged

peppsac closed this Jul 26, 2018

zarov deleted the refactor_sse branch December 7, 2018 09:13


		MODE_3D: 2,

		computeFromBox3(camera, box3, matrix, geometricError, mode) {


		const size = computeSizeFromGeometricError(box3, geometricError);

		const sse = compute(size, matrix, camera, distance, mode == this.MODE_3D);

Refactor(core): unify SSE computation methods #674

Refactor(core): unify SSE computation methods #674

Conversation

peppsac commented Mar 1, 2018

peppsac commented Mar 1, 2018

gchoqueux left a comment • 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

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

Choose a reason for hiding this comment

peppsac Mar 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peppsac commented Mar 6, 2018

peppsac commented Mar 6, 2018

gchoqueux commented Mar 6, 2018

peppsac commented Mar 6, 2018

peppsac commented Mar 7, 2018

gchoqueux commented Mar 9, 2018

peppsac commented Mar 9, 2018 • edited Loading

gchoqueux commented Mar 13, 2018

peppsac commented Mar 13, 2018

gchoqueux commented Mar 13, 2018

peppsac commented Mar 13, 2018 • edited Loading

gchoqueux commented Mar 13, 2018

peppsac commented Apr 3, 2018

mbredif left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peppsac commented Jul 2, 2018

gchoqueux left a comment •

edited

Loading

peppsac Mar 2, 2018 •

edited

Loading

peppsac commented Mar 9, 2018 •

edited

Loading

peppsac commented Mar 13, 2018 •

edited

Loading