Access `Window` from `Pixels`?

[emilyyyylime]

With the new 0.30 winit ApplicationHandler API using a Pixels<'a> for any lifetime other than 'static just got significantly harder, because all app state must now exist within one struct, which must exist before opening the window. The current workaround is to store an Arc<Window> and create the Pixels<'static> from it; but it would be far easier if the Pixels could own the window and simply have an accessor through which application code can use its own Window. Otherwise a self-referencing struct would be required with the Pixels referencing the Window stored alongside it in the same struct.

Anyways I'm proposing Pixels::window(&self) + Pixels::window_mut(&mut self) that return a reference to the winit::window::Window the Pixels was created with

[parasyte]

I have been thinking about this for a very long time. This proposal is what the game-loop crate did before version 1.0.0. They decided to change it to take Arc<Window> in https://github.com/tuzz/game-loop/issues/9.

It seems that the motivation behind this proposal might be a misunderstanding. The only changes this would have are the removal of a named lifetime and the addition of new methods. The removal of the named lifetime is unacceptable. The additional methods are fine but may be unnecessary. It's a lot to unpack why these things are true, so I will try to provide as much detail as I can.

Analysis

First, let's try to determine if there is a misunderstanding. Giving Pixels<'win> a lifetime is very useful because it allows the window to be borrowed temporarily. (E.g. borrowed from the stack. Which is how all examples using winit <= "0.29" work.) That explains the existence of the named lifetime. I need this capability because I don't want to be forced into shared ownership when it is not required.

Importantly, callers are under no obligation to use a lifetime shorter than 'static on the Pixels<'win> that they construct. In the case of winit's new Applicationhandler trait, users probably want 'static. Indeed, you want 'static. That is the only interpretation for the 'win lifetime if Pixels were to own the Window. I believe this is the crux of the misunderstanding, specifically because you stated:

using a Pixels<'a> for any lifetime other than 'static just got significantly harder

But what you are asking is not supporting a lifetime shorter than 'static. The API already allows that by design. E.g., it can borrow from the stack. I do not know how to approach the proposal if it is based on this misunderstanding.

Second, let's assume for argument's sake that the 'win lifetime is just confusing and how it might be addressed. One way is to force it to be exactly 'static, as this proposal does, so that the lifetime does not have to be named. This is the same as defining a type alias:

type Pixels = pixels::Pixels<'static>;

Now everywhere you name this Pixels alias, it has a hidden 'static lifetime that you don't need to concern yourself with.

Third is the challenge of composition, as raised in the linked game-loop issue. The easiest way to compose types from multiple crates is shared ownership with Arc<Window> [^1]. Composition is kind of big deal. See: https://github.com/parasyte/pixels/issues/384#issuecomment-1843796443 and #221 [^2].

I'm lukewarm on the idea of the getter methods, but I see the appeal. Assuming composition is not required, the caller can treat Pixels as the window's owner. And, at least hypothetically, that simplifies implementing winit::application::ApplicationHandler sometimes.

Mandatory reference counting

The problem is that it is not possible for Pixels to both own the Window and a wgpu::Surface without being self-referential. It requires Arc<Window>. We would have to do the same thing that game-loop did. It is not coincidental that the introduction of ApplicationHandler came with Arc<Window> baggage.

And taking Arc<Window> means that the getter methods are just a different way to access the same Arc<Window> that you started with. Calling into question the utility of the getters.

I'm trying not to rant, but I have been strongly opposed to winit's ApplicationHandler trait because it compels users to resolve the self-referential struct problem, most commonly with Arc<Window>. In my opinion, moving ownership of the Window handle deeper inside application state is the wrong direction. Tautologically, the window owns its contents, not the other way around!

Conclusion

I need to reiterate that I see the appeal, and I empathize with making the API as easy as possible to use. But I think a combination of misunderstanding the 'win lifetime parameter and underestimating the difficulty of balancing the tension between flexibility and simplicity makes this proposal far more restrictive, despite its appearance.

Specifically, it makes borrowing from the stack forbidden, and reference counting compulsory.

