Viewer: add a way to match POV of other cameras (from gltf file loaded for example) #16076

cournoll · 2025-01-16T16:31:15Z

This change introduces a way to interpolate the viewer's default camera to other existing cameras in the scene. The primary goal of this feature is to enable matching the POV of cameras defined in a GLTF file, without switching the active camera to try to keep a "simple" experience for non-expert users.

getArcRotateCameraInfos function compute the alpha, beta, and radius angles, as well as the target point for a camera that is not an ArcRotateCamera.
For target selection, I used a straightforward approach, the target point is determined based on the camera's forward ray. If an intersection with the meshes from AssetContainer is found, the first hit point is used as the target. If no intersection is detected, a fallback target is calculated by projecting the distance between the camera and the AssetContainer world center along the forward ray.

bjsplat · 2025-01-16T16:47:15Z

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

bjsplat · 2025-01-16T16:59:22Z

Snapshot stored with reference name:
refs/pull/16076/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16076/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16076/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/16076/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/16076/merge
https://gui.babylonjs.com/?snapshot=refs/pull/16076/merge
https://nme.babylonjs.com/?snapshot=refs/pull/16076/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/16076/merge#BCU1XR#0

bjsplat · 2025-01-16T17:11:24Z

WebGL2 visualization test reporter:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16076/merge/testResults/webgl2playwright/index.html

bjsplat · 2025-01-16T17:12:07Z

Visualization tests for WebGPU (Experimental)

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16076/merge/testResults/webgpuplaywright/index.html

ryantrem · 2025-01-16T20:41:21Z

I was thinking from our initial discussions that we would simply translate these cameras directly to hotspot entries on the element (rather than adding a new cameras concept). Any reason not to go that route? If we do, then I think a few changes to this PR would be good:

getArcRotateCameraInfos can be a private function in ViewerElement (just an internal implementation detail for now).
ViewerDetails could be updated such that the model property returns an object of type Model rather than AssetContainer (breaking change on Viewer, but I still think this is ok since it is @experimental and lower level with minimal current use outside of ViewerElement). Alternatively we could just add more properties to ViewerDetails that are model specific, like the model's bounding center point.
If we only want hotspots for the active model, then in ViewerElements onModelChanged handler, add to the hotspot collection camera-based hotspots for that model (and remove any previous camera-based hotspots).
If we want hotspots for all cameras from all loaded models, then I think it is a little trickier because we need to add camera-based hotspots any time a model is loaded and remove them any time a previously loaded model is removed from the scene.

Which behavior between #3 and #4 are you guys aiming for?

cournoll · 2025-01-17T09:01:23Z

I was thinking from our initial discussions that we would simply translate these cameras directly to hotspot entries on the element (rather than adding a new cameras concept). Any reason not to go that route?

Not really any specific reason. Initially, during development, I wanted a distinct visual indication so the user could understand that this behavior in the viewer was tied to the cameras in the scene. But once I finished my code and realized that focusing on a hotspot or matching a camera’s POV ultimately relies on the same principle (an interpolation) I understood that we could merge the two. :)

That said, I’m still wondering how we could clarify for users where the names and values of these "undeclared hotspots" originate.

cournoll · 2025-01-17T09:15:33Z

1. `getArcRotateCameraInfos` can be a private function in `ViewerElement` (just an internal implementation detail for now).

I had a similar feeling about that, so it works perfectly for me.

2. `ViewerDetails` could be updated such that the `model` property returns an object of type `Model` rather than `AssetContainer` (breaking change on `Viewer`, but I still think this is ok since it is @experimental and lower level with minimal current use outside of `ViewerElement`). Alternatively we could just add more properties to `ViewerDetails` that are model specific, like the model's bounding center point.

Yes, yes, yes! Having the bounding info in ViewerDetails would be very nice for the user. Got it, I’ll add that to this PR.

3. If we only want hotspots for the active model, then in `ViewerElement`s `onModelChanged` handler, add to the hotspot collection camera-based hotspots for that model (and remove any previous camera-based hotspots).

4. If we want hotspots for all cameras from all loaded models, then I think it is a little trickier because we need to add camera-based hotspots any time a model is loaded and remove them any time a previously loaded model is removed from the scene.

We will aiming for behavior 4, so i'll try to do something that stay consistent with the Babylon viewer approach and let us fit with our needed 👍

