refactor(render): flatten FilterChainController config structs

[K1ngst0m]

Delete PrechainResolutionConfig and AdapterBuildConfig wrapper structs. Pass vk::Extent2D and VulkanDeviceInfo/ChainConfig as direct parameters.

Summary by CodeRabbit

• Refactor
  • Internal restructuring of filter chain configuration handling to improve code organization and maintainability. No visible changes to end-user functionality.

[qodo-code-review]

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

Review Summary by Qodo

Flatten FilterChainController configuration structs

✨ Enhancement

Walkthroughs

Description

• Remove wrapper structs AdapterBuildConfig and PrechainResolutionConfig
• Pass VulkanDeviceInfo and ChainConfig as direct parameters
• Split make_filter_chain_build_config() into separate make_device_info() and
  make_chain_config() methods
• Update all call sites in controller, backend, and tests

Diagram

flowchart LR
  A["AdapterBuildConfig<br/>PrechainResolutionConfig"] -->|"Remove wrappers"| B["Direct Parameters"]
  B -->|"VulkanDeviceInfo"| C["recreate_filter_chain"]
  B -->|"ChainConfig"| C
  B -->|"vk::Extent2D"| D["set_prechain_resolution"]
  E["make_filter_chain_build_config"] -->|"Split into"| F["make_device_info"]
  E -->|"Split into"| G["make_chain_config"]

Loading

File Changes

1. src/render/backend/filter_chain_controller.hpp Refactoring +5/-12

Remove wrapper structs and flatten method signatures

• Remove AdapterBuildConfig struct definition
• Remove PrechainResolutionConfig struct definition
• Update recreate_filter_chain() signature to accept VulkanDeviceInfo and ChainConfig as
 separate parameters
• Update reload_shader_preset() signature to accept VulkanDeviceInfo and ChainConfig as
 separate parameters
• Update set_prechain_resolution() signature to accept vk::Extent2D directly instead of wrapper
 struct

src/render/backend/filter_chain_controller.hpp

---

2. src/render/backend/filter_chain_controller.cpp Refactoring +29/-27

Update function signatures and remove wrapper usage

• Update create_and_load_slot() to accept VulkanDeviceInfo and ChainConfig as separate
 parameters
• Refactor recreate_filter_chain() to use direct parameters and remove requested_config wrapper
 variable
• Refactor reload_shader_preset() to use direct parameters and pass them to lambda capture
• Update set_prechain_resolution() to accept vk::Extent2D directly and rename local variable
 from resolution to fc_resolution for clarity

src/render/backend/filter_chain_controller.cpp

---

3. src/render/backend/vulkan_backend.hpp Refactoring +4/-2

Split build config factory method

• Replace make_filter_chain_build_config() method with two separate methods
• Add make_device_info() method returning VulkanDeviceInfo
• Add make_chain_config() method returning ChainConfig

src/render/backend/vulkan_backend.hpp

---

View more (3)

4. src/render/backend/vulkan_backend.cpp Refactoring +24/-31

Update backend to use flattened parameters

• Update initialize_settings() to pass vk::Extent2D directly to set_prechain_resolution()
• Update init_filter_chain() to call separate make_device_info() and make_chain_config()
 methods
• Update set_prechain_resolution() to pass vk::Extent2D directly
• Update reload_shader_preset() to call separate factory methods
• Replace make_filter_chain_build_config() with make_device_info() and make_chain_config()
 implementations

src/render/backend/vulkan_backend.cpp

---

5. tests/render/test_filter_boundary_contracts.cpp 🧪 Tests +7/-6

Update contract tests for flattened signatures

• Update contract test to check for make_device_info() and make_chain_config() instead of
 make_filter_chain_build_config()
• Update assertions to check for source_resolution = resolution; instead of `source_resolution =
 config.requested_resolution;`
• Update assertions to check for active_slot.chain.set_prechain_resolution(&fc_resolution) instead
 of &resolution

tests/render/test_filter_boundary_contracts.cpp

---

6. tests/render/test_filter_chain_retarget.cpp 🧪 Tests +31/-14

Update test fixtures and calls for new signatures

• Add new TestBuildConfig struct to replace removed AdapterBuildConfig in tests
• Update make_adapter_build_config() return type to use TestBuildConfig
• Update configure_controller_runtime() to pass vk::Extent2D directly to
 set_prechain_resolution()
