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

feat: PolylinesLayer typed arrays input. #1949

Merged

Conversation

LeonidPolukhin
Copy link
Collaborator

  1. PolylinesLayer can accept typed arrays and use them as webgl attributes when possible.
  2. ZIncreasingDownwards flag is applied in the vertex shader.
  3. depthTest prop is added for uniformity in API with the other layers.

@LeonidPolukhin LeonidPolukhin added enhancement New feature or request AspenTech Task owned by AspenTech map-component Issues related to the map component. labels Mar 5, 2024
@LeonidPolukhin LeonidPolukhin self-assigned this Mar 5, 2024
@hkfb
Copy link
Collaborator

hkfb commented Mar 5, 2024

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

@LeonidPolukhin
Copy link
Collaborator Author

LeonidPolukhin commented Mar 5, 2024

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

Yes, I did. It generated three new images which I submitted but the test fails with disappearing image difference.
The last run: 0.00010850694444444444% different from snapshot (1 differing pixels)
However, in order to run snapshot update on windows with docker running on WSL I needed to modify package.json like this:
"storybook:test": "test-storybook --url http://host.docker.internal:6006/ --no-index-json"

@hkfb
Copy link
Collaborator

hkfb commented Mar 5, 2024

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

Yes, I did. It generated three new images which I submitted but the test fails with disappearing image difference. The last run: 0.00010850694444444444% different from snapshot (1 differing pixels) However, in order to run snapshot update on windows with docker running on WSL I needed to modify package.json like this: "storybook:test": "test-storybook --url http://host.docker.internal:6006/ --no-index-json"

Could be that the stories are unstable. The axes z order seems not completely stable. You can add the "no-test" tag to skip stories in the smoke tests

@LeonidPolukhin
Copy link
Collaborator Author

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

Yes, I did. It generated three new images which I submitted but the test fails with disappearing image difference. The last run: 0.00010850694444444444% different from snapshot (1 differing pixels) However, in order to run snapshot update on windows with docker running on WSL I needed to modify package.json like this: "storybook:test": "test-storybook --url http://host.docker.internal:6006/ --no-index-json"

Could be that the stories are unstable. The axes z order seems not completely stable. You can add the "no-test" tag to skip stories in the smoke tests

Skipped for now

@hkfb
Copy link
Collaborator

hkfb commented Mar 5, 2024

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

Yes, I did. It generated three new images which I submitted but the test fails with disappearing image difference. The last run: 0.00010850694444444444% different from snapshot (1 differing pixels) However, in order to run snapshot update on windows with docker running on WSL I needed to modify package.json like this: "storybook:test": "test-storybook --url http://host.docker.internal:6006/ --no-index-json"

Could be that the stories are unstable. The axes z order seems not completely stable. You can add the "no-test" tag to skip stories in the smoke tests

Skipped for now

Suggest also removing the corresponding snapshots.

@LeonidPolukhin
Copy link
Collaborator Author

Btw, did you use the docker container (npm run docker:storybook:test:update) to update the snapshots?

Yes, I did. It generated three new images which I submitted but the test fails with disappearing image difference. The last run: 0.00010850694444444444% different from snapshot (1 differing pixels) However, in order to run snapshot update on windows with docker running on WSL I needed to modify package.json like this: "storybook:test": "test-storybook --url http://host.docker.internal:6006/ --no-index-json"

Could be that the stories are unstable. The axes z order seems not completely stable. You can add the "no-test" tag to skip stories in the smoke tests

Skipped for now

Suggest also removing the corresponding snapshots.

Deleted

Copy link
Collaborator

@hkfb hkfb left a comment

Choose a reason for hiding this comment

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

Looks like there is some artifact in the new layer (to the right) compared to the previous. Is this intentional?

image

@LeonidPolukhin
Copy link
Collaborator Author

LeonidPolukhin commented Mar 6, 2024

Looks like there is some artifact in the new layer (to the right) compared to the previous. Is this intentional?

image

Indeed. Closed polylines are calculated on the backend by adding the first point as the last one and all the polylines are marked as "open" to prevent the same calculations in deck.gl. As all the polylines are rendered as open ones "z-fighting" can appear where two points coincide.

@LeonidPolukhin LeonidPolukhin merged commit 0829b5a into equinor:master Mar 6, 2024
9 checks passed
hkfb pushed a commit that referenced this pull request Mar 6, 2024
@hkfb
Copy link
Collaborator

hkfb commented Mar 6, 2024

🎉 This issue has been resolved in version subsurface-viewer@0.19.0 🎉

The release is available on GitHub release

@hkfb hkfb added the released label Mar 6, 2024
@LeonidPolukhin LeonidPolukhin deleted the PolylinesLayerImprovement branch June 18, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AspenTech Task owned by AspenTech enhancement New feature or request map-component Issues related to the map component. released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NGRM]: PolylinesLayer performance and memory usage improvements.
2 participants