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

Support resizing windows after creation #136

Merged
merged 19 commits into from Jan 1, 2023

Conversation

robbert-vdh
Copy link
Member

I've implemented this for Linux back in March, and I only just added Windows support. Still need to add macOS support, but I thought I'd submit a draft PR already because the changes needed for Windows support are pretty invasive. So we may want to discuss the best course of action first.

Resizing the window sends a WM_SIZE event and a couple other events to the window, so the window procedure and WindowState had to be changed to use interior mutability everywhere instead of wrapping the entire struct in a RefCell. This resolves most of the reentrant event call concerns from #130, but I'm not sure if it also prevents panics in that specific example To make this work there's now a deferred_tasks queue for handling tasks that are the result of a call to one of the Window methods in an event handler that may also indirectly emit other events. Perhaps Window::close() should also be implemented this way.

The Linux implementation was extremely straightforward, and I'll get to the macOS one next.

This closes #121.

And update the window info for any future events.
This is a prerequisite for being able to handle reentrant messages.
Everything is single threaded so there's no need to use Arcs.
@robbert-vdh
Copy link
Member Author

So one thing I noticed is that the behavior for window resize notifications is different between the different platforms. On Windows the window handler would receive an event after the resize had successfully taken place, and on Linux this didn't happen. I changed the Linux implementation so it also does that.

On Windows and macOS the implementations also sent you duplicate Event::Window(WindowEvent::Resized(window_info)) events even if the new size is the same as the old one, so I changed the behaviors there to also only send the event if something changed.

@robbert-vdh
Copy link
Member Author

So the Linux and Windows versions work great. The macOS version doesn't seem to resize the view at all (standalone windows are still resized, but the contents don't resize). Apple's docs on this are extremely sparse, so if anyone with more experience programming for their platform would like to chime in I'd appreciate it a lot!

To be in line with the Linux implementation.
To follow the same behavior as on Linux and Windows.
Now this works on all baseview platforms.
This fixes resizing with OpenGL applications.
Apparently macOS doesn't do this for you. If you don't do this then
parts of the bottom and right of the window will be cut off.
Otherwise the top row of pixels may contain flickering artifacts.
@robbert-vdh
Copy link
Member Author

Okay, everything should work great now. I'd appreciate it if others could give this a try as well! I've implemented support for this in my vizia fork which is used by nih_plug_vizia. All plugins in that repo that use Vizia will support resizing and you can also try running cargo run -p gain_gui_vizia --release.

@robbert-vdh robbert-vdh marked this pull request as ready for review December 2, 2022 19:19
@robbert-vdh robbert-vdh changed the title [WIP] Support resizing windows after creation Support resizing windows after creation Dec 2, 2022
@robbert-vdh
Copy link
Member Author

Oh and something I have not yet done in this PR but that might improve perceived responsiveness is to draw a new frame as soon as the window resize event is received. Right now the .on_frame() call lags a bit behind the resize.

src/macos/window.rs Outdated Show resolved Hide resolved
I somehow completely missed this the first five times I looked through
the docs.
src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Outdated Show resolved Hide resolved
src/win/window.rs Show resolved Hide resolved
src/win/window.rs Show resolved Hide resolved
This removes the need to pass through individual Rcs at the cost of
adding more lifetime parameters to the internals of the shared `Window`
struct and requiring an `Option` for the handler.
@glowcoil glowcoil merged commit 7001c25 into RustAudio:master Jan 1, 2023
robbert-vdh added a commit to robbert-vdh/vizia that referenced this pull request Jan 5, 2023
robbert-vdh added a commit to robbert-vdh/vizia that referenced this pull request Jan 5, 2023
geom3trik pushed a commit to vizia/vizia that referenced this pull request Jan 7, 2023
#248)

* Add groundwork for window resizing and user scale

These were originally 12 separate commits, but this was quite a hassle
when rebasing with all the frequent breaking changes on the main branch:

Target a baseview branch with window resizing

Add field on ApplicationRunner for new window size

This is needed because we don't own the baseview `window` object, so we
can't interact with it directly from within a view.

Add a WindowDescription field for scale factor

This would be a custom scale factor applied on top of the DPI scaling.
It's useful to keep track of this separately because otherwise dealing
with external DPI changes will be very difficult.

Use the user scale factor in vizia_baseview

This is additional scaling on top of the regular HiDPI scaling. This is
useful to allow GUIs to scale uniformly independently of the system's
HiDPI settings.

Handle resizing and scaling events in baseview

Only perform window resizing on Linux

Since the targetted baseview branch currently only implements it there.

Register the baseview window state model

And move it to a separate reference counted struct to make it happen.

Recompute props when window size or scale changes

Fix artifacts when resizing baseview GUIs

The logical width was used to compute the new physical height.

Print a warning when resizing on a non-Linux OS

Resize and rescale through cx instead of events

Changing a property instead of emitting an event makes the handling much
clearer and it also prevents multiple resizes from happening during a
single frame.

Add a user scale factor modifier

Now that you can no longer pass a `WindowDescription` directly.

Partially rewritten when rebasing on top of 5b0c9a4.

Partially rewritten when rebasing on top of 8299ccf.

* Update baseview for resizing support

* Implement resizing for the baseview backend

Now that RustAudio/baseview#136 has been merged.

* Add Context methods window size and scale factor

There was otherwise no way to obtain this information from a plain
`Context`.

* Rename Context::scale_factor to dpi_factor

To stay consistent with the rest of the renames and to be less
confusing. This is what you should use when working with pixels in
events and drawing code.

* Replace WindowResize with GeometryChanged

* WIP: Add a window resize example

* Rename window_resize example to user_scale

As requested.

* Add a Default impl for Context for doctests

* Add a window size control to the user scale demo
@helgoboss
Copy link

@robbert-vdh Thanks, the deferred_tasks queue is a good way to prevent the issue mentioned in #130, I just tested it! There's one small additional thing necessary to fix the issue: An API to let baseview consumers defer tasks, because the code in #130 which has reentrancy issues is not a part of the baseview crate itself. It's part of egui-baseview. I will make a PR.

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.

Resizing the window through a WindowHandle
3 participants