Conversation
…ir into feature/wayland-rs-event-gen
…ir into feature/wayland-rs-event-gen
…ir into feature/wayland-rs-event-gen
There was a problem hiding this comment.
Pull request overview
This PR restructures the wayland_rs Rust/C++ CXX bridge so generated Wayland interface C++ classes can be associated with Rust “extension” structs, enabling a future path for emitting Wayland events from C++ via Rust to clients.
Changes:
- Switch the CXX bridge entrypoint from
src/lib.rsto generatedsrc/ffi.rsand update the demo to includeffi.rs.h. - Add build-script generation for
middleware.rs(Rust extension structs) andffi.rs(CXX bridge definitions), plus anassociate()method on each generated C++ interface. - Refactor protocol parsing and code generation helpers; add
ffi_fwd.hgeneration to avoid circular includes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wayland/wayland_rs/src/lib.rs | Replaces inline module declarations with include!() of generated dispatch.rs, protocols.rs, middleware.rs, and ffi.rs. |
| src/wayland/wayland_rs/example/main.cpp | Updates the demo to include the new generated CXX header ffi.rs.h. |
| src/wayland/wayland_rs/build_script/protocol_parser.rs | Sorts protocol XML inputs for deterministic generation and simplifies iteration. |
| src/wayland/wayland_rs/build_script/protocol_middleware_generation.rs | Adds generator for Rust “Ext” wrappers that forward event calls to wayland_server protocol objects. |
| src/wayland/wayland_rs/build_script/main.rs | Major generation pipeline rework: middleware + ffi generation, ffi_fwd.h, new dispatch storage model (Arc<Mutex<...>>), and CXX bridge moved to src/ffi.rs. |
| src/wayland/wayland_rs/build_script/helpers.rs | Extracts shared helpers for writing generated Rust/C++ output and naming helpers. |
| src/wayland/wayland_rs/build_script/ffi_generation.rs | Generates the unified #[cxx::bridge] mod ffi with Rust->C++ and C++->Rust declarations. |
| src/wayland/wayland_rs/build_script/cpp_builder.rs | Updates C++ signature generation (return-by-value for unique_ptr, introduces rust::Box<T>, sanitizes identifiers) and pins C++ method receivers in Rust bindings. |
| src/wayland/wayland_rs/CMakeLists.txt | Points CMake at src/ffi.rs as the bridge source and adjusts include directories for the demo target. |
| src/wayland/wayland_rs/.gitignore | Updates ignored generated Rust files (ffi.rs, middleware.rs) and removes old ffi_cpp.rs. |
robert-ancell
left a comment
There was a problem hiding this comment.
I've read through this; there's a lot going on and I can only follow parts of it. I can't see any issues but it needs a second opinion / someone more knowledgeable in Rust to have a look too.
RAOF
left a comment
There was a problem hiding this comment.
The actual code generation code is fine.
I'm still digesting the code that it's generating; it seems at this point that we probably need more documentation about the high-level theory of what's happening here, because it's very much non-obvious.
| pub fn dash_to_snake(name: &str) -> String { | ||
| name.replace('-', "_") | ||
| } | ||
|
|
||
| pub fn dash_to_snake_ident(name: &str) -> Ident { | ||
| format_ident!("{}", dash_to_snake(name)) | ||
| } |
There was a problem hiding this comment.
These are unused, right?
Incidentally, if there's much more case manipulation needed, heck might be a good choice.
There was a problem hiding this comment.
dash_to_snake_ident is still used
I was going to leave this as a task for later where I write a big diagram/markdown of what is getting generated and why. I am waiting for "later" because things feel prone to change now, and I don't want to keep rewriting it |
…n bits with comments
|
@RAOF I update to use pin_mut, UniquePtr, and with comments to make things easier to follow :) |
TICS Quality Gate✔️ Passedmir
|
RAOF
left a comment
There was a problem hiding this comment.
I need GitHub to make it more clear when I haven't actually submitted the review!
This is somewhat weird code, but the weirdness is now documented, and once we've got the whole thing implemented we can see if anything can be simplified.
What's new?
src/middleware.rs, which contains Rust structs that ferry data from Rust to C++src/ffi.rs, which contains:associatemethod to each Wayland interface's C++ class. This class will "associate" theBox<InterfaceExt>struct with the C++ class so that the C++ class can call methods on its underlying Rust implementationinclude/ffi_fwd.hto get around a circular dependency issue where the generated C++ headers want to include the Rust headers, but the Rust headers want to include the generated C++ headers. This came about because the<Interface>Implclass now hold an<Interface>Extbox.How to test
cargo buildfromsrc/wayland/wayland_rssrc/middleware.rssrc/ffi.rssrc/dispatch.rsChecklist