• Update all test calls to recreate_filter_chain() and reload_shader_preset() to pass separate
 parameters

tests/render/test_filter_chain_retarget.cpp

---

[qodo-code-review]

Code Review by Qodo

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

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the FilterChainController API boundary to accept decomposed configuration parameters instead of aggregated structures. The changes split AdapterBuildConfig into separate VulkanDeviceInfo and ChainConfig parameters, and replace PrechainResolutionConfig with direct vk::Extent2D values. All call sites and tests are updated accordingly.

Changes

Cohort / File(s)	Summary
Filter Chain Controller src/render/backend/filter_chain_controller.hpp, src/render/backend/filter_chain_controller.cpp	Refactored public API to decompose AdapterBuildConfig and PrechainResolutionConfig nested structs. Updated recreate_filter_chain() and reload_shader_preset() to accept VulkanDeviceInfo and ChainConfig separately. Changed set_prechain_resolution() to accept vk::Extent2D directly. Updated internal slot creation and initialization logic to use passed parameters instead of dereferencing aggregated structures.
Vulkan Backend src/render/backend/vulkan_backend.hpp, src/render/backend/vulkan_backend.cpp	Removed make_filter_chain_build_config() helper method. Added separate make_device_info() and make_chain_config() private helpers. Updated all filter chain initialization and reload call sites to invoke controller methods with decomposed parameters from the new helper functions.
Tests tests/render/test_filter_boundary_contracts.cpp, tests/render/test_filter_chain_retarget.cpp	Introduced TestBuildConfig struct to aggregate device info and chain config for test convenience. Updated test control flow to call controller methods with separated parameters. Changed prechain resolution assertions from PrechainResolutionConfig wrapper to direct vk::Extent2D values. Updated string-based structural assertions to match renamed internal variables in generated implementation text.

Possibly related PRs

• PR#141: Refactors the VulkanBackend ↔ FilterChainController boundary, changing prechain-resolution APIs and controller invocation patterns.
• PR#113: Modifies FilterChainController/VulkanBackend boundary by changing how device and chain configuration are represented and passed between components.
• PR#108: Modifies filter-chain backend callsites and APIs, including changes to how prechain resolution and chain/device configuration are passed to the FilterChainController.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

Review effort 3/5

Poem

🐰 Configs split like morning mist,
Once bundled tight, now gently kissed,
Device and chain in freedom dance,
Extent's a value, not a prance—
Refactored clean, the APIs sing!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.76% 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 summarizes the main change: removing wrapper config structs and flattening parameters in FilterChainController.

_{✏️ 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-fcc

---

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.}

[coderabbitai]

🧹 Nitpick comments (3)

src/render/backend/vulkan_backend.cpp (1)

577-584: Small readability refactor: avoid fetching prechain resolution twice.

current_prechain_resolution() is called twice in the same return object. Caching it once would make this helper slightly clearer.

