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.
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
MRG, ENH: Add a screenshot button to the notebook 3d backend #8708
MRG, ENH: Add a screenshot button to the notebook 3d backend #8708
Changes from 35 commits
d236c1c
95ab3ec
82076aa
07be3c7
a5cbed0
a284f43
e3198f7
ed67a41
1a13ea7
0fd9744
4dd9fc9
4073c1d
11663a2
9ca7736
9101cb2
12a729b
e2667af
395bda1
9e18f6f
3c2f5c0
a22f5b3
a52f8e9
69069d7
ab544b7
d22c1c3
7456b41
031d4dc
54fe438
972b8a8
930fdcf
3c0b083
2c17547
34c804c
8e927e7
5d34669
1e28b73
2a96cdd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@GuillaumeFavelier this seems to work for both code paths now on Linux and macOS for me (notebook and standard renderer)
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.
This is awesome news!
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.
I'm not sure how critical the iteration over interactive levels was so I killed it to save time now that this isn't marked
slowtest
.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.
Notebook always renders non-HiDPI. At some point we should look into how to enable HiDPI support in the notebook -- basically we need the background renderer to be
ratio
(usually 2) times the size, and we need to tell it to display the image in native resolution (rather than treating physical and logical pixels the same). I suspect this is supported at some level in browsers, but TBD whether ipyvtk_simple allows setting this display resolution factor (i.e., to make images that are 600x600 pixels show up in a 300x300 logical pixel HTML element). Could be a cool PR there. Then we'd need a way to probe the physical-to-logical pixel ratio. Ideally this would come from the notebook but if not, we can quickly import Qt and query the screens and take a best guess...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.
HiDPI aside, on Linux I was getting values that were off by ~7%. It would be good if this were as good as the non-notebook backend (which uses
atol=3
instead of thertol=0.1
here).