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.
Please open a PR to gl-vis/gl-surface3d and provide the link here.
Thank you!
There was a problem hiding this comment.
There was a problem hiding this comment.
gl-surface3d@1.6.2 is published. Please install new patch.
|
Some tests are failing. |
hborchardt
force-pushed
the
hborchardt/3dsurface_offset_precision
branch
from
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.
| - 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. |
|
For reference, this is the added test image rendered with current |
|
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:
After:
