Skip to content

Commit 3f292d3

Browse files
Spruill-1Copilot
andcommitted
v1.7.3: clock controls on graph load + visibleWhen pin extension
Two minor UI bugs fixed: 1. Clock node loaded from a saved graph was missing its play/pause button and progress bar. EffectNode::isClock is derived from the ShaderLab descriptor at node-creation and isn't serialized; the ResetAfterGraphLoad restore loop ran AFTER SetGraph triggered a RebuildLayout, so the layout was computed with isClock=false. Move the restore before SetGraph. 2. Conditionally-visible input pins (e.g. ICtCp Gamut Map's TargetRedPrimary when TargetGamut=Custom) didn't appear on the canvas after a Properties-panel dropdown change. Two fixes: - UI markDirty enqueue priority Low -> Normal + SetNeedsRedraw() + m_forceRender = true so the canvas paint follows the rebuild. - GuiEngineCommandSink::OnNodeChanged also rebuilds layout when the node has any visibleWhen-conditional parameters, so MCP /graph/set-property gets the same behaviour. Direct call (no UI marshalling) since the hook already runs on the render thread inside the dispatch closure. Bump Version.h, Package.appxmanifest, copilot-instructions to 1.7.3. Tests 183/183 pass; headless smoke ratio 2.500. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 536da88 commit 3f292d3

7 files changed

Lines changed: 57 additions & 10 deletions

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ Active development centers on **tone-mapping and color-correction effects author
134134
- **Effect registry**: Singleton with 40+ built-in D2D effects across 9 categories. Case-insensitive name lookup.
135135
- **ShaderLab effects library**: 33 built-in effects in `Effects/ShaderLabEffects.h/.cpp` across categories: Analysis (Heatmaps + Scopes + Statistics + Tone-Mapping), Color Processing (Gamut Map + ICtCp Gamut Map + Scale), Source / Generator, Composition (Split Comparison), and the data-only Parameter / Clock / Numeric Expression / Random / Working Space nodes. Embedded HLSL with shared color math from `Effects/ColorMath.cpp`. Auto-compiled at first use; bytecode cached on disk under `%LOCALAPPDATA%\ShaderLab\bytecode\` (decision #58 catalog → see [builtin-catalog.md](../docs/effects/builtin-catalog.md) for the full per-effect type table).
136136
- **MCP server**: JSON-RPC 2.0 server on port 47808 (47809 for headless to avoid shared-machine conflicts). The server itself + 20 engine-pure routes live in `Engine/Mcp/{McpHttpServer,EngineMcpRoutes}.{h,cpp}`; 16 UI-coupled / host-specific routes stay in `MainWindow.McpRoutes.cpp`. Both hosts register the same engine-side route set through the same `IEngineCommandSink` interface (decision #58). Engine-side routes are uniform: pure mutation closures dispatched via `sink.Dispatch`, with 8 event hooks (`OnNodeAdded`, `OnNodeRemoved`, `OnNodeChanged`, `OnGraphCleared`, `OnGraphLoaded`, `OnGraphStructureChanged`, `OnCustomEffectRecompiled`, `OnDisplayProfileChanged`) the GUI overrides to keep its UI in sync.
137-
- **Versioning**: `Version.h` defines app version (currently **1.7.2**) and graph format version (2). Both are stored in saved graphs. Forward compatibility check on load. `EngineExport.h::SHADERLAB_ENGINE_ABI_VERSION` is independent — bumped manually on engine ABI breaks; mismatch between header and DLL aborts startup with a friendly message-box.
137+
- **Versioning**: `Version.h` defines app version (currently **1.7.3**) and graph format version (2). Both are stored in saved graphs. Forward compatibility check on load. `EngineExport.h::SHADERLAB_ENGINE_ABI_VERSION` is independent — bumped manually on engine ABI breaks; mismatch between header and DLL aborts startup with a friendly message-box.
138138
- **Refresh-rate-driven render loop on the worker thread**: the render worker `std::jthread` runs the graph evaluate at the active monitor's refresh rate (clamped to 60–240 Hz). Dirty-gated: skips evaluate when no nodes changed, no output window is open, and `m_forceRender` is false. The UI thread runs a `DispatcherQueueTimer` at the same rate, but its body is just "drain dispatcher + blit offscreen + Present1" — sub-ms cost. The interval is re-applied on every display change so dragging the window across monitors picks up the new rate.
139139
- **`ProcessDeferredCompute` requires an active D2D draw session**: it calls `dc->DrawImage` internally to pre-render the upstream chain into an FP32 bitmap, and outside `BeginDraw`/`EndDraw` that DrawImage silently no-ops. The GUI's `RenderFrame`, the headless host's `runEval` / `RunRender`, and the test bench all wrap accordingly.
140140
- **Ctrl+Enter** compiles shader from the editor TextBox.

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ Format follows [Keep a Changelog](https://keepachangelog.com/).
55

66
## [Unreleased]
77

8+
## [1.7.3] - 2026-05-10
9+
10+
### Fixed
11+
12+
- **Clock node controls missing after loading a saved graph.** `EffectNode::isClock` is derived from the ShaderLab effect descriptor at node-creation time and is intentionally not serialized. `MainWindow::ResetAfterGraphLoad` was restoring the flag *after* `m_nodeGraphController.SetGraph(&m_graph)` triggered a layout pass, so `RebuildLayout` saw `isClock=false` for every freshly-deserialized Clock node and computed a regular-parameter-shaped visual without the play/pause button or progress bar. Move the restore loop to run *before* `SetGraph` so the layout sees the correct flag the first time.
13+
- **Conditionally-visible input pins (e.g. `TargetRedPrimary` on `ICtCp Gamut Map` when `TargetGamut == Custom`) didn't appear on the canvas after a Properties-panel dropdown change.** Two fixes: (1) the Properties-panel `markDirty` handler bumped its post-edit `RebuildLayout` enqueue from `DispatcherQueuePriority::Low` to `Normal` and explicitly calls `SetNeedsRedraw()` + sets `m_forceRender` so the canvas repaint follows the rebuild instead of being starved behind the 60 Hz render tick; (2) the engine `GuiEngineCommandSink::OnNodeChanged` hook now also rebuilds the layout when the changed node has any `visibleWhen`-conditional parameters, so MCP-driven `/graph/set-property` calls get the same node-extension behaviour as the Properties-panel dropdown. The hook is already running inside a render-dispatcher closure so it touches `m_nodeGraphController` directly without re-marshalling.
14+
815
## [1.7.2] - 2026-05-10
916

1017
### Removed

MainWindow.GraphFileIo.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -888,10 +888,14 @@ namespace winrt::ShaderLab::implementation
888888
// Close all existing output windows.
889889
m_outputWindows.clear();
890890

891-
m_nodeGraphController.SetGraph(&m_graph);
892-
893-
// Restore isClock flag from ShaderLab effect descriptors
894-
// (not serialized in JSON, derived from effect definition).
891+
// Restore isClock flag from ShaderLab effect descriptors BEFORE
892+
// SetGraph triggers a layout pass. The flag isn't serialized in
893+
// JSON (it's derived from the effect definition), so a freshly-
894+
// deserialized Clock node has isClock=false. RebuildLayout reads
895+
// node.isClock to decide whether to allocate space for the
896+
// play/pause button + progress bar; if it runs first the visual
897+
// ends up sized as a regular parameter node and the controls
898+
// never appear until something else triggers another layout.
895899
{
896900
auto& lib = ::ShaderLab::Effects::ShaderLabEffects::Instance();
897901
for (auto& node : const_cast<std::vector<::ShaderLab::Graph::EffectNode>&>(m_graph.Nodes()))
@@ -905,6 +909,8 @@ namespace winrt::ShaderLab::implementation
905909
}
906910
}
907911

912+
m_nodeGraphController.SetGraph(&m_graph);
913+
908914
m_graph.MarkAllDirty();
909915
PopulatePreviewNodeSelector();
910916

MainWindow.McpRoutes.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,34 @@ namespace winrt::ShaderLab::implementation
203203
});
204204
}
205205

206-
void MainWindow::GuiEngineCommandSink::OnNodeChanged(uint32_t /*nodeId*/)
206+
void MainWindow::GuiEngineCommandSink::OnNodeChanged(uint32_t nodeId)
207207
{
208208
window->m_forceRender = true;
209+
210+
// If the changed node has any visibleWhen-conditional parameters,
211+
// a property change might flip a pin's visibility. Rebuild the
212+
// node-graph layout so new input pins materialize on the canvas.
213+
//
214+
// OnNodeChanged is already running inside a render-dispatcher
215+
// closure (the /graph/set-property route DispatchSync's into the
216+
// worker before invoking this hook), so it is safe to touch
217+
// m_graph and m_nodeGraphController here directly -- the worker
218+
// is paused for the duration. We just need to tell the controller
219+
// to repaint via m_needsRedraw (set by RebuildLayout itself).
220+
if (auto* n = window->m_graph.FindNode(nodeId))
221+
{
222+
if (n->customEffect.has_value())
223+
{
224+
for (const auto& p : n->customEffect->parameters)
225+
{
226+
if (!p.visibleWhen.empty())
227+
{
228+
window->m_nodeGraphController.RebuildLayout();
229+
break;
230+
}
231+
}
232+
}
233+
}
209234
}
210235

211236
void MainWindow::GuiEngineCommandSink::OnGraphCleared()

MainWindow.xaml.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3514,13 +3514,22 @@ namespace winrt::ShaderLab::implementation
35143514
if (hasVisibleWhen || isParameterNode || (n && n->effectClsid.has_value() &&
35153515
IsEqualGUID(n->effectClsid.value(), CLSID_D2D1Histogram)))
35163516
{
3517+
// Normal priority (was Low) so the panel + node-graph
3518+
// rebuild doesn't get starved behind the 60 Hz UI tick
3519+
// when the user flips a visibleWhen trigger. With Low
3520+
// priority the rebuild could sit in the queue
3521+
// indefinitely on a busy render path -- the user sees
3522+
// the Properties panel reflect their change but the
3523+
// canvas pins don't materialize.
35173524
this->DispatcherQueue().TryEnqueue(
3518-
winrt::Microsoft::UI::Dispatching::DispatcherQueuePriority::Low,
3525+
winrt::Microsoft::UI::Dispatching::DispatcherQueuePriority::Normal,
35193526
[this]() {
35203527
if (!m_isShuttingDown)
35213528
{
35223529
UpdatePropertiesPanel();
35233530
m_nodeGraphController.RebuildLayout();
3531+
m_nodeGraphController.SetNeedsRedraw();
3532+
m_forceRender = true;
35243533
}
35253534
});
35263535
}

Package.appxmanifest

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<Identity
1010
Name="ShaderLab"
1111
Publisher="CN=ShaderLab"
12-
Version="1.7.2.0" />
12+
Version="1.7.3.0" />
1313

1414
<mp:PhoneIdentity PhoneProductId="a1b2c3d4-e5f6-7890-abcd-ef1234567890" PhonePublisherId="00000000-0000-0000-0000-000000000000"/>
1515

Version.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ namespace ShaderLab
1414
{
1515
constexpr uint32_t VersionMajor = 1;
1616
constexpr uint32_t VersionMinor = 7;
17-
constexpr uint32_t VersionPatch = 2;
17+
constexpr uint32_t VersionPatch = 3;
1818

1919
// Human-readable version string.
20-
constexpr const wchar_t* VersionString = L"1.7.2";
20+
constexpr const wchar_t* VersionString = L"1.7.3";
2121

2222
// Graph format version. Increment when serialization format changes.
2323
// Graphs saved with a higher format version cannot be loaded by older apps.

0 commit comments

Comments
 (0)