From ff1b7519a020eb5884b6b1d1140715870e1d09ac Mon Sep 17 00:00:00 2001 From: Aiden Date: Wed, 6 May 2026 12:44:22 +1000 Subject: [PATCH] Added bad shader warning instead of hard fail --- README.md | 2 +- .../runtime/RuntimeHost.cpp | 25 +++++---- .../runtime/RuntimeHost.h | 1 + .../shader/ShaderPackageRegistry.cpp | 52 +++++++++++++++++-- .../shader/ShaderPackageRegistry.h | 7 ++- .../shader/ShaderTypes.h | 10 ++++ shaders/broken-shader-example/shader.json | 15 ++++++ shaders/broken-shader-example/shader.slang | 4 ++ tests/ShaderPackageRegistryTests.cpp | 36 ++++++++++++- ui/src/components/ShaderPicker.jsx | 19 +++++-- ui/src/styles.css | 22 ++++++++ 11 files changed, 170 insertions(+), 23 deletions(-) create mode 100644 shaders/broken-shader-example/shader.json create mode 100644 shaders/broken-shader-example/shader.slang diff --git a/README.md b/README.md index 33d5ae5..5bfc827 100644 --- a/README.md +++ b/README.md @@ -249,9 +249,9 @@ If neither variable is set, the workflow falls back to the repo-local defaults u - Audio. - Improve text rendering. - Genlock. +- Don't hardfail on shader fail - Find a better UI library. - Logs. - Continue source cleanup/refactoring. Pass 1 done - Display the control URL in the Windows app, ideally clickable, without rendering it on the video output. - Support a separate sound shader `.slang` file in shader packages. -![alt text](image.png) diff --git a/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.cpp b/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.cpp index 1ac9538..79cdeee 100644 --- a/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.cpp +++ b/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.cpp @@ -1602,12 +1602,14 @@ bool RuntimeHost::ScanShaderPackages(std::string& error) { std::map packagesById; std::vector packageOrder; + std::vector packageStatuses; ShaderPackageRegistry registry(mConfig.maxTemporalHistoryFrames); - if (!registry.Scan(mShaderRoot, packagesById, packageOrder, error)) + if (!registry.Scan(mShaderRoot, packagesById, packageOrder, packageStatuses, error)) return false; mPackagesById.swap(packagesById); mPackageOrder.swap(packageOrder); + mPackageStatuses.swap(packageStatuses); for (auto it = mPersistentState.layers.begin(); it != mPersistentState.layers.end();) { @@ -1854,18 +1856,19 @@ JsonValue RuntimeHost::BuildStateValue() const root.set("performance", performance); JsonValue shaderLibrary = JsonValue::MakeArray(); - for (const std::string& shaderId : mPackageOrder) + for (const ShaderPackageStatus& status : mPackageStatuses) { - auto shaderIt = mPackagesById.find(shaderId); - if (shaderIt == mPackagesById.end()) - continue; - JsonValue shader = JsonValue::MakeObject(); - shader.set("id", JsonValue(shaderIt->second.id)); - shader.set("name", JsonValue(shaderIt->second.displayName)); - shader.set("description", JsonValue(shaderIt->second.description)); - shader.set("category", JsonValue(shaderIt->second.category)); - if (shaderIt->second.temporal.enabled) + shader.set("id", JsonValue(status.id)); + shader.set("name", JsonValue(status.displayName)); + shader.set("description", JsonValue(status.description)); + shader.set("category", JsonValue(status.category)); + shader.set("available", JsonValue(status.available)); + if (!status.available) + shader.set("error", JsonValue(status.error)); + + auto shaderIt = mPackagesById.find(status.id); + if (status.available && shaderIt != mPackagesById.end() && shaderIt->second.temporal.enabled) { JsonValue temporal = JsonValue::MakeObject(); temporal.set("enabled", JsonValue(true)); diff --git a/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.h b/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.h index 3f6636f..223a80b 100644 --- a/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.h +++ b/apps/LoopThroughWithOpenGLCompositing/runtime/RuntimeHost.h @@ -151,6 +151,7 @@ private: std::filesystem::path mPatchedGlslPath; std::map mPackagesById; std::vector mPackageOrder; + std::vector mPackageStatuses; bool mReloadRequested; bool mCompileSucceeded; std::string mCompileMessage; diff --git a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.cpp b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.cpp index b3322b7..045c724 100644 --- a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.cpp +++ b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.cpp @@ -554,6 +554,36 @@ bool ParseParameterDefinitions(const JsonValue& manifestJson, ShaderPackage& sha return true; } + +std::string UniqueUnavailableShaderId(const std::filesystem::path& manifestPath, const std::string& parsedId) +{ + const std::string fallbackId = manifestPath.parent_path().filename().string(); + const std::string baseId = parsedId.empty() ? fallbackId : parsedId; + return baseId + "@invalid:" + fallbackId; +} + +ShaderPackageStatus BuildUnavailableStatus(const std::filesystem::path& manifestPath, const ShaderPackage& partialPackage, const std::string& packageError) +{ + ShaderPackageStatus status; + status.id = UniqueUnavailableShaderId(manifestPath, partialPackage.id); + status.displayName = !partialPackage.displayName.empty() ? partialPackage.displayName : manifestPath.parent_path().filename().string(); + status.description = partialPackage.description; + status.category = !partialPackage.category.empty() ? partialPackage.category : "Unavailable"; + status.available = false; + status.error = packageError; + return status; +} + +ShaderPackageStatus BuildAvailableStatus(const ShaderPackage& shaderPackage) +{ + ShaderPackageStatus status; + status.id = shaderPackage.id; + status.displayName = shaderPackage.displayName; + status.description = shaderPackage.description; + status.category = shaderPackage.category; + status.available = true; + return status; +} } ShaderPackageRegistry::ShaderPackageRegistry(unsigned maxTemporalHistoryFrames) @@ -561,10 +591,16 @@ ShaderPackageRegistry::ShaderPackageRegistry(unsigned maxTemporalHistoryFrames) { } -bool ShaderPackageRegistry::Scan(const std::filesystem::path& shaderRoot, std::map& packagesById, std::vector& packageOrder, std::string& error) const +bool ShaderPackageRegistry::Scan( + const std::filesystem::path& shaderRoot, + std::map& packagesById, + std::vector& packageOrder, + std::vector& packageStatuses, + std::string& error) const { packagesById.clear(); packageOrder.clear(); + packageStatuses.clear(); if (!std::filesystem::exists(shaderRoot)) { @@ -583,19 +619,27 @@ bool ShaderPackageRegistry::Scan(const std::filesystem::path& shaderRoot, std::m ShaderPackage shaderPackage; if (!ParseManifest(manifestPath, shaderPackage, error)) - return false; + { + packageStatuses.push_back(BuildUnavailableStatus(manifestPath, shaderPackage, error)); + error.clear(); + continue; + } if (packagesById.find(shaderPackage.id) != packagesById.end()) { - error = "Duplicate shader id found: " + shaderPackage.id; - return false; + packageStatuses.push_back(BuildUnavailableStatus(manifestPath, shaderPackage, "Duplicate shader id found: " + shaderPackage.id)); + continue; } packageOrder.push_back(shaderPackage.id); + packageStatuses.push_back(BuildAvailableStatus(shaderPackage)); packagesById[shaderPackage.id] = shaderPackage; } std::sort(packageOrder.begin(), packageOrder.end()); + std::sort(packageStatuses.begin(), packageStatuses.end(), [](const ShaderPackageStatus& left, const ShaderPackageStatus& right) { + return left.displayName < right.displayName; + }); return true; } diff --git a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.h b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.h index 2ed3a00..fc6b864 100644 --- a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.h +++ b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderPackageRegistry.h @@ -12,7 +12,12 @@ class ShaderPackageRegistry public: explicit ShaderPackageRegistry(unsigned maxTemporalHistoryFrames); - bool Scan(const std::filesystem::path& shaderRoot, std::map& packagesById, std::vector& packageOrder, std::string& error) const; + bool Scan( + const std::filesystem::path& shaderRoot, + std::map& packagesById, + std::vector& packageOrder, + std::vector& packageStatuses, + std::string& error) const; bool ParseManifest(const std::filesystem::path& manifestPath, ShaderPackage& shaderPackage, std::string& error) const; private: diff --git a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderTypes.h b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderTypes.h index a8118c6..cd6fbae 100644 --- a/apps/LoopThroughWithOpenGLCompositing/shader/ShaderTypes.h +++ b/apps/LoopThroughWithOpenGLCompositing/shader/ShaderTypes.h @@ -93,6 +93,16 @@ struct ShaderPackage std::filesystem::file_time_type manifestWriteTime; }; +struct ShaderPackageStatus +{ + std::string id; + std::string displayName; + std::string description; + std::string category; + bool available = false; + std::string error; +}; + struct RuntimeRenderState { std::string layerId; diff --git a/shaders/broken-shader-example/shader.json b/shaders/broken-shader-example/shader.json new file mode 100644 index 0000000..1cbf9dc --- /dev/null +++ b/shaders/broken-shader-example/shader.json @@ -0,0 +1,15 @@ +{ + "id": "broken-shader-example", + "name": "Broken Shader Example", + "description": "Intentionally invalid shader package used to verify that bad shaders appear as errors without blocking the app.", + "category": "Diagnostics", + "entryPoint": "shadeVideo", + "parameters": [ + { + "id": "badToggle", + "label": "Bad Toggle", + "type": "boolean", + "default": true + } + ] +} diff --git a/shaders/broken-shader-example/shader.slang b/shaders/broken-shader-example/shader.slang new file mode 100644 index 0000000..1e5beb4 --- /dev/null +++ b/shaders/broken-shader-example/shader.slang @@ -0,0 +1,4 @@ +float4 shadeVideo(ShaderContext context) +{ + return context.sourceColor; +} diff --git a/tests/ShaderPackageRegistryTests.cpp b/tests/ShaderPackageRegistryTests.cpp index 1539913..6cb10c4 100644 --- a/tests/ShaderPackageRegistryTests.cpp +++ b/tests/ShaderPackageRegistryTests.cpp @@ -1,5 +1,6 @@ #include "ShaderPackageRegistry.h" +#include #include #include #include @@ -187,9 +188,39 @@ void TestDuplicateScan() ShaderPackageRegistry registry(4); std::map packages; std::vector order; + std::vector statuses; std::string error; - Expect(!registry.Scan(root, packages, order, error), "duplicate package ids are rejected"); - Expect(error.find("Duplicate shader id") != std::string::npos, "duplicate scan error is clear"); + Expect(registry.Scan(root, packages, order, statuses, error), "duplicate package ids do not fail the whole scan"); + Expect(packages.size() == 1, "first duplicate package remains available"); + Expect(statuses.size() == 2, "duplicate package is surfaced in shader status list"); + Expect(std::any_of(statuses.begin(), statuses.end(), [](const ShaderPackageStatus& status) { + return !status.available && status.error.find("Duplicate shader id") != std::string::npos; + }), "duplicate scan error is shown on unavailable package"); + + std::filesystem::remove_all(root); +} + +void TestInvalidPackageDoesNotFailScan() +{ + const std::filesystem::path root = MakeTestRoot(); + WriteShaderPackage(root, "good", R"({ "id": "good", "name": "Good", "parameters": [] })"); + WriteShaderPackage(root, "bad", R"({ + "id": "bad", + "name": "Bad", + "parameters": [{ "id": "enabled", "label": "Enabled", "type": "boolean" }] + })"); + + ShaderPackageRegistry registry(4); + std::map packages; + std::vector order; + std::vector statuses; + std::string error; + Expect(registry.Scan(root, packages, order, statuses, error), "invalid package does not fail the whole scan"); + Expect(packages.find("good") != packages.end(), "valid package remains available"); + Expect(packages.find("bad") == packages.end(), "invalid package is not available for rendering"); + Expect(std::any_of(statuses.begin(), statuses.end(), [](const ShaderPackageStatus& status) { + return status.id.find("bad") != std::string::npos && !status.available && status.error.find("Unsupported parameter type") != std::string::npos; + }), "invalid package is surfaced with its parse error"); std::filesystem::remove_all(root); } @@ -203,6 +234,7 @@ int main() TestInvalidTemporalSettings(); TestDisabledTemporalSettingsAreIgnored(); TestDuplicateScan(); + TestInvalidPackageDoesNotFailScan(); if (gFailures != 0) { diff --git a/ui/src/components/ShaderPicker.jsx b/ui/src/components/ShaderPicker.jsx index 6913ac8..fbb4578 100644 --- a/ui/src/components/ShaderPicker.jsx +++ b/ui/src/components/ShaderPicker.jsx @@ -7,7 +7,7 @@ function matchesShader(shader, query) { return true; } - return [shader.name, shader.id, shader.category, shader.description] + return [shader.name, shader.id, shader.category, shader.description, shader.error] .filter(Boolean) .some((value) => value.toLowerCase().includes(normalizedQuery)); } @@ -17,6 +17,10 @@ function shaderSummary(shader) { return "Search available shaders"; } + if (shader.available === false) { + return shader.error || "Shader is unavailable"; + } + return shader.description || "No description"; } @@ -25,7 +29,8 @@ function ShaderOptionContent({ shader }) { <> {shader.name} - {shader.category ? {shader.category} : null} + {shader.available === false ? Error : null} + {shader.available !== false && shader.category ? {shader.category} : null} {shaderSummary(shader)} @@ -60,7 +65,9 @@ export function ShaderPicker({ id, label = "Shader", shaders, value, onChange }) {selectedShader?.name ?? "Choose shader"} - {selectedShader?.category ? ( + {selectedShader?.available === false ? ( + Error + ) : selectedShader?.category ? ( {selectedShader.category} ) : null} @@ -88,10 +95,14 @@ export function ShaderPicker({ id, label = "Shader", shaders, value, onChange })