Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Renderer redesign v6 #1352

Closed
ascent12 opened this issue Nov 1, 2018 · 20 comments
Closed

Renderer redesign v6 #1352

ascent12 opened this issue Nov 1, 2018 · 20 comments

Comments

@ascent12
Copy link
Member

ascent12 commented Nov 1, 2018

I've been doing some casual research into some DRM/Vulkan related stuff and came up with an idea.

With the changes required to the renderer for Vulkan etc., there is going to be a very significant difference between the rendering paths of the DRM backend and the rest of them, where we need to handle buffers ourselves in DRM, and use a provided swapchain (EGLSurface or VkSurface+VkSwapchain) for the others.

Having two code paths is kind of crap, so why don't we just use one?
Basically, we throw EGLSurfaces/VkSurface/VkSwapchains out entirely, and instead write our own swapchain.

We allocate two (or more?) GBM buffers, wrap them in our swapchain, and use the appropriate backend-specific way to display it. This can work for all backends, not just DRM.

Wayland

Use zwp_linux_dmabuf_v1 and maybe wp_presentation directly.
This does require the DRM render node to be given to us, which is supposed to come in the next version of the protocol.
zwp_linux_explicit_syncronization would also be relevant, whenever that ends up being done.

X11

Use the dri3 and present extension directly. The dri3 extension can give us the DRM render node.

DRM

Business as usual.

Headless

Just pick some arbitrary render node and use that.

Potential future backends

GBM buffers are essentially dmabufs, so any backend which can handle dmabufs can work.
They're also mmapable, so this doesn't exclude purely software based backends and renderers.

Advantages

  • Opens the possibility of Multiple renderers at the same time #778 to be revived again, or at least sharing a rendering node and EGL/VK context between different backend types.
  • Adds a lot of consistency between backends; moves a lot of code that would be DRM-specific.
  • Makes the API between EGL and Vulkan more similar. We're just dealing with EGLImages and VkImages, which are analogues to each other, and there is no such concept of a native platform.
  • Potentially allows non-dmabuf based screencapture protocols to be removed, as we will now be certain we have a dmabuf.

Disadvantages

  • More work for us to do. We have to do all of the X11 and Wayland bits that Mesa would have provided for us. This includes things like damage handling.
    I've already done some investigation to how to do it on X11, and it doesn't seem that hard. I'm sure everyone also understands Wayland, so that's easy too.
@emersion
Copy link
Member

emersion commented Nov 1, 2018

What about software renderers?

Potentially allows non-dmabuf based screencapture protocols to be removed, as we will now be certain we have a dmabuf.

Note that some clients don't want DMA-BUFs, because they need data in main memory. For grim for instance, DMA-BUFs would be a burden.

@ddevault
Copy link
Contributor

ddevault commented Nov 1, 2018

Note that some clients don't want DMA-BUFs, because they need data in main memory. For grim for instance, DMA-BUFs would be a burden.

Agreed.

Otherwise this idea looks pretty solid, I like it.

@emersion
Copy link
Member

emersion commented Nov 1, 2018

Daniel Stone said on IRC it's not a good idea to use GBM for shm.

15:01 <daniels> emersion: gbm won't do shm alloc for you, no
15:01 <emersion> hmm
15:01 <emersion> so for a software renderer we shouldn't use gbm?
15:01 <daniels> right, because there's no guarantee the buffers will be usable for you

@nyorain
Copy link
Contributor

nyorain commented Nov 6, 2018

fwiw i like the idea as well. It would indeed match the way things can be done (hopefully soon that is) with vulkan more naturally and allow to share more code between vulkan/gl but i'm not sure things are ready for this yet and that good abstractions justifiy the drawbacks. Software renderers and using wlroots on weird platforms may be theoretical now but shouldn't we try to keep everything as easily extendable as possible? E.g. for vulkan, do we know that VK_EXT_image_drm_format_modifier will be implemented for all hardware/drivers we want to support?
But with those things cleared up i would really like to see this work (will gladly rework the vulkan code for this then).

@ascent12
Copy link
Member Author

ascent12 commented Nov 7, 2018

I thought I would go over some of the things that I've currently added in my PR and a bit of the rationale behind them.
None of this is final. Any feedback or ideas are welcome.

wlr_allocator + wlr_image

Instead of going straight GBM, this is a type that loosely wraps over it (and other allocators). All it does it "allocate" and "deallocate", and wlr_image is a wrapper over that allocation. It deliberately does not do anything else; I believe trying to wrap over any of the actual allocator details would be a mistake, and lead to an extremely clunky and unwieldy interface.

Renderers would directly create and use one of the types below, as they really need to know about the details of what allocator they're using.

This is basically required so wlr_swapchain is not tied to a specific allocator, and is mostly just boilerplate.

