Skip to content

openusd: propagate dependencies OpenGL#414827

Merged
NickCao merged 1 commit intoNixOS:masterfrom
qbisi:openusd
Jun 24, 2025
Merged

openusd: propagate dependencies OpenGL#414827
NickCao merged 1 commit intoNixOS:masterfrom
qbisi:openusd

Conversation

@qbisi
Copy link
Contributor

@qbisi qbisi commented Jun 7, 2025

Part of VTK 9.2.6 → 9.4.2: #407910

Related upstream pull request: PixarAnimationStudios/OpenUSD#3648

Why OpenUSD should propagate OpenGL and X11:

OpenUSD exports the target usd_ms, which links to the target MaterialXRenderGlsl, defined in MaterialX.
As we can see in the MaterialX source, MaterialXRenderGlsl links to additional OpenGL and X11 targets.

Since not all MaterialX targets defined in MaterialXTargets.cmake require OpenGL/X11, it is not the responsibility of MaterialX to propagate these dependencies via find_dependency(OpenGL/X11) in their materialxConfig.cmake module.

In contrast, pxrTargets,cmake (OpenUSD) only defines one target (usd_ms) that links to MaterialXRenderGlsl. This is why OpenUSD should explicitly propagate the OpenGL and X11 dependencies.


Why OpenUSD is blocking the upgrade of VTK from 9.2.6 to 9.4.2 in the package f3d:

VTK > 9.2 uses Glad to find the available OpenGL implementation via dlopen, rather than linking to OpenGL::GL. As a result, vtkConfig.cmake no longer propagates OpenGL via find_dependency(OpenGL).

However, we do need the OpenGL target when compiling f3d with OpenUSD support. Since OpenUSD does not correctly propagate OpenGL, we miss the OpenGL::GL target during the CMake configurePhase.


I have tried to contact the OpenUSD upstream to fix this problem first, but they insist on blaming it on nixpkgs/materialx.

Therefore, let's fix the problem ourselves.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 7, 2025
@nix-owners nix-owners bot requested review from ShaddyDC and gador June 7, 2025 19:21
@ShaddyDC
Copy link
Contributor

sorry for taking so long to get to this.

Since not all MaterialX targets defined in MaterialXTargets.cmake require OpenGL/X11, it is not the responsibility of MaterialX to propagate these dependencies via find_dependency(OpenGL/X11) in their materialxConfig.cmake module.

but wouldn't we instead want to find_package it there just without the REQUIRED flag so it's non-blocking? like, if those are what actually requires these dependencies, that seems like the place to address the problem to me. otherwise, other dependents are liable to run into the same issue, and we won't know when we can remove the patch from this package (unless your pr there gets applied upstream, i suppose, which seems like it might happen). it seems like the usd guys agree and hence opened AcademySoftwareFoundation/MaterialX#2421, right?
can you clarify why you think this is the right place to apply the patch and not in materialx?

otherwise, it looks like this approach brings us closer in line with arch, which seems fine to me, so i'll approve if you have a good reason

@qbisi
Copy link
Contributor Author

qbisi commented Jun 10, 2025

but wouldn't we instead want to find_package it there just without the REQUIRED flag so it's non-blocking?

Do you mean we can patch upstream materialx to use non-blocking find_dependency(OpenGL) in their MaterialConfig.cmake file, I have not taken this approch into account before. Now I am bit understood why openusd dev opened AcademySoftwareFoundation/MaterialX#2421.

Also, I think the archlinux patch is more accurate to propagate dependency OpenGL, since disable PXR_ENABLE_GL_SUPPORT
will skip linking MaterialXRenderGlsl which cover the case in my pull request to openusd.

@ShaddyDC
Copy link
Contributor

Do you mean we can patch upstream materialx to use non-blocking find_dependency(OpenGL) in their MaterialConfig.cmake file

somewhere in materialx, yes. i haven't looked into the project to know if i'd add it to that specific file

Also, I think the archlinux patch is more accurate to propagate dependency OpenGL, since disable PXR_ENABLE_GL_SUPPORT
will skip linking MaterialXRenderGlsl which cover the case in my pull request to openusd.

that does seem smart!

@ShaddyDC
Copy link
Contributor

actually, looks like it should already find opengl? https://github.com/howetuft/MaterialX/blob/97505091af01390b888905237ff6aef432bff2dc/source/MaterialXRenderGlsl/CMakeLists.txt#L34

weird that we'd still need to add it then.

@qbisi
Copy link
Contributor Author

qbisi commented Jun 10, 2025

IMO,
find_package(OpenGL) is used by the materialx project itself to link target OpenGL::OpenGL.
while find_dependency(OpenGL) is commonly used in *config.cmake module to propagate dependencies to downstream packages.

However, by looking into the generated file MaterialXConfig.cmake in nixpkgs#materialx, i did not find any word matching "find_dependency(".

@qbisi
Copy link
Contributor Author

qbisi commented Jun 10, 2025

Still, i am confused on which package should declare the dependency opengl. namely openusd or materialx.

If materialx should, materialx should support optional components.
e.g. when downstream package make use of materialx.

  • find_package(MaterialX OPTIONAL_COMPONENTS opengl) will trigger find_dependency(OpenGL REQUIRED)
  • find_package(MaterialX) will not trigger find_dependency(OpenGL REQUIRED).

@ShaddyDC
Copy link
Contributor

hmm, interesting
i was under the assumption that if you in your CMakeLists.txt find a package and link against it, cmake would automatically propagate that and ensure that any dependents would also find it to successfully link against, but it's been a while i've worked with it, so ig not

at this point, i don't have a strong preference for how it should be done, so pick whichever seems best to you!

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 414827
Commit: 044563c8a994e2a2e32765caf993ee0904e317ab


x86_64-linux

✅ 19 packages built:
  • blender
  • blender-hip
  • blendfarm
  • exhibit
  • f3d
  • f3d.man
  • f3d_egl
  • f3d_egl.man
  • openusd
  • python312Packages.f3d
  • python312Packages.f3d.man
  • python312Packages.f3d_egl
  • python312Packages.f3d_egl.man
  • python312Packages.openusd
  • python313Packages.f3d
  • python313Packages.f3d.man
  • python313Packages.f3d_egl
  • python313Packages.f3d_egl.man
  • python313Packages.openusd

@Aleksanaa Aleksanaa changed the title openusd: propagate dependencies OpenGL and X11 openusd: propagate dependencies OpenGL Jun 10, 2025
@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 11, 2025
@qbisi qbisi mentioned this pull request Jun 17, 2025
13 tasks
@NickCao NickCao merged commit 498ac10 into NixOS:master Jun 24, 2025
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants