MaxVerevkin / wayrs

A simple wayland library
MIT License
14 stars 2 forks source link

Making wayrs-client generic over different wayland transports #8

Open madushan1000 opened 9 months ago

madushan1000 commented 9 months ago

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.

MaxVerevkin commented 9 months ago

Will it be a breaking change if the new generic has a default, like the allocator in Vec<T, A=Global>?

madushan1000 commented 9 months ago

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 :)

madushan1000 commented 9 months ago

@MaxVerevkin do you have any suggestions on how to handle async_fd field in Connection struct? I'm thinking of moving it to BufferedSocket struct and adding helper functions to manipulate it.

MaxVerevkin commented 9 months ago

@MaxVerevkin do you have any suggestions on how to handle async_fd field in Connection struct? I'm thinking of moving it to BufferedSocket struct and adding helper functions to manipulate it.

Hard to say since I don't know which problems you've encountered, and how the alternative transport you are working on works.

madushan1000 commented 9 months ago

The biggest problem is the virtio protocol won't use a file descriptor, it works by basically opening /dev/dri/card0 drm device and then doing a bunch of IOCTLs and memcpys to copy wayland packets back and forth. I looked into implementing file descriptor related traits for this but that looks like a lot of work. If there is no other way, I'll end up doing it anyway.

MaxVerevkin commented 9 months ago

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.

MaxVerevkin commented 9 months ago

Also, may I ask what you are planning to do with this? Are you writing a forwarding compositor? Just curious.

madushan1000 commented 9 months ago

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.

MaxVerevkin commented 9 months ago

Thanks for the link!

To receive you have to do a blocking IOCTL and poll the channel, see the receiving data section.

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 poll()-able. If this is true everything is good, and Connection can use that fd for the async stuff.

And yes, I'm trying to write a forwarding compositor for android. Something like waydroid but running in a vm.

Cool! Since you will also write server-side wayland code, do you think it would be worth it to make BufferedSocket generic enough to be used on the server side too? I've written one wayland server (https://github.com/MaxVerevkin/ewc), but I basically copy-pasted the socket code, adapting it as needed.

By the way, I don't mind a v2 release. For example I wanted to slightly change the Proxy trait so that I could reuse argument buffers from events too. And maybe change some of ArgValue variants.

madushan1000 commented 9 months ago

The ioctl is not blocking, the read is, right?

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 :)

do you think it would be worth it to make BufferedSocket generic enough to be used on the server side too?

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.

MaxVerevkin commented 9 months ago
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>;
}