💡 Suggested cleanup

 auto VulkanBackend::make_chain_config() const
     -> backend_internal::FilterChainController::ChainConfig {
+    const auto prechain = m_filter_chain_controller.current_prechain_resolution();
     return {
         .target_format = static_cast<VkFormat>(m_render_output.swapchain_format),
         .frames_in_flight = backend_internal::RenderOutput::MAX_FRAMES_IN_FLIGHT,
-        .initial_prechain_width = m_filter_chain_controller.current_prechain_resolution().width,
-        .initial_prechain_height = m_filter_chain_controller.current_prechain_resolution().height,
+        .initial_prechain_width = prechain.width,
+        .initial_prechain_height = prechain.height,
     };
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/render/backend/vulkan_backend.cpp` around lines 577 - 584, In
VulkanBackend::make_chain_config cache the result of
m_filter_chain_controller.current_prechain_resolution() into a local variable
(e.g., auto res = m_filter_chain_controller.current_prechain_resolution()) and
use res.width and res.height in the returned ChainConfig instead of calling
current_prechain_resolution() twice to improve readability and avoid duplicate
calls.

src/render/backend/filter_chain_controller.cpp (1)

567-570: Consider extracting shared chain-config normalization.

The stage mask + prechain-dimension assignment logic is duplicated across recreate/reload paths. A small helper would reduce drift risk if this policy evolves.

♻️ Suggested direction

+auto normalize_chain_config(FilterChainController::ChainConfig config,
+                            bool prechain_policy_enabled,
+                            bool effect_stage_policy_enabled,
+                            vk::Extent2D source_resolution)
+    -> FilterChainController::ChainConfig {
+    config.initial_stage_mask =
+        stage_mask_from_policy(prechain_policy_enabled, effect_stage_policy_enabled);
+    config.initial_prechain_width = source_resolution.width;
+    config.initial_prechain_height = source_resolution.height;
+    return config;
+}

Then use it in both methods before slot creation/job submission.

Also applies to: 724-727

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/render/backend/filter_chain_controller.cpp` around lines 567 - 570, The
duplicated normalization of chain_config (assigning
chain_config.initial_stage_mask via
stage_mask_from_policy(prechain_policy_enabled, effect_stage_policy_enabled) and
setting chain_config.initial_prechain_width/height from
source_resolution.width/height) should be extracted into a small helper function
(e.g., normalize_chain_config or init_chain_config_dimensions) and called from
both the recreate and reload code paths before slot creation/job submission;
update uses of chain_config, stage_mask_from_policy, prechain_policy_enabled,
effect_stage_policy_enabled, and source_resolution to call the helper so the
same logic is centralized and prevents drift.

tests/render/test_filter_chain_retarget.cpp (1)

180-183: Consider renaming make_adapter_build_config to match the new model.

This helper now returns split device/chain inputs, so a name like make_test_build_config (or similar) would reduce conceptual drift from the removed adapter wrapper type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/render/test_filter_chain_retarget.cpp` around lines 180 - 183, Rename
the helper function make_adapter_build_config to a name that reflects it now
returns split device/chain inputs (suggestion: make_test_build_config), update
all references/usages to the new name, and adjust any related test identifiers
or comments; locate the function definition make_adapter_build_config and its
callers in tests/render/test_filter_chain_retarget.cpp and replace with the
chosen new name to remove the outdated "adapter" terminology while preserving
the same signature and return type (TestBuildConfig).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/render/backend/filter_chain_controller.cpp`:
- Around line 567-570: The duplicated normalization of chain_config (assigning
chain_config.initial_stage_mask via
stage_mask_from_policy(prechain_policy_enabled, effect_stage_policy_enabled) and
setting chain_config.initial_prechain_width/height from
source_resolution.width/height) should be extracted into a small helper function
(e.g., normalize_chain_config or init_chain_config_dimensions) and called from
both the recreate and reload code paths before slot creation/job submission;
update uses of chain_config, stage_mask_from_policy, prechain_policy_enabled,
effect_stage_policy_enabled, and source_resolution to call the helper so the
same logic is centralized and prevents drift.

In `@src/render/backend/vulkan_backend.cpp`:
- Around line 577-584: In VulkanBackend::make_chain_config cache the result of
m_filter_chain_controller.current_prechain_resolution() into a local variable
(e.g., auto res = m_filter_chain_controller.current_prechain_resolution()) and
use res.width and res.height in the returned ChainConfig instead of calling
current_prechain_resolution() twice to improve readability and avoid duplicate
calls.

In `@tests/render/test_filter_chain_retarget.cpp`:
- Around line 180-183: Rename the helper function make_adapter_build_config to a
name that reflects it now returns split device/chain inputs (suggestion:
make_test_build_config), update all references/usages to the new name, and
adjust any related test identifiers or comments; locate the function definition
make_adapter_build_config and its callers in
tests/render/test_filter_chain_retarget.cpp and replace with the chosen new name
to remove the outdated "adapter" terminology while preserving the same signature
and return type (TestBuildConfig).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de056b78-6e6a-4ff0-b417-c32cf2e3d381

📥 Commits

Reviewing files that changed from the base of the PR and between 5db959d and a3fdde4.

📒 Files selected for processing (6)

• src/render/backend/filter_chain_controller.cpp
• src/render/backend/filter_chain_controller.hpp
• src/render/backend/vulkan_backend.cpp
• src/render/backend/vulkan_backend.hpp
• tests/render/test_filter_boundary_contracts.cpp
• tests/render/test_filter_chain_retarget.cpp