Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileTransforms with absolute paths should work with configs from OCIOZ archive files #2111

Open
doug-walker opened this issue Jan 3, 2025 · 0 comments
Assignees

Comments

@doug-walker
Copy link
Collaborator

When an OCIOZ archive file is used as the config, FileTransforms are loaded from the .ocioz file. Currently, if an absolute path is used with a FileTransform, it won't work since it won't exist in the archive (OCIO restricts what it archives to only support paths that are relative to the working directory).

However, an application sometimes needs to load LUT files that are not part of the config (e.g., the user simply wants to apply a stand-alone LUT file). Currently the only way to do this is to create the Processor using a config that is different from the .ocioz file. This complicates the situation for applications since they would need to have different code paths based on whether the config is an archive or not. However, the intent for archives is that they be usable the same as a normal config.

The proposal is that FileTransforms that use absolute paths should be loaded directly from the file system and not via the .ocioz file. This should apply to all ConfigIOProxy workflows, not only OCIOZ archives.

Here is a test that may be added at line 438 in tests/cpu/OCIOZArchive_tests.cpp that demonstrates the problem:

        {
            const std::string filePath = OCIO::GetTestFilesDir() + "/matrix_example4x4.ctf";
            OCIO::FileTransformRcPtr transform = OCIO::FileTransform::Create();
            transform->setSrc(filePath.c_str());
            OCIO::ConstProcessorRcPtr processor = cfg->getProcessor(transform);
            OCIO::ConstTransformRcPtr tr = processor->createGroupTransform()->getTransform(0);
            auto mtx = OCIO::DynamicPtrCast<const OCIO::MatrixTransform>(tr);
            OCIO_REQUIRE_ASSERT(mtx);
            mtx->getMatrix(mat);
            OCIO_CHECK_EQUAL(mat[0], 3.24);
        }
cozdas added a commit to autodesk-forks/OpenColorIO that referenced this issue Jan 9, 2025
- Do not use config proxy for absolute paths while computing file hash or loading LUT data.
- Added the unit test provided in the ticket.

Signed-off-by: cuneyt.ozdas <[email protected]>
doug-walker added a commit to autodesk-forks/OpenColorIO that referenced this issue Jan 24, 2025
commit 9fdae400748b265529436862338dcd4bf96c6d6c
Author: cuneyt.ozdas <[email protected]>
Date:   Mon Jan 13 10:32:09 2025 -0800

    - Minor cleanup
    - Added a test for absolute path to inexistent file.

    Signed-off-by: cuneyt.ozdas <[email protected]>

commit 451e8b800ac63187cdc80238f0e665415d664825
Author: cuneyt.ozdas <[email protected]>
Date:   Fri Jan 10 20:12:20 2025 -0800

    - Ah! make_unique is a c++14 feature and we support C++11. I wonder why windows build is configured to use c++14+ while other platforms use C++11. Replacing make_unique with the new syntax to make the other platforms happy too.

    Signed-off-by: cuneyt.ozdas <[email protected]>

commit 58e7f8fbf69f0880c69c2ca05736627c8d2b7972
Author: cuneyt.ozdas <[email protected]>
Date:   Fri Jan 10 18:35:23 2025 -0800

    - Changing the logic so that for abs paths we first try the configProxy and if that fails fall back to file system. For relative paths, we don't fall back to file system though, proxy is expected to handle those.
    - Removed the unnecessary closeLutStream() function. We're using unique pointers, that means RAII is in place. The whole idea behind RAII is we don't need to worry about the cleanup or the type of the object wrapped by the RAII handler (unique_ptr in this case).
    - Cleaned up some unnecessary conversions, type shuffling and copies around the code I touched.
    - Cleaned up some unsafe type casts which are prone to dereferencing null pointers.

    Signed-off-by: cuneyt.ozdas <[email protected]>

commit e6f0e0862ad6cb693a1338cbe798ff9b9c7f3cb8
Author: cuneyt.ozdas <[email protected]>
Date:   Thu Jan 9 10:12:36 2025 -0800

    Ticket AcademySoftwareFoundation#2111
    - Do not use config proxy for absolute paths while computing file hash or loading LUT data.
    - Added the unit test provided in the ticket.

    Signed-off-by: cuneyt.ozdas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants