Skip to content

feat: SG-45327: jxl cmake support via oiio - #1419

Open
shanesmith-dwa wants to merge 2 commits into
AcademySoftwareFoundation:mainfrom
dreamworksanimation:jpegxlcmake
Open

shanesmith-dwa wants to merge 2 commits into
AcademySoftwareFoundation:mainfrom
dreamworksanimation:jpegxlcmake

Conversation

@shanesmith-dwa

Copy link
Copy Markdown

Summarize your change.

CMake changes assuming that libjxl is preinstalled and forwards the library on to be built with OpenImageIO.
Assisted-by: Copilot / Opus 4.8

Describe the reason for the change.

To support the JPEG XL image format

Describe what you have tested and on which operating system.

Reading in a jxl file. Successfully tested on Rocky Linux 9.7

Add a list of changes, and note any that might need special attention during the review.

CMake requires that JXL_ROOT provide the path to libjxl. libjxl v0.11.1 tested

jxl.cmake maintains the existing header structure i.e.
Copyright (C) 2026 Autodesk, Inc. All Rights Reserved.
Should the copyright holder be updated / omitted?

Assisted-by: copilot / opus 4.8
Signed-off-by: Shane Smith <shane.smith@dreamworks.com>
@cedrik-fuoco-adsk cedrik-fuoco-adsk changed the title feat: jxl cmake support via oiio feat: SG-45327: jxl cmake support via oiio Sep 18, 2026
@cedrik-fuoco-adsk cedrik-fuoco-adsk added PR: Acknowledged New PR has been acknowledge by the TSC devdays26 labels Sep 18, 2026
@eloisebrosseau eloisebrosseau added community Contribution from the Open RV Community PR: Planned_P1 PR will be review and set soon. Expect 1 to 4 weeks delay. labels Sep 18, 2026
@cedrik-fuoco-adsk

Copy link
Copy Markdown
Contributor

Hi @shanesmith-dwa, thank you for participating in the dev days!

We are tracking this PR and the other PRs you've created. The testing might take some time, but we should be able to review them sooner.

Thank you for the contribution!

Comment thread cmake/dependencies/CMakeLists.txt Outdated
Comment on lines +58 to +63
# JPEG XL support is optional: set JXL_ROOT to a pre-installed library to enable.
SET(JXL_ROOT
""
CACHE PATH "Root of a pre-installed jxl library used by OpenImageIO"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed once the dependencies matches the other similar dependencies.

@cedrik-fuoco-adsk

Copy link
Copy Markdown
Contributor

The main concern is that this doe not follow how the other dependencies are wired, and I'd like to see it brought in line before we merge.

Make libjxl a managed dependency

libjxl belongs in the same category as openjpeg, openjph, webp, etc. We pin it, download it and build it. Eventually, it might change with Conan, but not yet.

openjpeg is the cleanest example to copy. The flow would look like this:

  • cmake/dependencies/jxl.cmake: RV_CREATE_STANDARD_DEPS_VARIABLES then RV_FIND_DEPENDENCY (find-first, honours RV_DEPS_PREFER_INSTALLED), then either INCLUDE(build/jxl.cmake) or the found-package path, then RV_STAGE_DEPENDENCY_LIBS + RV_ADD_IMPORTED_LIBRARY.
  • cmake/dependencies/build/jxl.cmake: Just the ExternalProject_Add
  • cmake/dependencies/CYCOMMON.cmake: RV_DEPS_JXL_VERSION and RV_DEPS_JXL_DOWNLOAD_HASH

I realised that build JXL from source is more work, but that's the current flow we want for the dependencies.

Assisted-by: copilot / opus 4.8
Signed-off-by: Shane Smith <shane.smith@dreamworks.com>
@shanesmith-dwa

Copy link
Copy Markdown
Author

Thanks @cedrik-fuoco-adsk. I have refactored the PR to address the move from an optional dependency to a managed dependency. I do have some follow up questions.

  1. libjxl has some external dependencies. The PR uses ExternalProject_Add with git and git submodules (libjxl recommended https://github.com/libjxl/libjxl/blob/main/BUILDING.md) rather than as a download and separate RV dependencies. I wanted to double check that this is acceptable?
  2. for the brotli external dependency there is an option to build it statically (embeds to libjxl) or as a shared library. does openrv have a preference?
  3. earlier I asked about the CMake header structure for new files. This PR omits the Autodesk copyright holder comment however I would appreciate some guidance please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Contribution from the Open RV Community devdays26 PR: Acknowledged New PR has been acknowledge by the TSC PR: Planned_P1 PR will be review and set soon. Expect 1 to 4 weeks delay.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants