refactor(render): reduce VulkanBackend forwarding surface

[K1ngst0m]

Summary by CodeRabbit

• Refactor

  • Reorganized how internal rendering components interact to improve code maintainability and structure.

• Tests

  • Updated test assertions to reflect new internal architecture.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0a918db5-c78b-453a-bed2-4f881f354aee

📥 Commits

Reviewing files that changed from the base of the PR and between 6c7776e and 65ad046.

📒 Files selected for processing (4)

• src/app/application.cpp
• src/render/backend/vulkan_backend.cpp
• src/render/backend/vulkan_backend.hpp
• tests/render/test_filter_boundary_contracts.cpp

💤 Files with no reviewable changes (1)

• src/render/backend/vulkan_backend.cpp

---

📝 Walkthrough

Walkthrough

VulkanBackend's public API is refactored to expose internal components (VulkanContext, RenderOutput, ExternalFrameImporter, FilterChainController) via accessor methods instead of providing direct getter/setter methods for Vulkan handles, filter controls, prechain resolution, and frame metadata. Application class is updated to route all filter control, preset path, and prechain operations through filter_chain_controller() and to source Vulkan handles and swapchain configuration from vulkan_context() and render_output().

Changes

Cohort / File(s)	Summary
API Surface Refactoring src/render/backend/vulkan_backend.hpp, src/render/backend/vulkan_backend.cpp	Removed 20+ public accessor methods for Vulkan handles, swapchain properties, filter controls, prechain resolution, frame metadata, and preset paths. Added new public methods that return const references to internal components: vulkan_context(), render_output(), frame_importer(), filter_chain_controller(). Relocated set_prechain_resolution() to public API.
Application Integration src/app/application.cpp	Updated all filter control, preset path, and prechain operation calls to route through filter_chain_controller(). Updated ImGui initialization to retrieve Vulkan instance/device/queue handles and swapchain configuration from vulkan_context() and render_output(). Swapchain extent/format queries in resize/rebuild paths switched to use render_output().swapchain_extent and render_output().swapchain_format. Prechain import extent switched from get_captured_extent() to frame_importer().import_extent.
Test Boundary Contracts tests/render/test_filter_boundary_contracts.cpp	Updated compile-time signature assertions to validate filter-control methods against FilterChainController instead of VulkanBackend. Adjusted boundary checks to forbid direct filter_chain() references and to verify use of filter_chain_controller() accessor in headers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• refactor(chain): split filter runtime by responsibility #126: Splits chain runtime and exposes FilterChainController, directly enabling the component-based API restructuring in this PR.
• refactor(render): split monolithic backend into subcomponents #113: Likely introduced the refactored internal components (VulkanContext, RenderOutput, ExternalFrameImporter) that this PR now exposes via accessors.
• feat(chain): add nearest-neighbor prechain downsampling mode #118: Adds prechain filter control handling in filter-chain components; this PR's routing of control calls through filter_chain_controller() integrates with that functionality.

Suggested labels

Review effort 3/5, Failed compliance check

Poem

🐰 Through gates of control, the paths transform,
Components emerge from the Vulkan storm!
From scattered methods, now facades arise—
FilterChainController meets the wise.
Refactored APIs dance and gleam,
Application sings a cleaner dream. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.35% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the core refactoring: reducing VulkanBackend's public API surface by replacing multiple individual accessors with component-level interfaces.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev/refactor-vkbk-surface

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

✨ Enhancement

Walkthroughs

Description

• Remove forwarding methods from VulkanBackend to reduce surface
• Expose internal components via accessor methods for direct access
• Update Application to use filter_chain_controller(), vulkan_context(), render_output(), and
  frame_importer() accessors
• Update tests to reflect new FilterChainController method locations

Diagram

flowchart LR
  VB["VulkanBackend"]
  FCC["FilterChainController"]
  VC["VulkanContext"]
  RO["RenderOutput"]
  EFI["ExternalFrameImporter"]
  App["Application"]
  
  VB -- "filter_chain_controller()" --> FCC
  VB -- "vulkan_context()" --> VC
  VB -- "render_output()" --> RO
  VB -- "frame_importer()" --> EFI
  App -- "uses accessors" --> VB

Loading

File Changes

1. src/app/application.cpp Refactoring +33/-25

Update Application to use VulkanBackend accessors

• Replace direct VulkanBackend method calls with accessor-based calls to internal components
• Update filter control operations to use filter_chain_controller() accessor
• Update Vulkan context access to use vulkan_context() accessor
• Update render output access to use render_output() accessor
• Update frame importer access to use frame_importer() accessor

