refactor: extract post processing menu

Author: robertvokac

NEXT.md

@@ -19,13 +19,13 @@ replacing the former flat `src/MeshCraft/MeshCraftApplication_*.cpp` layout.
 a compatibility forwarder. Actual UI components currently cover Validation,
 Registry results, Toolbar tools/display/snap-surface/proportional/grid controls,
 Properties delegation, and the View-menu panel/overlay/direction/focus/
-Camera-Bookmarks/Walk-Mode presentation, plus the Add/CSG and Help menu
+Camera-Bookmarks/Walk-Mode/Bloom-SSAO presentation, plus the Add/CSG and Help menu
 presentation and the Edit-history, clipboard, object-action, and
 selection-action groups, the Select-by-Type/Tag/Material, Align-Selection,
 Distribute-Selection, and Mirror-Selection submenus, and the Copy-Properties,
 Convert-to-Definition, Export-Subtree, Break-Instance, Drop-to-Ground,
 Snap-to-Grid, Group-Scale, Linear-Array, and Scatter-Along-Curve items plus the
-Group/Ungroup pair, plus every File-menu action through Open Recent. The detailed
+Group/Ungroup pair and every File-menu action through Open Recent. The detailed
 toolbar Snap interval contents and the remaining MenuBar sections are still
 application-owned. Camera bookmark, walk,
 document, undo, clipboard, selection, grid, and dialog state have **not** moved
@@ -100,11 +100,12 @@ before when explicitly requested (`SYS-W14-##` rows).
 
 ## 2. Current status
 
-- **Last full build: clean after the current Phase 13 File-exit
-  slice.** Testing is enabled in the current Ninja Release tree, and
+- **Last full build: clean after the current Phase 13 File-open-recent and
+  View-Bloom/SSAO slices.** Testing is enabled in the current Ninja Release tree, and
   `CCACHE_DISABLE=1 cmake --build b-release -j4` linked all targets successfully
   on EASYGL. Alternate-backend runtime qualification remains blocked.
-- **Tests:** the fresh Release tree registers 181 tests. All passed in two
+- **Tests:** the fresh Release tree registers 181 tests. All passed again after
+  the current Phase 13 slices in two
   disjoint groups: 147/147 non-render tests and 34/34 render-labelled tests
   under Xvfb (with local loopback/X11 socket access). SVG-specific
   verification passes with `-j4`: external and inline SVG export to glTF PNGs,
@@ -170,7 +171,7 @@ before when explicitly requested (`SYS-W14-##` rows).
   by application/UI ownership, and is extracting UI presentation through
   narrow contexts. Validation, Registry results, Toolbar controls, Properties
   delegation, and the View-menu directions/focus/overlays/panels/
-  Camera-Bookmarks/Walk-Mode presentation, plus the Add/CSG and Help menu
+  Camera-Bookmarks/Walk-Mode/Bloom-SSAO presentation, plus the Add/CSG and Help menu
   presentation and the Edit-history, clipboard, object-action, and
   selection-action groups, the Select-by-Type/Tag/Material, Align-Selection,
   Distribute-Selection, and Mirror-Selection submenus, and the Copy-Properties,
