[Question] Minimal example with drm-rs(dev) and gbm.rs

[pum-purum-pum-pum]

I want to use code in dev branch of drm-rs (which seems to be far ahead from master) with gbm.rs lib. It should be pretty straightforward I beleive (given that the draft already exists on gmb page), but just in case: is there any example that already does this?

[Drakulix]

I have transferred this issue, because integration with drm-rs is done in gbm.rs and therefor this is more suited to be discussed here.

There is unfortunately no small example. smithay makes use of gbm.rs in combination with drm-rs though.

Most of that happens in the current GbmSurface-code, which creates gbm buffers and surfaces and attaches them to a drm-rs framebuffer like you would expect.
It also uses the buffer objects userdata to store the framebuffer handles for caching reasons.

This code is going to change a lot and not use surfaces anymore and only buffers, but hopefully one of these links can give you some idea.

If you want to modify one of the drm-rs examples to use gbm.rs instead of a dumb-buffer for rendering, I would happily accept a PR adding that to this repository.

[Zerowalker]

I'm a bit lost myself on this.
i'm trying to translate a code i got in c++ to this, and it utilizes gbm_surface.
The purpose is to draw on on it with egl.

So you get the display and device then initialize egl against it and then you can start drawing with OpenGL.

Are you supposed to use a framebuffer instead when you do it in rust, it's a bit confusing how to translate it all?

[Drakulix]

Are you supposed to use a framebuffer instead when you do it in rust, it's a bit confusing how to translate it all?

No, not at all. If you want to use a surface (which depends on your use-case, there is a reason smithay is doing this manually with buffers and a drm framebuffer), that is totally doable!

You basically create a GbmDevice from a DrmDevice (which is the same step as gbm_create_device with a drm-node file descriptor) and then you can call Device::create_surface (or its variants create_surface_with_modifiers or create_surface_with_modifiers2), which is the same as gbm_surface_create (or gbm_surface_create_with_modifiers, etc).

You can then use the AsRaw implementatation of the GbmDevice and the GbmSurface to get raw-pointers, which you can give to your egl-code.
(This would be nicer if both types would implement the corresponding traits from the raw_window_handle-crate, which is supported by many graphic libraries, but the pointers work the same way.)

How you then get your egl display, context, surface depends on how you bind to egl. There are some crates for this, smithay has it's own egl-abstractions/wrappers and there is also always the option to generate raw-bindings using the gl_generator-crate yourself.

Does that help?

[Zerowalker]

I got it working, it took a lot of trial and error and i just mimicked what was going on in the original code.
It's for Raspberry Pi so it might be a bit of a special case.
The messiest part for me is the DRM/GBM setup, there's most likely a much better way to do it than how i did it.

I will post parts of the code here if it might help or if it might just be crap.

DRM/GBM

use drm::control::connector::State;
use drm::control::Device as ControlDevice;
use drm::SystemError;
use gbm::Surface;
use log::debug;

use crate::gl::GraphicsDisplaySize;

pub struct DrmState {
    pub device: gbm::Device<Card>,
    pub surface: Surface<()>,
    pub crtc: drm::control::crtc::Info,
    pub conn: drm::control::connector::Info,
    pub display_size: GraphicsDisplaySize,
}

/// A simple wrapper for a device node.
pub struct Card(std::fs::File);
/// Simple helper methods for opening a `Card`.
impl Card {
    pub fn open(path: &str) -> Self {
        let mut options = std::fs::OpenOptions::new();
        options.read(true);
        options.write(true);
        Card(options.open(path).unwrap())
    }
}

impl drm::Device for Card {}
impl ControlDevice for Card {}

/// Implementing `AsRawFd` is a prerequisite to implementing the traits found
/// in this crate. Here, we are just calling `as_raw_fd()` on the inner File.
impl std::os::unix::io::AsRawFd for Card {
    fn as_raw_fd(&self) -> std::os::unix::io::RawFd {
        self.0.as_raw_fd()
    }
}

fn get_crtc(gbm: &gbm::Device<Card>) -> Result<drm::control::crtc::Info, SystemError> {
    for l in gbm.resource_handles()?.crtcs {
        let crtc = gbm.get_crtc(l)?;
        if crtc.mode().is_some() {
            return Ok(crtc);
        }
    }
    Err(SystemError::InvalidArgument)
}

fn get_conn(gbm: &gbm::Device<Card>) -> Result<drm::control::connector::Info, SystemError> {
    for i in gbm.resource_handles()?.connectors {
        let conn = gbm.get_connector(i, true)?;
        if conn.state() == State::Connected {
            return Ok(conn);
        }
    }
    Err(SystemError::InvalidArgument)
}