src/app/application.cpp

---

2. src/render/backend/vulkan_backend.cpp Refactoring +0/-27

Remove VulkanBackend forwarding methods

• Remove forwarding method implementations for filter control operations
• Remove get_prechain_resolution(), list_filter_controls(), set_filter_control_value(),
 reset_filter_control_value(), and reset_filter_controls() methods

src/render/backend/vulkan_backend.cpp

---

3. src/render/backend/vulkan_backend.hpp Refactoring +13/-40

Replace forwarding methods with component accessors

• Remove individual forwarding methods for Vulkan context, render output, and filter control
 operations
• Add vulkan_context() accessor returning const reference to VulkanContext
• Add render_output() accessor returning const reference to RenderOutput
• Add frame_importer() accessor returning const reference to ExternalFrameImporter
• Add filter_chain_controller() accessors (both const and non-const) for direct access
• Remove current_preset_path(), instance(), physical_device(), device(), graphics_queue(),
 graphics_queue_family(), swapchain_format(), swapchain_extent(), swapchain_image_count(),
 get_prechain_resolution(), get_captured_extent(), gpu_index(), gpu_uuid(), and
 consume_chain_swapped() methods

src/render/backend/vulkan_backend.hpp

---

View more (1)

4. tests/render/test_filter_boundary_contracts.cpp 🧪 Tests +12/-15

Update tests for FilterChainController method locations

• Update type aliases to reference FilterChainController instead of VulkanBackend
• Update static_assert checks to verify FilterChainController method signatures
• Update test assertion to check for m_filter_chain_controller. instead of filter_chain()
• Update test to verify filter_chain_controller() accessor exists in header

tests/render/test_filter_boundary_contracts.cpp

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Mutable controller exposed 🐞 Bug ✓ Correctness

Description

VulkanBackend::filter_chain_controller() returns a mutable reference to
backend_internal::FilterChainController, allowing non-backend code to call lifecycle/orchestration
APIs and bypass the sequencing enforced by VulkanBackend::render()/shutdown(). This can violate
GPU synchronization/resource-lifetime invariants and cause incorrect rendering behavior or
hard-to-debug crashes if any future call site uses more than the intended “control” subset of the
controller API.

Code

src/render/backend/vulkan_backend.hpp[R84-86]

+    [[nodiscard]] auto filter_chain_controller() -> backend_internal::FilterChainController& {
+        return m_filter_chain_controller;
    }
