diff --git a/docs/adr/0001-viewport-color-correction.md b/docs/adr/0001-viewport-color-correction.md index 1a9e4cb..5208b8c 100644 --- a/docs/adr/0001-viewport-color-correction.md +++ b/docs/adr/0001-viewport-color-correction.md @@ -1,6 +1,6 @@ # ADR 0001 — Custom Viewport Color Correction (sRGB / OCIO) -- **Status:** Accepted +- **Status:** Superseded (2026-06-30) — replaced by the stageView.py approach (HdxColorCorrectionTask via RenderParams) - **Date:** 2026-06-27 - **Component:** `src/core/UsdSceneRenderer.cpp` (`ViewportColorCorrector`, `ApplyViewportColorCorrection`) - **Commit:** `fb1ae9d` @@ -181,3 +181,14 @@ expected corrected (non-black) image and sRGB is unchanged. Headless automated verification is blocked by the stale test-target CMake config (missing USD include dirs); restoring those targets would let an OCIO-vs-sRGB center-pixel test assert this automatically. + +## Supersession Note (2026-06-30) + +This ADR is superseded. The custom `ViewportColorCorrector` was removed in favour +of the approach used in usdview's `stageView.py`: `colorCorrectionMode`, +`ocioDisplay`, `ocioView`, `ocioColorSpace`, and `ocioLook` are now passed +directly to `UsdImagingGLRenderParams` and `SetColorCorrectionSettings()`, +delegating correction to Hydra's `HdxColorCorrectionTask`. The two failure modes +documented in D1 (black OCIO output, bind-stack corruption) may resurface; the +try/catch guard around `Render()` remains in place to mitigate the bind-stack +issue. diff --git a/src/core/UsdSceneRenderer.cpp b/src/core/UsdSceneRenderer.cpp index 3791d5b..ab32b34 100644 --- a/src/core/UsdSceneRenderer.cpp +++ b/src/core/UsdSceneRenderer.cpp @@ -28,16 +28,12 @@ #include #include -#include - #include #include #include #include #include -namespace OCIO = OCIO_NAMESPACE; - namespace UsdLayerManager { // --------------------------------------------------------------------------- @@ -143,271 +139,6 @@ static GLuint LinkProgram(GLuint vs, GLuint fs) { return prog; } -// =========================================================================== -// ViewportColorCorrector — custom GL color correction (sRGB / OCIO) -// =========================================================================== -// -// Replaces HdxColorCorrectionTask. The scene is rendered linear; Apply() -// samples that linear texture and draws a corrected fullscreen triangle into -// the currently-bound FBO. For OCIO it uses the OCIO GPU shader API directly -// (GpuShaderDesc) so the exact transform — including 1D/3D LUTs — is built and -// bound under our control, independent of Hydra. -class ViewportColorCorrector { -public: - enum class Mode { sRGB, OpenColorIO }; - - ~ViewportColorCorrector() { DestroyGLResources(); } - - /// Draw a corrected fullscreen quad into the bound FBO sampling srcTex - /// (single-sample, linear RGBA). Caller has set the GL viewport. - /// Returns true if a pass was drawn. On OCIO build failure it falls back - /// to the sRGB encode so the viewport degrades gracefully (never black). - bool Apply(GLuint srcTex, Mode mode, - const std::string& disp, const std::string& view, - const std::string& cs, const std::string& look) - { - if (!EnsureCommon()) return false; - - GLuint prog = m_srgbProgram; - std::vector* luts = nullptr; - if (mode == Mode::OpenColorIO && EnsureOcioProgram(disp, view, cs, look)) { - prog = m_ocioProgram; - luts = &m_ocioLuts; - } - if (!prog) return false; - - // Save the GL state we touch. - GLboolean depthTest = glIsEnabled(GL_DEPTH_TEST); - GLboolean blend = glIsEnabled(GL_BLEND); - GLboolean depthMask = GL_TRUE; - glGetBooleanv(GL_DEPTH_WRITEMASK, &depthMask); - glDisable(GL_DEPTH_TEST); - glDisable(GL_BLEND); - glDepthMask(GL_FALSE); - - glUseProgram(prog); - glActiveTexture(GL_TEXTURE0); - glBindTexture(GL_TEXTURE_2D, srcTex); - glUniform1i(glGetUniformLocation(prog, "uTex"), 0); - - if (luts) { - int unit = 1; - for (const auto& l : *luts) { - glActiveTexture(GL_TEXTURE0 + unit); - glBindTexture(l.target, l.id); - GLint loc = glGetUniformLocation(prog, l.sampler.c_str()); - if (loc >= 0) glUniform1i(loc, unit); - ++unit; - } - glActiveTexture(GL_TEXTURE0); - } - - glBindVertexArray(m_vao); - glDrawArrays(GL_TRIANGLES, 0, 3); - glBindVertexArray(0); - glUseProgram(0); - - // Restore state. - if (depthTest) glEnable(GL_DEPTH_TEST); - if (blend) glEnable(GL_BLEND); - glDepthMask(depthMask); - return true; - } - - void DestroyGLResources() { - DestroyOcioResources(); - if (m_srgbProgram) { glDeleteProgram(m_srgbProgram); m_srgbProgram = 0; } - if (m_vao) { glDeleteVertexArrays(1, &m_vao); m_vao = 0; } - } - -private: - struct LutTex { GLuint id; GLenum target; std::string sampler; }; - - bool EnsureCommon() { - if (m_vao == 0) glGenVertexArrays(1, &m_vao); - if (m_srgbProgram == 0) { - GLuint vs = CompileShader(kFullscreenVS, GL_VERTEX_SHADER); - GLuint fs = CompileShader(kSrgbFS, GL_FRAGMENT_SHADER); - if (vs && fs) m_srgbProgram = LinkProgram(vs, fs); - } - return m_vao != 0 && m_srgbProgram != 0; - } - - bool EnsureOcioProgram(const std::string& disp, const std::string& view, - const std::string& cs, const std::string& look) - { - const std::string key = disp + "|" + view + "|" + cs + "|" + look; - if (m_ocioProgram && key == m_ocioKey) return true; - if (key == m_ocioFailedKey) return false; - - DestroyOcioResources(); - m_ocioKey.clear(); - - std::string fragText; - OCIO::GpuShaderDescRcPtr desc; - try { - OCIO::ConstConfigRcPtr config = OCIO::GetCurrentConfig(); - if (!config) throw std::runtime_error("no current OCIO config"); - const char* srcCS = cs.empty() ? OCIO::ROLE_SCENE_LINEAR : cs.c_str(); - - OCIO::ConstProcessorRcPtr proc = config->getProcessor( - srcCS, disp.c_str(), view.c_str(), OCIO::TRANSFORM_DIR_FORWARD); - OCIO::ConstGPUProcessorRcPtr gpu = proc->getDefaultGPUProcessor(); - - desc = OCIO::GpuShaderDesc::CreateShaderDesc(); - desc->setLanguage(OCIO::GPU_LANGUAGE_GLSL_1_3); - desc->setFunctionName("OCIODisplay"); - desc->setResourcePrefix("ocio_"); - gpu->extractGpuShaderInfo(desc); - fragText = desc->getShaderText(); - } catch (const std::exception& e) { - LOG_WARNING("Custom OCIO build failed (" + key - + "), using sRGB: " + std::string(e.what())); - m_ocioFailedKey = key; - return false; - } - - // Assemble the fragment shader: OCIO declares its samplers + the - // OCIODisplay(vec4) function; our main() samples the linear input and - // runs it through. - std::string fs = "#version 130\n" - "uniform sampler2D uTex;\n" - "in vec2 vUv;\n" - "out vec4 outColor;\n" - + fragText + - "\nvoid main(){ outColor = OCIODisplay(texture(uTex, vUv)); }\n"; - - GLuint vsh = CompileShader(kFullscreenVS, GL_VERTEX_SHADER); - GLuint fsh = CompileShader(fs.c_str(), GL_FRAGMENT_SHADER); - GLuint prog = (vsh && fsh) ? LinkProgram(vsh, fsh) : 0; - if (!prog) { - LOG_WARNING("Custom OCIO shader compile/link failed for " + key - + " — using sRGB"); - m_ocioFailedKey = key; - return false; - } - - // Upload the LUT textures OCIO requested. - if (!CreateOcioTextures(desc)) { - glDeleteProgram(prog); - DestroyOcioResources(); - m_ocioFailedKey = key; - return false; - } - - m_ocioProgram = prog; - m_ocioKey = key; - LOG_INFO("Custom OCIO program built: " + key - + " (" + std::to_string(m_ocioLuts.size()) + " LUTs)"); - return true; - } - - bool CreateOcioTextures(const OCIO::GpuShaderDescRcPtr& desc) { - // 3D LUTs (RGB). - for (unsigned i = 0; i < desc->getNum3DTextures(); ++i) { - const char* texName = nullptr; const char* samplerName = nullptr; - unsigned edgelen = 0; OCIO::Interpolation interp = OCIO::INTERP_LINEAR; - desc->get3DTexture(i, texName, samplerName, edgelen, interp); - const float* values = nullptr; - desc->get3DTextureValues(i, values); - if (!values || edgelen == 0 || !samplerName) return false; - - GLuint id = 0; - glGenTextures(1, &id); - glBindTexture(GL_TEXTURE_3D, id); - glTexImage3D(GL_TEXTURE_3D, 0, GL_RGB32F, edgelen, edgelen, edgelen, - 0, GL_RGB, GL_FLOAT, values); - GLint filt = (interp == OCIO::INTERP_NEAREST) ? GL_NEAREST : GL_LINEAR; - glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_MIN_FILTER, filt); - glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_MAG_FILTER, filt); - glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); - glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); - glTexParameteri(GL_TEXTURE_3D, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); - m_ocioLuts.push_back({ id, GL_TEXTURE_3D, samplerName }); - } - - // 1D / 2D LUTs. - for (unsigned i = 0; i < desc->getNumTextures(); ++i) { - const char* texName = nullptr; const char* samplerName = nullptr; - unsigned width = 0, height = 0; - OCIO::GpuShaderDesc::TextureType channel = OCIO::GpuShaderDesc::TEXTURE_RGB_CHANNEL; - OCIO::Interpolation interp = OCIO::INTERP_LINEAR; - desc->getTexture(i, texName, samplerName, width, height, channel, interp); - const float* values = nullptr; - desc->getTextureValues(i, values); - if (!values || width == 0 || !samplerName) return false; - - const bool isRed = (channel == OCIO::GpuShaderDesc::TEXTURE_RED_CHANNEL); - const GLint internal = isRed ? GL_R32F : GL_RGB32F; - const GLenum format = isRed ? GL_RED : GL_RGB; - const GLint filt = (interp == OCIO::INTERP_NEAREST) ? GL_NEAREST : GL_LINEAR; - const GLenum target = (height > 1) ? GL_TEXTURE_2D : GL_TEXTURE_1D; - - GLuint id = 0; - glGenTextures(1, &id); - glBindTexture(target, id); - if (target == GL_TEXTURE_2D) { - glTexImage2D(GL_TEXTURE_2D, 0, internal, width, height, 0, - format, GL_FLOAT, values); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); - } else { - glTexImage1D(GL_TEXTURE_1D, 0, internal, width, 0, - format, GL_FLOAT, values); - } - glTexParameteri(target, GL_TEXTURE_MIN_FILTER, filt); - glTexParameteri(target, GL_TEXTURE_MAG_FILTER, filt); - glTexParameteri(target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); - m_ocioLuts.push_back({ id, target, samplerName }); - } - return true; - } - - void DestroyOcioResources() { - for (auto& l : m_ocioLuts) glDeleteTextures(1, &l.id); - m_ocioLuts.clear(); - if (m_ocioProgram) { glDeleteProgram(m_ocioProgram); m_ocioProgram = 0; } - m_ocioKey.clear(); - m_ocioFailedKey.clear(); - } - - static const char* kFullscreenVS; - static const char* kSrgbFS; - - GLuint m_vao = 0; - GLuint m_srgbProgram = 0; - GLuint m_ocioProgram = 0; - std::string m_ocioKey; - std::string m_ocioFailedKey; - std::vector m_ocioLuts; -}; - -// Attribute-less fullscreen triangle; UV in [0,2] covers the [0,1] screen. -const char* ViewportColorCorrector::kFullscreenVS = R"(#version 130 -out vec2 vUv; -void main() { - vec2 p = vec2(float((gl_VertexID << 1) & 2), float(gl_VertexID & 2)); - vUv = p; - gl_Position = vec4(p * 2.0 - 1.0, 0.0, 1.0); -} -)"; - -// Linear → sRGB encode (matches HdxColorCorrectionTask's sRGB path). -const char* ViewportColorCorrector::kSrgbFS = R"(#version 130 -uniform sampler2D uTex; -in vec2 vUv; -out vec4 outColor; -vec3 lin2srgb(vec3 c) { - vec3 lo = c * 12.92; - vec3 hi = 1.055 * pow(max(c, vec3(0.0)), vec3(1.0/2.4)) - 0.055; - bvec3 cut = lessThanEqual(c, vec3(0.0031308)); - return mix(hi, lo, vec3(cut)); -} -void main() { - vec4 c = texture(uTex, vUv); - outColor = vec4(lin2srgb(c.rgb), c.a); -} -)"; - // --------------------------------------------------------------------------- // GLSL sources // --------------------------------------------------------------------------- @@ -478,9 +209,6 @@ UsdSceneRenderer::~UsdSceneRenderer() { DestroyBBoxResources(); DestroyCamWireResources(); DestroyLightWireResources(); - m_colorCorrector.reset(); // deletes its GL program / LUT textures - if (m_ccLinearFBO) glDeleteFramebuffers(1, &m_ccLinearFBO); - if (m_ccLinearTex) glDeleteTextures(1, &m_ccLinearTex); } // =========================================================================== @@ -837,9 +565,8 @@ void UsdSceneRenderer::Render(int width, int height) m_drawTarget = pxr::GlfDrawTarget::New(pxr::GfVec2i(width, height), wantMSAA); if (!m_drawTarget) { LOG_ERROR("Failed to create GlfDrawTarget"); return; } m_drawTarget->Bind(); - // RGBA16F so the scene is stored linear with HDR headroom: we render - // linear (Hydra correction disabled) and apply our own color correction - // afterwards, which needs values outside [0,1] for OCIO/ACES. + // RGBA16F: Hydra's internal render buffers carry linear HDR values; + // RGBA16F on the draw target preserves precision in the corrected output. m_drawTarget->AddAttachment("color", GL_RGBA, GL_FLOAT, GL_RGBA16F); m_drawTarget->AddAttachment("depth", GL_DEPTH_COMPONENT, GL_FLOAT, GL_DEPTH_COMPONENT32F); @@ -971,11 +698,28 @@ void UsdSceneRenderer::Render(int width, int height) m_renderParams.forceRefresh = m_forceRefresh; m_renderParams.clipPlanes = m_clipPlanes; - // Color correction is done by our own GL post-process (ApplyViewport- - // ColorCorrection below), not HdxColorCorrectionTask — so Hydra always - // renders linear ("disabled"). This bypasses the hdx OCIO path entirely. - m_renderParams.colorCorrectionMode = pxr::TfToken("disabled"); - m_renderer->SetColorCorrectionSettings(pxr::TfToken("disabled")); + // Color correction — stageView.py approach: pass the actual mode + OCIO + // params to HdxColorCorrectionTask via RenderParams and SetColorCorrection- + // Settings (mirrors stageView.renderSinglePass). + pxr::TfToken ccToken("disabled"); + if (m_colorCorrectionMode == ColorCorrectionMode::sRGB) + ccToken = pxr::TfToken("sRGB"); + else if (m_colorCorrectionMode == ColorCorrectionMode::OpenColorIO) + ccToken = pxr::TfToken("openColorIO"); + + m_renderParams.colorCorrectionMode = ccToken; + if (m_colorCorrectionMode == ColorCorrectionMode::OpenColorIO) { + m_renderParams.ocioDisplay = pxr::TfToken(m_ocioDisplay); + m_renderParams.ocioView = pxr::TfToken(m_ocioView); + m_renderParams.ocioColorSpace = pxr::TfToken(m_ocioColorSpace); + m_renderParams.ocioLook = pxr::TfToken(m_ocioLook); + } + m_renderer->SetColorCorrectionSettings( + ccToken, + pxr::TfToken(m_ocioDisplay), + pxr::TfToken(m_ocioView), + pxr::TfToken(m_ocioColorSpace), + pxr::TfToken(m_ocioLook)); // Guard the Hydra render: if it throws, the m_drawTarget->Unbind() below // would be skipped, permanently unbalancing the GlfDrawTarget bind stack @@ -987,11 +731,6 @@ void UsdSceneRenderer::Render(int width, int height) } m_forceRefresh = false; - // --- Custom color-correction post-process (linear → sRGB / OCIO) --- - if (m_colorCorrectionMode != ColorCorrectionMode::Disabled) { - ApplyViewportColorCorrection(width, height); - } - // --- Optional grid overlay --- if (m_showGrid) { // When MSAA is active, render the grid into the MSAA FBO so it is @@ -1020,66 +759,6 @@ void UsdSceneRenderer::Render(int width, int height) m_drawTarget->Unbind(); } -// =========================================================================== -// Custom color-correction post-process -// =========================================================================== - -void UsdSceneRenderer::ApplyViewportColorCorrection(int width, int height) -{ - if (!m_drawTarget) return; - - if (!m_colorCorrector) - m_colorCorrector = std::make_unique(); - - // (Re)create the single-sample linear copy texture + its FBO on size change. - if (m_ccLinearTex == 0 || m_ccLinearW != width || m_ccLinearH != height) { - if (m_ccLinearTex == 0) glGenTextures(1, &m_ccLinearTex); - glBindTexture(GL_TEXTURE_2D, m_ccLinearTex); - glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA16F, width, height, 0, - GL_RGBA, GL_FLOAT, nullptr); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); - glBindTexture(GL_TEXTURE_2D, 0); - - if (m_ccLinearFBO == 0) glGenFramebuffers(1, &m_ccLinearFBO); - glBindFramebuffer(GL_FRAMEBUFFER, m_ccLinearFBO); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, - GL_TEXTURE_2D, m_ccLinearTex, 0); - glBindFramebuffer(GL_FRAMEBUFFER, 0); - m_ccLinearW = width; - m_ccLinearH = height; - } - - // Resolve MSAA (no-op otherwise) so the color attachment holds the linear - // single-sample image, then copy it into m_ccLinearTex — sampling and - // writing the same texture in one pass is illegal, hence the copy. - m_drawTarget->Resolve(); - glBindFramebuffer(GL_READ_FRAMEBUFFER, m_drawTarget->GetFramebufferId()); - glBindFramebuffer(GL_DRAW_FRAMEBUFFER, m_ccLinearFBO); - glBlitFramebuffer(0, 0, width, height, 0, 0, width, height, - GL_COLOR_BUFFER_BIT, GL_NEAREST); - - // Draw the corrected result back into the draw-target render FBO (the MSAA - // FBO when multisampling, so it resolves together with the overlays drawn - // on top of it afterwards). - GLuint dstFbo = m_drawTarget->HasMSAA() - ? m_drawTarget->GetFramebufferMSId() - : m_drawTarget->GetFramebufferId(); - glBindFramebuffer(GL_FRAMEBUFFER, dstFbo); - glViewport(0, 0, width, height); - - ViewportColorCorrector::Mode mode = - (m_colorCorrectionMode == ColorCorrectionMode::OpenColorIO) - ? ViewportColorCorrector::Mode::OpenColorIO - : ViewportColorCorrector::Mode::sRGB; - - m_colorCorrector->Apply(m_ccLinearTex, mode, - m_ocioDisplay, m_ocioView, - m_ocioColorSpace, m_ocioLook); -} - // =========================================================================== // Output // =========================================================================== diff --git a/src/core/UsdSceneRenderer.h b/src/core/UsdSceneRenderer.h index 6e3ca60..c46ea3e 100644 --- a/src/core/UsdSceneRenderer.h +++ b/src/core/UsdSceneRenderer.h @@ -25,10 +25,6 @@ namespace UsdLayerManager { -/// Custom viewport color-correction post-process (sRGB / OCIO) implemented in -/// our own GL shader instead of HdxColorCorrectionTask. Defined in the .cpp. -class ViewportColorCorrector; - /// Bounding box display mode for selected prims. enum class BBoxMode { None, ///< No bounding boxes drawn @@ -392,17 +388,6 @@ private: pxr::SdfPathVector m_cachedCameraPaths; bool m_cameraCacheDirty = true; - // --- Custom color-correction post-process --------------------------------- - // The scene is rendered linear (Hydra correction "disabled") and corrected - // by our own GL shader, bypassing HdxColorCorrectionTask. ApplyViewport- - // ColorCorrection() resolves the linear result into m_ccLinearTex, then - // draws a corrected fullscreen quad back into the draw-target FBO. - void ApplyViewportColorCorrection(int width, int height); - std::unique_ptr m_colorCorrector; - GLuint m_ccLinearFBO = 0; ///< FBO wrapping m_ccLinearTex (correction input) - GLuint m_ccLinearTex = 0; ///< single-sample linear copy of the Hydra output - int m_ccLinearW = 0; - int m_ccLinearH = 0; }; } // namespace UsdLayerManager