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

Finish WindowHandle to WinCtx migration #95

Closed
raphlinus opened this issue Aug 3, 2019 · 3 comments
Closed

Finish WindowHandle to WinCtx migration #95

raphlinus opened this issue Aug 3, 2019 · 3 comments

Comments

@raphlinus
Copy link
Contributor

This is a continuation of a stream of work started in #52 and #76. That kicked off a refactor where we're passing down a mutable reference to a WinCtx in window handler functions, rather than having shell and window handler share a reference to a WindowHandle and use interior mutability to access it.

We've migrated some functions to WinCtx (invalidation, text factory, and setting cursor), but others remain in WindowHandle. It'd be nice to migrate most of the rest, or maybe even all of them and remove WindowHandle entirely.

Important remaining methods on WindowHandle, with notes on how to handle them:

  • close. Shouldn't provide any special difficulty, just needs per-platform implementation.

  • hi-dpi methods. These also shouldn't be difficult, but I'd like to change the conventions a little. Right now, they're expressed (in a Windows-centric way) as a dpi value, with a nominal value of 96.0. I'd rather use a dpi scaling factor, with a nominal value of 1.0.

  • file dialogs. These are trickier. It would be possible to just move the existing API over, but the idea of doing a blocking call to create a modal dialog from inside the window handler creates problems. A better API would be to request a file dialog (as a method on WinCtx), and pass in a closure which should be called on completion of the dialog. The implementation would then wait until control passes up from the window handler, and then do the blocking call from inside the shell, specifically, while not holding a mutable borrow on WindowState. This would let handlers run (eg for painting the window) while the modal dialog is up. It would also adapt much more easily to a non-modal dialog, which might be better UX.

  • idle handle (for external events). This is probably more of a convenience, as the idle handle will continue to be implemented with a shared reference and interior mutability.

  • show. This doesn't need to be called by the window handler, only by whoever created the window. Possibly we split this up so the object returned by WindowBuilder::build() is different than the one internal to the window handler. This could get slightly more interesting when we have multiple windows. Also, it's possible we simplify the API and make the need to this method go away, all windows are implicitly shown right after WindowBuilder::build (we might need to dig into whether there are useful things that should be done after build but before show).

@edwin0cheng
Copy link
Collaborator

edwin0cheng commented Jan 23, 2020

file dialogs. These are trickier. It would be possible to just move the existing API over, but the idea of doing a blocking call to create a modal dialog from inside the window handler creates problems. A better API would be to request a file dialog (as a method on WinCtx), and pass in a closure which should be called on completion of the dialog. The implementation would then wait until control passes up from the window handler, and then do the blocking call from inside the shell, specifically, while not holding a mutable borrow on WindowState. This would let handlers run (eg for painting the window) while the modal dialog is up. It would also adapt much more easily to a non-modal dialog, which might be better UX.

I want to work on this one because I have a usage which is the same as the problem described in #115 which I want to fix.

@teddemunnik
Copy link
Contributor

It seems like WinCtx does't exist anymore, is this issue still valid?

@cmyr
Copy link
Member

cmyr commented Mar 17, 2020

you're right, this is outdated. thanks!

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

No branches or pull requests

4 participants