impl DrmState {
    pub fn swap_buffers(&self) {
        let bo = unsafe { self.surface.lock_front_buffer().unwrap() };
        let fb = self.device.add_framebuffer(&bo, 24, 32).unwrap();
        self.device
            .set_crtc(
                self.crtc.handle(),
                Some(fb),
                (0, 0),
                &[self.conn.handle()],
                self.crtc.mode(),
            )
            .unwrap();
    }
}
pub fn init_drm() -> DrmState {
    use gbm::{BufferObjectFlags, Device, Format};
    // ... init your drm device ...
    let drm = Card::open("/dev/dri/card0");

    // init a GBM device
    let gbm = Device::new(drm).unwrap();

    let crtc = get_crtc(&gbm).expect("drm: failed to get crtc");
    let conn = get_conn(&gbm).expect("drm: failed to get connector");

    // get display width/height
    let (w, h) = crtc.mode().unwrap().size();

    // create a surface
    let surface = gbm
        .create_surface::<()>(
            w as u32,
            h as u32,
            Format::Xrgb8888,
            BufferObjectFlags::SCANOUT | BufferObjectFlags::RENDERING,
        )
        .unwrap();

    debug!("crtc: {:?}", crtc);
    debug!("current encoder: {:?}", conn.current_encoder());

    DrmState {
        device: gbm,
        surface,
        crtc,
        conn,
        display_size: GraphicsDisplaySize {
            width: w as u32,
            height: h as u32,
        },
    }
}

EGL

pub fn init_egl(
    device: khronos_egl::NativeDisplayType,
    surface: khronos_egl::NativeWindowType,
) -> EGLData {
    let egl = khronos_egl::Instance::new(khronos_egl::Static);
    let egl_display = egl.get_display(device).expect("egl: get_display failed");
    egl.initialize(egl_display).expect("egl: initialize failed");

    // egl config attributes
    let egl_attribs = [
        khronos_egl::RED_SIZE,
        8,
        khronos_egl::GREEN_SIZE,
        8,
        khronos_egl::BLUE_SIZE,
        8,
        khronos_egl::DEPTH_SIZE,
        8,
        khronos_egl::RENDERABLE_TYPE,
        khronos_egl::OPENGL_ES2_BIT,
        khronos_egl::NONE,
    ];
    let mut configs = Vec::with_capacity(
        egl.matching_config_count(egl_display, &egl_attribs)
            .expect("egl: matching_counfig_count failed") as usize,
    );
    egl.choose_config(egl_display, &egl_attribs, &mut configs)
        .expect("egl: choose_config failed");

    let egl_config = configs
        .iter()
        .find(|e| {
            if let Ok(id) = egl.get_config_attrib(egl_display, **e, khronos_egl::NATIVE_VISUAL_ID) {
                id == gbm::Format::Xrgb8888 as i32
            } else {
                false
            }
        })
        .cloned()
        .expect("egl: no matching config found");

    egl.bind_api(khronos_egl::OPENGL_ES_API)
        .expect("egl: bind_api failed");
    unsafe {
        gl_loader_helper!(
            glCreateShader,
            glCompileShader,
            glShaderSource,
            glGetShaderiv,
            glCreateProgram,
            glAttachShader,
            glLinkProgram,
            glGetProgramiv,
            glDeleteProgram,
            glGetProgramInfoLog,
            glUseProgram,
            glClearColor,
            glClear,
            glGenBuffers,
            glBindBuffer,
            glGetAttribLocation,
            glGetUniformLocation,
            glBufferData,
            glVertexAttribPointer,
            glEnableVertexAttribArray,
            glUniform2fv,
            glUniform1f,
            glDrawElements
        );
    };

    let egl_context_attribs = [khronos_egl::CONTEXT_CLIENT_VERSION, 2, khronos_egl::NONE];
    let egl_context = egl
        .create_context(egl_display, egl_config, None, &egl_context_attribs)
        .expect("egl: create_context failed");

    debug!("egl context created");

    let egl_buffer = unsafe {
        egl.create_window_surface(egl_display, egl_config, surface, None)
            .expect("egl: create_window_surface failed")
    };
    debug!("egl window surface created");
    egl.make_current(
        egl_display,
        Some(egl_buffer),
        Some(egl_buffer),
        Some(egl_context),
    )
    .expect("egl: make_current failed");
    EGLData {
        egl,
        egl_buffer,
        egl_display,
    }
}

in egl here's the device/display i used.

    let egl_data = gl::init_egl(
        drm_state.device.as_raw() as khronos_egl::NativeDisplayType,
        drm_state.surface.as_raw() as khronos_egl::NativeWindowType,
    );

and when drawing i just swap egl buffers and then in the drm.
It's a bit messy as the functions here are taken from different mod files so it might be confusing,
but hopefully it gets the idea at least.

[Drakulix]

That looks mostly fine (though I would have to see the whole code to judge if everything is used correctly), the only thing I would try to avoid is creating a new framebuffer for every swap (and never freeing them).

The buffers returned by the GbmSurface actually cycle through, so you can use BufferObject::set_userdata and BufferObject::user_data to cache them!
Basically on every swap you would check if user_data returns a framebuffer or None and either use that or create a new one and store it inside the buffer. (The framebuffer is just an id and implements Copy, so there is no need to deal with any references here.)
That way you never create more framebuffers then you actually need.

[Zerowalker]

That's great that the impressions is that it seems mostly fine, so at least i'm not completely of track:)
Understandable that you need the whole code to judge, it's quite messy and i'm trying to clean it up but it's not going too well,
i will try upload the project and share it here.

the only thing I would try to avoid is creating a new framebuffer for every swap (and never freeing them).

I'm assuming you mean the DRMState::swap_buffers part right?
I wasn't sure how to clean up stuff and thought it was handled by Rust (Drop) as the code isn't normally unsafe i think,
Before i took some example code which utilized create_buffer_object instead of create_surface,
and that wasn't unsafe. Is it perhaps that it will cleanup the buffer_object automatically and that in itself frees the framebuffer?

Not sure what you mean with the GbmSurface actually cycle through etc,
do you mean that i only need to create the surface/buffer and add them to the framebuffer once and then just swap them?
Cause that was one thing i found odd, as it looks like i re-create the framebuffer all the time, it would make more sense to make it once and then just access them (by locking like you do the surface).
(But then again i'm not that great at graphics stuff so i get that my understanding of the steps is quite limited).

I will try to adjust based on your feedback and also upload the project in some way:)

EDIT:

Do you mean something like this for the swap_buffers?

        let mut bo = unsafe { self.surface.lock_front_buffer().unwrap() };

        let fb = bo
            .userdata()
            .unwrap()
            .map(ToOwned::to_owned)
            .unwrap_or_else(|| {
                let fb = self.device.add_framebuffer(&bo, 24, 32).unwrap();
                bo.set_userdata(fb).unwrap();
                fb
            });

        self.device
            .set_crtc(
                self.crtc.handle(),
                Some(fb),
                (0, 0),
                &[self.conn.handle()],
                self.crtc.mode(),
            )
            .unwrap();

[Drakulix]

I'm assuming you mean the DRMState::swap_buffers part right?

yes.

I wasn't sure how to clean up stuff and thought it was handled by Rust (Drop) as the code isn't normally unsafe i think,
Before i took some example code which utilized create_buffer_object instead of create_surface,
and that wasn't unsafe. Is it perhaps that it will cleanup the buffer_object automatically and that in itself frees the framebuffer?

It is not unsafe from rust's perspective to leak memory! That is a safe operation (though mostly this is indeed prevented by Drop).
In the framebuffer case there is no way to implement Drop, because the framebuffer is a kernel object. We just get an id, that is not meaningful other than for referrring to said object through the drm api. So you have to call drm::control::Device::destroy_framebuffer manually to free it.
(Note, that with the GbmSurface-api you are essentially forced to leak the framebuffers, because you have no control over when the internal buffer objects of the surface are released.)

Not sure what you mean with the GbmSurface actually cycle through etc,

The GbmSurface will internally re-use BufferObjects. So multiple calls to lock_front_buffer will return the same buffers (as long as the surface was not resized, etc)!

do you mean that i only need to create the surface/buffer and add them to the framebuffer once and then just swap them?

The other way around, you can create a framebuffer, associated with a front buffer, and attach that to the front buffer object. (Theoretically you could also just create two framebuffers and swap them, but there is no guarantee, that the GbmSurface will never give you a new buffer you haven't seen before. In those cases your framebuffers would be wrong.)

Do you mean something like this for the swap_buffers?

Yes that should work! Exactly!

[Zerowalker]

We just get an id, that is not meaningful other than for referrring to said object through the drm api. So you have to call drm::control::Device::destroy_framebuffer manually to free it.

Can't you just wrap the id (Handle) and on Drop it destroys it?
I get that might have an invalid id if it's dropped manually though, but at least it should be cleaned up i think.

(Note, that with the GbmSurface-api you are essentially forced to leak the framebuffers, because you have no control over when the internal buffer objects of the surface are released.)

That feels odd, not saying it's not like that though, not sure i quite understand it.
Do you mean you can clean the surface itself, but the framebuffers you get from locking it is out of control?

Yes that should work! Exactly!

Yay:)
Though i must say that it doesn't make much sense to me with the userdata.
Is it just that the userdata is convenient as it can store something along with the surface,
or does it actually do something storing it like that?

[Drakulix]

Can't you just wrap the id (Handle) and on Drop it destroys it?

The problem with that is, that cleaning up requires the DrmDevice. We could store the file-descriptor from the device, that is necessary to do that, inside the proposed Handle, but you would not want to clone that, because that invites a whole bunch of issues. And borrowing it would give every Handle derived from a DrmDevice a lifetime, which is also a nightmare to use.

Believe me, we have experimented a lot with the api and this is the best we came up with...

Do you mean you can clean the surface itself, but the framebuffers you get from locking it is out of control?

Right, because libgbm (the C-library this binds to) internally frees the buffers.

Is it just that the userdata is convenient as it can store something along with the surface,

Correct. And because the gbm-surface sometimes returns you buffers, that you have already seen this is just a convenient way to create exactly one framebuffer per buffer object in use.