-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Improving the quality of surface objects to better follow the data by using LCM or highly composite numbers #3281
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
d1065b5
increased resolution for gl-surface3d plots
archmoj 7acba04
baseline improvements
archmoj bc8f75b
increased time outfor gl-surface3d objects
archmoj e4a676c
minimal fix for issue 2713
archmoj 08b24fd
updated baseline using minimal fix
archmoj 51ca3e3
before more improvements based on compiste numbers
archmoj b534987
adding getter functions
archmoj b18959c
now using identical pad function inside gl-surface3d - and YES no nee…
archmoj 9cd9de8
linking new gl-surface3d module
archmoj fc619b3
function changes
archmoj 13cb09c
improved logic
archmoj c0cf4d1
using least common multiple between min steps of x and y axes
archmoj 8511400
finding identical best resolution between two axes steps
archmoj 66a85e1
updated surface baselines using new refine logic
archmoj 0c1e2b2
improved logic needed to reduce high-res cases
archmoj c09b40c
improved logic needed to reduce high-res cases - fixed syntax
archmoj bd15b4e
improved logic needed to reduce high-res cases - baseline
archmoj 8d89eb3
apply max res when reduction reached lower bound
archmoj 1832abf
using gl-surface3d 1.3.8
archmoj 292d48c
removed unsed input data in ibm mock
archmoj 9bf1495
correct ibm baseline
archmoj b8e99b8
apply min resolution for uniform grids instead of 128
archmoj 670f15e
fix dstRes
archmoj 97a9368
revert resDst for the case of 2
archmoj ae88c63
separated LCMs for X and Y
archmoj ff6c393
fix for issue 2713
archmoj 1f07e9a
updated baselines for issue 2713
archmoj e0f5de2
fixing issue 2208 as well
archmoj 59a51f5
new baseline with refine disabled to keep high resolution in one dire…
archmoj 174458a
adding more primes and optimized function to return faster
archmoj bd8caf1
big prepration to solve floating issues
archmoj f0c7092
reuse get functions
archmoj fcb5a86
transalte used in colorscale
archmoj ad1208b
applying offset in spikes
archmoj ac319ef
better fix for issue 2208 and prepration for issue-0179 and surfaces …
archmoj 69f250e
updated baseline
archmoj bd966d8
revised fix for issue 2208
archmoj b7bfd7a
fix worldOffset
archmoj 288e709
apply world coordinate
archmoj 7da5c08
world > object
archmoj 93fb239
using new log
archmoj 6719353
using new gl-surface3d branch
archmoj 5a5c55b
using changes in gl-surface3d branch
archmoj 926e1a0
baselines updated using new gl-surface3d
archmoj bb51eb7
removed unsed var
archmoj f73b758
updated surface3d module
archmoj 0271961
fix fragment and contour shaders
archmoj bd3b9da
turn off refinements
archmoj 144768c
fix for mesh refinement
archmoj 088a2c7
Merge pull request #3299 from plotly/issue-0179
archmoj 7b302d3
fix to revert ribbons
archmoj 601bbcd
fix robbine baseline...
archmoj b8047c3
using mediump shaders and fixed the typo of the week
archmoj be99193
fixes for parametric cases e.g. torus
archmoj 999ab09
function syntax
archmoj 7a051c9
baseline changes using new function
archmoj 0305c8d
added new mock with baseline using v.1.42.5
archmoj f227e2b
baseline using branch
archmoj 8763093
added parametric surface mock and old baseline using release 1.42.5
archmoj 779aeca
slightly better baseline using branch and objectOffset
archmoj 9c65eef
Merge remote-tracking branch 'origin/master' into issue-2713 to use c…
archmoj 109344f
correct baseline using this branch
archmoj 8860759
removed unused midValue attribute
archmoj 4ea04d5
fixing the function to use totalDist
archmoj 4608966
improve mock for prime edge cases
archmoj 392ec3e
correct baselines using surface updates
archmoj 6140b37
removed objectOffset bound modifications from gl3d scene
archmoj 8317455
improved parametric surface mock
archmoj b50e4a3
correct new surface baselines
archmoj f948f45
bump gl-surface version to 1.4.0
archmoj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified
BIN
+358 Bytes
(100%)
...aselines/gl3d_mesh3d_surface3d_scatter3d_line3d_error3d_log_reversed_ranges.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm. I was about to ask if you noticed any performance deterioration on your branch. I guess this here answers my question. 🤔
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.
... would there be a way to increase the resolution just for traces that need it? In other words, is a way to determine if a given
z
2D array requires extra resolution?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.
One idea could be to allow resetting this resolution via the api.
I can imagine of auto-detect solutions; but considering animations I suggest we keep this constant.
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.
@etpinard Rather than using
1024
, now by simply using120
instead of `128' the performance is untouched; while the quality of the graphs are remarkably improved. If we want to go further than that the next magic values that could be applied are 180, 240, 360, 720 and 840.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.
Fantastic! Could you explain briefly why
120
is better than128
?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.
128 = 27 so it is divisible by (1 + 7) numbers {1, 2, 4, 8, 16, 32, 64, 128}
120 = 23x31x51 therefore divisible by (1+3)x(1+1)x(1+1) = 16 numbers {1, 2, 3, 4, 5, 6, 8, 10, 12, 15, 16, 20, 24, 30, 60, 120}.
So it is more likely that the refined grid match actual data points in this case.
https://en.wikipedia.org/wiki/Highly_composite_number
Base on this investigation I am now thinking to rewriting the function that refines scale.
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.
Wow @archmoj is the man! Thanks!