feat(underground#WIP): dynamic opacity to visualize underground and add hideSkirt parameter #2108

AnthonyGlt · 2023-06-01T14:19:29Z

2nd Version of #2077
Not completely optimized.
The underground option can't be enabled by default but it can be once the view is loaded (using a button)

…ideSkirt parameter

Working

AnthonyGlt · 2023-06-01T14:22:30Z

src/Controls/PlanarControls.js

@@ -372,6 +373,7 @@ class PlanarControls extends THREE.EventDispatcher {
        }
        if (onMovement) {
            this.view.dispatchEvent({ type: PLANAR_CONTROL_EVENT.MOVED });
+            this.view.dispatchEvent({ type: VIEW_EVENTS.CAMERA_MOVED });


this could be in another PR. This event is dispatch in GlobeControls when there is movement, it should be dispatch from PlanarControls too

Indeed it is a good point to dispatch the same event here. However, what could be done in another PR is to dispatch the same properties as with GlobeControls here (talking about range, heading etc).
This should however be done when working on controls harmonization.

AnthonyGlt · 2023-06-01T14:23:42Z

src/Utils/CameraUtils.js

@@ -101,13 +100,6 @@ class CameraRig extends THREE.Object3D {
        } else {
            this.camera.matrixWorld.decompose(camera.position, camera.quaternion, camera.scale);
        }
-        view.dispatchEvent({


this is removed because it's dispatched in the controls (Planar/GlobeControls) directly
Like this, we don't dispatch the event when there is tiles loading or other updates without camera movements

This was dispatched here because, if I remember well, CameraUtils can move the camera without the use of Controls. Therefore, dispatching it here allows user to be aware of a camera movement if it originates from CameraUtils.
The typical use case is for the Navigation widget, which has to update its compass and 2D/3D toggle if the camera is moved through CameraUtils.

Oh... You're right, but this dispatch is trigger too often (like I said, when tiles are loading or event when there is no movement). I'll see how I can do

mgermerie

Thanks for the PR.
The TileLayer options are only visible while in debug mode if I'm correct. Could you therefore add a button on your example (a bit like what you did for the 3dTiles classification example) to toggle dynamic opacity so that it appears on the website (which is not in debug mode) ?

mgermerie · 2023-06-30T08:09:20Z

src/Controls/PlanarControls.js

@@ -372,6 +373,7 @@ class PlanarControls extends THREE.EventDispatcher {
        }
        if (onMovement) {
            this.view.dispatchEvent({ type: PLANAR_CONTROL_EVENT.MOVED });
+            this.view.dispatchEvent({ type: VIEW_EVENTS.CAMERA_MOVED });


Indeed it is a good point to dispatch the same event here. However, what could be done in another PR is to dispatch the same properties as with GlobeControls here (talking about range, heading etc).
This should however be done when working on controls harmonization.

mgermerie · 2023-06-30T08:22:22Z

src/Core/View.js

+    setUndergroundVisualization(trigger) {
+        const atmo = this.getLayerById('atmosphere');
+        if (trigger) {
+            this.tileLayer.updateTiledLayerOpacity({ target: this });


Perhaps we should add a return point if this.tileLayer is not defined (when user is not using a prefab View) ? As it is done in View.getPickingPositionFromDepth. WDYT ?

mgermerie · 2023-06-30T08:27:59Z

src/Layer/TiledGeometryLayer.js

+        if (config.altitudeForZeroOpacity === undefined || config.altitudeForZeroOpacity === null) {
+            this.altitudeForZeroOpacity = 420;
+        } else {
+            this.altitudeForZeroOpacity = config.altitudeForZeroOpacity;
+        }


Could you simplify with this.altitudeForZeroOpacity = config.altitudeForZeroOpacity ?? 420; ?

mgermerie · 2023-06-30T08:29:35Z

src/Layer/TiledGeometryLayer.js

+        if (event == null) {
+            return;
+        }


As the method is called through an event listener, is it possible that event might actually be null ?

mgermerie · 2023-06-30T08:36:54Z

src/Layer/TiledGeometryLayer.js

+            return;
+        }
+        const view = event.target;
+        const distance = CameraUtils.getTransformCameraLookingAtTarget(view, view.controls.camera).range;


I just thought about orthographic camera, for which the range has no meaning (since only the fov of the camera is changed).
@AnthonyGlt, @jailln, do you think we should work (in another PR) on adapting the dynamic opacity feature to orthographic cameras or else adapting CameraUtils so that range is computed from camera fov (which would rigorously be a wrong value but would ease development for features that must support both perspective and orthographic cameras) ?

mgermerie · 2023-06-30T08:49:29Z

src/Utils/CameraUtils.js

@@ -101,13 +100,6 @@ class CameraRig extends THREE.Object3D {
        } else {
            this.camera.matrixWorld.decompose(camera.position, camera.quaternion, camera.scale);
        }
-        view.dispatchEvent({


This was dispatched here because, if I remember well, CameraUtils can move the camera without the use of Controls. Therefore, dispatching it here allows user to be aware of a camera movement if it originates from CameraUtils.
The typical use case is for the Navigation widget, which has to update its compass and 2D/3D toggle if the camera is moved through CameraUtils.

mgermerie · 2023-06-30T08:53:19Z

src/Core/View.js

+
+
+            if (atmo) {
+                this.scene.background = new THREE.Color(0x000000);


Why do we need to set this ? The default value is null which already results in a black background if I'm wright. Also, if setting this here, we should set it back to null on line 1162.
Moreover, by testing on view_3d_map example, switching atmo.visible does not seem to have an effect.

In view_3d_map for example, when I don't change the background we get:

When I don't change the atmo:

When I don't change anything:

With my edits:

It will be better to see the data I think.

Although, you are right about switching back to the previous value at line 1162

mgermerie · 2023-06-30T08:59:20Z

src/Layer/TiledGeometryLayer.js

+            get: () => _hideSkirt,
+            set: (value) => {
+                _hideSkirt = value;
+                this.#hideExistingSkirt(value);


Perhaps we should directly add the method's content here instead of defining a method if it is only called here. WDYT ?

AnthonyGlt and others added 15 commits June 1, 2023 14:49

feat(underground): dynamic opacity to visualize underground and add h…

feb1993

…ideSkirt parameter

move to tiledGeo + remove hidingSkirt func

0e5cd98

check if event null

b1ebdff

bug fixe when elevation

1d47685

use view instead of target

f23cf59

remove clamp + loading import

8426979

revert handleCollision in GlobeView

d840d24

move underground options

1b00c39

use range

c52cf18

Working

remove CAMERA_MOVED

250d52a

use let instead of var

5902c1f

clean code

c96f535

add rig camera arg

a405efa

remove import

ff9026f

remove import view_events

8ac288c

AnthonyGlt commented Jun 1, 2023

View reviewed changes

mgermerie reviewed Jun 30, 2023

View reviewed changes

jailln assigned AnthonyGlt Sep 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(underground#WIP): dynamic opacity to visualize underground and add hideSkirt parameter #2108

feat(underground#WIP): dynamic opacity to visualize underground and add hideSkirt parameter #2108

AnthonyGlt commented Jun 1, 2023

AnthonyGlt Jun 1, 2023 •

edited

Loading

mgermerie Jun 30, 2023

AnthonyGlt Jun 1, 2023 •

edited

Loading

mgermerie Jun 30, 2023

AnthonyGlt Jul 26, 2023

mgermerie left a comment

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

mgermerie Jun 30, 2023

AnthonyGlt Jul 26, 2023

mgermerie Jun 30, 2023



		if (atmo) {
		this.scene.background = new THREE.Color(0x000000);

feat(underground#WIP): dynamic opacity to visualize underground and add hideSkirt parameter #2108

Are you sure you want to change the base?

feat(underground#WIP): dynamic opacity to visualize underground and add hideSkirt parameter #2108

Conversation

AnthonyGlt commented Jun 1, 2023

AnthonyGlt Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AnthonyGlt Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgermerie 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

Choose a reason for hiding this comment

AnthonyGlt Jun 1, 2023 •

edited

Loading

AnthonyGlt Jun 1, 2023 •

edited

Loading