Skip to content
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

[WGLMakie] Add automatic figure resizing #3042

Closed
wants to merge 4 commits into from

Conversation

staticfloat
Copy link
Contributor

Description

This implements basic resizing support for WGLMakie figures. This has primarily been tested in the VSCode plot pane and the JSServe auto-launched browser window (using Chrome and Safari).

The implementation creates a new resize event that the javascript frontend can send back to the Julia backend, with a computed size (in pixels) that the backend should resize to. Additionally, the javascript throttle() function is altered to no longer drop the last few events when throttled, it instead delays the last event until the end of the throttling period, which is important when resizing quickly so that the final size is faithfully recorded.

Fixes #1318

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@staticfloat
Copy link
Contributor Author

NOTE: I've run deno bundle in order to be able to use this branch on projects, but the diff to the bundled source seems pretty large; perhaps I'm using a different version of deno or something?

This implements basic resizing support for WGLMakie figures.  This has
primarily been tested in the VSCode plot pane and the JSServe
auto-launched browser window (using Chrome and Safari).

The implementation creates a new `resize` event that the javascript
frontend can send back to the Julia backend, with a computed size (in
pixels) that the backend should resize to.  Additionally, the javascript
`throttle()` function is altered to no longer drop the last few events
when throttled, it instead delays the last event until the end of the
throttling period, which is important when resizing quickly so that the
final size is faithfully recorded.
@SimonDanisch
Copy link
Member

Cool! :) Will need to test this, to make sure it doesn't behave weirdly on pluto etc...

but the diff to the bundled source seems pretty large; perhaps I'm using a different version of deno or something?

I think is bound to happen when checking in something automatically generated.
I still need to get around to make this nicer, but I had the problem that I needed JSServe + WGLMakie to run on systems that have no deno binaries, so not having to install deno on the target platform was needed and therefore checking in the bundles.

Ideally, the bundles would be artifacts, I think @fonsp did something like that.

@SimonDanisch
Copy link
Member

Welp, pluto doesn't like it ^^
image
And IJulia neither.
If nobody has a smart way to make this work in IJulia/Pluto, while allowing to auto resize in the right contexts (or to the right size?), I'd say we make this optional for now via WGLMakie.activate!(resize_to_parent=true) or set_theme!(WGLMakie=(; resize_to_parent=true)).
Open to a better name, but window / screen seems wrong, since it's not really resizing to a window, but just to the html parent it's inserted in.

@SimonDanisch SimonDanisch mentioned this pull request Jul 4, 2023
I tried to make this work everywhere, but the layout changes too much
between JSServe, Pluto, VSCode, etc.... so just special-case when we're
inside of VSCode as I think that's the most egregious example
@staticfloat
Copy link
Contributor Author

@SimonDanisch as an alternative to #3044, what do you think of this latest commit; it simplifies things somewhat by forcing all elements between the canvas and the html tag to have height: 100% applied, which allows the resizing calculation to be done much more simply. The most dubious thing about this is that it's basically triggering off of something to figure out if we're in a VSCode plot panel or not. I gave up trying to make it work with JSServe, as there are too many pathways where we may or may not get elements without a height: 100% applied.

@SimonDanisch
Copy link
Member

Hm, I think I'd rather have more complicated height/width calculation then having to mess with css/html that we don't own (and relying on finding the VSCode div).

@kbarros
Copy link
Contributor

kbarros commented Jul 6, 2023

It looks like the aspect ratio of the rendered image changes with the panel dimensions. See attached two images. Is there a way to specify a fixed aspect ratio to avoid this distortion? Tested on Mac/VSCode.

Edit: This changing aspect ratio differs from the existing GLMakie behavior.

image image

@SimonDanisch
Copy link
Member

What axis type is this?

@kbarros
Copy link
Contributor

kbarros commented Jul 7, 2023

What axis type is this?

LScene: https://github.com/SunnySuite/Sunny.jl/blob/main/src/Plotting.jl#L5

@SimonDanisch
Copy link
Member

What happens if you do the same with GLMakie? I thought LScene shouldn't change the aspect ratio.
But since resizing should work pretty similar between GLMakie and this PR, I'd be surprised if the aspect ratio change is due to this PR.

@kbarros
Copy link
Contributor

kbarros commented Jul 7, 2023

If I try with GLMakie then the aspect ratio is preserved. (But the image inside VSCode is static, i.e. not interactive)

@kbarros
Copy link
Contributor

kbarros commented Jul 9, 2023

Similar distortion happens inside Jupyter notebook (see attached image). However, it's exciting to see that this PR is so close to working!

image

@SimonDanisch
Copy link
Member

Merged in #3044

@staticfloat staticfloat deleted the sf/vscode_resizing branch August 31, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WGLMakie Resize Feature
3 participants