-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Improve numerical stability of 3D projection #6998
Conversation
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.
Thanks for the PR. |
Please add plotly.js/test/image/compare_pixels_test.js Lines 176 to 181 in 8b09892
test-baselines-virtual-webgl pass.
|
stackgl_modules/package.json
Outdated
"gl-cone3d": "^1.6.0", | ||
"gl-error3d": "^1.0.16", | ||
"gl-axes3d": "github:hborchardt/gl-axes3d#83caafc058b2d60b6006e26432967c411c125fe1", | ||
"gl-cone3d": "github:hborchardt/gl-cone3d#e9728cd068e0dfe11b92c5785974cd463f78c01c", |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-cone3d@1.6.1
stackgl_modules/package.json
Outdated
"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", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-error3d@1.0.17
stackgl_modules/package.json
Outdated
"gl-heatmap2d": "^1.1.1", | ||
"gl-line3d": "1.2.1", | ||
"gl-mesh3d": "^2.3.1", | ||
"gl-line3d": "github:hborchardt/gl-line3d#27123949f65e2d8deab6a6e131030073ed9cdf02", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-line3d@1.2.2
stackgl_modules/package.json
Outdated
"gl-line3d": "1.2.1", | ||
"gl-mesh3d": "^2.3.1", | ||
"gl-line3d": "github:hborchardt/gl-line3d#27123949f65e2d8deab6a6e131030073ed9cdf02", | ||
"gl-mesh3d": "github:hborchardt/gl-mesh3d#be2c7fcb68e70e001bbfb8021380d0d7ac75b1c9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-mesh3d@2.3.2
stackgl_modules/package.json
Outdated
"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", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-scatter3d@1.4.1
stackgl_modules/package.json
Outdated
"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", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-spikes3d@1.0.11
stackgl_modules/package.json
Outdated
"gl-streamtube3d": "^1.4.1", | ||
"gl-surface3d": "^1.6.0", | ||
"gl-spikes3d": "github:hborchardt/gl-spikes3d#9ad06b2dd48fa19d2faa54e5645c2e4bf5e6157c", | ||
"gl-streamtube3d": "github:hborchardt/gl-streamtube3d#29c914cc0d88fc5dffce62dcd8169f38115d078b", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now: gl-streamtube3d@1.4.2
stackgl_modules/package.json
Outdated
"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", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please install the patch.
npm i gl-surface3d@1.6.1
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. Would you please paste two short videos interacting with |
For the before vs after video you may fork and tweak this codepen. |
stackgl_modules/package.json
Outdated
"gl-axes3d": "^1.7.0", | ||
"gl-cone3d": "^1.6.0", | ||
"gl-error3d": "^1.0.16", | ||
"gl-axes3d": "github:hborchardt/gl-axes3d#fcc1c5c49db3de6445020a63a5d76403ec5a2b98", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please install the patch.
npm i gl-axes3d@1.7.1
@hborchardt , please install new patches of |
Excellent community PR. Cc: @Coding-with-Adam Thanks very much @hborchardt |
Plotly.js v2.33.0 is out: https://github.com/plotly/plotly.js/releases/tag/v2.33.0 |
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