Making wayrs-client generic over different wayland transports #8
Comments
|
Will it be a breaking change if the new generic has a default, like the allocator in |
|
Oh I didn't realize that was a thing, I'll try to set a default and see if I can avoid breaking the public api, I'll make a PR when I'm done. Thank you :) |
|
@MaxVerevkin do you have any suggestions on how to handle |
Hard to say since I don't know which problems you've encountered, and how the alternative transport you are working on works. |
|
The biggest problem is the virtio protocol won't use a file descriptor, it works by basically opening |
|
How do you know then a new message is available to be read? You can open a draft PR so I can have some idea how wayland over virtio works, since I've never used it. |
|
Also, may I ask what you are planning to do with this? Are you writing a forwarding compositor? Just curious. |
|
There is an overview of the protocol here. To receive you have to do a blocking IOCTL and poll the channel, see the receiving data section. I didn't actually start writing the virtio stuff yet because I couldn't settle on a proper abstraction for transport. I was doing something like this, but looks like that's not going to be enough because of the async functions. pub trait WaylandSocket: AsRawFd {
fn connect() -> Result<Self, ConnectError>
where
Self: Sized;
fn get_free_msg_args_as_ref(&mut self) -> &mut Vec<Vec<ArgValue>>;
fn flush(&mut self, mode: IoMode) -> io::Result<()>;
fn peek_message_header(&mut self, mode: IoMode) -> io::Result<MessageHeader>;
fn write_message(&mut self, msg: Message, mode: IoMode) -> Result<(), SendMessageError>;
fn recv_message(
&mut self,
header: MessageHeader,
iface: &'static Interface,
version: u32,
mode: IoMode,
) -> io::Result<Message>;
}
impl WaylandSocket for BufferedSocket {
fn connect() -> Result<Self, ConnectError>
where
Self: Sized,
{
Self::connect()
}
fn get_free_msg_args_as_ref(&mut self) -> &mut Vec<Vec<ArgValue>> {
&mut self.free_msg_args
}
fn peek_message_header(&mut self, mode: IoMode) -> io::Result<MessageHeader> {
self.peek_message_header(mode)
}
fn recv_message(
&mut self,
header: MessageHeader,
iface: &'static Interface,
version: u32,
mode: IoMode,
) -> io::Result<Message> {
self.recv_message(header, iface, version, mode)
}
fn write_message(&mut self, msg: Message, mode: IoMode) -> Result<(), SendMessageError> {
self.write_message(msg, mode)
}
fn flush(&mut self, mode: IoMode) -> io::Result<()> {
self.flush(mode)
}
}And yes, I'm trying to write a forwarding compositor for android. Something like waydroid but running in a vm. |
|
Thanks for the link!
The ioctl is not blocking, the read is, right? It doesn't explicitly state this, but I would be very surprised if the fd you are supposed to read 8 bytes from is not
Cool! Since you will also write server-side wayland code, do you think it would be worth it to make By the way, I don't mind a v2 release. For example I wanted to slightly change the |
Oh yes, that's true. I will try to actually implement the cross domain protocol using the device fd and see how far I can get, thanks for the advice :)
I think this would be useful in general, but not for what I'm doing. My compositor will accept AIDl calls from android system compositor(surfaceflinger) and basically translate them back and forth into wayland calls. |
is it required to send messages one-by-one? If no (from a quick look at |
|
I think that makes sense, I will refactor once I have a working example. |
|
I went ahead and made the low-level wayrs/wayrs-core/src/transport.rs Lines 34 to 49 in 9f865b2
|
|
Nice, I will rebase my work on this :D I have something slimier locally now. for |
|
I'm not sure about |
|
yeah I think that will work fine. |
|
I ran into another problem, while getting the example to work. virtio protocol requires clients use dma-buf. virtio-gpu mesa driver only supports egl 1.4, I'm not exactly sure why because the iris driver supports egl 1.5. And wayrs-egl requires egl 1.5. So wayrs egl clients doesn't work on guests properly. I'm trying to get wayrs-egl running on egl 1.4 but I couldn't wrap my head around all the egl stuff yet. I think I can just use shm and copy the data to a dma-buf buffer. but I would like to avoid that if possible. |
|
Lowering the required EGL version should be possible, I've chosen 1.5 because it was the latest but at the same time pretty old (10 years old?). The important part is having the required extensions. https://docs.rs/wayrs-egl/latest/wayrs_egl/ has links to all used extensions. |
|
Am I interpriting this correctly? looks like 2 extensions( |
|
No. |
|
Okay, so all the extensions are there. this is the error I got when I just set the egl version to 1.4. looks like the format table is just empty. |
|
Can you show the output with |
btw this is just running in a vm guest using the compositor in the guest. not using the virto wayland protocol |
|
I've added some debug prints in d2d70de, could you try it? |
|
|
That's weird. What what does this print? diff --git a/wayrs-egl/examples/triangle.rs b/wayrs-egl/examples/triangle.rs
index 367f209..6e9bfc3 100644
--- a/wayrs-egl/examples/triangle.rs
+++ b/wayrs-egl/examples/triangle.rs
@@ -370,6 +370,8 @@ impl DmabufFeedbackHandler for State {
let egl_display = EglDisplay::new(self.linux_dmabuf, render_node).unwrap();
+ dbg!(egl_display.supported_formats());
+
let format_table = self.surf.dmabuf_feedback.format_table();
let mut formats = HashMap::<Fourcc, Vec<u64>>::new();
|
|
|
Huh, is zero modifier implicitly sported? Does this work? Not sure how correct that is, though. diff --git a/wayrs-egl/src/egl.rs b/wayrs-egl/src/egl.rs
index 1d17db4..07dfede 100644
--- a/wayrs-egl/src/egl.rs
+++ b/wayrs-egl/src/egl.rs
@@ -243,6 +243,10 @@ unsafe fn get_supported_formats(
}
unsafe { mods_buf.set_len(mods_len as usize) };
+ if mods_buf.is_empty() {
+ mods_buf.push(0);
+ }
+
retval.insert(Fourcc(format as u32), mods_buf);
}
|
|
still similar issue, I wonder if this is a crosvm issue, I'll try running it in qemu too. |
|
What does the compositor advertise then? diff --git a/wayrs-egl/examples/triangle.rs b/wayrs-egl/examples/triangle.rs
index 367f209..a444ab6 100644
--- a/wayrs-egl/examples/triangle.rs
+++ b/wayrs-egl/examples/triangle.rs
@@ -391,6 +391,12 @@ impl DmabufFeedbackHandler for State {
.entry(Fourcc(fmt.fourcc))
.or_default()
.push(fmt.modifier);
+ } else {
+ eprintln!(
+ "Format {:?}:{} not supported by EGL",
+ Fourcc(fmt.fourcc),
+ fmt.modifier
+ );
}
}
eprintln!(
Apparently not. Maybe assuming that if no modifiers where given, then all modifiers from the tranche are supported would work? Can you also try: diff --git a/wayrs-egl/src/egl.rs b/wayrs-egl/src/egl.rs
index 1d17db4..8121a8c 100644
--- a/wayrs-egl/src/egl.rs
+++ b/wayrs-egl/src/egl.rs
@@ -151,7 +151,7 @@ impl EglDisplay {
/// Check whether a fourcc/modifier pair is supported
pub fn is_format_supported(&self, fourcc: Fourcc, modifier: u64) -> bool {
match self.supported_formats.get(&fourcc) {
- Some(mods) => mods.contains(&modifier),
+ Some(mods) => mods.is_empty() || mods.contains(&modifier),
None => false,
}
} |
with the egl-mod-invalid branch |
|
Added more prints... |
|
yeah the context creation is the problem |
|
Maybe |
|
oh yeah, now it works! nice. Thanks :D |
|
Turns out |
|
thank you for the fix :) |
|
@MaxVerevkin looks like I have to do a custom implementation of ps: dma-buf metadata differs between virto-gpu driver and host gpu driver, so I have to do some ioctls and figure out the host values and use that in the above call. |
|
Not sure what your mean, the |
|
I want to have two implementations of params::add for params::add<D, UnixStream> and params::add<D, VirtgpuTransport>. So the client doesn't have to know about the implementation details of how to fix the buffer metadata. |
|
The code generated by I don't know what exactly you mean by "fix the buffer metadata", I'm not even sure how you allocate buffers. A client using virtio will have to figure out the correct params, sure. And virtio seems to be niche enough to not be explicitly supported by the core project. If everything is generic enough, then a separate crate may implement all the virtio quirks. |
|
The problem I'm having now is when I'm using the underlying technical issue is that when a program asks to allocate a dma-buf on virtio-gpu, it doesn't actually allocate anything. It just calls down to virglrenderer running on the host, and virgl allocates a dma-buf on host gpu. so guest metadata and host metadata about the same buffer can be different(ex: intel GPU drivers expect stride to be 16 byte aligned but virtio-gpu driver doesn't). I have to query the correct metadata from the host, and use it in |
|
The problem is that |
|
I think that's fine, allocations still go trough guest gbm, things happening underneath is pretty much hidden from the guest. I also manged to get the client working by patching metadata checks in host mesa driver. So wayrs-egl is already working fine. |
|
Okay, I just don't understand how it works. Are there any docs? |
|
I couldn't find good docs on any of this unfortunately. there are some info about the virglrenderer here. From what I understand, when a egl client on guest wants to allocate a dma-buf, it'll request the buffer via virgl mesa driver. mesa will request a buffer from virtio-gpu kernel driver. kernel driver will request a buffer from the hypervisor(crosvm or qemu). hypervisor will request a buffer form virglrenderer running on the host, and it will finally request a real buffer on the host gpu using host mesa driver. |
|
I got it to work. I'll update the draft pr with the changes later. |
|
Nice! |
|
I've hit another snag :/ so far I've been using an ubuntu VM to test the virtio suff, now I have to start developing it on android which is where I intended to use it. |
That's bad but it is not critical. AFAIK
No,
|
|
My compositor would just pass the buffers rendered by android to the host compositor and hopefully don't have to do any allocations. Android will do all the buffer allocations. Maybe I can get away with just using About Looks like bellow are the available extensions on android. |
|
It doesn't support any
That's bad. Regarding GBM, is this related? How do you get buffers from the android compositor? As dmabufs with known params? If so, and if you don't want to render anything additional (postprocessing?), sure, you can skip gbm/egl thing and just forward the buffers. |
yes, android supports both minigbm and mesa libgbm. But clients access it via the system graphics allocator service. Composer clients sends buffers using this interface, it uses a I'll try the pass trough proxy approach first. I might have to do some post processing later(mutter doesn't support the graphics buffer format android uses properly AFAIK, so colors are off). I'll figure it out later. |
|
I tried a couple of ways to get async working, but I couldn't. The gpu fd doesn't seem to support epoll well. for read, it works fine. But for writes, it just waits forever. I guess because you can just write to a gpu driver fd whenever you want. so waiting for write availability is sort of meaningless. Cross domain wayland channel is a GEM object, you can convert it to a fd by using an ioctl. So I tried using two fds for read(gpu fd), and write(cross domain channel fd). But this caused 100% cpu usage. I think there is a "proper" way to do async stuff in gpu workloads using gpu fences. But I don't understand the documentation on this at all. So I think I'll just keep using the sync wayland client for now. |
|
If writing never blocks, then there is no problem at all. I'm pretty sure gpu fences have nothing to do with this. |
|
Actually this seems like a nice optimization even for unixstream, so I did that in 8b078fe. |
|
Interesting, I will give it another try. So the trasnport channels virtio-gpu uses to send/receive messages from host wayland are GEM objects. Looks like I can use gpu fences to synchronize access to these in an async way just like with actual graphics buffers. But I probably won't need this. |
|
I tried with your latest commits to main, and async works okay now. It still uses three time the cpu as unix sockets implementation (~12% vs %4 cpu). But I think that's probably some other overhead. Nice! |
|
I created a separate crate for this transport https://github.com/madushan1000/wayrs-client-transport-virtgpu, |
Currently wayrs-client crate only supports unix socket as the transport for wayland protocol.
Right now, I'm trying to modify the crate to be generic over the transport so I can use it with virtio-gpu cross-domain protocol(used to allow programs running on crosvm/qemu guests to open windows in host wayland compositor).
Are you interested in merging the changes here? It will break the public crate api, because things like Connection struct will have to be generic over the transport.
The text was updated successfully, but these errors were encountered: