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

Improve numerical stability of 3D projection #6998

Merged

Conversation

hborchardt
Copy link
Contributor

When the position vector has large values that are cancelled out by large values in the model matrix, it is more numerically stable to first multiply the position by the model matrix instead of multiplying the matrices together first.

This should always yield equivalent or more accurate results.

Use forks of the gl-vis libraries that use the problematic pattern.

Helps with #3306
See gl-vis/gl-axes3d#23

When the position vector has large values that are cancelled out
by large values in the model matrix, it is more numerically stable
to first multiply the position by the model matrix instead of
multiplying the matrices together first.

Use forks of the gl-vis libraries that use the problematic pattern.
@archmoj
Copy link
Contributor

archmoj commented May 15, 2024

Thanks for the PR.
Please download baselines.tar from https://app.circleci.com/pipelines/github/plotly/plotly.js/10465/workflows/5b8aa5a0-5c2e-4237-b231-604b94893e2b/jobs/229556/artifacts
Then extract the files and replace them in test/image/baselines; commit and push.

@archmoj archmoj added bug something broken community community contribution status: reviewable labels May 15, 2024
@archmoj
Copy link
Contributor

archmoj commented May 15, 2024

Please add gl3d_mesh3d_surface3d_scatter3d_line3d_error3d_log_reversed_ranges to this list:

'gl3d_ibm-plot',
'gl3d_isosurface_2surfaces-checker_spaceframe',
'gl3d_opacity-scaling-spikes',
'gl3d_cone-wind',
'gl3d_isosurface_math',
'gl3d_scatter3d-blank-text'
which would help test-baselines-virtual-webgl pass.

"gl-cone3d": "^1.6.0",
"gl-error3d": "^1.0.16",
"gl-axes3d": "github:hborchardt/gl-axes3d#83caafc058b2d60b6006e26432967c411c125fe1",
"gl-cone3d": "github:hborchardt/gl-cone3d#e9728cd068e0dfe11b92c5785974cd463f78c01c",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please open pull requests on all gl-vis/* dependencies and paste the links for every line change in package.json file. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-error3d": "^1.0.16",
"gl-axes3d": "github:hborchardt/gl-axes3d#83caafc058b2d60b6006e26432967c411c125fe1",
"gl-cone3d": "github:hborchardt/gl-cone3d#e9728cd068e0dfe11b92c5785974cd463f78c01c",
"gl-error3d": "github:hborchardt/gl-error3d#ee55620d17f3cc7dfb104823c904f5070aba79ed",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-heatmap2d": "^1.1.1",
"gl-line3d": "1.2.1",
"gl-mesh3d": "^2.3.1",
"gl-line3d": "github:hborchardt/gl-line3d#27123949f65e2d8deab6a6e131030073ed9cdf02",
Copy link
Contributor Author

@hborchardt hborchardt May 15, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-line3d": "1.2.1",
"gl-mesh3d": "^2.3.1",
"gl-line3d": "github:hborchardt/gl-line3d#27123949f65e2d8deab6a6e131030073ed9cdf02",
"gl-mesh3d": "github:hborchardt/gl-mesh3d#be2c7fcb68e70e001bbfb8021380d0d7ac75b1c9",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-plot2d": "^1.5.0",
"gl-plot3d": "^2.4.7",
"gl-pointcloud2d": "^1.0.3",
"gl-scatter3d": "^1.4.0",
"gl-scatter3d": "github:hborchardt/gl-scatter3d#086177000f405bc72f171350f9c1efa6eb523e29",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-select-box": "^1.0.4",
"gl-shader": "4.3.1",
"gl-spikes2d": "^1.0.2",
"gl-streamtube3d": "^1.4.1",
"gl-surface3d": "^1.6.0",
"gl-spikes3d": "github:hborchardt/gl-spikes3d#9ad06b2dd48fa19d2faa54e5645c2e4bf5e6157c",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-streamtube3d": "^1.4.1",
"gl-surface3d": "^1.6.0",
"gl-spikes3d": "github:hborchardt/gl-spikes3d#9ad06b2dd48fa19d2faa54e5645c2e4bf5e6157c",
"gl-streamtube3d": "github:hborchardt/gl-streamtube3d#29c914cc0d88fc5dffce62dcd8169f38115d078b",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

"gl-surface3d": "^1.6.0",
"gl-spikes3d": "github:hborchardt/gl-spikes3d#9ad06b2dd48fa19d2faa54e5645c2e4bf5e6157c",
"gl-streamtube3d": "github:hborchardt/gl-streamtube3d#29c914cc0d88fc5dffce62dcd8169f38115d078b",
"gl-surface3d": "github:hborchardt/gl-surface3d#0728e33c0296c602b147e8f1ea8f1bfe7040c61e",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please install the patch.

@archmoj
Copy link
Contributor

archmoj commented May 15, 2024

Looking at the diffs of baselines (https://app.circleci.com/pipelines/github/plotly/plotly.js/10465/workflows/5b8aa5a0-5c2e-4237-b231-604b94893e2b/jobs/229556/artifacts), the changes are not remarkable on most of except the one ussing dates and larger numbers which is a good sign of fixing the bug.
For reference I pasted the diff of gl3d_world-cals mock below.
diff-gl3d_world-cals

Would you please paste two short videos interacting with gl3d_world-cals or other mock showing before and after effects?

@archmoj
Copy link
Contributor

archmoj commented May 15, 2024

For the before vs after video you may fork and tweak this codepen.

"gl-axes3d": "^1.7.0",
"gl-cone3d": "^1.6.0",
"gl-error3d": "^1.0.16",
"gl-axes3d": "github:hborchardt/gl-axes3d#fcc1c5c49db3de6445020a63a5d76403ec5a2b98",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please install the patch.

@hborchardt
Copy link
Contributor Author

For gl3d_world-cals, I scaled the z axis values to be within a few hours, to make the problem apparent.
On master:
old.webm
On this branch:
new.webm

Unfortunately this did not fix the jagged edges of the surface plot - would have been nice.

@archmoj
Copy link
Contributor

archmoj commented May 15, 2024

@hborchardt , please install new patches of gl-vis modules and we should be good to go 🚀

draftlogs/6998_fix.md Outdated Show resolved Hide resolved
@archmoj
Copy link
Contributor

archmoj commented May 16, 2024

Excellent community PR. Cc: @Coding-with-Adam

Thanks very much @hborchardt
🎖️ 🏅 🥇 🥈 🥉 🏆 ⭐ 🌟 for each of those many dependency fixes :)

@archmoj archmoj merged commit cfccbcc into plotly:master May 16, 2024
1 check failed
@archmoj
Copy link
Contributor

archmoj commented May 29, 2024

Plotly.js v2.33.0 is out: https://github.com/plotly/plotly.js/releases/tag/v2.33.0
Thanks for your contribution.

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

Successfully merging this pull request may close these issues.

2 participants