Skip to content

feat: per-layer bounding box tracking with removal support #2573

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rubenthoms
Copy link
Collaborator

  • Replaces global bounding box reducer with per-layer tracking
  • Automatically recomputes unioned bounding box via useMemo
  • Adds debounced cleanup effect to remove bounding boxes for unmounted layers
  • Adds Storybook story to demonstrate dynamic camera adjustment on add/remove

- Replaces global bounding box reducer with per-layer tracking
- Automatically recomputes unioned bounding box via useMemo
- Adds debounced cleanup effect to remove bounding boxes for unmounted layers
- Adds Storybook story to demonstrate dynamic camera adjustment on add/remove
@rubenthoms rubenthoms requested review from hkfb and w1nklr June 6, 2025 12:09
@rubenthoms rubenthoms self-assigned this Jun 6, 2025
@rubenthoms rubenthoms added the enhancement New feature or request label Jun 6, 2025
@rubenthoms rubenthoms changed the title feat: per-layer bounding boxes with removal support feat: per-layer bounding box tracking with removal support Jun 6, 2025
@w1nklr
Copy link
Collaborator

w1nklr commented Jun 6, 2025

The use case for us is that data is retrieved asynchronously per data type.
Roughly, the app wants to display surfaces (faults), and wells. These entities are known by there uris.

Viewers component issues 2 GraphQL calls to retrieve faults and wells.
Due to async data fetching, re-rendering occurs when data arrives. Say first the wells and then the faults.

This means for webviz, there are 3 rendering steps:

  • first rendering is without data but with decoration
  • second re-rendering comes with wells => this computes the camera settings from a view direction and the 3D bounding box.
  • third re-rendering already has the wells but with new surfaces => what should be the behavior ???

The current implementation was driven by the following rationale (but implementation could be poor/wrong ;)):

  1. if the user did not interact with the camera, it makes sense to update the camera settings to see the whole data
  2. if the user did change the point of view, it should be respected. At this point adding/removing data, thus modifying the bounding box, should not impact the camera.

As you noticed, in case 1 does only grow the bounding box. This can be discussed (actually, I don't really mind, since our use case does only add data due to async fetching).
But we really need to respect case 2. once the user starts interacting with the camera, adding or removing data should not change the point of view.

Note: I did not yet review the PR to see its impact. I just wanted to explain the current behavior.

@rubenthoms
Copy link
Collaborator Author

rubenthoms commented Jun 10, 2025

The use case for us is that data is retrieved asynchronously per data type. Roughly, the app wants to display surfaces (faults), and wells. These entities are known by there uris.

Viewers component issues 2 GraphQL calls to retrieve faults and wells. Due to async data fetching, re-rendering occurs when data arrives. Say first the wells and then the faults.

This means for webviz, there are 3 rendering steps:

  • first rendering is without data but with decoration
  • second re-rendering comes with wells => this computes the camera settings from a view direction and the 3D bounding box.
  • third re-rendering already has the wells but with new surfaces => what should be the behavior ???

The current implementation was driven by the following rationale (but implementation could be poor/wrong ;)):

  1. if the user did not interact with the camera, it makes sense to update the camera settings to see the whole data
  2. if the user did change the point of view, it should be respected. At this point adding/removing data, thus modifying the bounding box, should not impact the camera.

As you noticed, in case 1 does only grow the bounding box. This can be discussed (actually, I don't really mind, since our use case does only add data due to async fetching). But we really need to respect case 2. once the user starts interacting with the camera, adding or removing data should not change the point of view.

Note: I did not yet review the PR to see its impact. I just wanted to explain the current behavior.

@w1nklr Thanks for the detailed explanation. I see your point with case 2 and that makes total sense. Our use case is that we might change fields in between. Hence, the total bounding box is becoming very large, respecting irrelevant/outdated data. I think our main concern is that homing does not work properly anymore in this case. We can probably work around by recreating the viewer component and, hence, resetting the bounding box. However, in my opinion, homing should in any case only respect the existing (optionally maybe even only visible?) layers. A good solution would be to keep track of the bounding boxes of only existing layers and if the user changed the camera. No auto-apply when the camera was changed but still support for proper homing. We can leave this open for discussion/better solutions.

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

Successfully merging this pull request may close these issues.

2 participants