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

Draft for handling epsilon-dependence of orientation #11937

Closed
wants to merge 1 commit into from

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Apr 16, 2024

Description

This changes the camera.lookAt implementation so that the orientation does no longer depends (so strongly) on certain "epsilon" values of the target position. Some additonal details are laid out in #11488 .

The fact that the title and the name of the branch contain the word draft should be a clear indicator:

This is not supposed to be merged!

This should just serve as a basis for figuring out how to solve the issue sensibly.

As pointed out in the comments to the issue: There is no one-fits-all solution. The whole zoomTo-functionality just assumes that everything happens on the surface of the WGS84 ellipsoid. When a tileset has a distance of (0,0,100) or (5000,0,0) from the center of the ellipsoid, then trying to compute the ENU orientation from the point on the surface that is closest to this point hardly makes sense. Tilesets that are not "geolocated" (but have their center at the origin) will usually not be handled gracefully by that function. But basically every Sandcastle in the world will use the zoomTo/flyTo functionality. So every change here will inevitably be a "breaking change" in terms of the behavior.

That said .... what is currently implemented here will do the following:

  • It will compute the distance of the target (i.e. the center of the bounding sphere to zoom to) to the origin.
  • It will compute the distance of the eye point to the target (based on the offset that is passed in)
  • It will compute a "weight" from that. When the eye is "far away" from the target, then the target position will less affect the orientiation of the camera.

In terms of "testing", I focussed on the case of a tileset with a bounding sphere that covers the whole earth, but is centered at different positions. These are created (on the fly) in the sandcastle that is given below, with the

  createSampleOption([0, 0, 0]),
  createSampleOption([1e-15, 1e-15, 1e-15]),
  ...
  createSampleOption([0, 100, 0]),
  ...
  createSampleOption([0, 0, 14000000]),

lines.

Some examples of the old behavior, showing that the orientation strongly depends on that magic "epsilon":

Cesium S2 Orientation OLD

Some examples of the new behavior:

Cesium S2 Orientation NEW

To emphasize this again: This is not a 'proposed solution'. The approach feels pretty arbitrary, and I could come up with a million cases where this will lead to "unexpected results". (And the functionality of flyTo is not covered here at all...)

The goal is mainly to encourage people to chime in, saying what could be "The Right Orientation" for each of the createSampleOption cases in this sandcastle:

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

let currentTileset;

function createTilesetObject(center) {
  const tilesetObject = {
    asset: {
      version: "1.1",
    },
    geometricError: 1e10,
    root: {
      refine: "ADD",
      boundingVolume: {
        sphere: [
          center[0],
          center[1],
          center[2],
          12000000,
        ],
      },
      geometricError: 1.0,
    },
  };
  return tilesetObject;
}

function createTilesetUrl(center) {
  const tilesetObject = createTilesetObject(center);
  const url =
    "data:application/json;base64," + btoa(JSON.stringify(tilesetObject));
  return url;
}

function zoomToTilesetFixed(targetTileset) {
  const boundingSphere = targetTileset.boundingSphere;
  const transform = Cesium.Matrix4.fromTranslation(boundingSphere.center);
  const offset = new Cesium.Cartesian3(boundingSphere.radius, 0, 0);
  viewer.camera.lookAtTransform(transform, offset);
}

async function createTileset(center) {
  if (Cesium.defined(currentTileset)) {
    viewer.scene.primitives.remove(currentTileset);
    currentTileset = undefined;
  }
  const url = createTilesetUrl(center);
  currentTileset = viewer.scene.primitives.add(
    await Cesium.Cesium3DTileset.fromUrl(url, {
      debugShowBoundingVolume: true,
    })
  );
  viewer.zoomTo(currentTileset);
  //zoomToTilesetFixed(currentTileset);
}

function createSampleOption(center) {
  return {
    text: center.toString(),
    onselect: async function () {
      try {
        await createTileset(center);
      } catch (e) {
        console.log(e);
      }
    },
  };
}

const sampleOptions = [
  createSampleOption([0, 0, 0]),
  createSampleOption([1e-15, 1e-15, 1e-15]),
  createSampleOption([1e-12, 1e-15, 1e-15]),
  createSampleOption([1e-15, 1e-12, 1e-15]),
  createSampleOption([1e-10, 1e-10, 1e-10]),
  createSampleOption([1e-5, 1e-5, 1e-5]),
  createSampleOption([0, 0, 1]),
  createSampleOption([0, 1, 0]),
  createSampleOption([1, 0, 0]),
  createSampleOption([100, 0, 0]),
  createSampleOption([0, 100, 0]),
  createSampleOption([0, 0, 100]),
  createSampleOption([10000, 0, 0]),
  createSampleOption([0, 10000, 0]),
  createSampleOption([0, 0, 10000]),
  createSampleOption([1000000, 0, 0]),
  createSampleOption([0, 1000000, 0]),
  createSampleOption([0, 0, 1000000]),
  createSampleOption([10000000, 0, 0]),
  createSampleOption([0, 10000000, 0]),
  createSampleOption([0, 0, 10000000]),
  createSampleOption([14000000, 0, 0]),
  createSampleOption([0, 14000000, 0]),
  createSampleOption([0, 0, 14000000]),
];
Sandcastle.addToolbarMenu(sampleOptions);

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have update the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code
Copy link

Thank you for the pull request, @javagl!

✅ We can confirm we have a CLA on file for you.

@javagl
Copy link
Contributor Author

javagl commented Sep 19, 2024

The approach that is shown here is a draft. Most of the value of this PR might be the sandcastle with the test cases. But it is still (too) far away from a robust, generic, agreed-upon, universal solution to the problem to warrant keeping it open any longer. (Or to put it that way: Whatever we're going to do: It will be vastly different from what is currently done in this PR).

@javagl javagl closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant