smithay icon indicating copy to clipboard operation
smithay copied to clipboard

Implement SinglePixelBuffer protocol

Open PolyMeilex opened this issue 3 years ago • 7 comments

Tested with anvil's viewporter implementation, and everything appear to work like a charm.

Draft because:

  • Waiting for wayland-rs protocols release.
  • Needs better docs.

PolyMeilex avatar Aug 25 '22 10:08 PolyMeilex

What is the reason behind creating a Texture for the buffer? For Gles2Renderer we already have a solid color shader that could be used to draw a solid color.

cmeissl avatar Aug 25 '22 10:08 cmeissl

What is the reason behind creating a Texture for the buffer? For Gles2Renderer we already have a solid color shader that could be used to draw a solid color.

Mostly for simplicity, creating texture does not require adding any special paths into the rendering flow, it's just import and render_texture_from_to. The buffer can be treated by users like any other. I would like to keep one render method for everything, and currently our render method is based around TextureId.

wlroots and Mutter do the same, Weston single pixel buffers on the other hand have special treatment. As their rendering flow is based on WestonBuffer rather than Texture, so the special case is basically transparent there.

PolyMeilex avatar Aug 25 '22 11:08 PolyMeilex

What is the reason behind creating a Texture for the buffer? For Gles2Renderer we already have a solid color shader that could be used to draw a solid color.

Mostly for simplicity, creating texture does not require adding any special paths into the rendering flow, it's just import and render_texture_from_to. The buffer can be treated by users like any other. I would like to keep one render method for everything, and currently our render method is based around TextureId.

wlroots and Mutter do the same, Weston single pixel buffers on the other hand have special treatment. As their rendering flow is based on WestonBuffer rather than Texture, so the special case is basically transparent there.

Background for my question is that I will need to do some hole punching using the renderer for supporting drm underlay planes. I thought about sharing some code with single pixel buffer for that and I don't like the idea of creating temporary textures for hole punching. But probably it would't be that bad if the texture is cached, it could use the same texture for every hole anyway. BUT that would also require some changes to support disable blending. So in the end the better way to handle that is to create a custom render element which just uses the existing clear function on the Frame.

So :+1: from me

cmeissl avatar Aug 25 '22 11:08 cmeissl

I think I have to agree with @cmeissl here.

I don't necessarily want to add new API calls or even break the existing one, but if we don't need a texture here (and already have a render function), I don't really want this to work this way.

As a compromise to not break the api, we could change the Gles2Texture into an enum with a normal texture variant and a solid color. In that case render_texture_from_to should always just render the color (with blending, so just calling clear is not enough here) to the dst rectangle.

That way import_buffer could still return a TextureId (though only ImportAll would work, if we don't add something like ImportSolid) and render_texture_from_to would also still work.

But given that it is not really a texture, that kinda feels like a hack, though given the Renderer is an abstraction, that is fine by me.

Drakulix avatar Aug 25 '22 16:08 Drakulix

I think I have to agree with @cmeissl here.

I don't necessarily want to add new API calls or even break the existing one, but if we don't need a texture here (and already have a render function), I don't really want this to work this way.

As a compromise to not break the api, we could change the Gles2Texture into an enum with a normal texture variant and a solid color. In that case render_texture_from_to should always just render the color (with blending, so just calling clear is not enough here) to the dst rectangle.

That way import_buffer could still return a TextureId (though only ImportAll would work, if we don't add something like ImportSolid) and render_texture_from_to would also still work.

But given that it is not really a texture, that kinda feels like a hack, though given the Renderer is an abstraction, that is fine by me.

I attempted this, but encountered some problems, if we lie that solid color is a gles texture then one would expect dmabuf export_texture method to work, the first solution that comes to mind is creating a temporary texture to hold the pixel that would be exported as dma, but that's what we tried to avoid in the first place. So it feels kinda hacky.

Maybe adding render_solid method to the renderer is not as bad as I initially though? Compositors could also use it to render some simple quads. draw_surface_tree would have to be modified to support that tho.

Any thoughts?

PolyMeilex avatar Sep 04 '22 22:09 PolyMeilex

I attempted this, but encountered some problems, if we lie that solid color is a gles texture then one would expect dmabuf export_texture method to work, the first solution that comes to mind is creating a temporary texture to hold the pixel that would be exported as dma, but that's what we tried to avoid in the first place. So it feels kinda hacky.

Yeah, that is indeed a problem.

Maybe adding render_solid method to the renderer is not as bad as I initially though? Compositors could also use it to render some simple quads. draw_surface_tree would have to be modified to support that tho.

Any thoughts?

This sounds like a proper solution. We already have a frame.clear() function anyway. render_solid is not too far off.

Drakulix avatar Sep 05 '22 10:09 Drakulix

Rebased, and I dropped all of the rendering related code. tbh I have no idea how to plug this into draw_solid method previous 1x1 buffer approach just used existing code path for buffers and textures. So anyone feel free to pick this up instead of me, the Wayland code should be good to go.

PolyMeilex avatar Mar 26 '23 20:03 PolyMeilex