is it required to send messages one-by-one? If no (from a quick look at sommelier I couldn't find any evidence that it is), I think an API with just sendmsg/recvmsg would be better and avoid a lot of duplication.

madushan1000 commented 9 months ago

I think that makes sense, I will refactor once I have a working example.

MaxVerevkin commented 8 months ago

I went ahead and made the low-level BufferedSocket generic over https://github.com/MaxVerevkin/wayrs/blob/9f865b27d2f27bf43b21427d4e8a4d35c3e55be1/wayrs-core/src/transport.rs#L34-L49

Connection is not generic yet, though.

madushan1000 commented 8 months ago

Nice, I will rebase my work on this :D I have something slimier locally now. for connect, I just added a connect method to the Tansport trait and called implementation specific connect from BufferedSocket::connect

MaxVerevkin commented 8 months ago

I'm not sure about connect being part of Transport, since it may be used by both clients and servers, and connect doesn't make sense for servers. Perhaps we can define a supertrait in wayrs-client, like trait ClientTransport: Transport { fn connect() -> Result<Self> }, which is specific to clients?

madushan1000 commented 8 months ago

yeah I think that will work fine.

madushan1000 commented 8 months ago

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.

MaxVerevkin commented 8 months ago

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.

madushan1000 commented 8 months ago

Am I interpriting this correctly? looks like 2 extensions(EGL_KHR_platform_gbm, and GL_OES_EGL_image are missing from virtio-gpu driver.

XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 eglinfo
EGL client extensions string:
    EGL_EXT_client_extensions, EGL_EXT_device_base,
    EGL_EXT_device_enumeration, EGL_EXT_device_query, EGL_EXT_platform_base,
    EGL_EXT_platform_device, EGL_EXT_platform_wayland, EGL_EXT_platform_x11,
    EGL_EXT_platform_xcb, EGL_KHR_client_get_all_proc_addresses,
    EGL_KHR_debug, EGL_KHR_platform_gbm, EGL_KHR_platform_wayland,
    EGL_KHR_platform_x11, EGL_MESA_platform_gbm,
    EGL_MESA_platform_surfaceless

GBM platform:
EGL API version: 1.4
EGL vendor string: Mesa Project
EGL version string: 1.4
EGL client APIs: OpenGL OpenGL_ES
EGL driver name: virtio_gpu
EGL extensions string:
    EGL_ANDROID_blob_cache, EGL_ANDROID_native_fence_sync, EGL_EXT_buffer_age,
    EGL_EXT_image_dma_buf_import, EGL_EXT_image_dma_buf_import_modifiers,
    EGL_EXT_pixel_format_float, EGL_KHR_cl_event2, EGL_KHR_config_attribs,
    EGL_KHR_context_flush_control, EGL_KHR_create_context,
    EGL_KHR_create_context_no_error, EGL_KHR_fence_sync,
    EGL_KHR_get_all_proc_addresses, EGL_KHR_gl_colorspace,
    EGL_KHR_gl_renderbuffer_image, EGL_KHR_gl_texture_2D_image,
    EGL_KHR_gl_texture_3D_image, EGL_KHR_gl_texture_cubemap_image,
    EGL_KHR_image, EGL_KHR_image_base, EGL_KHR_image_pixmap,
    EGL_KHR_no_config_context, EGL_KHR_reusable_sync,
    EGL_KHR_surfaceless_context, EGL_KHR_wait_sync,
    EGL_MESA_configless_context, EGL_MESA_drm_image,
    EGL_MESA_image_dma_buf_export, EGL_MESA_query_driver,
    EGL_WL_bind_wayland_display
MaxVerevkin commented 8 months ago

No. EGL_KHR_platform_gbm is listed in the snippet you provided. And GL_OES_EGL_image should be listed under "OpenGL core profile extensions", not "EGL extensions string". Just grep it.

madushan1000 commented 8 months ago

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.

thread 'main' panicked at wayrs-egl/examples/triangle.rs:403:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

Can you show the output with WAYLAND_DEBUG=1?

madushan1000 commented 8 months ago
$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 WAYLAND_DEBUG=1 ./triangle
[wayrs]  -> wl_display@1v1.get_registry(new id wl_registry@2)
[wayrs]  -> wl_display@1v1.sync(new id wl_callback@3)
[wayrs] wl_registry@2v1.global(1, "wl_compositor", 5)
[wayrs] wl_registry@2v1.global(2, "wl_drm", 2)
[wayrs] wl_registry@2v1.global(3, "wl_shm", 1)
[wayrs] wl_registry@2v1.global(4, "wl_output", 4)
[wayrs] wl_registry@2v1.global(5, "zxdg_output_manager_v1", 3)
[wayrs] wl_registry@2v1.global(6, "wl_data_device_manager", 3)
[wayrs] wl_registry@2v1.global(7, "zwp_primary_selection_device_manager_v1", 1)
[wayrs] wl_registry@2v1.global(8, "wl_subcompositor", 1)
[wayrs] wl_registry@2v1.global(9, "xdg_wm_base", 6)
[wayrs] wl_registry@2v1.global(10, "gtk_shell1", 5)
[wayrs] wl_registry@2v1.global(11, "wp_viewporter", 1)
[wayrs] wl_registry@2v1.global(12, "wp_fractional_scale_manager_v1", 1)
[wayrs] wl_registry@2v1.global(13, "zwp_pointer_gestures_v1", 3)
[wayrs] wl_registry@2v1.global(14, "zwp_tablet_manager_v2", 1)
[wayrs] wl_registry@2v1.global(15, "wl_seat", 8)
[wayrs] wl_registry@2v1.global(16, "zwp_relative_pointer_manager_v1", 1)
[wayrs] wl_registry@2v1.global(17, "zwp_pointer_constraints_v1", 1)
[wayrs] wl_registry@2v1.global(18, "zxdg_exporter_v2", 1)
[wayrs] wl_registry@2v1.global(19, "zxdg_importer_v2", 1)
[wayrs] wl_registry@2v1.global(20, "zxdg_exporter_v1", 1)
[wayrs] wl_registry@2v1.global(21, "zxdg_importer_v1", 1)
[wayrs] wl_registry@2v1.global(22, "zwp_linux_dmabuf_v1", 4)
[wayrs] wl_registry@2v1.global(23, "wp_single_pixel_buffer_manager_v1", 1)
[wayrs] wl_registry@2v1.global(24, "zwp_keyboard_shortcuts_inhibit_manager_v1", 1)
[wayrs] wl_registry@2v1.global(25, "zwp_text_input_manager_v3", 1)
[wayrs] wl_registry@2v1.global(26, "wp_presentation", 1)
[wayrs] wl_registry@2v1.global(27, "xdg_activation_v1", 1)
[wayrs] wl_registry@2v1.global(28, "zwp_idle_inhibit_manager_v1", 1)
[wayrs] wl_callback@3v1.done(160)
[wayrs]  -> wl_registry@2v1.bind(22, new id 4@zwp_linux_dmabuf_v1v4)
[wayrs]  -> wl_registry@2v1.bind(1, new id 5@wl_compositorv5)
[wayrs]  -> wl_registry@2v1.bind(9, new id 6@xdg_wm_basev4)
[wayrs]  -> wl_compositor@5v5.create_surface(new id wl_surface@7)
[wayrs]  -> zwp_linux_dmabuf_v1@4v4.get_surface_feedback(new id zwp_linux_dmabuf_feedback_v1@8, 7)
[wayrs]  -> xdg_wm_base@6v4.get_xdg_surface(new id xdg_surface@9, 7)
[wayrs]  -> xdg_surface@9v4.get_toplevel(new id xdg_toplevel@10)
[wayrs]  -> wl_surface@7v5.set_buffer_transform(6)
[wayrs]  -> xdg_toplevel@10v4.set_app_id("wayrs-egl")
[wayrs]  -> xdg_toplevel@10v4.set_title("TITLE")
[wayrs]  -> wl_surface@7v5.commit()
[wayrs] wl_display@1v1.delete_id(3)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.format_table(fd 4, 240)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.main_device(<array>)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.tranche_target_device(<array>)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.tranche_flags(0)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.tranche_formats(<array>)
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.tranche_done()
[wayrs] zwp_linux_dmabuf_feedback_v1@8v4.done()
[wayrs] xdg_toplevel@10v4.configure_bounds(1920, 1048)
[wayrs] xdg_toplevel@10v4.configure(0, 0, <array>)
[wayrs] xdg_surface@9v4.configure(10)
thread 'main' panicked at wayrs-egl/examples/triangle.rs:403:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

btw this is just running in a vm guest using the compositor in the guest. not using the virto wayland protocol

MaxVerevkin commented 8 months ago

I've added some debug prints in https://github.com/MaxVerevkin/wayrs/commit/d2d70def6627296e4a0eef0ef962f142aeb6e6bd, could you try it?

madushan1000 commented 8 months ago
$XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
Feedback tranche contains 15 formats, 0 of them supported by EGL
thread 'main' panicked at wayrs-egl/examples/triangle.rs:410:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

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();
madushan1000 commented 8 months ago
$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
[wayrs-egl/examples/triangle.rs:372:9] egl_display.supported_formats() = {
    XB48: [],
    AB4H: [],
    R16 : [],
    XR30: [],
    AR30: [],
    RG16: [],
    GR32: [],
    AB30: [],
    AB48: [],
    XB24: [],
    XR24: [],
    GR88: [],
    AB24: [],
    R8  : [],
    AR24: [],
    XB30: [],
    XB4H: [],
}
Feedback tranche contains 15 formats, 0 of them supported by EGL
thread 'main' panicked at wayrs-egl/examples/triangle.rs:411:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

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);
     }
