Introduce MultiGPU renderer #503

Drakulix · 2022-03-01T12:48:43Z

Best reviewed commit by commit:

45d1ee3

Adds a new trait Offscreen to our renderer module, that describes the feature to create offscreen-framebuffers to render into. (Actually rendering to them is handled by Bind and Renderer as previously.)

05059d3
f25c19a

Adds two implementations of Offscreen (and matching Bind implementations) to our Gles2Renderer.

Gles2Texture allow now to render into textures, which is useful to do some post-processing.
Gles2Renderbuffer, which is faster than a texture, but cannot be sampled from.

eb0a25c

Renames ImportShm to ImportMem and adds functions to import bitmaps, that don't belong to a wl_buffer.

25f59e1

Introduces a new ExportMem trait that allows copying textures and from bound framebuffers into memory
and implements it for our Gles2Renderer

4bd7f4d

Prequisite for ea9aabd. Adds the ability to convert from egl-images to dmabuf. (We previously only supported the other way around.)

ea9aabd

Introduces a new ExportDma trait, that analogous to ExportMem (and to ImportDma and ImportMem being the opposites) allows to export textures and framebuffers to dmabufs. Again implemented for our Gles2Renderer

ea2a9ac

Changes the DrmNode type to not contain a file descriptor, essentially only describing a drm-device. Can be used across the code base to uniquely identify drm-nodes, but is currently a bit under-used (except for the multigpu module).

This also makes the X11-backend api a little more awkward, but that is meant to switch to rendering via a EGLSurface in the future anyway.

95adeaf

Removes the 'static-constraint on the Renderer provided to Space::render_output by not using trait-objects for custom RenderElements anymore. A new macro is provided to generate a collating enum-type for downstream. This is required, given the MultiRenderer borrows internal Renderers from the GpuManager.

e9493f7
58443b4

Introduce a new multigpu-module, containing the GpuManager, MultiRenderer and some accompanying types.
Also implements its use in anvil's udev-backend.

The idea is, that a GpuManager enumerates available rendering devices for a given Api (only Egl+Gles2 is provided at the moment) and then allows to create MultiRenderers from two gpus represented by DrmNodes. The render-gpu, where compositing will take place and the target-gpu, where the result will reside on. Client buffers will be imported by any source-gpu, if it is available to the used api.

Almost all of the processing required to move stuff between gpus is done transparently in the background and using the new Offscreen- and Export*-traits, meaning this should theoretically just work with a future Vulkan-based renderer.

One exception is the GpuManager::early_import function, that can be used to pro-actively start copy-operations of client buffers on commit as opposed to during compositing, risking running late on the frame.

c10fa70
1ba365d
a64c55e
feecb91
8b4ebb2

A bunch of smaller fixes and additions...

Todo

Use GpuManager::early_import in anvil
~~Sometimes windows moved between outputs of different gpus will take a second to show up, probably a caching issue, because performance so far seems to be really smooth for a first attempt.~~ fixed
Changelog

Questions

@i509VCB Are the new traits realistically compatible with vulkan the way they are or do they need some obvious adjustments?
Can we find some more people to test this on different hardware configurations?

src/backend/renderer/mod.rs

anvil/src/udev.rs

src/desktop/space/mod.rs

src/backend/renderer/gles2/mod.rs

elinorbgr · 2022-03-06T14:41:44Z

src/backend/renderer/gles2/mod.rs

@@ -784,6 +784,89 @@ impl ImportShm for Gles2Renderer {
        .map_err(Gles2Error::BufferAccessError)?
    }

+    fn import_memory(&mut self, data: &[u8], size: Size<i32, Buffer>) -> Result<Gles2Texture, Gles2Error> {


It seems to be there is quite a bit of repetition, I'm thinking we might:

change the import_memory / update_memory to take as argument a &[u8] and a BufferData

make import_shm_buffer use import_memory / update_memory

would it make sense?

Sadly it does, more work for me! :)

src/backend/renderer/mod.rs

src/utils/geometry.rs

elinorbgr · 2022-03-06T14:53:57Z

src/utils/geometry.rs

+    }
+}
+
+impl std::ops::Add for Transform {


Okay, this is extremely subjective, but Add does not feel like the appropriate operator for this to me. I think Mul would make more sense.

So 90 * 90 = 180? Feels weird to me.
Also Flipped * Flipped = Normal?

Yeah, but that clearly my inner mathematician speaking (seeing composition of rotations as multiplication of complex numbers, and likewise, -1 * -1 = 1 makes sense to me for flipping). But I'm clearly not going to die on that hill. 🙂

Well, I am indecisive about this, do we already have similar cases for composition in the library? Because at least it should be consistent.

src/desktop/space/mod.rs

PolyMeilex · 2022-03-07T17:36:32Z

anvil/src/udev.rs


    // setup the timer
    let timer = Timer::new().unwrap();

+    std::mem::drop(renderer);


Is there any particular reason for this drop? If so, could we get a comment about the reason?
If this is just a stylistic choice, then don't mind me.

Yes, because the renderer borrows gpus mutably, so without the drop rustc complains, that it cannot move gpus into the UdevData in the next line.

Added a comment to explain this.

Yes, because the renderer borrows gpus mutably, so without the drop rustc complains, that it cannot move gpus into the UdevData in the next line.

Just checked and it compiles just fine for me, so rustc does not complain for me, maybe it'a a certain feature combination? (I'm building the default feature set of anvil)

Hmm, maybe some recent changes made this obsolete? I will remove it and see what CI think about it.

i509VCB · 2022-03-07T17:38:49Z

src/backend/x11/mod.rs

@@ -313,7 +314,7 @@ impl X11Handle {
    /// Returns the DRM node the X server uses for direct rendering.
    ///
    /// The DRM node may be used to create a [`gbm::Device`] to allocate buffers.
-    pub fn drm_node(&self) -> Result<DrmNode, X11Error> {
+    pub fn drm_node(&self) -> Result<(DrmNode, RawFd), X11Error> {


OwnedFd in std is currently unstable. We should probably wrap the fd in something so we can't leak it since the method gives you ownership of the fd.

Well that is also how e.g. IntoRawFd works, ownership is implicit.
But yes, we could probably wrap it.

Note on that, I want to change the X11-backend soon to use an EGLSurface, which will not require downstream to access the RawFd anymore anyway. I will thus skip wrapping the RawFd in this PR and deal with it in an upcoming one.

src/backend/allocator/dmabuf.rs

src/backend/egl/display.rs

i509VCB · 2022-03-07T17:49:29Z

src/backend/renderer/gles2/mod.rs

+        Ok(texture)
+    }
+
+    fn update_memory(


Should this work on an EGLImage or Dmabuf backed texture?

It might be in the case of the Gles2Renderer (though it likely depends on the EGLImage/Dmabuf),
but the docs of the trait clearly state that:

/// This function *may* error, if (but not limited to): /// - The texture was not created using either [`ImportMem::import_shm_buffer`] or [`ImportMem::import_memory`]. /// ...

src/backend/renderer/multigpu/mod.rs

Add the possibility of importing memory slices to the ImportShm trait, just like the ImportDma trait supports importing Dmabufs and not only dmabuf-based WlBuffers. Also rename ImportShm to ImportMem to better reflect the new functionality.

i509VCB reviewed Mar 1, 2022

View reviewed changes

src/backend/renderer/mod.rs Show resolved Hide resolved

Drakulix force-pushed the feature/multigpu branch from 58443b4 to 3a9836e Compare March 1, 2022 16:48

i509VCB reviewed Mar 1, 2022

View reviewed changes

src/backend/renderer/mod.rs Outdated Show resolved Hide resolved

Drakulix force-pushed the feature/multigpu branch 8 times, most recently from 60b38b7 to fb4e467 Compare March 3, 2022 18:22

i509VCB reviewed Mar 3, 2022

View reviewed changes

anvil/src/udev.rs Outdated Show resolved Hide resolved

src/desktop/space/mod.rs Outdated Show resolved Hide resolved

Drakulix requested review from i509VCB, elinorbgr and PolyMeilex March 4, 2022 14:44

elinorbgr reviewed Mar 6, 2022

View reviewed changes

PolyMeilex reviewed Mar 7, 2022

View reviewed changes

i509VCB reviewed Mar 7, 2022

View reviewed changes

Drakulix added 12 commits March 10, 2022 13:01

renderer: Introduce Offscreen render targets

7ec9c8d

gles2: Add textures as Offscreen render targets

32f8909

gles2: Add renderbuffer as Offscreen target

acd26bd

renderer: ImportShm -> ImportMem

3b704eb

Add the possibility of importing memory slices to the ImportShm trait, just like the ImportDma trait supports importing Dmabufs and not only dmabuf-based WlBuffers. Also rename ImportShm to ImportMem to better reflect the new functionality.

renderer: Introduce ExportMem trait

79f9f94

egl: Add function to export egl images

bc973e9

renderer: Introduce ExportDma

7bc81da

allocator: Implemented for boxed variants

dffc75e

drm: Remove file descriptor from DrmNode

bf0f59c

gles2: Fix rendering of inverted textures

2b1218f

utils: Allow Transforms to add up

0b20587

renderer: Export/ImportMem support flipped buffers

a3e377e

Drakulix added 2 commits March 11, 2022 16:54

desktop: Remove Renderer: 'static constraint

0e5612c

anvil: RenderElement is not using relative coordinates anymore

075b67b

Drakulix force-pushed the feature/multigpu branch 4 times, most recently from dadde8a to 1dffaa6 Compare March 14, 2022 15:04

Drakulix added 21 commits March 14, 2022 16:13

anvil/udev: Use MultiGPU renderer

4d2d17f

renderer: Add id() to track different textures

7cc6bd8

renderer: Add utils::import_surface_tree

79c52ba

utils: Cleanup generics of draw_* helpers

72d1be3

anvil: Make use of early_import

d0ad545

renderer: split of wl_buffer-specific Import fns

3446c0b

multigpu: Remove source param to not depend on wl

572101f

changelog: Add multigpu additions

2592f6b

gles2: Fix feature gates

be5f243

multigpu: Add AsRef/Mut to access underlying impl

aff3c32

cargo: Add new renderer_multi feature

882c9bf

wlcs_anvil: Adjust for multigpu changes

2551073

desktop: Add meta attribs to custom_elements

1e91749

desktop: Allow custom_elements! to be generic

c609baa

address review comments

1bdefca

space: Allow custom_elements! to cast wrapped renderers.

59cffd0

egl: Add render_node query to EGLDevice

22c5ea7

egl: refactor: move node selection into EGLDevice

36020ef

anvil: Use udev-nodes for everything but rendering

279c2fb

anvil/udev: Fix crash on device_changed

76946c4

anvil/udev: Allow overriding the primary gpu

2e0ae16

Drakulix force-pushed the feature/multigpu branch from 1dffaa6 to 2e0ae16 Compare March 14, 2022 15:13

Drakulix merged commit 1151eea into master Mar 14, 2022

Drakulix deleted the feature/multigpu branch June 23, 2022 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce MultiGPU renderer #503

Introduce MultiGPU renderer #503

Drakulix commented Mar 1, 2022 •

edited

Loading

elinorbgr Mar 6, 2022

Drakulix Mar 7, 2022

elinorbgr Mar 6, 2022

Drakulix Mar 7, 2022

elinorbgr Mar 11, 2022

Drakulix Mar 11, 2022

PolyMeilex Mar 7, 2022

Drakulix Mar 9, 2022

Drakulix Mar 11, 2022

PolyMeilex Mar 14, 2022

Drakulix Mar 14, 2022

i509VCB Mar 7, 2022

Drakulix Mar 9, 2022

Drakulix Mar 10, 2022

i509VCB Mar 7, 2022

Drakulix Mar 9, 2022

Introduce MultiGPU renderer #503

Introduce MultiGPU renderer #503

Conversation

Drakulix commented Mar 1, 2022 • edited Loading

Todo

Questions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Drakulix commented Mar 1, 2022 •

edited

Loading