wlr_gbm_allocator

(I'm thinking of renaming this to just wlr_gbm)
Does what you think it does. It creates a GBM device, and allocates buffers for you. It is considered a utility type for renderers, and anyone using the wlr_renderer interface would generally not need to touch this directly.

This could be used by either an OpenGL or Vulkan renderer, or anything that can deal with dmabufs.

Some new backend functions were added specifically for this type:

  • get_render_fd: Gets the file descriptor to use with gbm_create_device().
  • attach_gbm: Gets the backend to add anything required for this buffer to be usable for presentation.
    • drmModeAddFB2(withModifiers) for DRM,
    • create a wl_buffer via wp_linux_dmabuf for wayland
    • create an xcb_pixmap_t via dri3 for x11
    • headless probably doesn't need to do anything.
  • detach_gbm: Just the opposite of above.

wlr_shm_allocator

This is just a possible type we can add, but I'm not going to do it as part of the PR.
Another renderer utility type, but mainly useful for pure software renderers. The buffers would be directly accessible from the CPU.

Some new allocator-specific functions would be added to the backend (also not part of the PR).
It would boil down to

  • Create a dumb buffer + drmModeAddFB2 for DRM
  • Create a wl_buffer via wl_shm for wayland
  • Create an xcb_pixmap_t via shm for x11
  • Just malloc a buffer for headless.

We'd only support linear XRGB8888 or ARGB8888 for this allocator. Supporting anything else seems like it'd be a huge pain.

It would be possible to write an OpenGL/Vulkan renderer which uses this allocator, but it would be suboptimal and probably pointless.

wlr_egl

This has been "demoted" to a pure utility type for writing OpenGL renderers. If you're not writing exactly that, you would not be touching this type.

Much of the generality has been removed and it is now GBM specific.

A nice little consequence of the new design is now that we're doing surfaceless rendering, we don't need to deal with EGLConfigs anymore. I hated those damn things.

Since we're dealing with non-default framebuffers in OpenGL, people writing their own renderers are free to attach their own depth or stencil buffers, making wlr_renderer_create_func_t unnecessary.

wlr_output_present

Replaces the old "swap buffers" thing. Asks the output to use our buffer, and it will notify us when we're free to use it again.
It'll work very closely to wl_surface.attach + wl_surface.commit + wl_buffer.release from the wayland protocol.

I also want this to become the point where direct-scanout or anything similar happens. If we want to scan something out, we just take a wlr_image created from a client buffer, and pass it here.

Damage still needs to be added to this interface.

wlr_format_set

Just a set of formats + modifiers. I'll also add a function which takes the union of this set.
We could get the set of what the backend is capable of presenting, plus the set of what the renderer is capable of drawing, and union them to get the set of formats we could use for our buffers.

Other notes

  • I'll probably remove glgen.sh + glapi.txt. I'm trying to do everything properly and check extension strings, and it's probably more lines of code to keep this around instead of just doing it directly.
  • I'm not sure if or how I'm going to change how wlr_texture works.
  • I would like to remove wlr_renderer from as many types as I can (this is an old point of mine). I'll see if it's viable.
  • I want to add fence support to most of the interfaces.
  • I want to make everything support modifiers.

@emersion
Copy link
Member

emersion commented Nov 7, 2018

Replaces the old "swap buffers" thing. Asks the output to use our buffer, and it will notify us when we're free to use it again.

I think it makes sense to keep the frame event for this interface. I don't think release belongs here.

@ascent12
Copy link
Member Author

ascent12 commented Nov 7, 2018

I think it makes sense to keep the frame event for this interface

Right.

I don't think release belongs here.

'release' and 'frame' aren't quite the same thing, so they're currently separate.

@emersion
Copy link
Member

emersion commented Nov 7, 2018

"Release" belongs to the wlr_image IMHO. There should be a wlr_image_release that triggers a signal on the wlr_image, which could be used by the wlr_swapchain to update its state.

"Frame" really means "please draw". On Wayland this is wl_surface.frame. On DRM this could be emitted just like it is currently right after pageflips, or a little bit later if we implement presentation scheduling.

  • "Frame" is emitted by the output
  • The compositor renders
  • wlr_output_present is called

@ascent12
Copy link
Member Author

ascent12 commented Nov 7, 2018

Ok, that makes sense.

@emersion
Copy link
Member

emersion commented Nov 7, 2018

Forgot to say: overall this looks good™.

(I'm thinking of renaming this to just wlr_gbm)

This would be kind of inconsistent with our other naming conventions (e.g. we have wlr_drm_backend, not wlr_drm).

This has been "demoted" to a pure utility type for writing OpenGL renderers. If you're not writing exactly that, you would not be touching this type.

+1, it has already become more-or-less this.

wlr_format_set

Instead of a list of lists, an alternate design would be to have a list of (format, modifier). Maybe this can simplify the interface.

I'm trying to do everything properly and check extension strings

+1

I would like to remove wlr_renderer from as many types as I can (this is an old point of mine). I'll see if it's viable.

Rationale?

Maybe it'll become possible to write a renderer-less compositor? A use-case would be to use wlroots to build a display manager (or anything else that already has a buffer ready for presentation).

I want to add fence support to most of the interfaces.

+1, the explicit synchronization protocol will be merged soon in wayland-protocols

I want to make everything support modifiers.

+1

@ascent12
Copy link
Member Author

ascent12 commented Nov 7, 2018

Rationale?
Maybe it'll become possible to write a renderer-less compositor? A use-case would be to use wlroots to build a display manager (or anything else that already has a buffer ready for presentation).

I've always wanted wlr_renderer to just be our shitty drawing API that people with simple requirements can use, but not actually be required for anything to actually be done. Just like how SDL has their awful rendering functions, but if you actually want to do anything complicated, you just go for straight OpenGL.

@ascent12
Copy link
Member Author

I thought I'd go into more detail about something I want to do as part of this, which I briefly mentioned on IRC.

Basically, I was thinking that we could move all of the wlr_buffer related operations to live inside of the renderer. The renderer is responsible for advertising the globals, and nobody else really needs to care how buffers are allocated at all.

This isn't actually that different to how it currently works, since the renderer handles zwp_linux_dmabuf_v1 and wl_drm (EGL), but now wl_shm would be like that too.

  • Add struct wlr_shm which handles everything related to wl_shm, taken from what currently exists in wlr_buffer.
    Probably will use a callback for allocation and write operations.
  • Change wlr_linux_dmabuf_v1 to take a wlr_gbm_allocator and wlr_format_set instead of a wlr_renderer.
    I may move the type to live inside render/ instead of types/, just for make it clear who's supposed to be using it.
  • wlr_egl is technically a wl_buffer factory too, but doesn't need to change much.

With all of this living inside the renderer, wlr_buffer probably doesn't need to exist at all, and the user can just deal with the wl_resource.

// It doesn't matter how this buffer was made; the renderer knows about it.
struct wlr_texture *wlr_texture_from_wl_buffer(struct wlr_renderer *renderer, struct wl_resource *buffer);
void wlr_texture_apply_damage(struct wlr_texture, pixman_region32_t *damage);

@ascent12
Copy link
Member Author

After more thought and investigation about how zwp_linux_explicit_synchronization_v1 is going to work, I realised that wlr_texture is going to need to know some wl_surface state as opposed to just wl_buffer.
I'd rather not expose too many of the gory details of dma-fences to users (unless they want to by not using wlr_renderer), because it'd probably be fairly complicated and require a bunch of back and forth between the renderer and wlr_surface, which is one of the things I'm trying to remove.
I was thinking that it could become

struct wlr_texture *wlr_texture_from_surface(struct wlr_renderer *renderer, struct wlr_surface *surface);

and the wlr_texture can listen to the commit signal, and update itself accordingly. When the texture is used, it'll use the most up-to-date buffer that has been committed without the user needing to pass anything else around.

Although there are a few things that probably need to be thought out more, particularly involving buffer releases and possibly using older versions of a client's buffer.

@ddevault
Copy link
Contributor

Can you speak to some of the rationale behind that constraint?

@ascent12
Copy link
Member Author

The explicit sync protocol works per-commit and is defined in terms of a wl_surface.
The complexity I don't want to force on the users is checking whether the renderer even supports explicit sync, dealing with unsignalled fences, and making sure to release them properly.
A situation that now arises from explicit sync is a client giving us several buffers all with unsignalled fences within the same frame. Ideally we want to use the latest buffer that's ready at the time we want to render, and not just rely on the latest one given to us.

Perhaps something like

/*
 * Texture is no longer something we render directly with, but is really a container for dealing with
 * the rendering part of a wlr_surface. We could change the name.
 */
struct wlr_texture *wlr_texture_from_surface(struct wlr_renderer *renderer, struct wlr_surface *surface);

/*
 * This is what we actually render with. It's not the same as the wlr_buffer which currently exists,
 * but I like this name, because it corresponds to a wl_buffer.
 * It will return the latest frame ready for rendering. All older frames that aren't referenced are also released.
 * It's reference counted.
 */
struct wlr_buffer *wlr_texture_get_buffer(struct wlr_texture *tex);
struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer);
// -1 if there is no fence
void wlr_buffer_release(struct wlr_buffer *buffer, int release_fence);

@Ongy
Copy link

Ongy commented Feb 12, 2019

sway and waymonad at the very least already have a "last usable buffer" kinda concept for the atomic tree update stuff.
And that'll have to live outside wlroots either way afaict. I haven't looked at the explicit fencing, but is it enough of a hassle to warrant entangling considering that code already exists in compositors?

@emersion emersion changed the title [RFC] Renderer redesign v6 Renderer redesign v6 Sep 18, 2020
@emersion
Copy link
Member

Initial renderer v6 redesign merged for DRM and headless. Will work on Wayland and X11.

emersion added a commit to emersion/wlroots that referenced this issue Nov 26, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit to emersion/wlroots that referenced this issue Nov 26, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
@emersion
Copy link
Member

Now that I've done the (yet-to-be-merged) work to convert all backends to wlr_swapchain, I'm starting to plan for the next step: remove any wlr_renderer usage from the backends. Instead wlr_output would be responsible for maintaining the swapchain. Backends will only consume and display wlr_buffers.

For this to happen, we need a few things to happen:

  • Expose the DRM device used by the backend to be able to create the wlr_renderer with the correct render node.
  • Expose the available formats for the primary plane.
  • Expose the available formats for the cursor plane.
  • Expose the cursor buffer size limitations.
  • We'd need to move the WLR_DRM_NO_MODIFIERS fallback logic out of the DRM backend. Not sure how this can work yet.

I hope we can at some point get rid of the per-plane formats getters and replace them with per-layer feedback the backend could send. That way this could be useful for regular clients too via dmabuf-hints.

emersion added a commit to emersion/wlroots that referenced this issue Dec 1, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit to emersion/wlroots that referenced this issue Dec 3, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit to emersion/wlroots that referenced this issue Dec 4, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit to emersion/wlroots that referenced this issue Dec 7, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit to emersion/wlroots that referenced this issue Dec 7, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
@emersion
Copy link
Member

emersion commented Dec 8, 2020

I've started implementing the plan outlined above in #2507 and #2505. Unfortunately because of multi-GPU support it doesn't seem like we'll be able to completely remove the renderer bits from the DRM backend.

The compositor only uses the primary GPU for rendering. Someone needs to blit the buffers to the secondary GPUs. wlr_output can't really do it, because it doesn't know whether the backend needs a blit or not. The more conservative choice is to keep the blit in the backend.

We'll need a new plan once we expose everything to compositors. Maybe expose the secondary GPUs, let the compositors handle the blit, and integrate the logic into a helper?

emersion added a commit to emersion/wlroots that referenced this issue Dec 11, 2020
The cursor surface still uses a wl_egl_window.

References: swaywm#1352
emersion added a commit that referenced this issue Dec 13, 2020
The cursor surface still uses a wl_egl_window.

References: #1352
berylline added a commit to berylline/wlroots that referenced this issue Jan 21, 2021
Just another part of swaywm#2624 and swaywm#1352 that I felt needed to be done.

Breaking change:

Both EGLint *config_atrribs and egl->config no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 21, 2021
Just another part of swaywm#2624 and swaywm#1352 that I felt needed to be done.

Breaking change:

Both "EGLint *config_attribs" and "egl->config" no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 22, 2021
Just another part of swaywm#2624 and swaywm#1352 that I felt needed to be done.

Breaking change:

Both "EGLint *config_attribs" and "egl->config" no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 22, 2021
Just another bit of EGL housekeeping done for swaywm#1352 and swaywm#2624.

Breaking changes:

"wlr_egl_context->draw_surface" and "wlr_egl_context->read_surface" have been deleted and no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 26, 2021
Just another part of swaywm#2624 and swaywm#1352 that I felt needed to be done.

Breaking change:

Both "EGLint *config_attribs" and "egl->config" no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 26, 2021
Just another bit of EGL housekeeping done for swaywm#1352 and swaywm#2624.

Breaking changes:

"wlr_egl_context->draw_surface" and "wlr_egl_context->read_surface" have been deleted and no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 28, 2021
Just another bit of EGL housekeeping done for swaywm#1352 and swaywm#2624.

Breaking changes:

"wlr_egl_context->draw_surface" and "wlr_egl_context->read_surface" have been deleted and no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 29, 2021
Just another bit of EGL housekeeping done for swaywm#1352 and swaywm#2624.

Breaking changes:

"wlr_egl_context->draw_surface" and "wlr_egl_context->read_surface" have been deleted and no longer exist.
berylline added a commit to berylline/wlroots that referenced this issue Jan 29, 2021
Just another bit of EGL housekeeping done for swaywm#1352 and swaywm#2624.

Breaking changes:

"wlr_egl_context->draw_surface" and "wlr_egl_context->read_surface" have been deleted and no longer exist.
@emersion
Copy link
Member

This is now done.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

5 participants