-
-
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 precision of surface3d worldPosition calculation #6999
Improve precision of surface3d worldPosition calculation #6999
Conversation
When the objectOffset is very large compared to the values in the localCoordinate, adding it to the localCoordinate loses precision in the localCoordinate. As the addition of the objectOffset was immediately undone by application of the model matrix, it is better to combine the model matrix and the translation by the objectOffset into one matrix and apply it directly to the localCoordinate.
Awesome. I was thinking about this. Thank you very much! |
Please download and extract |
stackgl_modules/package.json
Outdated
@@ -28,7 +28,7 @@ | |||
"gl-spikes2d": "^1.0.2", | |||
"gl-spikes3d": "^1.0.11", | |||
"gl-streamtube3d": "^1.4.2", | |||
"gl-surface3d": "^1.6.1", | |||
"gl-surface3d": "github:hborchardt/gl-surface3d#6c75a5b44781c2aaec543237312df1f14274feac", |
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 a PR to gl-vis/gl-surface3d
and provide the link here.
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.
gl-surface3d@1.6.2
is published. Please install new patch.
Some tests are failing. |
c9ac34b
to
e12bb74
Compare
To lock this bug from happening again, I suggest you add a new mock inside |
I accidentally reverted the change to the |
draftlogs/6999_fix.md
Outdated
@@ -0,0 +1 @@ | |||
- Fix numerical precision in 3D surface plots [[6999](https://github.com/plotly/plotly.js/pull/6999)] |
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.
- Fix numerical precision in 3D surface plots [[6999](https://github.com/plotly/plotly.js/pull/6999)] | |
- Fix numerical precision of drawing surface trace [[6999](https://github.com/plotly/plotly.js/pull/6999)], | |
with thanks to @hborchardt for the contribution! |
I'd love to include this PR in the upcoming |
Hi, I can do it later today. When building the test case I saw that there
are other problems when the time range is on the order seconds or minutes,
and I was playing around trying if I can fix it as well, but it is more
complicated. So I will add a test with a time range of about an hour which
should be good.
Mojtaba Samimi ***@***.***> schrieb am Fr. 17. Mai 2024 um
17:46:
… I'd love to include this PR in the upcoming plotly.js release.
@hborchardt <https://github.com/hborchardt> Wondering if you are going to
complete this PR today?
I.e. install new gl-surface3d package and if possible adding a test.
If not, I may add those commits.
Thank you!
—
Reply to this email directly, view it on GitHub
<#6999 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AP2VDRP5DQ6VAGQRLODNNELZCYQ3ZAVCNFSM6AAAAABH2VPMY2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJXHA4DAMRQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks for the quick reply. |
Excellent bug fix! |
Plotly.js v2.33.0 is out: https://github.com/plotly/plotly.js/releases/tag/v2.33.0 |
When the objectOffset is very large compared to the values in the localCoordinate, adding it to the localCoordinate loses precision in the localCoordinate. As the addition of the objectOffset was immediately undone by application of the model matrix, it is better to combine the model matrix and the translation by the objectOffset into one matrix and apply it directly to the localCoordinate.
Again, it's an issue mostly present when rendering data from a small time range.
Before:
![image](https://private-user-images.githubusercontent.com/66408901/331322435-b49dbf85-ebee-47ec-aa65-6c60163f9782.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjU0MjU3OTksIm5iZiI6MTcyNTQyNTQ5OSwicGF0aCI6Ii82NjQwODkwMS8zMzEzMjI0MzUtYjQ5ZGJmODUtZWJlZS00N2VjLWFhNjUtNmM2MDE2M2Y5NzgyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA5MDQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwOTA0VDA0NTEzOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWIxYzUyNGU5MTkzZGU0NWI0NWIwOTJjNjBmZjRkMDQwNjJlNzU0NTcwNmFkZmE3OGQxNjg2NTk4NzhlMzU2OGQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.j3kI0VX9UykPdlLOaayPdgNQUOKVZqOnIYdt32vujFY)
After:
![image](https://private-user-images.githubusercontent.com/66408901/331321453-68431da0-a13d-49bb-b64d-9bacd60afd2d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjU0MjU3OTksIm5iZiI6MTcyNTQyNTQ5OSwicGF0aCI6Ii82NjQwODkwMS8zMzEzMjE0NTMtNjg0MzFkYTAtYTEzZC00OWJiLWI2NGQtOWJhY2Q2MGFmZDJkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA5MDQlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwOTA0VDA0NTEzOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTg2MjgzOGU4Yjg1YTQ1MmI2NTE3MDBiZTI0M2VlMzgxMGNiODgzMWM2OTU3M2MyNDc1MTc3NjhmNzUzYmJlOTgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.GiXd1xrSmJMyRcQ2jxxQEe2T-1HWeR1kWn6-3Ob1_Lg)