packages/tools/viewer/src/viewer.ts

ryantrem · 2025-01-16T20:20:10Z

packages/tools/viewer/src/viewer.ts

+     */
+    public async getArcRotateCameraInfos(camera: Camera): Promise<Nullable<ViewerArcRotateCameraInfos>> {
+        await import("core/Culling/ray");
+        const ray = camera.getForwardRay(100, camera.getWorldMatrix(), camera.globalPosition);


This is equivalent to just getForwardRay() (which we do in another part of this file).

It seems that the default origin of the forward ray is actually the camera position rather than the camera’s global position.

packages/tools/viewer/src/viewer.ts

ryantrem · 2025-01-17T23:39:21Z

We will aiming for behavior 4, so i'll try to do something that stay consistent with the Babylon viewer approach and let us fit with our needed 👍

Ok, I guess in this case we can just check Scene.cameras and watch Scene.onNewCameraAddedObservable and Scene.onCameraRemovedObservable. I think we should probably also have a new property/attribute like cameras-as-hotspots that is disabled by default (e.g. you have to explicitly opt into this behavior).

Remove useless ray parameters Factor code on radius calculation Use _tempVectors for computation Handle mesh predicate as parameter of function getArcRotateCameraInfos

Add property camerasAsHotSpots to active cameras hot spots Fix camera forward ray

Add bounding infos into the model

ryantrem · 2025-01-21T21:26:07Z

packages/dev/core/src/Cameras/arcRotateCamera.ts

+     * @param offset The directional offset between the source position and the target position
+     * @returns The alpha angle in radians
+     */
+    public static computeAlpha(offset: Vector3): number {


Since these functions don't access this, can we just make them module level functions?

ryantrem · 2025-01-21T21:30:35Z

packages/tools/viewer/src/viewer.ts

+export type ModelBoundingInfo = {
+    worldExtents: {
+        min: Vector3;
+        max: Vector3;
+    };
+    worldSize: Vector3;
+    worldCenter: Vector3;
+};


A few thoughts on this new type:

We don't expose Vector3 directly for other parts of the interface (like hotspots). Could we switch these to [x: number, y: number, z: number] like we have elsewhere?

Make it readonly either in the definition or in the usage (since it is a stored/persistent value, not a transient value that is just returned from a function with a single usage)? I think definition would be ok, but either works.

To make this more re-usable (in case in the future we want to use it for something not in world space, or for something other than models), could we rename to not include "world" or "model"?

Suggested change

export type ModelBoundingInfo = {

worldExtents: {

min: Vector3;

max: Vector3;

};

worldSize: Vector3;

worldCenter: Vector3;

};

export type BoundingInfo = {

extents: Readonly<{

readonly min: readonly [x: number, y: number, z: number];

readonly max: readonly [x: number, y: number, z: number];

}>;

readonly size: readonly [x: number, y: number, z: number];

readonly center: readonly [x: number, y: number, z: number];

};

And then in the usage below, have worldBounds?: BoundingInfo

ryantrem · 2025-01-21T21:48:42Z

packages/tools/viewer/src/viewerElement.ts

+    set hotSpots(value: Record<string, HotSpot>) {
+        this._hotSpots = value;
+    }
+
+    get hotSpots() {
+        return this._hotSpots;
+    }


Any reason for changing this to have a getter/setter?

ryantrem · 2025-01-21T21:49:02Z

packages/tools/viewer/src/viewerElement.ts

-     * A string value that encodes one or more hotspots.
-     */
+    private _hotSpots: Record<string, HotSpot> = {};
+
    @property({


Did we lose this doc comment?

ryantrem · 2025-01-21T21:53:28Z

packages/tools/viewer/src/viewerElement.ts

@@ -1125,6 +1136,27 @@ export abstract class ViewerElement<ViewerClass extends Viewer = Viewer> extends
                    this._dispatchCustomEvent("animationprogresschange", (type) => new Event(type));
                });

+                details.scene.onNewCameraAddedObservable.add((camera) => {
+                    if (camera.uniqueId !== details.camera.uniqueId && this.camerasAsHotSpots) {


Can we just compare the camera instance rather than the uniqueId?

Suggested change

if (camera.uniqueId !== details.camera.uniqueId && this.camerasAsHotSpots) {

if (camera !== details.camera && this.camerasAsHotSpots) {

ryantrem · 2025-01-21T21:56:26Z

packages/tools/viewer/src/viewerElement.ts

@@ -1125,6 +1136,27 @@ export abstract class ViewerElement<ViewerClass extends Viewer = Viewer> extends
                    this._dispatchCustomEvent("animationprogresschange", (type) => new Event(type));
                });

+                details.scene.onNewCameraAddedObservable.add((camera) => {
+                    if (camera.uniqueId !== details.camera.uniqueId && this.camerasAsHotSpots) {


With the current impl, if you load some models and then toggle camerasAsHotSpots, it will not actually affect the hotSpots. I think it would be a good improvement to factor our the handlers to functions like addCameraHotSpot and removeCameraHotSpot and then call them for these observables, but also when camerasAsHotSpots is enabled/disabled (probably in update if the property has changed).

ryantrem · 2025-01-21T21:59:38Z

packages/tools/viewer/src/viewer.ts

+    /**
+     * Updates the bounding info for the model by computing its maximum extents, size, and center considering animation, skeleton, and morph targets.
+     */
+    private _updateModelBoundingInfo(): void {


Instead of having this function mutate this._modelInfo, can we:

Move it out to a utility function (within this file, but not inside the class) that takes an AssetContainer and returns a BoundingInfo

Call it in loadModel and set the bounding info for the model when it is loaded

Make the worldBounds property of the ModelInfo part of the readonly section (since it will always be available as it would be computed when the model loads)

ryantrem · 2025-01-21T22:00:21Z

packages/tools/viewer/src/viewer.ts

+     * @param predicate Used to define predicate for selecting meshes and instances (if exist)
+     * @returns An object containing the alpha, beta, radius and target properties, or null if no model found
+     */
+    public async getArcRotateCameraInfos(camera: Camera, predicate?: MeshPredicate): Promise<Nullable<ViewerArcRotateCameraInfos>> {


As we discussed a bit on the side, can we move this now to be a private function inside viewerElement.ts since ViewerElement will now have access to the bounding info through the model info?

cournoll added 2 commits January 16, 2025 17:42

Interpolate default camera to other scene cameras

dd2f30e

Change func generateArcRotateCamera in getArcRotateCameraInfos

471b841

cournoll force-pushed the viewer-select-cameras branch from 1a18638 to 471b841 Compare January 16, 2025 16:45

ryantrem reviewed Jan 17, 2025

View reviewed changes

cournoll added 8 commits January 20, 2025 12:25

Fix ci remove imports from core/index in element

ab441d8

Remove useless ray parameters Factor code on radius calculation Use _tempVectors for computation Handle mesh predicate as parameter of function getArcRotateCameraInfos

Observe scene cameras to add hot spots

1bd1120

Add property camerasAsHotSpots to active cameras hot spots Fix camera forward ray

Fix comments on getArcRotateCameraInfos

f10edcd

Return a Model instead of an AssetContainer in viewer details

92cab4b

Add bounding infos into the model

Repalce _modelWorldCenter by the model bounding infos

306b57d

Optimize radius calculation

1b809d7

Factorisation of the compute of the alpha and the beta

5160345

Clean comments

09e3119

ryantrem reviewed Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Viewer: add a way to match POV of other cameras (from gltf file loaded for example) #16076

Viewer: add a way to match POV of other cameras (from gltf file loaded for example) #16076

cournoll commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

ryantrem commented Jan 16, 2025

cournoll commented Jan 17, 2025 •

edited

Loading

cournoll commented Jan 17, 2025 •

edited

Loading

ryantrem Jan 16, 2025

cournoll Jan 20, 2025

ryantrem commented Jan 17, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

ryantrem Jan 21, 2025

	if (camera.uniqueId !== details.camera.uniqueId && this.camerasAsHotSpots) {
	if (camera !== details.camera && this.camerasAsHotSpots) {

Viewer: add a way to match POV of other cameras (from gltf file loaded for example) #16076

Are you sure you want to change the base?

Viewer: add a way to match POV of other cameras (from gltf file loaded for example) #16076

Conversation

cournoll commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

bjsplat commented Jan 16, 2025

ryantrem commented Jan 16, 2025

cournoll commented Jan 17, 2025 • edited Loading

cournoll commented Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryantrem commented Jan 17, 2025

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

cournoll commented Jan 17, 2025 •

edited

Loading

cournoll commented Jan 17, 2025 •

edited

Loading