Details

Reviewers

Ray Molenkamp (LazyDodo)
Brecht Van Lommel (brecht)

Commits

rB91bd63a196ce: CMake: Reduce dependencies of USD

Summary

PXR_ENABLE_OSL_SUPPORT=OFF: OpenShadingLanguage is an optional
dependency of the Imaging module. However, since that module was
included for its support for converting primitive shapes (sphere, cube,
etc.) to geometry, OSL is not necessary. Disabling it will make it simpler
to build Blender; currently only Cycles uses OSL.

PXR_ENABLE_GL_SUPPORT=OFF: GL support on Linux also links to X11
libraries (report). Enabling it would break headless or Wayland-only builds.
OpenGL support would be useful if someone wants to work on a Hydra
viewport in Blender; when that's actually being worked on, we could
patch in a new PXR_ENABLE_X11_SUPPORT option (to separate OpenGL from
X11) and contribute it upstream.

PXR_BUILD_OPENIMAGEIO_PLUGIN=OFF: It's used for loading image textures in Hydra Storm / Embree renderers which we don't use.

For future reference: re-enabling OSL would require:

Add -DOSL_ROOT=${LIBDIR}/osl and set -DPXR_ENABLE_OSL_SUPPORT=ON in USD_EXTRA_ARGS.
Add external_osl to the add_dependencies() call.

Diff Detail

Repository

rB Blender

Branch

tmp-usd-disable-gl-osl (branched from master)

Build Status

Buildable 21872
Build 21872: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) requested review of this revision.Apr 28 2022, 2:49 PM

Sybren A. Stüvel (sybren) created this revision.

Harbormaster completed remote builds in B21869: Diff 50947.Apr 28 2022, 2:49 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Apr 28 2022, 2:50 PM

Required change:
Hard to leave a comment on things not in the patch, but you forgot to clean up the dependencies section just below this code :)

Observational warning:
This will just move your problem from: "if you build without CYCLES_OSL it'll give you a linker error" to "if you build without OIIO it'll give you a linker error" is this really any better?

This revision now requires changes to proceed.Apr 28 2022, 2:55 PM

Brecht Van Lommel (brecht) accepted this revision.Apr 28 2022, 2:57 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)Apr 28 2022, 3:00 PM

I don't think there's a good reason to keep OpenImageIO, so we might as well disable that too. It's used for loading image textures in Hydra Storm / Embree renderers which we don't use.

I would like to keep the OpenSubdiv dependency since that is used by the Cycles Hydra render delegate. For that we could add this to our top level CMakeLists.txt:

set_and_warn_dependency(WITH_OPENSUBDIV WITH_USD            OFF)

In D14792#397666, @Ray Molenkamp (LazyDodo) wrote:

Required change:
Hard to leave a comment on things not in the patch, but you forgot to clean up the dependencies section just below this code :)

👍

Observational warning:
This will just move your problem from: "if you build without CYCLES_OSL it'll give you a linker error" to "if you build without OIIO it'll give you a linker error" is this really any better?

Given that we don't even use OSL in USD right now (at least we don't have any concrete examples that would require OSL support), yeah to me that's different.

I'm not against enabling OSL in USD, but then I feel we also should just link Blender against OSL when USD is enabled. And that I think is only a good idea when we have a concrete use case.

Remove other references to OSL

Harbormaster completed remote builds in B21870: Diff 50948.Apr 28 2022, 3:11 PM

Given that we don't even use OSL in USD right now

yeah but by that logic, we're using none of the imaging module right now, you want to disable the whole thing?

I'd like to see @Michael Kowalski (makowalski) chime in here and determine "is it useful to us and may we use it in the near future" lets not base it on if it's currently being used.

Also disable OIIO.

I've tested building with WITH_CYCLES=OFF, WITH_OPENIMAGEIO=OFF,
and WITH_USD=ON, and it works!

Harbormaster completed remote builds in B21871: Diff 50949.Apr 28 2022, 3:19 PM

Sybren A. Stüvel (sybren) planned changes to this revision.Apr 28 2022, 3:20 PM

Add set_and_warn_dependency() and remove external_openimageio dependency

Harbormaster completed remote builds in B21872: Diff 50950.Apr 28 2022, 3:22 PM

Sybren A. Stüvel (sybren) retitled this revision from CMake: Build USD without dependency of OSL or X11 to CMake: Reduce dependencies of USD to what's actually needed by Blender.Apr 28 2022, 3:25 PM

Sybren A. Stüvel (sybren) edited the summary of this revision. (Show Details)

Sybren A. Stüvel (sybren) added projects: BF Blender (3.2), USD, Platforms, Builds, Tests & Devices.

In D14792#397679, @Ray Molenkamp (LazyDodo) wrote:

yeah but by that logic, we're using none of the imaging module right now, you want to disable the whole thing?

I'd like to see @Michael Kowalski (makowalski) chime in here and determine "is it useful to us and may we use it in the near future" lets not base it on if it's currently being used.

We are using Hydra for Cycles standalone to read USD files now, so I would like to keep it enabled.

In D14792#397679, @Ray Molenkamp (LazyDodo) wrote:

yeah but by that logic, we're using none of the imaging module right now, you want to disable the whole thing?

I'd like to see @Michael Kowalski (makowalski) chime in here and determine "is it useful to us and may we use it in the near future" lets not base it on if it's currently being used.

I've updated the description to read:

However, since [the Imaging] module was included for its support for converting primitive shapes (sphere, cube, etc.) to geometry, OSL is not necessary.

Earlier it read "..., it's not necessary", where it was ambiguous what "it" referred to. Support for USD primitives is a concrete use case.

In D14792#397695, @Sybren A. Stüvel (sybren) wrote:

In D14792#397679, @Ray Molenkamp (LazyDodo) wrote:

yeah but by that logic, we're using none of the imaging module right now, you want to disable the whole thing?

I'd like to see @Michael Kowalski (makowalski) chime in here and determine "is it useful to us and may we use it in the near future" lets not base it on if it's currently being used.

I've updated the description to read:

However, since [the Imaging] module was included for its support for converting primitive shapes (sphere, cube, etc.) to geometry, OSL is not necessary.

Earlier it read "..., it's not necessary", where it was ambiguous what "it" referred to. Support for USD primitives is a concrete use case.