feat(input): present selected non-vulkan surface via compositor dmabuf#69
Merged
feat(input): present selected non-vulkan surface via compositor dmabuf#69
Conversation
- add compositor swapchain render + dma-buf export for selected surface - expose presented frames through InputForwarder for viewer fallback - map DRM FourCC formats to Vulkan formats for import path - reset cached surface frames on selection changes or teardown - add OpenSpec proposal/design/spec/tasks for compositor capture
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
📝 WalkthroughWalkthroughAdds a compositor-based presentation path for non‑Vulkan Wayland/XWayland surfaces: render the selected surface into a headless swapchain, export frames as DMA‑BUF with metadata, cache the last presented frame, and expose it via InputForwarder for the viewer; falls back to input-only forwarding if presentation fails. Changes
Sequence DiagramsequenceDiagram
participant Surface as Wayland/XWayland<br/>Surface
participant Comp as CompositorServer<br/>(Compositor thread)
participant Swap as wlr_swapchain<br/>(Headless output)
participant Forwarder as InputForwarder
participant App as Application<br/>(Main thread)
participant Viewer as Viewer Renderer<br/>(Vulkan)
Surface->>Comp: surface commit
Comp->>Comp: check selected/input target
Comp->>Swap: render_surface_to_frame()
Swap->>Swap: acquire buffer
Comp->>Comp: render via wlr_render_pass
Comp->>Comp: export DMA-BUF attrs (format,stride,modifier)
Comp->>Comp: duplicate FD, cache SurfaceFrame (frame_number)
App->>Forwarder: tick_frame()
Forwarder->>Comp: get_presented_frame(last_seen)
Comp-->>Forwarder: SurfaceFrame (if newer)
Forwarder-->>App: std::optional<SurfaceFrame>
App->>App: drm_to_vk_format() and decide rebuild
App->>Viewer: import DMA-BUF + metadata, render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@openspec/changes/add-non-vulkan-surface-present/proposal.md`:
- Around line 9-21: Add the missing spec delta to the "What Changes" section:
insert a bullet describing the new `compositor-capture` spec (mark as "new" and
"breaking" if applicable) directly under the existing bullets in the "What
Changes" list so it appears alongside the other changes rather than only in the
"Impact" section; reference the exact spec name `compositor-capture` in that
bullet to match the Impact entry.
In
`@openspec/changes/add-non-vulkan-surface-present/specs/compositor-capture/spec.md`:
- Around line 6-23: Update each scenario to use the single-line "- **WHEN** ...
- **THEN** ..." step format instead of separate GIVEN/AND/WHEN/THEN bullets: for
"Scenario: Present selected surface" replace the four bullets with one WHEN/THEN
line (e.g. WHEN a non-Vulkan client surface is connected and selected via the
surface selector and the compositor produces a new frame - THEN the viewer
presents the selected surface), for "Scenario: Presentation unavailable" combine
the GIVEN/WHEN/THEN into one WHEN/THEN line reflecting that compositor
presentation cannot be initialized and input forwarding continues without
presenting non-Vulkan frames, and for "Scenario: Export compositor frame via
DMA-BUF" make a single WHEN/THEN line that states when the compositor renders a
frame for the selected surface it exports a DMA-BUF with the required metadata
and the viewer imports/presents it without CPU readback; apply the same
single-line WHEN/THEN pattern to the other scenarios referenced (lines 28-31).
In `@src/app/application.cpp`:
- Around line 563-571: The CaptureFrame's frame_number is being hardcoded to 0
causing timeline semaphore sync issues; update the assignment in the block that
fills surface_capture so that surface_capture.frame_number is set from the
source frame (use m_surface_frame->frame_number), ensuring any needed type
conversion to match surface_capture.frame_number's type; modify the code around
the surface_capture population (the lines that set
width/height/stride/offset/format/modifier/frame_number/dmabuf_fd) to propagate
the real frame number instead of 0 so Vulkan backend timeline semaphore
synchronization remains correct.
In `@src/util/drm_format.hpp`:
- Around line 1-28: Replace the C Vulkan API usage in drm_to_vk_format with
vulkan-hpp types: change the include from <vulkan/vulkan.h> to
<vulkan/vulkan.hpp>, change the function return type from VkFormat to vk::Format
(function drm_to_vk_format), and return the corresponding vk::Format enum values
(e.g., vk::Format::eB8G8R8A8Unorm, vk::Format::eR8G8B8A8Unorm,
vk::Format::eA2R10G10B10UnormPack32, vk::Format::eA2B10G10R10UnormPack32,
vk::Format::eR5G6B5UnormPack16, vk::Format::eUndefined) instead of VK_FORMAT_*
constants; keep the DRM switch cases (DRM_FORMAT_*) unchanged.
Comment on lines
+9
to
+21
| ## What Changes | ||
|
|
||
| - Render the selected non-Vulkan surface into a headless compositor output and export a DMA-BUF. | ||
| - Expose the latest compositor-presented frame via `InputForwarder` without altering input routing. | ||
| - When no Vulkan capture frame is available, feed compositor DMA-BUF frames into the existing | ||
| Vulkan viewer render path. | ||
| - Keep the Vulkan layer capture pipeline unchanged. | ||
| - Fall back to input-only mode if compositor presentation is unavailable. | ||
|
|
||
| ## Impact | ||
|
|
||
| - Affected specs: `compositor-capture` (new) | ||
| - Affected code: |
There was a problem hiding this comment.
List spec deltas under “What Changes.”
Guidelines require spec deltas to be listed in the “What Changes” section. Please add a bullet there (and mark breaking if applicable), rather than only under Impact.
✅ Example addition
## What Changes
+- Spec deltas: compositor-capture (ADDED)
- Render the selected non-Vulkan surface into a headless compositor output and export a DMA-BUF.As per coding guidelines, spec deltas must appear under “What Changes.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## What Changes | |
| - Render the selected non-Vulkan surface into a headless compositor output and export a DMA-BUF. | |
| - Expose the latest compositor-presented frame via `InputForwarder` without altering input routing. | |
| - When no Vulkan capture frame is available, feed compositor DMA-BUF frames into the existing | |
| Vulkan viewer render path. | |
| - Keep the Vulkan layer capture pipeline unchanged. | |
| - Fall back to input-only mode if compositor presentation is unavailable. | |
| ## Impact | |
| - Affected specs: `compositor-capture` (new) | |
| - Affected code: | |
| ## What Changes | |
| - Spec deltas: compositor-capture (ADDED) | |
| - Render the selected non-Vulkan surface into a headless compositor output and export a DMA-BUF. | |
| - Expose the latest compositor-presented frame via `InputForwarder` without altering input routing. | |
| - When no Vulkan capture frame is available, feed compositor DMA-BUF frames into the existing | |
| Vulkan viewer render path. | |
| - Keep the Vulkan layer capture pipeline unchanged. | |
| - Fall back to input-only mode if compositor presentation is unavailable. | |
| ## Impact | |
| - Affected specs: `compositor-capture` (new) | |
| - Affected code: |
🤖 Prompt for AI Agents
In `@openspec/changes/add-non-vulkan-surface-present/proposal.md` around lines 9 -
21, Add the missing spec delta to the "What Changes" section: insert a bullet
describing the new `compositor-capture` spec (mark as "new" and "breaking" if
applicable) directly under the existing bullets in the "What Changes" list so it
appears alongside the other changes rather than only in the "Impact" section;
reference the exact spec name `compositor-capture` in that bullet to match the
Impact entry.
Comment on lines
+6
to
+23
| #### Scenario: Present selected surface | ||
| - **GIVEN** a non-Vulkan client surface is connected to the compositor | ||
| - **AND** the surface is selected via the existing surface selector | ||
| - **WHEN** the compositor produces a new frame | ||
| - **THEN** the viewer presents the selected surface | ||
|
|
||
| #### Scenario: Presentation unavailable | ||
| - **GIVEN** compositor presentation cannot be initialized | ||
| - **WHEN** non-Vulkan clients connect for input | ||
| - **THEN** input forwarding continues without presenting non-Vulkan frames | ||
|
|
||
| ### Requirement: DMA-BUF Export for Compositor Frames | ||
| The compositor capture path SHALL export frames using DMA-BUF for zero-copy presentation. | ||
|
|
||
| #### Scenario: Export compositor frame via DMA-BUF | ||
| - **WHEN** the compositor renders a frame for the selected surface | ||
| - **THEN** it exports a DMA-BUF with width, height, format, stride, and modifier metadata | ||
| - **AND** the viewer imports and presents the frame without CPU readback |
There was a problem hiding this comment.
Scenario steps must use the WHEN/THEN single-line format.
The scenarios currently use GIVEN/AND/WHEN/THEN bullets, but the spec format requires each step to be a single - **WHEN** ... - **THEN** ... line. Please update all scenarios accordingly.
✅ Example rewrite (apply similar pattern to all scenarios)
#### Scenario: Present selected surface
-- **GIVEN** a non-Vulkan client surface is connected to the compositor
-- **AND** the surface is selected via the existing surface selector
-- **WHEN** the compositor produces a new frame
-- **THEN** the viewer presents the selected surface
+- **WHEN** a non-Vulkan client surface is connected to the compositor, the surface is selected via the existing surface selector, and the compositor produces a new frame - **THEN** the viewer presents the selected surfacePlease apply the same WHEN/THEN step format to the remaining scenarios. As per coding guidelines, please use the required scenario step format.
Also applies to: 28-31
🤖 Prompt for AI Agents
In
`@openspec/changes/add-non-vulkan-surface-present/specs/compositor-capture/spec.md`
around lines 6 - 23, Update each scenario to use the single-line "- **WHEN** ...
- **THEN** ..." step format instead of separate GIVEN/AND/WHEN/THEN bullets: for
"Scenario: Present selected surface" replace the four bullets with one WHEN/THEN
line (e.g. WHEN a non-Vulkan client surface is connected and selected via the
surface selector and the compositor produces a new frame - THEN the viewer
presents the selected surface), for "Scenario: Presentation unavailable" combine
the GIVEN/WHEN/THEN into one WHEN/THEN line reflecting that compositor
presentation cannot be initialized and input forwarding continues without
presenting non-Vulkan frames, and for "Scenario: Export compositor frame via
DMA-BUF" make a single WHEN/THEN line that states when the compositor renders a
frame for the selected surface it exports a DMA-BUF with the required metadata
and the viewer imports/presents it without CPU readback; apply the same
single-line WHEN/THEN pattern to the other scenarios referenced (lines 28-31).
K1ngst0m
force-pushed
the
dev/non-vulkan-present
branch
from
2dc4135 to
94a1d67
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
PR Type
Enhancement
Description
Add compositor swapchain rendering and DMA-BUF export for non-Vulkan surfaces
Expose presented frames through InputForwarder for fallback rendering path
Map DRM FourCC formats to Vulkan formats for seamless import
Reset cached surface frames on selection changes or teardown
Include OpenSpec proposal, design, specification, and task documentation
Diagram Walkthrough
File Walkthrough
8 files
Add surface frame tracking and state membersIntegrate compositor frames into render pipelineDefine SurfaceFrame struct and presentation APIImplement swapchain rendering and DMA-BUF exportAdd get_presented_frame method to public APIForward get_presented_frame to compositor serverDefine DRM FourCC format constants and helpersMap DRM formats to Vulkan format enums4 files
Document feature proposal and design rationaleDetail implementation decisions and trade-offsDefine requirements and acceptance scenariosTrack implementation tasks and verification stepsSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.