Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

anvil/udev: Dynamic support for pixman rendererer #1497

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Aug 5, 2024

With the changes here, the anvil udev backend will use pixman for rendering if ANVIL_USE_PIXMAN is set.

To do:

  • Figure out how to deal with Send bound on MemoryRenderBuffer, and no Send for PixmanTexture
  • Fix build with debug feature (need to abstract texture)
  • Make render node optional, and automatically use pixman if hardware acceleration isn't present
  • Pixman support in x11/winit backends

It's not entirely clear how best to abstract renderers dynamically:

  • Should an enum wrapper implement the Renderer trait?
    • How does that impact performance or binary size compare to dispatching to a render function twice like this currently does?
    • Will also need enums over texture, frame, error; and probably a panic if these types are mismatched in some way
  • Would any changes to Renderer and related traits make it easier to abstract over, and dispatch at runtime to different renderers?

@cmeissl
Copy link
Collaborator

cmeissl commented Aug 7, 2024

I tried to create a wrapper enum implementing the renderer traits. Bind is the most "problematic" as this can be different per renderer.

Macro based implementation: https://github.com/cmeissl/smithay/blob/3834756520cfb8db5c21b9b5485d16bdc7cdab6e/src/backend/renderer/auto.rs
Usage of the enum: https://github.com/cmeissl/smithay/blob/3834756520cfb8db5c21b9b5485d16bdc7cdab6e/anvil/src/render.rs#L28

@ids1024
Copy link
Member Author

ids1024 commented Aug 7, 2024

I was thinking a macro might make sense. That looks handy. Although macro rules macros like that get a bit complicated.

@ids1024
Copy link
Member Author

ids1024 commented Dec 4, 2024

Updated on top of #1600.

Testing this I see a crash:

2024-12-04T18:08:44.814186Z  WARN anvil::udev: Error during rendering: ContextLost(BufferDestroyed)
thread 'main' panicked at anvil/src/udev.rs:1613:30:
Rendering loop lost: The underlying buffer has been destroyed

I guess that's an existing issue with the pixman renderer, and not something changed here? Hm.

@ids1024
Copy link
Member Author

ids1024 commented Dec 5, 2024

Updated so the debug feature builds, and all the feature combinations tested on CI now work. Still needs a fix for that crash.

Exactly how best to abstract over multiple renderers remains an open question.

This removes a bit of duplication between backends. There seems to be no
need to separate these unless fps is used for other things.
The `RendererRef` and `Texture` enums provide one way to abstract over
different renderers.

This splits `FpsElement` into a seperate `Fps` that isn't a render
element, and isn't specific to one renderer/texture type.
@ids1024 ids1024 changed the title WIP anvil/udev: Dynamic support for pixman rendererer anvil/udev: Dynamic support for pixman rendererer Dec 9, 2024
@ids1024
Copy link
Member Author

ids1024 commented Dec 9, 2024

This seems to be working correctly, including with the debug feature.

We may want to change how renderers are abstracted dynamically in the future (and provide some helper for this in Smithay), but this seems good for now, to have a way to test Pixman rendering properly.

@ids1024 ids1024 marked this pull request as ready for review December 9, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants