Skip to content

fix: use picked coordinate instead of unprojecting screen coordinates when multipicking#2649

Merged
HansKallekleiv merged 5 commits into
equinor:masterfrom
HansKallekleiv:fix-multiview-picking
Nov 3, 2025
Merged

fix: use picked coordinate instead of unprojecting screen coordinates when multipicking#2649
HansKallekleiv merged 5 commits into
equinor:masterfrom
HansKallekleiv:fix-multiview-picking

Conversation

@HansKallekleiv

@HansKallekleiv HansKallekleiv commented Oct 23, 2025

Copy link
Copy Markdown
Collaborator

Relates to equinor/webviz#1229

The multi-picking hook used 2d screen coordinates to unproject to world coordinates.
For 3d view we need the z-value as well.
https://deck.gl/docs/api-reference/core/viewport#unproject

Fix:
Use coordinates from first picked object as reference point.
If picking outside, uses the default.

);

const worldCoordinate =
activePickingInfo[0]?.coordinate ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer using ?? instead of ||

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@w1nklr I am struggling to run the smoke tests locally. I assume this has to be done through the docker to reproduce the same environment? I cant get this approach to work.

npm run docker:build && npm run docker:storybook:test
...
[test-storybook] It seems that your Storybook instance is not running at: http://host.docker.internal:6006/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is storybook running locally?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it doesn't resolve host.docker.internal?

Can you try:
npm run docker:storybook:test -- npm run storybook:test -- --url http://localhost:6006/ --no-index-json --updateSnapshot

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to run storybook with npm run storybook -- --host 0.0.0.0
Its probably because of WSL, which uses its own VM and interface. Docker runs in its own VM. Windows is the host.
I.e. they have different localhost. Binding to 0.0.0.0 opens it up to all interfaces.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started on a PR to run both storybook and test-runner with docker-compose: #2619

Should make it more reliable, but also some drawbacks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this there was no need to run the smoke test after all. They shouldnt change.
Ready for review now.

@HansKallekleiv HansKallekleiv requested a review from w1nklr October 27, 2025 07:09
@HansKallekleiv HansKallekleiv merged commit d87f339 into equinor:master Nov 3, 2025
9 checks passed
@HansKallekleiv HansKallekleiv deleted the fix-multiview-picking branch November 3, 2025 12:26
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

Successfully merging this pull request may close these issues.

3 participants