From 196a2bb0ddf960c3b353a3190a56094f0e5a0c66 Mon Sep 17 00:00:00 2001 From: indigo Date: Sat, 11 Jul 2026 17:39:14 +0800 Subject: [PATCH] Fix node thumbnails not converging with progressive renderers; add colorSpace and Sdr-enum widgets - NodeThumbnailCache/MaterialPreviewRenderer: keep a material node thumbnail stale (re-pumped each frame) until the shared preview renderer actually converges, instead of caching after one render pass. Progressive delegates (Arnold/Cycles/Embree) need multiple passes to accumulate past their first noisy sample, so thumbnails previously froze on an early frame and only "fixed themselves" when toggling thumbnail display forced a fresh render. - MaterialEditorPanel: add a Color Space row under Asset-typed shader inputs, authoring the attribute's colorSpace metadata from the active OCIO config's color spaces. - MaterialManager: surface Sdr-declared allowed values (enums) per shader input so the property panel can render a dropdown instead of free text. - CMakePresets.json: build the release preset with debug info (RelWithDebInfo) for easier diagnosis. Co-Authored-By: Claude Sonnet 5 --- CMakePresets.json | 2 +- src/core/MaterialManager.cpp | 8 +- src/core/MaterialManager.h | 4 + src/ui/MaterialEditorPanel.cpp | 124 +++++++++++++++++++++++++++-- src/ui/MaterialEditorPanel.h | 17 ++++ src/ui/MaterialPreviewRenderer.cpp | 18 +++-- src/ui/MaterialPreviewRenderer.h | 16 ++++ src/ui/NodeThumbnailCache.cpp | 33 +++++++- src/ui/NodeThumbnailCache.h | 20 +++++ 9 files changed, 227 insertions(+), 15 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 5a37e0b..d970eed 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -36,7 +36,7 @@ "description": "Optimized release build", "inherits": "default", "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", + "CMAKE_BUILD_TYPE": "RelWithDebInfo", "BUILD_TESTS": "OFF" } }, diff --git a/src/core/MaterialManager.cpp b/src/core/MaterialManager.cpp index d02957a..4074510 100644 --- a/src/core/MaterialManager.cpp +++ b/src/core/MaterialManager.cpp @@ -195,8 +195,12 @@ ShaderGraphSnapshot MaterialManager::GetShaderGraph(const pxr::SdfPath& material if (sdrNode) { node.category = DeriveCategory(sdrNode); for (const pxr::TfToken& inputName : sdrNode->GetShaderInputNames()) { - if (auto* prop = sdrNode->GetShaderInput(inputName)) - node.inputs.push_back({inputName.GetString(), prop->GetTypeAsSdfType().GetSdfType()}); + if (auto* prop = sdrNode->GetShaderInput(inputName)) { + ShaderPinInfo pin{inputName.GetString(), prop->GetTypeAsSdfType().GetSdfType()}; + for (const auto& option : prop->GetOptions()) + pin.allowedValues.push_back(option.first.GetString()); + node.inputs.push_back(std::move(pin)); + } } for (const pxr::TfToken& outputName : sdrNode->GetShaderOutputNames()) { if (auto* prop = sdrNode->GetShaderOutput(outputName)) diff --git a/src/core/MaterialManager.h b/src/core/MaterialManager.h index 34d4368..76b3992 100644 --- a/src/core/MaterialManager.h +++ b/src/core/MaterialManager.h @@ -31,6 +31,10 @@ struct ShaderNodeTypeInfo { struct ShaderPinInfo { std::string name; pxr::SdfValueTypeName typeName; + /// Sdr-declared allowed values (e.g. UsdUVTexture's sourceColorSpace: + /// "raw"/"sRGB"/"auto"), empty when the property isn't enum-constrained. + /// Drives a dropdown instead of a free-text field in the property panel. + std::vector allowedValues; }; /// Hypershade-style pin display mode for a node in the graph canvas. diff --git a/src/ui/MaterialEditorPanel.cpp b/src/ui/MaterialEditorPanel.cpp index a85e5af..63a4c5a 100644 --- a/src/ui/MaterialEditorPanel.cpp +++ b/src/ui/MaterialEditorPanel.cpp @@ -8,6 +8,7 @@ #include "../core/commands/RenamePrimCommand.h" #include "../utils/Logger.h" #include "../utils/FileDialog.h" +#include "../utils/OcioConfigParser.h" #include #include #include @@ -365,6 +366,14 @@ void MaterialEditorPanel::SetColorCorrectionFromPrefs(int ccMode, const std::str const std::string& ocioColorSpace, const std::string& ocioLook) { m_preview.SetColorCorrection(ccMode, ocioDisplay, ocioView, ocioColorSpace, ocioLook); + // Cached for the per-frame NodeThumbnailCache::SyncRenderSettings() call + // in RenderNodeGraphCanvas, which keeps in-canvas node thumbnails + // matching this same color correction (see its declaration). + m_ccMode = ccMode; + m_ocioDisplay = ocioDisplay; + m_ocioView = ocioView; + m_ocioColorSpace = ocioColorSpace; + m_ocioLook = ocioLook; } void MaterialEditorPanel::Render() { @@ -570,6 +579,18 @@ void MaterialEditorPanel::RenderSelectedNodeProperties() { RenderInputValueWidget(*node, input, prim); ImGui::PopItemWidth(); } + + if (input.typeName == pxr::SdfValueTypeNames->Asset) { + ImGui::TableNextRow(ImGuiTableRowFlags_None, rowH); + ImGui::TableSetColumnIndex(1); + ImGui::AlignTextToFramePadding(); + ImGui::TextUnformatted("Color Space"); + ImGui::TableSetColumnIndex(2); + ImGui::PushItemWidth(-FLT_MIN); + RenderColorSpaceWidget(*node, input, prim); + ImGui::PopItemWidth(); + } + ImGui::PopID(); } ImGui::EndTable(); @@ -645,11 +666,39 @@ void MaterialEditorPanel::RenderInputValueWidget(const ShaderGraphNode& node, std::string s; if (current.IsHolding()) s = current.UncheckedGet(); else if (current.IsHolding()) s = current.UncheckedGet().GetString(); - char buf[512]; - std::snprintf(buf, sizeof(buf), "%s", s.c_str()); - if (ImGui::InputText(widgetId.c_str(), buf, sizeof(buf), ImGuiInputTextFlags_EnterReturnsTrue)) { - if (type == tn->String) newValue = std::string(buf); - else newValue = pxr::TfToken(buf); + + if (!input.allowedValues.empty()) { + // Sdr-declared enum (e.g. UsdUVTexture's sourceColorSpace: + // raw/sRGB/auto) — a dropdown of exactly the legal values instead + // of free text, which could author anything the renderer ignores. + // A selection is one atomic edit (no drag-in-progress concept for + // a combo), so it's committed inline here — like the Asset browse + // button below — instead of through the drag/type commit + // machinery at the end of this function. + if (ImGui::BeginCombo(widgetId.c_str(), s.c_str())) { + for (const std::string& option : input.allowedValues) { + bool selected = (option == s); + if (ImGui::Selectable(option.c_str(), selected) && !selected) { + m_preEditValue = current; + m_preEditWasAuthored = authored; + pxr::UsdShadeShader shader(prim); + if (shader) + shader.CreateInput(pxr::TfToken(input.name), input.typeName) + .Set(type == tn->String ? pxr::VtValue(option) : pxr::VtValue(pxr::TfToken(option))); + CommitInputEdit(node, input); + } + if (selected) ImGui::SetItemDefaultFocus(); + } + ImGui::EndCombo(); + } + return; + } else { + char buf[512]; + std::snprintf(buf, sizeof(buf), "%s", s.c_str()); + if (ImGui::InputText(widgetId.c_str(), buf, sizeof(buf), ImGuiInputTextFlags_EnterReturnsTrue)) { + if (type == tn->String) newValue = std::string(buf); + else newValue = pxr::TfToken(buf); + } } } else if (type == tn->Asset) { std::string s; @@ -715,6 +764,64 @@ void MaterialEditorPanel::RenderInputValueWidget(const ShaderGraphNode& node, CommitInputEdit(node, input); } +void MaterialEditorPanel::RenderColorSpaceWidget(const ShaderGraphNode& node, const ShaderPinInfo& input, + const pxr::UsdPrim& prim) { + if (!m_stage || !m_commandHistory) return; + pxr::UsdAttribute attr = prim.GetAttribute(pxr::TfToken("inputs:" + input.name)); + if (!attr) return; + + const std::string current = attr.HasColorSpace() ? attr.GetColorSpace().GetString() : std::string(); + + // Listed from the active OCIO config ($OCIO — the same one driving + // viewport/preview color correction) rather than a fixed raw/sRGB/auto + // set: those are only meaningful to UsdUVTexture's own sourceColorSpace + // input, while this authors the renderer-agnostic colorSpace attribute + // metadata, so the choices should span whatever color spaces are + // actually defined and thus consistently interpretable across every + // render delegate (Storm, Arnold, Cycles, ...) rather than being tied to + // one shading backend's built-in vocabulary. + const OcioConfig& ocfg = GetCurrentOcioConfig(); + static const std::vector kFallback = {"raw", "sRGB"}; + const std::vector& colorSpaces = ocfg.valid && !ocfg.colorSpaces.empty() + ? ocfg.colorSpaces : kFallback; + + auto commit = [&](const std::string& newValue) { + pxr::UsdStageRefPtr stage = m_stage; + pxr::SdfPath nodePath = node.path; + pxr::TfToken attrName = attr.GetName(); + std::string oldValue = current; + m_commandHistory->Push(std::make_unique( + "Set color space on " + node.path.GetName() + "." + input.name, + [stage, nodePath, attrName, newValue]() { + pxr::UsdAttribute a = stage->GetPrimAtPath(nodePath).GetAttribute(attrName); + if (!a) return; + if (newValue.empty()) a.ClearColorSpace(); + else a.SetColorSpace(pxr::TfToken(newValue)); + }, + [stage, nodePath, attrName, oldValue]() { + pxr::UsdAttribute a = stage->GetPrimAtPath(nodePath).GetAttribute(attrName); + if (!a) return; + if (oldValue.empty()) a.ClearColorSpace(); + else a.SetColorSpace(pxr::TfToken(oldValue)); + })); + }; + + if (ImGui::BeginCombo("##colorSpace", current.empty() ? "(unset)" : current.c_str())) { + if (ImGui::Selectable("(unset)", current.empty()) && !current.empty()) + commit(std::string()); + if (current.empty()) ImGui::SetItemDefaultFocus(); + for (const std::string& cs : colorSpaces) { + const bool selected = (cs == current); + if (ImGui::Selectable(cs.c_str(), selected) && !selected) + commit(cs); + if (selected) ImGui::SetItemDefaultFocus(); + } + ImGui::EndCombo(); + } + if (ImGui::IsItemHovered() && ocfg.valid) + ImGui::SetTooltip("Color spaces from the active OCIO config"); +} + void MaterialEditorPanel::CommitInputEdit(const ShaderGraphNode& node, const ShaderPinInfo& input) { if (!m_stage || !m_commandHistory) return; @@ -1418,6 +1525,13 @@ void MaterialEditorPanel::RenderNodeGraphCanvas() { const ImVec2 viewCenterScreen(regionPos.x + regionSize.x * 0.5f, regionPos.y + regionSize.y * 0.5f); + // Keep node thumbnails' renderer/color-correction matching the big + // preview swatch (cheap no-op unless something actually changed) before + // pumping, so a change this frame invalidates stale entries in time to + // be picked up by the pump below. + m_thumbnails.SyncRenderSettings(m_preview.GetRendererPlugin(), m_ccMode, + m_ocioDisplay, m_ocioView, m_ocioColorSpace, m_ocioLook); + // Drain finished texture decodes (GL upload), render one stale material // thumbnail, and prune dead entries — GL/FBO work kept outside the // node-editor's Begin/End so it never touches mid-draw ImGui state. diff --git a/src/ui/MaterialEditorPanel.h b/src/ui/MaterialEditorPanel.h index c308c31..d852f76 100644 --- a/src/ui/MaterialEditorPanel.h +++ b/src/ui/MaterialEditorPanel.h @@ -108,6 +108,14 @@ private: void RenderInputValueWidget(const ShaderGraphNode& node, const ShaderPinInfo& input, const pxr::UsdPrim& prim); void CommitInputEdit(const ShaderGraphNode& node, const ShaderPinInfo& input); + /// A "Color Space" row (raw/sRGB/auto/unset) under an Asset-typed input, + /// authoring the attribute's colorSpace metadata (UsdAttribute:: + /// SetColorSpace) — the generic, renderer-agnostic mechanism Hydra reads + /// for texture color space (see UsdImagingDataSourceAttributeColorSpace), + /// and the only one available for shader types with no separate + /// sourceColorSpace input, e.g. MaterialX image nodes. + void RenderColorSpaceWidget(const ShaderGraphNode& node, const ShaderPinInfo& input, + const pxr::UsdPrim& prim); /// Hypershade-style browser: lists every material on the stage; the /// selected one is loaded into the graph work area via "Show Graph" /// (or double-click). @@ -202,6 +210,15 @@ private: CommandHistory* m_commandHistory = nullptr; IconManager* m_iconManager = nullptr; + /// Last color correction pushed via SetColorCorrectionFromPrefs, cached + /// so RenderNodeGraphCanvas can pass it to NodeThumbnailCache every frame + /// (keeps in-canvas node thumbnails matching m_preview/the app global). + int m_ccMode = 1; // ColorCorrectionMode cast to int (1 = sRGB), mirrors AppPreferences' default + std::string m_ocioDisplay; + std::string m_ocioView; + std::string m_ocioColorSpace; + std::string m_ocioLook; + /// Material currently shown in the graph work area (empty = none). pxr::SdfPath m_materialPath; /// Material highlighted in the browser list; becomes m_materialPath when diff --git a/src/ui/MaterialPreviewRenderer.cpp b/src/ui/MaterialPreviewRenderer.cpp index ac4713c..d70f25a 100644 --- a/src/ui/MaterialPreviewRenderer.cpp +++ b/src/ui/MaterialPreviewRenderer.cpp @@ -592,6 +592,17 @@ void MaterialPreviewRenderer::SetColorCorrection(int ccMode, const std::string& m_dirty = true; } +void MaterialPreviewRenderer::SetRendererPlugin(const pxr::TfToken& pluginId) { + if (pluginId == m_renderer.GetCurrentRendererId()) return; + m_renderer.SetRendererPlugin(pluginId); + UpdateDomeOrientation(); // pole convention differs per delegate + m_dirty = true; +} + +pxr::TfToken MaterialPreviewRenderer::GetRendererPlugin() const { + return m_renderer.GetCurrentRendererId(); +} + void MaterialPreviewRenderer::RenderRendererDropdown(float width) { pxr::TfToken currentId = m_renderer.GetCurrentRendererId(); std::string displayName = currentId.IsEmpty() @@ -607,11 +618,8 @@ void MaterialPreviewRenderer::RenderRendererDropdown(float width) { std::string name = UsdSceneRenderer::GetRendererDisplayName(pluginId); if (name.empty()) name = pluginId.GetString(); bool selected = (pluginId == currentId); - if (ImGui::MenuItem(name.c_str(), nullptr, selected) && !selected) { - m_renderer.SetRendererPlugin(pluginId); - UpdateDomeOrientation(); // pole convention differs per delegate - m_dirty = true; - } + if (ImGui::MenuItem(name.c_str(), nullptr, selected) && !selected) + SetRendererPlugin(pluginId); } ImGui::EndPopup(); } diff --git a/src/ui/MaterialPreviewRenderer.h b/src/ui/MaterialPreviewRenderer.h index 6a1a288..aea3064 100644 --- a/src/ui/MaterialPreviewRenderer.h +++ b/src/ui/MaterialPreviewRenderer.h @@ -49,6 +49,22 @@ public: const std::string& ocioColorSpace, const std::string& ocioLook); + /// Switches the shader-ball's render delegate (what RenderRendererDropdown + /// does on click). No-op if already this plugin. Exposed so other + /// consumers of a material preview (e.g. NodeThumbnailCache's in-canvas + /// thumbnails) can mirror this renderer's current choice. + void SetRendererPlugin(const pxr::TfToken& pluginId); + /// Current render delegate (empty until EnsureInitialized() has run). + pxr::TfToken GetRendererPlugin() const; + + /// True once the last Render() call's image is final. Progressive + /// delegates (Arnold/Cycles/Embree) return false for many calls after a + /// material/renderer change while they accumulate samples — callers that + /// cache the rendered texture (e.g. NodeThumbnailCache) must keep + /// re-rendering until this is true, or they'll cache an early noisy (or + /// still-black) frame. + bool IsConverged() const { return m_renderer.IsConverged(); } + /// Renderer-delegate picker UI (matches the viewport's dropdown pattern). /// width: ImGui item width (-1 = fill available). void RenderRendererDropdown(float width = -1.0f); diff --git a/src/ui/NodeThumbnailCache.cpp b/src/ui/NodeThumbnailCache.cpp index 4994b21..eda4b4e 100644 --- a/src/ui/NodeThumbnailCache.cpp +++ b/src/ui/NodeThumbnailCache.cpp @@ -1,6 +1,5 @@ #include "NodeThumbnailCache.h" #include "../utils/GLExt.h" -#include "../utils/Logger.h" #include #include @@ -243,6 +242,32 @@ ImTextureID NodeThumbnailCache::GetMaterialThumbnail(const pxr::UsdStageRefPtr& return e.texId; } +void NodeThumbnailCache::SyncRenderSettings(const pxr::TfToken& rendererPlugin, int ccMode, + const std::string& ocioDisplay, const std::string& ocioView, + const std::string& ocioColorSpace, const std::string& ocioLook) { + const bool changed = !m_renderSettingsInitialized || + rendererPlugin != m_lastRendererPlugin || ccMode != m_lastCcMode || + ocioDisplay != m_lastOcioDisplay || ocioView != m_lastOcioView || + ocioColorSpace != m_lastOcioColorSpace || ocioLook != m_lastOcioLook; + if (!changed) return; + + m_renderSettingsInitialized = true; + m_lastRendererPlugin = rendererPlugin; + m_lastCcMode = ccMode; + m_lastOcioDisplay = ocioDisplay; + m_lastOcioView = ocioView; + m_lastOcioColorSpace = ocioColorSpace; + m_lastOcioLook = ocioLook; + + if (!rendererPlugin.IsEmpty()) + m_matRenderer.SetRendererPlugin(rendererPlugin); + m_matRenderer.SetColorCorrection(ccMode, ocioDisplay, ocioView, ocioColorSpace, ocioLook); + + // Every cached shader-ball thumbnail was rendered under the old settings. + for (auto& kv : m_matEntries) + kv.second.stale = true; +} + void NodeThumbnailCache::EnsureBlitFbos() { if (!m_readFbo) glGenFramebuffers(1, &m_readFbo); if (!m_drawFbo) glGenFramebuffers(1, &m_drawFbo); @@ -317,7 +342,11 @@ void NodeThumbnailCache::PumpMainThread() { const uint32_t src = m_matRenderer.Render(kThumbPx, kThumbPx); if (src != 0) { BlitToEntry(e, src); - e.stale = false; + // Progressive delegates (Arnold/Cycles/Embree) need several + // Render() calls to accumulate past their first noisy sample; + // keep re-pumping this entry until the image is final instead of + // caching whatever the first pass produced. + e.stale = !m_matRenderer.IsConverged(); } break; // one per frame } diff --git a/src/ui/NodeThumbnailCache.h b/src/ui/NodeThumbnailCache.h index d573488..79febdb 100644 --- a/src/ui/NodeThumbnailCache.h +++ b/src/ui/NodeThumbnailCache.h @@ -3,6 +3,7 @@ #include "MaterialPreviewRenderer.h" #include #include +#include #include #include #include @@ -53,6 +54,18 @@ public: const std::string& output, bool terminal, size_t revision); + /// Keeps the material shader-ball thumbnails' render delegate and color + /// correction in sync with the main preview swatch's current settings + /// (they'd otherwise render under this cache's own independent Hydra + /// instance's defaults, mismatching the preview and the app's global + /// color-correction). Cheap to call every frame: no-ops unless something + /// actually changed, and only then invalidates cached thumbnails so + /// PumpMainThread re-renders them under the new settings. An empty + /// rendererPlugin means "not resolved yet" and is left alone. + void SyncRenderSettings(const pxr::TfToken& rendererPlugin, int ccMode, + const std::string& ocioDisplay, const std::string& ocioView, + const std::string& ocioColorSpace, const std::string& ocioLook); + /// Main thread, once per frame, GL context current: drains finished /// decodes and uploads them, renders at most one stale material thumbnail, /// and prunes entries not requested since the previous pump. @@ -99,6 +112,13 @@ private: unsigned int m_readFbo = 0; // scratch GL FBOs for the render->cache blit unsigned int m_drawFbo = 0; + // Last render settings applied via SyncRenderSettings, so repeated + // per-frame calls with unchanged values are cheap no-ops. + bool m_renderSettingsInitialized = false; + pxr::TfToken m_lastRendererPlugin; + int m_lastCcMode = -1; + std::string m_lastOcioDisplay, m_lastOcioView, m_lastOcioColorSpace, m_lastOcioLook; + void EnsureBlitFbos(); void BlitToEntry(MatEntry& e, uint32_t srcTex); static void DeleteTex(ImTextureID& id);