@@ -191,6 +192,17 @@ before when explicitly requested (`SYS-W14-##` rows).
   --target MeshCraft` and `ctest --test-dir b-release -R '^mc3_commands$'
   --output-on-failure` passed; the latter already exercises recent-files'
   load, save, MRU de-duplication, cap, and restart round-trip contract.
+- **Recently implemented (2026-07-25):** `SYS-W3-01` Phase 13 moved the
+  `View → Bloom/SSAO` controls into `Application::UI::MenuBar` through
+  `ViewPostProcessingContext`. It receives the application-computed
+  ShaderEffect-capability gate and value snapshots, then reports changes only
+  through five setters; renderer capability checks and all post-processing
+  state remain application-owned. The supported/unsupported presentation,
+  labels, slider ranges, 140px widths, and `AlwaysClamp` safety guards are
+  unchanged. `CCACHE_DISABLE=1 cmake --build b-release -j4 --target MeshCraft`
+  and the Xvfb-hosted `bloom_test` render regression passed. The test-hook
+  comments now also correctly state that `MESHCRAFT_TEST_FORCE_POSTFX` forces
+  only Bloom; SSAO has its own `MESHCRAFT_TEST_FORCE_SSAO` hook.
 - **Recently implemented (2026-07-20 through 2026-07-25):** all 7
   raw-OpenGL(ES)-vs-CNA migrations, `AUD-082` through `AUD-088` — full
   detail with file:line evidence and
@@ -792,9 +804,9 @@ git stash pop && cmake --build b-release -j4 --target <affected-target>
 No actionable follow-up audit task remains: `AUD-089` through `AUD-091` are
 complete, while Android (`AUD-042`) is environment/owner deferred.
 `SYS-W3-01` has 12 completed subsystem phases and an active Phase 13 for
-application/UI ownership. The authorized Camera Bookmarks, Walk Mode, Help,
-Add/CSG, Edit-history, Edit-clipboard, and Edit-object-actions menu slices are
-implemented and verified, together with Edit-selection-actions,
+application/UI ownership. The authorized Camera Bookmarks, Walk Mode, View
+Bloom/SSAO, Help, Add/CSG, Edit-history, Edit-clipboard, and Edit-object-actions
+menu slices are implemented and verified, together with Edit-selection-actions,
 Edit-select-by-type/tag/material, Edit-copy-properties, Edit-grouping,
 Edit-convert-to-definition, Edit-export-subtree, Edit-break-instance,
 Edit-align-selection, Edit-distribute-selection, Edit-drop-to-ground,
@@ -1009,6 +1021,11 @@ pending-action state, dialog buffers and state, file loading, document
 replacement, selection and undo initialization, title and status updates, error
 handling, and the Ctrl+O keyboard route remain in their existing owners;
 `MenuBar` owns only the unchanged label, shortcut, and click dispatch.
+`ViewPostProcessingContext` exposes only the application-computed text-shader
+capability, Bloom/SSAO value snapshots, and setters. The application retains
+the capability check, effect state, CNA targets/effects, and rendering; the UI
+component preserves the supported/unsupported text, labels, ranges, widths,
+`AlwaysClamp` flags, and callback dispatch.
 `FileOpenRecentContext` exposes only current availability, a lazy recent-files
 provider, one open callback, and one clear callback. Recent-file storage and
 persistence, unsaved-change handling, pending-action state, file loading,
@@ -1027,13 +1044,8 @@ The sequence below is planning only. Per `CLAUDE.md`, each item must be
 described and explicitly confirmed immediately before implementation; finishing
 one item does not authorize the next.
 
-1. **View → Bloom/SSAO controls.** Audit and extract only the text-shader
-   effects block, including its supported and unsupported presentation. Preserve
-   the conditional controls, labels, slider ranges, `AlwaysClamp` flags, and
-   existing state changes; renderer-capability checks and effect state remain
-   application-owned.
-2. **Post-menu boundary audit.** Once the remaining File and View blocks are
-   complete, identify one new narrow presentation boundary before changing
+1. **Post-menu boundary audit.** The remaining File and View blocks are
+   complete; identify one new narrow presentation boundary before changing
    `MeshCraftApplication`. Do not start a broad application refactor.
 
 ### Tracked work that is not implementation-ready

include/MeshCraft/Application/UI/MenuBar.hpp

@@ -25,6 +25,20 @@ struct WalkModeContext {
     std::function<void()> toggle;
 };
 
+struct ViewPostProcessingContext {
+    bool textShaderEffectsSupported;
+    bool bloomEnabled;
+    float bloomStrength;
+    bool ssaoEnabled;
+    float ssaoStrength;
+    float ssaoRadius;
+    std::function<void(bool)> setBloomEnabled;
+    std::function<void(float)> setBloomStrength;
+    std::function<void(bool)> setSsaoEnabled;
+    std::function<void(float)> setSsaoStrength;
+    std::function<void(float)> setSsaoRadius;
+};
+
 struct HelpMenuContext {
     std::function<void()> openPreferences;
     std::function<void()> openCommandPalette;
@@ -313,6 +327,7 @@ class MenuBar final {
     static void drawFocusSelection(const std::function<void()>& focus);
     static void drawCameraBookmarks(const CameraBookmarksContext& context);
     static void drawWalkMode(const WalkModeContext& context);
+    static void drawViewPostProcessing(const ViewPostProcessingContext& context);
     static void drawHelpMenu(const HelpMenuContext& context);
 };
 

plan.md

@@ -164,7 +164,8 @@ still internally consistent.
    research pass.
 6. **SYS-W3-01 (P2/W3), Phase 13 is active:** application/UI ownership is
    being reduced one narrow presentation slice at a time. The authorized
-   Camera Bookmarks, Walk Mode, Help, Add/CSG, Edit-history, Edit-clipboard,
+   Camera Bookmarks, Walk Mode, View Bloom/SSAO, Help, Add/CSG, Edit-history,
+   Edit-clipboard,
    Edit-object-actions, Edit-selection-actions, Edit-select-by-type,
    Edit-select-by-tag/material, Edit-copy-properties, Edit-grouping,
    Edit-convert-to-definition, Edit-export-subtree, Edit-break-instance,
@@ -180,9 +181,8 @@ still internally consistent.
    File-exit menu slices are implemented and verified. Their state remains in
    the existing editor/application owners;
    `Application::UI::MenuBar` receives only the read-only values and callbacks
-   required for presentation. The remaining ordered Phase 13 queue is:
-   **View → Bloom/SSAO controls** (preserve the capability gate, ranges, and
-   clamping), then a fresh audit for a new narrow boundary.
+   required for presentation. The remaining ordered Phase 13 queue is a fresh
+   audit for a new narrow boundary.
    Every item requires its own confirmation per `CLAUDE.md`.
 
 ---
@@ -607,7 +607,10 @@ _All items in this workstream are DONE — archived to [`docs/history/plan_20260
   narrow delegation facade over the existing `Scene::PropertiesPanel`.
   `Application::UI::MenuBar` now owns the View-menu panel toggles, overlay
   toggles, direction choices, focus action, Camera Bookmarks presentation,
