ociocheck to check OpenColorIO library version first by meimchu · Pull Request #2310 · AcademySoftwareFoundation/OpenColorIO

meimchu · 2026-05-13T07:11:37Z

The idea behind this PR originated with me using ociocheck and getting stuck with config read error (#2303). So I just wanted clearer error message when the error was due to the file path provided being non-existent. Upon looking at the existing issues, I noticed there is an issue about removing pystring dependency (#2256). So I took this opportunity to get started by adding PathUtils::IsPathAbs(), PathUtils::NormalizePath()... etc with standard library filesystem and using them in Config. Also a change to ExceptionMissingFile raised if file does not exist. (Can revert back to just raise Exception if it's causing too much API change)

I would appreciate if @KevinJW has any thought about this as a starting point and the right track. Thank you! Fix #2303.

Signed-off-by: Mei Chu <meimchu@gmail.com>

KevinJW · 2026-05-13T09:49:10Z

    // working directory must be an absolute path.
    const char * workingDirectory = getWorkingDir();
-    if ((workingDirectory && !workingDirectory[0]) || !pystring::os::path::isabs(workingDirectory))
+    if ((workingDirectory && !workingDirectory[0]) || !IsPathAbs(workingDirectory))


My gut suggests we should deal with the switch away from the pystring implementation as a separate issue to cleaning up the error messages. Changing the implementation to use std::filesystem could have more subtle issues

No issue with that. I can reduce the scope of this specific change to just simply a better message if config file does not exists.

Reverted changes done to PathUtil which can be put in a separate PR.

KevinJW · 2026-05-13T09:54:08Z

    return (!hash.empty());
 }

+bool FileExists(const std::string & filename)


As an example of the subtlety When comparing this function with the other overload

bool FileExists(const std::string & filename, const Context & context)

Shows a much more complicated implementation which immediately makes me wonder why. Should we be dealing with the IOProxy here or not, if not then should the function have a different name? or at least a clear documentation stating why etc.

At this point I have not reasoned this through as I don't have the understanding of the IOProxy workings without studying it.

That is all good points. I cannot say for sure myself about that other particular FileExists overload. I see it being used for trying to find lut files with search_path context. I have no issue with it being a different name when we want to replace pystring::path usages!

Perhaps we can create our own internal pystring-like module that basically does some of those pystring tasks that we use then? That was sort of where I was heading by adding these path-related stuff to PathUtil.

Reverted changes done to PathUtil which can be put in a separate PR.

Signed-off-by: Mei Chu <meimchu@gmail.com>

meimchu · 2026-05-15T00:46:06Z

For now, I've reverted the more ambitious changes and opted for a targeted solution. I looked a little deeper into the existing PathUtils::FileExists function which seems caching file hash within a context. My shallow understanding is that it helps speed up the various lut files found via search_path. I also tried creating an empty context to try to utilize that same FileExists() with the config file path but it doesn't feel right to me.

The primary impact of this though is that ExceptionMissingFile() is raised if input file path does not exist. Relying on ifstream.fail() for other reasons why reading that file could fail.

Signed-off-by: Mei Chu <meimchu@gmail.com>

doug-walker

Thanks Mei!

Signed-off-by: Mei Chu <meimchu@gmail.com>

doug-walker · 2026-05-22T23:49:55Z

@meimchu , if you merge in the latest changes from the main branch it should fix the CI problem for you.

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

…ation#2315) * Fix 2023.3 Linux container break Signed-off-by: Doug Walker <doug.walker@autodesk.com> * Adjust to keep the name constant to use the existing GitHub branch rules Signed-off-by: Doug Walker <doug.walker@autodesk.com> --------- Signed-off-by: Doug Walker <doug.walker@autodesk.com>

…demySoftwareFoundation#2271) * Add DirectX 12 GPU backend for automated unit testing on Windows Introduce a DirectX 12 / HLSL rendering backend alongside the existing OpenGL / GLSL and Metal / MSL backends, enabling the GPU unit test suite to run natively on Windows without requiring an OpenGL context. Key changes: GraphicalApp abstract interface (graphicalapp.h/cpp) Backend-agnostic base class extracted from OglApp. OglApp and MetalApp now inherit from it. DxApp (dxapp.h/cpp) -- DirectX 12 backend Off-screen RGBA32F render target, full-screen triangle via SV_VertexID, staging readback, SM 6.0 DXC shader compilation. HLSLBuilder (hlsl.h/cpp) -- HLSL shader generation Translates GpuShaderDesc into HLSL pixel shaders with 1D and 3D LUT texture uploads in RGBA32F format. CMake integration OCIO_DIRECTX_ENABLED option, FetchContent for DirectX-Headers, auto-copy of DXC runtime DLLs to the test output directory. Test tolerance adjustments Minor epsilon increases for 4 tests due to DX12/SM6.0 FMA and pow() precision differences. All 263 GPU tests pass on the DirectX 12 backend. Build and run: # Configure (OCIO_DIRECTX_ENABLED defaults to ON on Windows) cmake -S . -B build -DCMAKE_BUILD_TYPE=Release # Build the GPU test binary cmake --build build --target test_gpu_exec --config Release # Run GPU tests with the DX12 backend ctest --test-dir build -C Release -R test_dx Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Fix post-rebase issues found in code review - HeadlessOglApp::printGraphicsInfo() was calling pure virtual base (crash on headless EGL) - graphicalapp.cpp included oglapp.h unconditionally; guard under OCIO_GL_ENABLED - tests/gpu/CMakeLists.txt early-return guard excluded Vulkan-only builds - Add missing test_vulkan ctest entry Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Minor additional comments, formatting and fixes. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Speed up DX12 GPU test backend (~19%) The DX12 test suite was noticeably slower than the OpenGL and Vulkan backends. Profiling the run showed the gap was almost entirely in DXC shader compilation, not in Present, fence waits, or DxcCreateInstance as initially suspected. Three low-risk changes: - Cache IDxcUtils and IDxcCompiler3 as DxApp members instead of recreating them on every setShader() call. The COM instances are thread-safe and perfectly reusable; recreating them per test added no value. - Compile the full-screen-triangle vertex shader exactly once and reuse the bytecode across all tests. The VSMain HLSL is a hard-coded SV_VertexID-driven triangle with no test-specific state — the bytecode is identical every time. Extracted into a new ensureVertexShaderCompiled() helper. This alone eliminated the biggest redundancy (263 duplicate VS compiles). - Present(1, 0) → Present(0, 0). VSync is meaningless for an off-screen test harness that reads back from a float render target. Locally the win shows up mostly in waitForPreviousFrame, which was being throttled by the swap-chain pipeline even on an invisible window. All 263/263 tests still pass; no tolerance changes, no DXIL codegen changes (except for a UTF8 fix), no precision risk. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Several small fixes tidying up the recently-added GPU test infrastructure. - Fix unused-variable warnings (fatal on macOS with warnings-as-errors): guard useDxRenderer and useVulkanRenderer declarations with the same ifdefs as their usage sites. useMetalRenderer stays unconditional because it's referenced on all platforms. - Propagate the MSVC+shared-libs PATH workaround to test_vulkan so it can find OpenColorIO_*.dll at runtime, matching what's already done for test_dx. - Upgrade the dxcompiler.dll detection message from STATUS to WARNING and rewrite it to name OCIO_DIRECTX_ENABLED and offer concrete recovery paths. The previous STATUS message was easy to miss, leaving users with a silent degradation until test_dx failed at runtime. - Rename the OpenGL ctest from test_gpu to test_opengl now that sibling backend-specific tests (test_dx, test_vulkan, test_metal) exist. The test_gpu_exec binary keeps its name since it's backend-agnostic and selects via CLI flags. - Declare OCIO_VULKAN_ENABLED as a first-class CMake option with mark_as_advanced, matching the existing OCIO_DIRECTX_ENABLED. It was previously used in conditionals without ever being declared, so it never appeared as a toggle in ccmake/cmake-gui. - Document both OCIO_DIRECTX_ENABLED and OCIO_VULKAN_ENABLED in docs/quick_start/installation.rst, noting that Vulkan requires an external SDK. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Integrate DirectX-Headers with OCIO's external-package pattern Previously InstallDirectXHeaders.cmake was included unconditionally from oglapphelpers/CMakeLists.txt, so DirectX-Headers was always fetched from GitHub regardless of whether the user had a local copy installed. There was no way to use a system install, a vendored copy, or an air-gapped build, and the dep didn't respect OCIO_INSTALL_EXT_PACKAGES. DirectX-Headers is now a first-class OCIO dependency, handled the same way as Imath, ZLIB, yaml-cpp, etc.: try find_package first, fall back to FetchContent only if not found and OCIO_INSTALL_EXT_PACKAGES allows it. Changes: - New share/cmake/modules/FindDirectX-Headers.cmake, modeled on FindImath.cmake. - InstallDirectXHeaders.cmake → InstallDirectX-Headers.cmake (the hyphen matches OCIO's Install convention). - oglapphelpers/CMakeLists.txt now calls ocio_handle_dependency(DirectX-Headers ...) with MIN_VERSION 1.606.0 (Windows SDK 22H2 era — old enough to cover most installed copies) and RECOMMENDED_VERSION 1.619.1 (the version OCIO pins and validates). For users: a local DirectX-Headers install can now be supplied via any of the standard CMake mechanisms — -DDirectX-Headers_DIR, -DDirectX-Headers_ROOT, -DDirectX-Headers_INCLUDE_DIR, or globally with -DOCIO_INSTALL_EXT_PACKAGES=NONE to forbid any network fetch. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Improve dxcompiler.dll diagnostics and allow overriding its path Addresses test crashes seen on stuck Windows 10 hosts caused by an old dxcompiler.dll shipped in that host's Windows SDK Redist. - Print the version of the found dxcompiler.dll at configure time so crash reports identify the exact DXC build without follow-up diagnostics. - Emit a standing hint pointing at the DirectX Shader Compiler releases page, which is the documented workaround. - New -DOCIO_DXCOMPILER_DLL=<path> overrides the Windows SDK Redist search, letting users supply a newer DLL pre-build instead of copying it by hand after. - Extracted the DXC-runtime logic into share/cmake/utils/LocateDXCompilerRuntime.cmake so tests/gpu/CMakeLists.txt stays focused on the test target. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Minor comment tweaks in LocateDXCompilerRuntime.cmake. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Use OCIO_DirectX-Headers_RECOMMENDED_VERSION in InstallDirectX-Headers.cmake ocio_install_dependency already propagates the RECOMMENDED_VERSION from the ocio_handle_dependency call site. Consume it instead of hardcoding the version a second time. Matches the pattern in Installyaml-cpp.cmake and Installpystring.cmake. Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> * Address local cleanup notes from PR AcademySoftwareFoundation#2271 Claude review. * Name CbvSrvHeapSize and throw in setShader if a shader needs more SRV slots than the heap holds. * Guard ~DxApp() so the GPU wait/CloseHandle are skipped when sync objects were never created (constructor partial-init). * Comment the 16-byte float4 stride used when packing UNIFORM_VECTOR_FLOAT/INT arrays into the HLSL constant buffer. * Only record m_windowClassName when RegisterClassExA actually succeeds, so cleanup won't unregister a class owned by another DxApp. * Drop the redundant trailing else in GPUUnitTest.cpp's shadingLanguage selector (initializer already covers it). Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> --------- Signed-off-by: Eric Renaud-Houde <eric.renaud.houde@gmail.com> Co-authored-by: Doug Walker <doug.walker@autodesk.com>

meimchu · 2026-05-23T04:39:45Z

@meimchu , if you merge in the latest changes from the main branch it should fix the CI problem for you.

Just did! Thank you for fixing that. Though I noticed my DCO check failed and maybe that's due to the fact that I did not git rebase with --signoff. Can that be rectified by simply doing another git commit with --signoff? I didn't realize I needed to rebase with signoff.

meimchu added 3 commits May 13, 2026 00:01

Add to PathUtils

d0fe0a7

Signed-off-by: Mei Chu <meimchu@gmail.com>

Python test switch to ExceptionMissingFile

1d35437

Signed-off-by: Mei Chu <meimchu@gmail.com>

Updated tests to use ExceptionMissingFile instead

57d8030

Signed-off-by: Mei Chu <meimchu@gmail.com>

meimchu marked this pull request as draft May 13, 2026 07:57

KevinJW reviewed May 13, 2026

View reviewed changes

Comment thread src/OpenColorIO/Config.cpp Outdated

KevinJW reviewed May 13, 2026

View reviewed changes

meimchu added 2 commits May 14, 2026 15:28

Revert PathUtil related changes.

008214a

Signed-off-by: Mei Chu <meimchu@gmail.com>

Remove PathUtils changes

b4fdc76

Signed-off-by: Mei Chu <meimchu@gmail.com>

meimchu marked this pull request as ready for review May 15, 2026 00:40

meimchu mentioned this pull request May 15, 2026

Proposal to remove pystring dependency #2256

Open

Switch exception raised for filepath not provided in CreateFromFile.

707375f

Signed-off-by: Mei Chu <meimchu@gmail.com>

doug-walker approved these changes May 20, 2026

View reviewed changes

Comment thread src/OpenColorIO/Config.cpp Outdated

Remove TODO

2d192d0

Signed-off-by: Mei Chu <meimchu@gmail.com>

doug-walker and others added 3 commits May 22, 2026 21:03

Add CVE to security (AcademySoftwareFoundation#2311)

a80e86f

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ociocheck to check OpenColorIO library version first#2310

ociocheck to check OpenColorIO library version first#2310
meimchu wants to merge 10 commits into
AcademySoftwareFoundation:mainfrom
meimchu:feature/config_exists_check

meimchu commented May 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

KevinJW May 13, 2026

Uh oh!

meimchu May 14, 2026

Uh oh!

meimchu May 14, 2026

Uh oh!

KevinJW May 13, 2026

Uh oh!

meimchu May 14, 2026 •

edited

Loading

Uh oh!

meimchu May 14, 2026

Uh oh!

meimchu commented May 15, 2026 •

edited

Loading

Uh oh!

doug-walker left a comment

Uh oh!

Uh oh!

doug-walker commented May 22, 2026

Uh oh!

meimchu commented May 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

meimchu commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

KevinJW May 13, 2026

Choose a reason for hiding this comment

Uh oh!

meimchu May 14, 2026

Choose a reason for hiding this comment

Uh oh!

meimchu May 14, 2026

Choose a reason for hiding this comment

Uh oh!

KevinJW May 13, 2026

Choose a reason for hiding this comment

Uh oh!

meimchu May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meimchu May 14, 2026

Choose a reason for hiding this comment

Uh oh!

meimchu commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

doug-walker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

doug-walker commented May 22, 2026

Uh oh!

meimchu commented May 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

meimchu commented May 13, 2026 •

edited

Loading

meimchu May 14, 2026 •

edited

Loading

meimchu commented May 15, 2026 •

edited

Loading