madushan1000 commented 8 months ago

still similar issue, I wonder if this is a crosvm issue, I'll try running it in qemu too.

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
[wayrs-egl/examples/triangle.rs:372:9] egl_display.supported_formats() = {
    XB30: [
        0,
    ],
    XB24: [
        0,
    ],
    AB30: [
        0,
    ],
    XB4H: [
        0,
    ],
    AB48: [
        0,
    ],
    XR24: [
        0,
    ],
    RG16: [
        0,
    ],
    GR32: [
        0,
    ],
    AR30: [
        0,
    ],
    R16 : [
        0,
    ],
    AB24: [
        0,
    ],
    XR30: [
        0,
    ],
    R8  : [
        0,
    ],
    AB4H: [
        0,
    ],
    AR24: [
        0,
    ],
    GR88: [
        0,
    ],
    XB48: [
        0,
    ],
}
Feedback tranche contains 15 formats, 0 of them supported by EGL
thread 'main' panicked at wayrs-egl/examples/triangle.rs:411:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

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!(

Huh, is zero modifier implicitly sported? Does this work? Not sure how correct that is, though.

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,
         }
     }
madushan1000 commented 8 months ago

still no,

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
[wayrs-egl/examples/triangle.rs:372:9] egl_display.supported_formats() = {
    XR30: [
        0,
    ],
    AB48: [
        0,
    ],
    XB30: [
        0,
    ],
    AR24: [
        0,
    ],
    GR32: [
        0,
    ],
    XB24: [
        0,
    ],
    XB4H: [
        0,
    ],
    AB30: [
        0,
    ],
    RG16: [
        0,
    ],
    AB4H: [
        0,
    ],
    R8  : [
        0,
    ],
    GR88: [
        0,
    ],
    XB48: [
        0,
    ],
    XR24: [
        0,
    ],
    AR30: [
        0,
    ],
    AB24: [
        0,
    ],
    R16 : [
        0,
    ],
}
Format RG16:72057594037927935 not supported by EGL
Format XR24:72057594037927935 not supported by EGL
Format AR24:72057594037927935 not supported by EGL
Format XB24:72057594037927935 not supported by EGL
Format AB24:72057594037927935 not supported by EGL
Format XR30:72057594037927935 not supported by EGL
Format AR30:72057594037927935 not supported by EGL
Format XB30:72057594037927935 not supported by EGL
Format AB30:72057594037927935 not supported by EGL
Format XB4H:72057594037927935 not supported by EGL
Format AB4H:72057594037927935 not supported by EGL
Format YUYV:72057594037927935 not supported by EGL
Format NV12:72057594037927935 not supported by EGL
Format P010:72057594037927935 not supported by EGL
Format YU12:72057594037927935 not supported by EGL
Feedback tranche contains 15 formats, 0 of them supported by EGL
thread 'main' panicked at wayrs-egl/examples/triangle.rs:417:22:
at least one supported format
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

72057594037927935 is DRM_FORMAT_MOD_INVALID, maybe it should be special-cased.

still no,

Event with the last patch? It was intended to be applied without the one which adds zeros. Try it from a clean git state.

madushan1000 commented 8 months ago

I applied the patch for a clean git tree now. same issue

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
Format YUYV:72057594037927935 not supported by EGL
Format NV12:72057594037927935 not supported by EGL
Format P010:72057594037927935 not supported by EGL
Format YU12:72057594037927935 not supported by EGL
Feedback tranche contains 15 formats, 11 of them supported by EGL
EGL v1.4
thread 'main' panicked at wayrs-egl/examples/triangle.rs:39:14:
called `Result::unwrap()` on an `Err` value: Egl(BadAttribute)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

A new error message, that's progress. I did some blind guessing what can be done on the https://github.com/MaxVerevkin/wayrs/tree/egl-mod-invalid branch. Can you try it?

MaxVerevkin commented 8 months ago

Oh wait. Context creation fails? What does eglinfo | grep 'OpenGL ES profile version' print?

madushan1000 commented 8 months ago
$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 eglinfo | grep 'OpenGL ES profile version'
OpenGL ES profile version: OpenGL ES 3.2 Mesa 23.2.1-1ubuntu3.1
OpenGL ES profile version: OpenGL ES 3.2 Mesa 23.2.1-1ubuntu3.1
OpenGL ES profile version: OpenGL ES 3.2 Mesa 23.2.1-1ubuntu3.1
OpenGL ES profile version: OpenGL ES 3.2 Mesa 23.2.1-1ubuntu3.1
OpenGL ES profile version: OpenGL ES 3.2 Mesa 23.2.1-1ubuntu3.1

