Skip to content

refactor(ui): inline ImGuiLayer callback type aliases #144

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.

Sign up for GitHub

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

Merged
K1ngst0m merged 1 commit into from
Mar 25, 2026
Merged

refactor(ui): inline ImGuiLayer callback type aliases #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion filter-chain
Open in desktop
Submodule filter-chain updated 11 files
+5 −0 CMakeLists.txt
+14 −4 CMakePresets.json
+29 −0 README.md
+1 −0 tools/CMakeLists.txt
+25 −0 tools/cli/CMakeLists.txt
+75 −0 tools/cli/image_io.cpp
+25 −0 tools/cli/image_io.hpp
+449 −0 tools/cli/main.cpp
+6 −0 tools/cli/stb_image_write_impl.cpp
+558 −0 tools/cli/vulkan_backend.cpp
+78 −0 tools/cli/vulkan_backend.hpp
19 changes: 11 additions & 8 deletions src/ui/imgui_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,16 @@ void ImGuiLayer::set_parameters(std::vector<ParameterState> params) {
m_state.parameters = std::move(params);
}

void ImGuiLayer::set_parameter_change_callback(ParameterChangeCallback callback) {
void ImGuiLayer::set_parameter_change_callback(
std::function<void(goggles::fc::FilterControlId, float)> callback) {
m_on_parameter_change = std::move(callback);
}

void ImGuiLayer::set_parameter_reset_callback(ParameterResetCallback callback) {
void ImGuiLayer::set_parameter_reset_callback(std::function<void()> callback) {
m_on_parameter_reset = std::move(callback);
}

void ImGuiLayer::set_prechain_change_callback(PreChainChangeCallback callback) {
void ImGuiLayer::set_prechain_change_callback(std::function<void(uint32_t, uint32_t)> callback) {
m_on_prechain_change = std::move(callback);
}

Expand Down Expand Up @@ -442,11 +443,13 @@ void ImGuiLayer::set_prechain_parameters(std::vector<goggles::fc::FilterControlD
m_state.prechain.pass_parameters = std::move(params);
}

void ImGuiLayer::set_prechain_parameter_callback(PreChainParameterCallback callback) {
void ImGuiLayer::set_prechain_parameter_callback(
std::function<void(goggles::fc::FilterControlId, float)> callback) {
m_on_prechain_parameter = std::move(callback);
}

void ImGuiLayer::set_prechain_scale_mode_callback(PreChainScaleModeCallback callback) {
void ImGuiLayer::set_prechain_scale_mode_callback(
std::function<void(ScaleMode, uint32_t)> callback) {
m_on_prechain_scale_mode = std::move(callback);
}

Expand All @@ -461,19 +464,19 @@ void ImGuiLayer::set_target_fps(uint32_t target_fps) {
}
}

void ImGuiLayer::set_target_fps_change_callback(TargetFpsChangeCallback callback) {
void ImGuiLayer::set_target_fps_change_callback(std::function<void(uint32_t)> callback) {
m_on_target_fps_change = std::move(callback);
}

void ImGuiLayer::set_surfaces(std::vector<compositor::SurfaceInfo> surfaces) {
m_surfaces = std::move(surfaces);
}

void ImGuiLayer::set_surface_select_callback(SurfaceSelectCallback callback) {
void ImGuiLayer::set_surface_select_callback(std::function<void(uint32_t)> callback) {
m_on_surface_select = std::move(callback);
}

void ImGuiLayer::set_surface_filter_toggle_callback(SurfaceFilterToggleCallback callback) {
void ImGuiLayer::set_surface_filter_toggle_callback(std::function<void(uint32_t, bool)> callback) {
m_on_surface_filter_toggle = std::move(callback);
}

Expand Down
45 changes: 18 additions & 27 deletions src/ui/imgui_layer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,6 @@ struct ShaderControlState {
PreChainState prechain;
};

using ParameterChangeCallback =
Copy link
Copy Markdown

@qodo-code-review qodo-code-review bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Public callback aliases removed 🐞 Bug ⚙ Maintainability

Deleting the public callback type aliases in goggles::ui will break any downstream code that
referenced goggles::ui::{ParameterChangeCallback, ...} even though the underlying std::function
signatures are unchanged. This is a source/ABI-facing API break because the header is exposed via
the goggles_ui target’s PUBLIC include directories.
Agent Prompt 
### Issue description
The PR removes the public callback type aliases (e.g., `goggles::ui::ParameterChangeCallback`) from `src/ui/imgui_layer.hpp`. Any downstream code that referenced these names will now fail to compile.

### Issue Context
Even if you prefer inline `std::function` types in method signatures/members, you can keep the aliases purely as compatibility shims.

### Fix Focus Areas
- src/ui/imgui_layer.hpp[70-120]
  - Reintroduce the removed `using ...Callback = std::function<...>;` aliases in `namespace goggles::ui` (optionally mark them `[[deprecated]]` with a clear message).
  - Keep the new inline `std::function` signatures if that’s the refactor goal; the aliases can remain unused internally but preserve downstream compatibility.

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

std::function<void(goggles::fc::FilterControlId control_id, float value)>;
using ParameterResetCallback = std::function<void()>;
using PreChainChangeCallback = std::function<void(uint32_t width, uint32_t height)>;
using PreChainParameterCallback =
std::function<void(goggles::fc::FilterControlId control_id, float value)>;
using PreChainScaleModeCallback = std::function<void(ScaleMode mode, uint32_t integer_scale)>;
using SurfaceSelectCallback = std::function<void(uint32_t surface_id)>;
using SurfaceFilterToggleCallback = std::function<void(uint32_t surface_id, bool enabled)>;
using TargetFpsChangeCallback = std::function<void(uint32_t target_fps)>;

class ImGuiLayer {
public:
[[nodiscard]] static auto create(SDL_Window* window, const ImGuiConfig& config,
Expand All @@ -113,16 +102,18 @@ class ImGuiLayer {
void set_current_preset(const std::filesystem::path& path);
void set_parameters(std::vector<ParameterState> params);

void set_parameter_change_callback(ParameterChangeCallback callback);
void set_parameter_reset_callback(ParameterResetCallback callback);
void set_prechain_change_callback(PreChainChangeCallback callback);
void set_parameter_change_callback(
std::function<void(goggles::fc::FilterControlId, float)> callback);
void set_parameter_reset_callback(std::function<void()> callback);
void set_prechain_change_callback(std::function<void(uint32_t, uint32_t)> callback);
void set_prechain_state(vk::Extent2D resolution, ScaleMode scale_mode, uint32_t integer_scale);
void set_prechain_parameters(std::vector<goggles::fc::FilterControlDescriptor> params);
void set_prechain_parameter_callback(PreChainParameterCallback callback);
void set_prechain_scale_mode_callback(PreChainScaleModeCallback callback);
void set_prechain_parameter_callback(
std::function<void(goggles::fc::FilterControlId, float)> callback);
void set_prechain_scale_mode_callback(std::function<void(ScaleMode, uint32_t)> callback);
void set_runtime_metrics(util::CompositorRuntimeMetricsSnapshot metrics);
void set_target_fps(uint32_t target_fps);
void set_target_fps_change_callback(TargetFpsChangeCallback callback);
void set_target_fps_change_callback(std::function<void(uint32_t)> callback);

[[nodiscard]] auto state() -> ShaderControlState& { return m_state; }
[[nodiscard]] auto state() const -> const ShaderControlState& { return m_state; }
Expand All @@ -133,8 +124,8 @@ class ImGuiLayer {
[[nodiscard]] auto is_globally_visible() const -> bool { return m_global_visible; }

void set_surfaces(std::vector<compositor::SurfaceInfo> surfaces);
void set_surface_select_callback(SurfaceSelectCallback callback);
void set_surface_filter_toggle_callback(SurfaceFilterToggleCallback callback);
void set_surface_select_callback(std::function<void(uint32_t)> callback);
void set_surface_filter_toggle_callback(std::function<void(uint32_t, bool)> callback);

void rebuild_for_format(vk::Format new_format);

Expand Down Expand Up @@ -168,14 +159,14 @@ class ImGuiLayer {

ShaderControlState m_state;
PresetTreeNode m_preset_tree;
ParameterChangeCallback m_on_parameter_change;
ParameterResetCallback m_on_parameter_reset;
PreChainChangeCallback m_on_prechain_change;
PreChainParameterCallback m_on_prechain_parameter;
PreChainScaleModeCallback m_on_prechain_scale_mode;
SurfaceSelectCallback m_on_surface_select;
SurfaceFilterToggleCallback m_on_surface_filter_toggle;
TargetFpsChangeCallback m_on_target_fps_change;
std::function<void(goggles::fc::FilterControlId, float)> m_on_parameter_change;
std::function<void()> m_on_parameter_reset;
std::function<void(uint32_t, uint32_t)> m_on_prechain_change;
std::function<void(goggles::fc::FilterControlId, float)> m_on_prechain_parameter;
std::function<void(ScaleMode, uint32_t)> m_on_prechain_scale_mode;
std::function<void(uint32_t)> m_on_surface_select;
std::function<void(uint32_t, bool)> m_on_surface_filter_toggle;
std::function<void(uint32_t)> m_on_target_fps_change;
std::vector<compositor::SurfaceInfo> m_surfaces;
util::CompositorRuntimeMetricsSnapshot m_runtime_metrics;
uint32_t m_target_fps = 60;
Expand Down
6 changes: 0 additions & 6 deletions tests/render/test_filter_boundary_contracts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ TEST_CASE("Filter chain boundary control contract coverage", "[filter_chain][bou
static_assert(std::is_same_v<decltype(&FCC::set_filter_control_value), SetSig>);
static_assert(std::is_same_v<decltype(&FCC::reset_filter_control_value), ResetSig>);

static_assert(std::is_same_v<goggles::ui::ParameterChangeCallback,
std::function<void(FilterControlId, float)>>);
static_assert(std::is_same_v<goggles::ui::PreChainParameterCallback,
std::function<void(FilterControlId, float)>>);
static_assert(
std::is_same_v<goggles::ui::TargetFpsChangeCallback, std::function<void(uint32_t)>>);
static_assert(std::is_same_v<decltype(goggles::ui::ParameterState{}.descriptor),
FilterControlDescriptor>);
static_assert(std::is_same_v<decltype(goggles::ui::PreChainState{}.pass_parameters),
Expand Down
Loading