-    [[nodiscard]] auto swapchain_image_count() const -> uint32_t {
-        return m_render_output.image_count();

Evidence

The PR adds a public mutable accessor to the full controller type. The controller type includes
orchestration methods
(advance_frame/check_pending_chain_swap/cleanup_retired_adapters/shutdown/retarget_filter_chain/etc.),
and VulkanBackend currently relies on calling these in a specific order inside render() and
shutdown(). Exposing the mutable controller makes it possible to invoke those methods outside that
order, bypassing the backend’s synchronization points (e.g., wait_all_frames() callbacks).

src/render/backend/vulkan_backend.hpp[73-90]
src/render/backend/filter_chain_controller.hpp[67-111]
src/render/backend/vulkan_backend.cpp[398-548]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`VulkanBackend::filter_chain_controller()` currently returns a **mutable** `backend_internal::FilterChainController&`. This exposes lifecycle/orchestration methods that `VulkanBackend` expects to own and sequence safely.

### Issue Context
`VulkanBackend::render()` calls `advance_frame()`, `check_pending_chain_swap(...)`, and `cleanup_retired_adapters()` in a controlled order. Returning a mutable controller reference makes it easy for future call sites to invoke these APIs out-of-order and break backend invariants.

### Fix Focus Areas
- src/render/backend/vulkan_backend.hpp[73-90]
- src/render/backend/vulkan_backend.cpp[398-548]
- src/render/backend/filter_chain_controller.hpp[67-111]

### Recommended change
- Remove the non-const `filter_chain_controller()` accessor, or restrict it so external code can only perform the intended narrow set of operations.
- Re-introduce a small, explicit `VulkanBackend` facade API for UI interactions (e.g., `list_filter_controls(stage)`, `set_filter_control_value(...)`, `reset_filter_controls()`, `current_preset_path()`, `consume_chain_swapped()`, `current_prechain_resolution()`), so orchestration remains owned by `VulkanBackend`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Internal structs leaked 🐞 Bug ⚙ Maintainability

Description

VulkanBackend now returns const& to
backend_internal::{VulkanContext,RenderOutput,ExternalFrameImporter} from a public header,
coupling external code to internal layouts and enabling accidental retention of references to
backend-owned state beyond the backend lifetime. This increases API/ABI fragility and makes future
internal refactors riskier than when callers only received value handles via narrow accessors.

Code

src/render/backend/vulkan_backend.hpp[R75-83]

+    [[nodiscard]] auto vulkan_context() const -> const backend_internal::VulkanContext& {
+        return m_vulkan_context;
    }
-    [[nodiscard]] auto graphics_queue_family() const -> uint32_t {
-        return m_vulkan_context.graphics_queue_family;
+    [[nodiscard]] auto render_output() const -> const backend_internal::RenderOutput& {
+        return m_render_output;
    }
-    [[nodiscard]] auto swapchain_format() const -> vk::Format {
-        return m_render_output.swapchain_format;
+    [[nodiscard]] auto frame_importer() const -> const backend_internal::ExternalFrameImporter& {
+        return m_external_frame_importer;
    }
-    [[nodiscard]] auto swapchain_extent() const -> vk::Extent2D {
-        return m_render_output.swapchain_extent;

Evidence

The PR introduces accessors that expose internal structs directly from the public VulkanBackend
API. These structs have public fields (e.g., Vulkan handles, swapchain fields, import state), and
consumers can now trivially take references to those fields (e.g., `auto& dev =
backend.vulkan_context().device;`) which become dangling when the backend is destroyed. This also
increases compile-time coupling because internal headers/types are now part of the public surface of
vulkan_backend.hpp.

src/render/backend/vulkan_backend.hpp[73-90]
src/render/backend/vulkan_context.hpp[15-49]
src/render/backend/render_output.hpp[15-100]
src/render/backend/external_frame_importer.hpp[12-46]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`VulkanBackend` exposes `backend_internal` structs by `const&` in its public header. This leaks internal layout, increases coupling, and enables consumers to accidentally keep references into backend-owned state.

### Issue Context
Several call sites only need a few fields (handles/extents/formats). Returning full internal structs unnecessarily expands the public surface and makes refactors risky.

### Fix Focus Areas
- src/render/backend/vulkan_backend.hpp[73-90]
- src/app/application.cpp[117-126]
- src/app/application.cpp[597-607]

### Recommended change
- Replace `vulkan_context()`, `render_output()`, and `frame_importer()` public accessors with narrow, value-returning getters for the specific fields needed by external code (e.g., `vk_instance()`, `vk_device()`, `swapchain_format()`, `swapchain_extent()`, `swapchain_image_count()`, `captured_extent()`).
- If you still want to avoid forwarding many methods, consider returning small POD snapshots (by value) instead of references to internal structs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. Mutable controller exposed 🐞 Bug ✓ Correctness

VulkanBackend::filter_chain_controller() returns a mutable reference to
backend_internal::FilterChainController, allowing non-backend code to call lifecycle/orchestration
APIs and bypass the sequencing enforced by VulkanBackend::render()/shutdown(). This can violate
GPU synchronization/resource-lifetime invariants and cause incorrect rendering behavior or
hard-to-debug crashes if any future call site uses more than the intended “control” subset of the
controller API.

Agent Prompt

### Issue description
`VulkanBackend::filter_chain_controller()` currently returns a **mutable** `backend_internal::FilterChainController&`. This exposes lifecycle/orchestration methods that `VulkanBackend` expects to own and sequence safely.

### Issue Context
`VulkanBackend::render()` calls `advance_frame()`, `check_pending_chain_swap(...)`, and `cleanup_retired_adapters()` in a controlled order. Returning a mutable controller reference makes it easy for future call sites to invoke these APIs out-of-order and break backend invariants.

### Fix Focus Areas
- src/render/backend/vulkan_backend.hpp[73-90]
- src/render/backend/vulkan_backend.cpp[398-548]
- src/render/backend/filter_chain_controller.hpp[67-111]

### Recommended change
- Remove the non-const `filter_chain_controller()` accessor, or restrict it so external code can only perform the intended narrow set of operations.
- Re-introduce a small, explicit `VulkanBackend` facade API for UI interactions (e.g., `list_filter_controls(stage)`, `set_filter_control_value(...)`, `reset_filter_controls()`, `current_preset_path()`, `consume_chain_swapped()`, `current_prechain_resolution()`), so orchestration remains owned by `VulkanBackend`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools