Reduce pystring usages

[meimchu]

First step of pystring removal by improving StringUtils (#2256)
StringUtils have been updated to include some pystring functionalities. Focused on replacing:

• pystring::split()
• pystring::strip()
• pystring::replace()
• pystring::mul()
• pystring::startswith()
• pystring::endswith()

These usages have also been replaced throughout the modules that use these as well. Only pystring::os usages should be left now.

Additional changes of note:

• Temporarily changed ci_workflow.yml. Additional explanation below.
• Added an additional include path for OpenFX build.

@KevinJW please take a look when you have a moment! Thank you.

[meimchu]

I have temporarily removed a couple of combinations of our CICD Linux tests here to at least allow more tests to run. Since one tests from the OS category can stop the other jobs from running.

There is an on-going discussions in Slack regarding some Linux combinations and the recent aswf/ci-ocio:2023 docker image update. That is not the focus of this PR so I have temporarily removed it as a distraction.

I will not merge this request without a conclusion is reached in that regards.

The initial message from Jean-Francois Panisset:

See my comments in #wg-ci, it does seem like the update of the aswf/ci-ocio:2023 image from 2023.2 to 2023.3 has "changed something", and now the self build of OpenEXR 3.4 (the version from 2025) is breaking since that makes use of C++ 20 features.

Quick workaround would be to pin this build to use the aswf/ci-ocio:2023.2 image which should restore the previous behavior, but more generally it would be interesting to figure out "what does building latest OCIO with VFX 2025 OpenEXR in a VFX 2023 environment" really mean, and is that something we need / want to support.

[KevinJW]

I wonder if something like the following might be 'better'/faster

std::string repeat_fast(const std::string& input, std::size_t n) {
    if (n == 0) {
        return {};
    }
    if (n == 1) {
        return input;
    }
    std::string result;
    const auto input_size = input.size();
    result.reserve(input_size * n);
    result = input;
    std::size_t current = 1;
    while (current * 2 <= n) {
        result += result;
        current *= 2;
    }
    const auto remaining = n - current;
    if (remaining > 0) {
        result += std::string_view(result.data(), input_size * remaining);
    }
    return result;
}

Edited because I nerd sniped myself and ran some performance tests
And again avoid an extra allocation

[KevinJW]

might want to call the function Repeat or Repeat_n rather than Multiply.

Note that reserve() can raise an exception if we would go over std::string::max_size() but likely we would run out of memory before that happens.

[meimchu]

What a wonderful suggestion. I just want to speak briefly on what I've done so far to get your eyes on it.

1. Implemented your function above as Repeat(). Made some small changes for syntax consistency.
2. I attempted to put a max_limit check as you've pointed out prior to reserve(). I chose not to throw an exception but I'm open to suggestions. It is a new behaviour compared to the old pystring::mul.
3. I have replaced the single StringUtils::Multiply() usage found in the code base (which was using pystring::mul() before).
4. I did do a quick benchmark testing between Multiply() and Repeat(). I got a string of 23 chars in length and repeated it 1000 times. The difference is significant and I like the pointer arithmetic in this implementation. I'm happy to remove Multiply() completely in favour of Repeat() if that is the desired decision.

Multiply() Total Time: 388.173 ms
Multiply() Avg Time:   0.0388174 ms/iter

Repeat() Total Time:   4.99192 ms
Repeat() Avg Time:     0.000499192 ms/iter

[KevinJW]

count should possibly be a size_t ? if we really need signed values then C++20 has std::ssize_t or we could use std::ptrdiff_t

[meimchu]

Thank you for this suggestion. I've replaced it with size_t as you've suggested to make it consistent with the other size_t usages. As well, I've removed a signed integer comparison ReplaceInPlace() accordingly.

[KevinJW]

ditto

[meimchu]

More size_t replacement as suggested!

[KevinJW]

and again?

[meimchu]

More size_t replacement as suggested!

[KevinJW]

Ah I didn't mean you needed to detect this case, I don't think you need this. if you remove it then the reserve() call below will deal with it. With this check you could think of it as silently "failing".

[meimchu]

Removed and let it YOLO! When I was testing this, I did mistakenly let it ran and it filled up my swap pretty fast. Also of note is that this is a private function only used in Op's std::string SerializeOpVec(const OpRcPtrVec & ops, int indent=0);. I have reservations about that function's indent parameter as int. As SerializeOpVec is internal and infrequently used, I have switched this to size_t to remove negative integer possibility. If a contributor decided to use Repeat() or SerializeOpVec() with a negative number though, then they're going to have a bad time!

[KevinJW]

I've not benchmarked it but it is possible that under some circumstances we might want to check || str.empty() to catch the case of appending an empty string many times. though the extra test probably slows down the function when n is small.

[meimchu]

I agree with you that doing an early check on str.empty() for early return makes sense. I ran a benchmark on both Multiply() and Repeat() with an empty string input and 10,000 repeats. Both Multiply() and Repeat() will return itself and Repeat() just barely beat out Multiply(). I think at this point, I feel pretty good about just removing Multiply() in favour of Repeat() if you agree with it.

Multiply() Total Time: 0.129125 ms
Multiply() Avg Time:   1.29125e-05 ms/iter

Repeat() Total Time:   0.119125 ms
Repeat() Avg Time:     1.19125e-05 ms/iter

[Copilot]

Pull request overview

This PR is a first step toward removing the pystring dependency by expanding StringUtils to cover several pystring string-manipulation helpers and replacing those usages across the codebase. It also refactors GPU test infrastructure to support multiple GPU backends and introduces an experimental DirectX 12 backend (plus supporting CMake and CI wiring).

Changes:

• Add/extend StringUtils helpers (trim-by-char-set, bounded replace, repeat/multiply) and replace several pystring::* call sites.
• Introduce a GraphicalApp abstraction and add a DirectX 12 GPU test backend (plus GPU test harness refactor and tolerance updates).
• Add CMake support for DirectX-Headers + DXC runtime discovery/copying and update docs/CI configuration accordingly.

Reviewed changes

Copilot reviewed 37 out of 40 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
vendor/openfx/OCIOUtils.h	Add StringUtils include for OpenFX integration.
vendor/openfx/OCIOUtils.cpp	Replace pystring helpers with StringUtils equivalents.
vendor/openfx/CMakeLists.txt	Add ${PROJECT_SOURCE_DIR}/src/ include path for utils/StringUtils.h.
tests/utils/StringUtils_tests.cpp	Expand unit tests to cover new trim/replace/repeat behavior.
tests/gpu/MatrixOp_test.cpp	Adjust epsilon to accommodate DirectX rounding differences.
tests/gpu/GPUUnitTest.cpp	Refactor to use GraphicalApp and add DirectX/Vulkan backend selection.
tests/gpu/FixedFunctionOp_test.cpp	Relax thresholds to account for DirectX precision differences.
tests/gpu/CMakeLists.txt	Split GPU tests per backend; add DXC runtime copying for DirectX.
tests/cpu/Op_tests.cpp	Add indentation-prefix assertion for SerializeOpVec.
src/utils/StringUtils.h	Add trim-by-char-set overloads, bounded replace, multiply/repeat.
src/OpenColorIO/transforms/FileTransform.cpp	Replace pystring::replace with StringUtils::Replace.
src/OpenColorIO/Op.h	Change SerializeOpVec indent type to size_t.
src/OpenColorIO/Op.cpp	Remove pystring usage; use StringUtils::Repeat for indentation.
src/OpenColorIO/OCIOZArchive.cpp	Replace pystring::lstrip with StringUtils::LeftTrim.
src/OpenColorIO/fileformats/FileFormatIridasLook.cpp	Replace pystring::strip with StringUtils::Trim.
src/OpenColorIO/Config.cpp	Replace pystring::split/startswith with StringUtils equivalents.
src/libutils/oglapphelpers/vulkanapp.h	Make VulkanApp derive from GraphicalApp and override API.
src/libutils/oglapphelpers/vulkanapp.cpp	Update Vulkan app to use GraphicalApp verbosity and buffer API.
src/libutils/oglapphelpers/oglapp.h	Make OglApp derive from GraphicalApp; rename Screen/Headless classes.
src/libutils/oglapphelpers/oglapp.cpp	Implement OglApp::CreateApp; rename APIs; adjust GLUT include workaround.
src/libutils/oglapphelpers/metalapp.mm	Update inheritance and verbosity calls to match refactor.
src/libutils/oglapphelpers/metalapp.h	Update base class name references after refactor.
src/libutils/oglapphelpers/hlsl.h	Add HLSLBuilder for DirectX LUT upload/allocation.
src/libutils/oglapphelpers/hlsl.cpp	Implement DirectX LUT texture upload + SRV creation.
src/libutils/oglapphelpers/graphicalapp.h	Add new abstract GPU-app interface shared by OGL/Vulkan/DX.
src/libutils/oglapphelpers/graphicalapp.cpp	Implement factory selection for GraphicalApp::CreateApp.
src/libutils/oglapphelpers/dxutils.h	Add D3D12 helper utilities (HRESULT, row pitch alignment).
src/libutils/oglapphelpers/dxapp.h	Add DirectX 12 GraphicalApp implementation declaration.
src/libutils/oglapphelpers/dxapp.cpp	Implement DirectX 12 backend for GPU tests (DXC compile, render, readback).
src/libutils/oglapphelpers/CMakeLists.txt	Build oglapphelpers conditionally for GL/DX/Vulkan; add DirectX-Headers dep.
src/apps/ociodisplay/main.cpp	Switch to GraphicalApp pointer type and updated method names.
src/apps/ocioconvert/main.cpp	Use GraphicalApp::CreateApp and updated buffer/verbose methods.
src/apps/ociochecklut/main.cpp	Use GraphicalApp API and updated method names.
share/cmake/utils/LocateDXCompilerRuntime.cmake	Locate/copy DXC runtime DLLs and report version for test runtime.
share/cmake/modules/install/InstallDirectX-Headers.cmake	Fetch/install DirectX-Headers when missing.
share/cmake/modules/FindDirectX-Headers.cmake	Find DirectX-Headers via config or header search; provide target fallback.
SECURITY.md	Move CVE history section and add new CVE entry.
docs/quick_start/installation.rst	Document new OCIO_DIRECTX_ENABLED / OCIO_VULKAN_ENABLED options.
CMakeLists.txt	Add OCIO_DIRECTX_ENABLED (Windows-only) and OCIO_VULKAN_ENABLED options.
.github/workflows/ci_workflow.yml	Allow container-tag override; disable fail-fast for matrix runs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cozdas]

Looks like commits
57a94a8 (DX12)
907a643 (Linux container)
f705d2e (CVE)
are replayed (cherry picked?) into the PR branch with different commit ID's and that confuses the PR diff.

I think rebasing your branch to main and force-pushing your branch should clear the duplicate changes and remove the extra changes listed here.