with the egl-mod-invalid branch

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
Feedback tranche contains 15 formats, 11 of them supported by EGL
EGL v1.4
thread 'main' panicked at wayrs-egl/examples/triangle.rs:39:14:
called `Result::unwrap()` on an `Err` value: Egl(BadAttribute)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

Added more prints...

madushan1000 commented 8 months ago

yeah the context creation is the problem

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
Feedback tranche contains 15 formats, 11 of them supported by EGL
EGL v1.4
eglCreateContext failed
thread 'main' panicked at wayrs-egl/examples/triangle.rs:39:14:
called `Result::unwrap()` on an `Err` value: Egl(BadAttribute)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
MaxVerevkin commented 8 months ago

Maybe EGL_CONTEXT_OPENGL_DEBUG is problematic, I've removed it temporarily.

madushan1000 commented 8 months ago

oh yeah, now it works! nice. Thanks :D

$ XDG_RUNTIME_DIR=/run/user/1000/ WAYLAND_DISPLAY=wayland-0 ./triangle
DRM render node: /dev/dri/renderD128
Feedback tranche contains 15 formats, 11 of them supported by EGL
EGL v1.4
OpenGL-ES v3.2
mod is invalid, using gbm_bo_create
mod is invalid, using gbm_bo_create
MaxVerevkin commented 8 months ago

Turns out EGL_CONTEXT_OPENGL_DEBUG was introduced only in 1.5. Fixed in #12.

madushan1000 commented 8 months ago

thank you for the fix :)

madushan1000 commented 8 months ago

@MaxVerevkin looks like I have to do a custom implementation of zwp_linux_buffer_params_v1@<>.add function, because I have to set correct stride and modifiers. I can't figure out how to do this for the auto generated functions. Any pointers?

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.

MaxVerevkin commented 8 months ago

Not sure what your mean, the params::add function accepts the stride and modifiers as arguments.

madushan1000 commented 8 months ago

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.

MaxVerevkin commented 8 months ago

The code generated by wayrs-scanner must not have any hidden logic and must represent the protocol as defined in the xml file.

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.

madushan1000 commented 8 months ago

The problem I'm having now is when I'm using wayrs-egl the client doesn't directly call ZwpLinuxBufferParamsV1::add. It's called here. I can add a new fix_metadata function to the ClientTransport trait and call it from Buffer::alloc, is that a good solution?

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 ZwpLinuxBufferParamsV1::add. Otherwise, host compositor refuses to import the buffer.

MaxVerevkin commented 8 months ago

The problem is that wayrs-egl is very GBM-centric, and if I understand correctly, in your case, you do not use it. Maybe it should be generic over the allocator, GBM being the default.

madushan1000 commented 8 months ago

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.

MaxVerevkin commented 8 months ago

Okay, I just don't understand how it works. Are there any docs?