rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.74k stars 890 forks source link

Semantics of getters/setters and events? #3690

Open madsmtm opened 4 months ago

madsmtm commented 4 months ago

I'm working on improving the internals of the event handling and queuing on macOS (and iOS), and couldn't seem to find any docs on what the expected behaviour actually is? Take the following example of modifying and querying the outer position of a window:

match event {
    WindowEvent::Moved(new_position) => {
        println!("moved to: ({}, {})", new_position.x, new_position.y);
    },
    WindowEvent::KeyboardInput { event: KeyEvent { state: ElementState::Pressed, .. }, .. } => {
        println!("setting position");
        window.set_outer_position(PhysicalPosition::new(100, 100));
        let new_position = window.outer_position().unwrap();
        println!("new position: ({}, {})", new_position.x, new_position.y);
    },
    _ => (),
}

There are a few possible ways this could behave:

  1. The value is immediately updated, and querying it yields the new value:
    setting position
    new position: (100, 100)
    moved to: (100, 100)
  2. The value is deferred to only be updated once the event executes:
    setting position
    new position: (712, 234) // Old value
    moved to: (100, 100)
  3. The event is executed immediately, and the new value is available afterwards:
    setting position
    moved to: (100, 100) // Emitted before window.set_outer_position returns
    new position: (100, 100)

Of these, I would argue that option 3 is the "better" option, as it allows the user to have one place where updates to the position is always executed, and every part of the system instantly remain up to date about the current state. It is, however, also not possible in Rust with the current event handler, since it'd be re-entrant (and is probably also pretty difficult to implement efficiently).

That leaves us with option 1 and 2, of which I'm honestly not sure which is the best one? I can see the arguments for both sides, with option 1 ensuring consistency within the section, while option 2 ensures consistency within the entire application (forcing you to do stuff that depends on a specific update in that event handler).

Currently though, I'm leaning towards option 1 as that's how the macOS backend currently works (mostly), and I think that's what users expect when they call a method called set_x.

Is that how it works on the other platforms too? And for all the other methods? I see that we have request_inner_size, I think because it works like option 2 on some platforms (X11? Windows?), can and should we change that to be more consistent?

daxpedda commented 4 months ago

Is that how it works on the other platforms too? And for all the other methods?

Not for Web, where any getter method will always return the current true value regardless of what the last thing is the user did. Though more often then not this turns out to look like 1. anyway because on Web most things are updated immediately.

I see that we have request_inner_size, I think because it works like option 2 on some platforms (X11? Windows?), ...

Yes, additionally because there is no guarantee that this is set to the value the user requested because e.g. its not allowed to be that big/small, or in the case of Web there is no way to accurately set the size, it might be a pixel off.

can and should we change that to be more consistent?

I think it would be best to basically make a list of every single method and let backend maintainers answer this question for every method. Though this is probably a lot of work.

But yes, I also believe we should be consistent here and document it when we are not, which we did in some places.


I also agree that we should stick with option 1., as long as viable, e.g. consistent on all platforms. Though it might not make sense for functions that can't predict what the outcome might be, e.g. with request_inner_size().

kchibisov commented 4 months ago

Well, reporting old value could be just incorrect because the windowing system could deny what you did, so you use something wrong and e.g. fail in validation, etc.

In general, only platforms where things are entirely client side can do that correctly and do 1 or 3. So I guess wayland/macOS can do so.

You also can not really do e.g. 3 unless you allow re-entrance.

About the option 1, it works the way you'd expect only on Wayland, the rest likely can reject it on server and you'll desync unless you try to correct the user. And the user could also easily enter the update cycle because they want to prevent the server change. Though, you can end up with that in a lot of cases right now anyway and we shouldn't prevent from it.

madsmtm commented 4 months ago

Rough conclusion from today's meeting: Option 1 has the desired semantics.

So taking the simpler example of set_title, the following assert should succeed:

window.set_title("foo");
assert_eq!(window.title(), "foo");

There are, naturally, cases where it might not, where the title might be updated in between those two calls, such as:

The point is not to rule out the above cases, the point is that the title is immediately available in the getter.

In short, the desired behaviour is that you will never get an old value, though you might get a newer value than the one you just set.

Does that match your understanding?


I think instead of making a list of which methods play by this rule, and which ones doesn't, I'm gonna make a test case or some feature-flagged assertions, or something, to ensure that this is the behaviour, and to make it easy to test if it is (in the future too).

kchibisov commented 4 months ago

On X11, the server is asynchronous, and might receive another event (from the user). (Or might just misbehave, and not set the title?)

It could be not applied instantly. In general, relying that title was actually changed is not a great idea because it's not guaranteed, etc, etc.

I'd just suggest that we model methods that are crucial, like Window::request_inner_size as close to their real semantics as possible, and for the rest we do best effort when it comes to consistency.

daxpedda commented 1 month ago

This has come up in #3861, where assert_eq!(Window::set_fullscreen(Some(Fullscreen::Borderless(None))), Window::fullscreen()) would fail.

This is again caused by the Web call actually being async, so we can't actually know if the request succeeded or not right away.