Add `Window::has_focus` by amrbashir · Pull Request #2531 · rust-windowing/winit

amrbashir · 2022-10-26T18:50:10Z

Porting tauri-apps/tao@7d2eeee

madsmtm

Thanks, looks good, only a few minor nits.

src/window.rs

kchibisov · 2022-11-03T17:02:35Z

I think it's dangerous to add this with blanket true impl to be honest, since users my start relying on it. Given that you simply want to get the last value of Focused(changed) event we should have the Api on all the platform implemented from the start.

amrbashir · 2022-11-04T18:32:20Z

Unfortunately, I don't have much experience with other platforms so I can't implement them, however I may be able to implement x11 if you steer me into the right direction.

kchibisov · 2022-11-04T18:54:51Z

@amrbashir I mean it's not about knowing though. You can see where we sent Fosused event and stash its result on a window, it's just rust and doesn't require extra knowledge at all. For example on Wayland you'd need to pass some Arc<AtomicBool> around, on X11 can likely something similar or maybe you can query things directly.

There're no platform specifics at all in the end.

amrbashir · 2022-11-04T19:06:45Z

cool, thought there might be a way to query the OS like macOS impl for example.

kchibisov · 2022-11-04T19:11:14Z

Maybe on X11, but be aware that active window isn't keyboard focused window sometimes, since you can hover the window with mouse and it'll be active, but won't have focus.

amrbashir · 2022-11-04T19:49:58Z

alright, x11, wayland and web, are implemented now but I was not able to test wayland.

kchibisov

While there's a way to know whether the window is focus or not on ios/Android, I guess the issue was that it's unclear how to hook into the current handlers, which is sort of understandable.

Would leave that to android and ios maintainers.

src/window.rs

examples/window.rs

kchibisov · 2022-11-05T02:46:57Z

src/platform_impl/linux/x11/window.rs

            max_inner_size: None,
            resize_increments: None,
            base_size: None,
+            has_focus: true,


Shouldn't this be false?

doesn't the window start initially with keyboard focus?

On macOS, the window is initially not focused (and window.has_focus() returns false), but it receives a WindowEvent::Focused shortly after launching (which also makes window.has_focus() return true)

This only affects x11, on macOS we query the OS directly. The reason I made x11 start with true is because Windows also starts with true and I think x11 is very similar to Windows in this matter, but I haven't had the chance to test x11 (I may test it later today).

src/platform_impl/linux/wayland/window/shim.rs

amrbashir · 2022-11-05T12:25:42Z

Android and iOS are now implemented, although I am not sure if a global state on Android is the way to go.

kchibisov

LGTM.

Would like to hear from android maintainers wrt the change and tracking
focus in a global state. @MarijnS95 or @msiglreith.

Also, you'd need a rebase, I guess.

MarijnS95 · 2022-11-20T13:16:53Z

Android can support multiple windows (called Activitys) so this should not go into a static global.

EDIT: I haven't checked what the right/updated way is to do this since #2444.

amrbashir · 2022-11-20T14:17:17Z

Android can support multiple windows (called Activitys) so this should not go into a static global.

I am aware of that but glancing over the code, the events are dispatched with a dummy window id and there is no way to differentiate between different activity events.

MarijnS95 · 2022-11-20T16:36:23Z

That's unfortunate to hear @amrbashir, I thought @rib's solution in #2444 would implicitly address this; or at least his crate opens the door for multi-window (multiple Activity instances) setup as that's relevant to some of my use-cases going forward.

Regardless, I thought at least in the past we had a per-window struct/storage location where I stored the RawWindowHandle lock.
Looks like @rib removed that (probably for good reason, though I have not had the opportunity to look at any of the code in detail yet), and looking back it seems to have been in EventLoop which is already a better place than a global static (but not per-Window, as I remembered), allowing for easier move to a per-window struct when we get to finally implementing this.

madsmtm

Haven't tested the iOS impl, but is very trivial, so accepted from my side. macOS also looks good, thanks for your work @amrbashir!

src/platform_impl/ios/window.rs

kchibisov · 2023-01-15T21:13:59Z

@amrbashir could you rebase it?

amrbashir · 2023-01-16T20:11:41Z

Should be fine now

amrbashir requested review from asterbloom, kchibisov, madsmtm and msiglreith as code owners October 26, 2022 18:50

amrbashir mentioned this pull request Oct 26, 2022

Tracking issue for different implementation between winit tauri-apps/tao#470

Open

22 tasks

madsmtm approved these changes Nov 2, 2022

View reviewed changes

src/window.rs Outdated Show resolved Hide resolved

src/window.rs Outdated Show resolved Hide resolved

src/window.rs Show resolved Hide resolved

amrbashir changed the title ~~On Windows and MacOS, add Window::has_focus~~ Add Window::has_focus Nov 4, 2022

amrbashir force-pushed the feat/has-focus branch 2 times, most recently from 754075f to fd1ffc3 Compare November 4, 2022 21:49

kchibisov requested changes Nov 5, 2022

View reviewed changes

kchibisov requested a review from MarijnS95 November 20, 2022 13:04

kchibisov approved these changes Nov 20, 2022

View reviewed changes

madsmtm approved these changes Nov 22, 2022

View reviewed changes

src/platform_impl/ios/window.rs Outdated Show resolved Hide resolved

kchibisov added this to the Version 0.28 milestone Dec 24, 2022

kchibisov approved these changes Jan 15, 2023

View reviewed changes

amrbashir requested a review from jackpot51 as a code owner January 16, 2023 19:59

On Windows and MacOS, add Window::has_focus

3798762

kchibisov force-pushed the feat/has-focus branch from 476f0cc to 3798762 Compare January 17, 2023 01:20

kchibisov merged commit a88d2e0 into rust-windowing:master Jan 17, 2023

Conversation

amrbashir commented Oct 26, 2022 • edited by madsmtm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madsmtm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kchibisov commented Nov 3, 2022

Uh oh!

amrbashir commented Nov 4, 2022

Uh oh!

kchibisov commented Nov 4, 2022

Uh oh!

amrbashir commented Nov 4, 2022

Uh oh!

kchibisov commented Nov 4, 2022

Uh oh!

amrbashir commented Nov 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kchibisov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kchibisov Nov 5, 2022

Choose a reason for hiding this comment

Uh oh!

amrbashir Nov 5, 2022

Choose a reason for hiding this comment

Uh oh!

madsmtm Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amrbashir Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amrbashir commented Nov 5, 2022

Uh oh!

kchibisov left a comment

Choose a reason for hiding this comment

Uh oh!

MarijnS95 commented Nov 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amrbashir commented Nov 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarijnS95 commented Nov 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madsmtm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kchibisov commented Jan 15, 2023

Uh oh!

amrbashir commented Jan 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

amrbashir commented Oct 26, 2022 •

edited by madsmtm

Loading

amrbashir commented Nov 4, 2022 •

edited

Loading

madsmtm Nov 23, 2022 •

edited

Loading

MarijnS95 commented Nov 20, 2022 •

edited

Loading

amrbashir commented Nov 20, 2022 •

edited

Loading

MarijnS95 commented Nov 20, 2022 •

edited

Loading