Prototype a Dispatch2 trait to dispatch delegate macros to oblivion#519
Conversation
ids1024
force-pushed
the
dispatch2
branch
2 times, most recently
from
4799e6d to
3307616
Compare
|
Rebased on top of #524, so the Not sure about how we want to handle |
Presumably the type argument for the `LoopHandle` is expected to match the the `D` type used for wayland-rs dispatch? Unless the calloop event loop isn't being used to dispatch Wayland events. Without `keyboard:` specified in `delegate_keyboard!`, this did use `KeyboardData<$ty>`, so in that case these are assumed to match.
For Smithay/wayland-rs#902, and eliminating `delegate_!` macros, we won't be able to implement `Dispatch` for a udata type that is not our own. So of letting the application define a data type wrapping the sctk one with a trait, reverse that. In some ways not having traits, etc. is a bit simpler here. This opens up the possibility for things like keyboard to use the builder pattern for all the various options that can be used when binding a keyboard. But that isn't changed here. Prior to this, `delegate_data_device!` doesn't seem to have had a way to implement dispatch for a custom type, despite having an extension trait. The input method code also seems to have been broken for custom types, not handling it in the delegate macro, not providing a method to create an instance with custom data, and assuming the default type in `.commit()`. These things are fixed here. `RequestDataExt` was defined a bit different from other things here, with multiple methods instead of one returning a shared struct. But it should still be fine to replace that with an immutable wrapper struct.
Smithay/wayland-rs#902 will replace `Dispatch` with a trait defined this way. `Dispatch2` is a prototype to prove that works, and an incremental change toward that. I think for `delegate_registry!` we'll want a change to the API wayland-client provides, so that is unchanged for now. This makes it simpler to add protocols to sctk, and seems overall a little bit better for client code using sctk. Once the wayland-rs update is done, the `Dispatch` type bounds will be cleaner, and the compiler should provide nicer error messages about missing `*Handler` traits.
|
I've cleaned up the history, added detailed commit messages, and added changelog entries. Along with the examples and tests, I think rust-windowing/winit#4567 verifies nothing seems to be wrong with how this works. And #525 confirms this seems to work smoothly with the wayland-rs release instead of providing a I think that confirms I can finish and merge Smithay/wayland-rs#902 and start moving towards actually releasing wayland-rs with these things.
This could be left for merge until after wayland-rs is released if preferred, but I think this should be okay to merge now. I guess sctk does breaking API changes a bit less than smithay, but it should be probably sctk won't need to do a release before the wayland-rs update anyway. |
ids1024
changed the title
Dispatch2 trait to dispatch delegate macros to oblivionDispatch2 trait to dispatch delegate macros to oblivion
2 participants
I've though of some alternate ways to define
Dispatch(as an alternative to the change in Smithay/smithay#1327). Today I had an idea for a way to prototype such a thing outside ofwayland-rs. Due to the orphan rule, we can't create any blanket impls ofDispatchthat are generic over theStatethe trait is implemented for (unless our crate is the one that defined the typeI). But we can provide adelegate_dispatch2!macro that will add such a blanket impl for a particular concrete type.Dispatch2is similar towayland_client::Dispatch, but it's now implemented for the object user data type instead of the state type, uses&selffor the argument pointer to the user data, and removes the extraState = Selftype bound.Implementing this for the user data seems reasonable, since we already need to pass a user data when sending a request that creates an object. So the type passed there indicates what dispatch implementation to use. (It would also work to use
State: Dispatch<UserData, I>instead ofUserData: Dispatch<I, State>; but it would not work to useState: Dispatch<I, UserData>whereIis a type from a different crate andStateis generic. That isn't allowed under the orphan rule. So of the alternatives that work, this draft uses the one that seemed best to me.)This draft uses
Dispatch2and eliminates the dispatch macros used in thesimple_windowexample, except:KeyboardDataExt,PointerDataExt,SurfaceDataExt,RequestDataExt(at least if this trait were part of wayland-rs, and not the same crate)GlobalListtype, since that is awayland-clienttype, not an sctk oneevent_created_child()is unchanged, but we should think of better ways to handle that too.It is still necessary to have
Dispatchbounds in methods of the*Statetypes that create objects, but not in the implementations ofDispatch2. Though type bounds shouldn't be needed anymore if this were moved to wayland-rs and replacedDispatch.If an API change like this is good, it could be incorporated in the next breaking update to wayland-rs. Does anyone see non-obvious issues with this?