-  and the Walk Mode menu item. The bookmark state remains the already-extracted
+  and the Walk Mode menu item. `ViewPostProcessingContext` also owns the
+  Bloom/SSAO presentation, receiving only the capability result, value
+  snapshots, and setters; the application retains effect state and rendering.
+  The bookmark state remains the already-extracted
   `Editor::CameraBookmarks`; a `CameraBookmarksContext` exposes only the
   read-only slots plus Save/Restore callbacks, leaving camera mutation and
   status reporting in the application. Likewise, `WalkModeContext` exposes
@@ -753,9 +756,10 @@ _All items in this workstream are DONE — archived to [`docs/history/plan_20260
   Macro Editor… item is now component-owned through `EditMacroEditorContext`,
   which exposes only one open-dialog callback. Dialog state, macro steps,
   recording and playback operations, macro context, file buffer, save/load
-  behavior, and status reporting remain in their existing owners. File, the
-  remaining Edit groups, and the remaining View controls are still
-  application-owned.
+  behavior, and status reporting remain in their existing owners. The File-menu
+  action slices and the View menu presentation are now component-owned; any
+  remaining extraction target must be selected by a fresh, narrow boundary audit
+  rather than assumed from its location in the menu.
   The historical audit references retain their former paths as time-accurate
   evidence.
   Static undo-audit and snapshot-lint path checks pass after their tracked
@@ -781,7 +785,7 @@ _All items in this workstream are DONE — archived to [`docs/history/plan_20260
   Edit-hide-selection, Edit-show-all-hidden, File-merge-scene,
   File-export-selection, File-export-GLB, File-export-OBJ, File-save,
   File-save-as, File-import-OBJ, File-new, File-open, File-open-recent, and
-  File-exit slices,
+  File-exit slices, plus the Bloom/SSAO controls,
   each incremental Release link and the same 147/147 + 34/34 partitions pass
   again.
   For the current MenuBar slices, the public UI header also compiles as a

src/MeshCraft/Application/Application.cpp

@@ -81,9 +81,9 @@ MeshCraftApplication::MeshCraftApplication(std::filesystem::path filePath, std::
     , autoExportPath_(std::move(exportPath))
     , autoExportCountdown_(autoExportPath_.empty() ? 0 : 2)
 {
-    // Test-only hook: bloom/SSAO are UI-menu-only toggles with no CLI/scene-
-    // file equivalent. Force both on for a screenshot so their CNA render
-    // targets and effects are exercised in headless regression tests too.
+    // Test-only hooks: Bloom and SSAO are UI-menu-only toggles with no CLI/
+    // scene-file equivalent. Keep their force switches independent so each
+    // headless regression test exercises only the effect it intends to cover.
     if (std::getenv("MESHCRAFT_TEST_FORCE_POSTFX")) {
         bloomEnabled_ = true;
     }

src/MeshCraft/Application/UI/MenuBar.cpp

@@ -531,33 +531,20 @@ float MeshCraftApplication::drawMenuBar()
             ImGui::Separator();
             UI::MenuBar::drawOverlays(showEdgeOverlay_, showWireframeMode_,
                                       showStatsOverlay_, shadowDebugEnabled_, snapEnabled_);
-            if (supportsTextShaderEffects()) {
-                ImGui::MenuItem("Bloom (emissive glow)", nullptr, &bloomEnabled_);
-                if (bloomEnabled_) {
-                    ImGui::SetNextItemWidth(140);
-                    // AlwaysClamp: without it, Ctrl+Click lets a typed value go
-                    // out of [min,max] (incl. negative), which the composite
-                    // shader has no other guard against (STAB-0325).
-                    ImGui::SliderFloat("  Strength##bloom", &bloomStrength_, 0.5f, 8.0f, "%.1f",
-                                       ImGuiSliderFlags_AlwaysClamp);
-                }
-                ImGui::MenuItem("SSAO (ambient occlusion)", nullptr, &ssaoEnabled_);
-                if (ssaoEnabled_) {
-                    ImGui::SetNextItemWidth(140);
-                    // AlwaysClamp: same out-of-bounds-via-Ctrl+Click risk as
-                    // bloom strength above (STAB-0326).
-                    ImGui::SliderFloat("  Strength##ssao", &ssaoStrength_, 0.0f, 1.0f, "%.2f",
-                                       ImGuiSliderFlags_AlwaysClamp);
-                    ImGui::SetNextItemWidth(140);
-                    // Same fix applied here too: a negative/zero radius from an
-                    // unclamped Ctrl+Click entry would break the SSAO sample
-                    // kernel, same root cause as the two strength sliders above.
-                    ImGui::SliderFloat("  Radius##ssao",   &ssaoRadius_,   0.05f, 2.0f, "%.2f",
-                                       ImGuiSliderFlags_AlwaysClamp);
-                }
-            } else {
-                ImGui::TextDisabled("Bloom and SSAO require cross-backend ShaderEffect support");
-            }
+            const UI::ViewPostProcessingContext viewPostProcessingContext{
+                .textShaderEffectsSupported = supportsTextShaderEffects(),
+                .bloomEnabled = bloomEnabled_,
+                .bloomStrength = bloomStrength_,
+                .ssaoEnabled = ssaoEnabled_,
+                .ssaoStrength = ssaoStrength_,
+                .ssaoRadius = ssaoRadius_,
+                .setBloomEnabled = [this](bool enabled) { bloomEnabled_ = enabled; },
+                .setBloomStrength = [this](float strength) { bloomStrength_ = strength; },
+                .setSsaoEnabled = [this](bool enabled) { ssaoEnabled_ = enabled; },
+                .setSsaoStrength = [this](float strength) { ssaoStrength_ = strength; },
+                .setSsaoRadius = [this](float radius) { ssaoRadius_ = radius; },
+            };
+            UI::MenuBar::drawViewPostProcessing(viewPostProcessingContext);
             UI::MenuBar::drawPanelToggles(showTimeline_, showRegistryPanel_,
                                           showAiPanel_, showValidationPanel_);
             ImGui::EndMenu();
@@ -1030,6 +1017,52 @@ void MenuBar::drawWalkMode(const WalkModeContext& context) {
     if (ImGui::MenuItem("Walk Mode", "F5", context.active)) context.toggle();
 }
 
+void MenuBar::drawViewPostProcessing(const ViewPostProcessingContext& context) {
+    if (!context.textShaderEffectsSupported) {
+        ImGui::TextDisabled("Bloom and SSAO require cross-backend ShaderEffect support");
+        return;
+    }
+
+    bool bloomEnabled = context.bloomEnabled;
+    if (ImGui::MenuItem("Bloom (emissive glow)", nullptr, &bloomEnabled)) {
+        context.setBloomEnabled(bloomEnabled);
+    }
+    if (bloomEnabled) {
+        float bloomStrength = context.bloomStrength;
+        ImGui::SetNextItemWidth(140);
+        // AlwaysClamp: without it, Ctrl+Click lets a typed value go out of
+        // [min,max] (including negative), which the composite shader has no
+        // other guard against (STAB-0325).
+        if (ImGui::SliderFloat("  Strength##bloom", &bloomStrength, 0.5f, 8.0f, "%.1f",
+                               ImGuiSliderFlags_AlwaysClamp)) {
+            context.setBloomStrength(bloomStrength);
+        }
+    }
+
+    bool ssaoEnabled = context.ssaoEnabled;
+    if (ImGui::MenuItem("SSAO (ambient occlusion)", nullptr, &ssaoEnabled)) {
+        context.setSsaoEnabled(ssaoEnabled);
+    }
+    if (ssaoEnabled) {
+        float ssaoStrength = context.ssaoStrength;
+        ImGui::SetNextItemWidth(140);
+        // AlwaysClamp: same out-of-bounds-via-Ctrl+Click risk as the bloom
+        // strength above (STAB-0326).
+        if (ImGui::SliderFloat("  Strength##ssao", &ssaoStrength, 0.0f, 1.0f, "%.2f",
+                               ImGuiSliderFlags_AlwaysClamp)) {
+            context.setSsaoStrength(ssaoStrength);
+        }
+        float ssaoRadius = context.ssaoRadius;
+        ImGui::SetNextItemWidth(140);
+        // A negative or zero radius from an unclamped Ctrl+Click entry would
+        // break the SSAO sample kernel; same root cause as the strength sliders.
+        if (ImGui::SliderFloat("  Radius##ssao", &ssaoRadius, 0.05f, 2.0f, "%.2f",
+                               ImGuiSliderFlags_AlwaysClamp)) {
+            context.setSsaoRadius(ssaoRadius);
+        }
+    }
+}
+
 void MenuBar::drawHelpMenu(const HelpMenuContext& context) {
     if (!ImGui::BeginMenu("Help")) return;
 

test/bloom.mc3.xml

@@ -4,9 +4,9 @@
      halo point just outside the box's own rendered silhouette is pure
      background (no geometry there at all) with an SSAO ao=1.0 fast-path
      (kSsaoFS returns vec4(1.0) unconditionally when the depth sample is the
-     far clear value), so it is unaffected by SSAO even when
-     MESHCRAFT_TEST_FORCE_POSTFX forces both post-effects on together.
-     Any brightening there can only be Bloom's additive glow. -->
+     far clear value), so it would be unaffected even if an SSAO test enabled
+     its separate hook. MESHCRAFT_TEST_FORCE_POSTFX enables only Bloom here,
+     so any brightening at that point can only be Bloom's additive glow. -->
 <mc3 version="0.3" model="BloomFixture">
 
   <environment>