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

Subwindow updates #1532

Merged
merged 8 commits into from
Jan 14, 2021
Merged

Subwindow updates #1532

merged 8 commits into from
Jan 14, 2021

Conversation

rjwittams
Copy link
Collaborator

Expose WindowLevel, add get_content_insets, track widgets window relative coordinates.
Add WindowSizePolicy to allow windows to size themselves to their content.
Make scroll contained things re layout if the scroll offset changes.

All of these are to let subwindows like dropdowns be laid out relative to their parent widget.

The drop down widget itself isn't here. It is currently in nursery locally.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. a few little things inline but nothing major. Thanks!

druid-shell/src/platform/gtk/window.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/mac/window.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/mac/window.rs Outdated Show resolved Hide resolved
druid-shell/src/window.rs Outdated Show resolved Hide resolved
druid/src/app.rs Show resolved Hide resolved
druid/src/contexts.rs Outdated Show resolved Hide resolved
druid/src/contexts.rs Outdated Show resolved Hide resolved
druid/src/core.rs Show resolved Hide resolved
druid/src/app.rs Outdated Show resolved Hide resolved
rjwittams and others added 6 commits January 14, 2021 14:02
doc fix

Co-authored-by: Colin Rofls <colin@cmyr.net>
doc fix

Co-authored-by: Colin Rofls <colin@cmyr.net>
doc fix

Co-authored-by: Colin Rofls <colin@cmyr.net>
doc fix

Co-authored-by: Colin Rofls <colin@cmyr.net>
@rjwittams rjwittams merged commit de5f88a into linebender:master Jan 14, 2021
@rjwittams rjwittams deleted the subwindow-updates branch January 14, 2021 15:04
@@ -13,6 +13,9 @@ You can find its changes [documented below](#070---2021-01-01).
- RichTextBuilder ([#1520] by [@Maan2003])
- `get_external_handle` on `DelegateCtx` ([#1526] by [@Maan2003])
- `AppLauncher::localization_resources` to use custom l10n resources. ([#1528] by [@edwin0cheng])
- Shell: get_content_insets and mac implementation ([#XXXX] by [@rjwittams])
- Contexts: to_window and to_screen (useful for relatively positioning sub windows) ([#XXXX] by [@rjwittams])
- WindowSizePolicy: allow windows to be sized by their content ([#XXXX] by [@rjwittams])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjwittams might want to pr something to fix these up?

@@ -156,7 +163,7 @@ impl WindowBuilder {
handler: None,
title: String::new(),
menu: None,
size: Size::new(500.0, 400.0),
size: Size::new(0., 0.),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See issue #1543: because of this change, windows don't appear at all on Mac if their size are not explicitly set.

@cmyr
Copy link
Member

cmyr commented Feb 18, 2021

@rjwittams I am hitting a mysterious exception thrown in a cocoa paint method that bisect has tracked down to de5f88a.

As far as I've tested this only happens on Catalina, although I can reproduce in runebender consistently: run the application, and then open two glyph windows; the app aborts when you try to open the second window.

I'm curious to know more about the rationale of the deferring of the size & position setting; that might help me track this down a bit further.

@rjwittams
Copy link
Collaborator Author

This was deferred in order to avoid reentrancy issues (wrt borrowing the app state I think? Due to the window handler going back into druid).
I was following the patterns in the Windows backend on this (@jneem pointed it out the pattern I think). However I think it might be dumb as it is, and maybe what we should do is defer the notifications, but do the cocoa stuff? I will take another look at it tomorrow.

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.

None yet

3 participants