Given the requirements for users to decide for themselves whether or not they want shared ownership or to borrow from the stack, the only interface that works (as far as I know) is what we have today.

Possible improvements

Maybe there is still something that can be done about confusion caused by the 'win lifetime. Exporting the above type alias is an option.

The downside is that it forces us to either unambiguously name Pixels<'win> and the type alias or export them from different namespaces, like pixels::owned::Pixels and pixels::borrowed::Pixels<'win>. Another downside is that exposing owned and borrowed variants adds a new kind of cognitive overhead: deciding when to choose one or the other. But the choice is an illusion. They are the same type!

JMS55's suggestion in https://github.com/parasyte/pixels/pull/221#issuecomment-963582468 is another option to consider. It's a bigger undertaking, and I don't know what the specific details will look like. It involves splitting pixels into several crates with varying levels of flexibility between "I want to control absolutely everything" to "I don't care about all of that, you do the work".

I'm at odds here. This is a very long way of saying that the only immediate improvement I can see is a fairly minor one that comes with its own downsides. I'm happy to accept feedback and discuss our options to find a better solution. This is the current situation as I understand it.

[^1]: FWIW, reference counting/shared ownership is not a workaround. It's a fundamental requirement to safely construct self-referential structures without the lifetime shenanigans of yoke and ouroboros.

[^2]: This PR does exactly the opposite of this proposal for wgpu state. It adds a new 'wgpu lifetime so that the state does not have to be owned by Pixels!

[emilyyyylime]

I see. It has been my (mis)understanding that when constructing Pixels from a moved Window, the Pixels object can directly hold the window inside of it. If I understand you correctly, that is not the case, or at least it could not be the case while allowing the end user to get access to the window. It's rather annoying that the new winit design almost forces self-referential structs seemingly for no significant gain—but I'm really not involved enough with this side of Rust and standardisation to know what was the reasoning and the considerations involved I guess

[emilyyyylime]

In terms of aiding newer users, I think after the examples are updated to the new API using Option<Arc<Window>> as in #403 and #411 this will mostly be a solved problem. Unfortunately winit_input_helper is still out of date, and I personally had problems getting it to compile with the latest winit.

[parasyte]

Yeah, winit_input_helper must be abandoned for winit 0.30 compatibility. I don't think there is any interest in updating the helper crate to conform to the ApplicationHandler trait.

I agree that Arc<Window> is not a terrible solution. It might just be the least bad when you are forced to use APIs that disallow naming a lifetime. In my brief experimentation with ApplicationHandler, I have been using two structs like this:

#[derive(Default)]
struct App {
    inner: Option<AppInner>,

    // More app state here...
}

struct AppInner {
    pixels: Pixels<'static>,
    window: Arc<Window>,
}

It's helpful to bundle both of these types under a single Option. They can be borrowed together at the top of the callback methods instead of unwrap() everywhere:

fn window_event(&mut self, event_loop: &ActiveEventLoop, _id: WindowId, event: WindowEvent) {
    let Some(AppInner { pixels, window }) = self.inner.as_mut() else {
        return;
    };

    // Use `pixels` and `window` here...
}

---

I do not know the motivation behind the ApplicationHandler trait, but it appears to have been introduced in https://github.com/rust-windowing/winit/issues/3432 And from this description alone, I can't really make sense of how the design fulfills the requirements. They are also collecting feedback in https://github.com/rust-windowing/winit/issues/3626.

[emilyyyylime]

I generally agree. I don't think any Pin solution will be worth users' time. On the winit feedback issue I think the pixels use case falls under tgross's comment

[nicoburns]

Is there any downside to using Arc<Window>? All of the methods on Window take &self so you never actually need mutable access to Window. And my understanding is that Winit devs are strongly considering internally reference counting Window in future (similar to wgpu types are now all internally Arcd and cheaply clonable).

[parasyte]

The biggest downsides for me are double indirection for every access and the potential for resource leaks with strong references (cycles, accumulation into collections, etc).

It would be different if windowing systems always provided ref counted handles. That might have been preferable. I guess they probably did not for efficiency reasons. (These platforms were designed in the 80’s and 90’s.)