Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Camera documentation, usability, and behavior issues #12346

Open
javagl opened this issue Nov 28, 2024 · 0 comments
Open

Camera documentation, usability, and behavior issues #12346

javagl opened this issue Nov 28, 2024 · 0 comments

Comments

@javagl
Copy link
Contributor

javagl commented Nov 28, 2024

I generally think that it's better to have fine-grained, specific issues. But the Camera class has some issues that are interrelated. So I think that it makes sense to summarize them here. These refer to usability, plain bugs, and the documentation. In doubt, this issue can be split up...


A broader issue of the documentation is about the coordinate systems ("reference frame"). It is often not clear

  • which coordinate system a property is given in and
  • how functions will affect the properties, depending on the "reference frame"

For example:

The documentation of position currently says

The position of the camera.

Without further details, it could be reasonable to assume that this is the position in world coordinates. What else should it be?

But then there is the documentation of positionWC, which says

Gets the position of the camera in world coordinates.

The documentation for position should mention that this is ~"the position relative to the reference frame" or so (details to be verified). It should have a link to the documentation of the transform property, explaining the effect of this property and its connection to other properties (like the position vs. positionWC).

Related aspects of the usability are

  • It's possible to set the position
  • It's not possible to set the positionWC
  • It's hardly possible to affect the positionWC sensibly (see next point) - at least, not without broader knowledge about the Camera class

The documentation of lookAtTransform says

Sets the camera position and orientation using a target and transformation matrix.

(and continues with details that are really hard to understand and visualize). But ... even the first statement is not true. It does not affect the position. At least not the positionWC. (It does affect the position, but the position afterwards is not necessarily the translation of the given matrix).

Consider this snippet:

const viewer = new Cesium.Viewer("cesiumContainer", {
  globe: false
});
const camera = viewer.camera;

console.log("Setting position (1,2,3)");
camera.position = new Cesium.Cartesian3(1, 2, 3);

console.log("Before lookAtTransform with (2,3,4)");
console.log("position   " + camera.position);
console.log("positionWC " + camera.positionWC);

const transform = Cesium.Matrix4.fromTranslation(
  new Cesium.Cartesian3(2, 3, 4),
  new Cesium.Matrix4());
camera.lookAtTransform(transform);

console.log("After lookAtTransform with (2,3,4)");
console.log("position   " + camera.position);
console.log("positionWC " + camera.positionWC);

which prints

Setting position (1,2,3)
Before lookAtTransform with (2,3,4)
position   (1, 2, 3)
positionWC (1, 2, 3)
After lookAtTransform with (2,3,4)
position   (-1, -1, -1)
positionWC (1, 2, 3)

The documentation should describe more clearly that this sets the "reference frame". Maybe it should even start with a clear

Sets the [link:transform] of this camera.

[Details]

The way how the position is affected (and that this call does not affect the positionWC) should be decribed more clearly, either in the documentation of lookAtTransform itself, or in the documentation of the transform that should be linked from there.


Setting the position = (0,0,0) will cause a

DeveloperError: normalized result is not a number

const viewer = new Cesium.Viewer("cesiumContainer");
const camera = viewer.camera;

camera.position = new Cesium.Cartesian3();
camera.direction = Cesium.Cartesian3.negate(Cesium.Cartesian3.UNIT_Z, new Cesium.Cartesian3());
camera.up = Cesium.Cartesian3.clone(Cesium.Cartesian3.UNIT_Y);
camera.frustum.fov = Cesium.Math.PI_OVER_THREE;
camera.frustum.near = 1.0;
camera.frustum.far = 2.0;

Setting position = (0,0,0) without a globe will cause a

TypeError: Cannot read properties of undefined (reading 'height')

const viewer = new Cesium.Viewer("cesiumContainer", {
  globe: false
});
const camera = viewer.camera;

camera.position = new Cesium.Cartesian3();
camera.direction = Cesium.Cartesian3.negate(Cesium.Cartesian3.UNIT_Z, new Cesium.Cartesian3());
camera.up = Cesium.Cartesian3.clone(Cesium.Cartesian3.UNIT_Y);
camera.frustum.fov = Cesium.Math.PI_OVER_THREE;
camera.frustum.near = 1.0;
camera.frustum.far = 2.0;

Note: This is the code from the 'Example' at the top of the Camera documentation!

Both of these are unaffected by things like
viewer.scene.screenSpaceCameraController.enableCollisionDetection = false;
or
viewer.depthTestAgainstTerrain = false;
which might be first guesses to resolve this...


Using an offset = (0,0,0) for lookAtTransform will cause a

DeveloperError: normalized result is not a number

const viewer = new Cesium.Viewer("cesiumContainer");
const camera = viewer.camera;

const transform = Cesium.Matrix4.clone(
  Cesium.Matrix4.IDENTITY);
const offset = new Cesium.Cartesian3(0, 0, 0);
camera.lookAtTransform(transform, offset);

This is not documented (and using undefined will work)


A specific case of these errors was mentioned in #11823, but this may become obsolete now.

I originally posted some of this in #11749 (comment), because it only referred to the given "Example" code at the top of the camera documentation, which, when applied in practice, causes the DeveloperError mentioned above.

Anecdotal: I did spend quite some time with ~"these issues" in a recent PR. The goal was to achieve a certain camera position in world coordinates. I eventually figured out that the difficulties had been caused by a missing
camera.lookAtTransform(Matrix4.IDENTITY);
That may be obvious in hindsight, but wasn't for me (and certainly isn't for those who have not digged through the Camera code and docs). The fact that this has to be called before setting the position is not obvious either, and is something that can definitely not be extracted from the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant