Conversation
c5dd789 to
195d216
Compare
|
Note that as the minimum required version is 2.8 I have to do some detection. |
|
Ah, nice. I have an open issue in the vcpkg regarding this: microsoft/vcpkg#20932 Does this PR provide a config that allows to choose what features to use? The vcpkg package had to patch the OSD project to handle this: microsoft/vcpkg#20871 |
It seems that Jack already took care of most of the features in microsoft/vcpkg#20895, is this what you are looking for ? Regarding OpenSubdiv vcpkg port, I plan to improve it with what is not yet supported (e.g. OpengGL backend) later after this PR. |
I was not aware of your issue but I feel this we aim for the same goal :) |
|
Hey @thomthom , I am working on microsoft/vcpkg#24467 which offers a CMake config in the current vcpkg port while we wait for this PR to move forward. |
|
Hi, would you submit a CLA Thanks! |
195d216 to
ee4d4e9
Compare
|
|
||
| endif() | ||
|
|
||
| # Expose preprocessor macro to the interface so that client know which feature are enabled |
There was a problem hiding this comment.
This portion is useful so that client consuming this any OpenSubdiv library can introspect which feature are enabled directly on the exported CMake targets.
For example, USD would benefit from this as they are checking if header exists and recompute those exact preprocessor macros because they are not exported
cf. https://github.com/PixarAnimationStudios/USD/blob/3b097e3ba8fabf1777a1256e241ea15df83f3065/cmake/modules/FindOpenSubdiv.cmake#L110-L117
67d7208 to
04ff84b
Compare
|
@davidgyu is there anything new on this PR ? |
|
Gently bumping this PR for any news @davidgyu ? |
|
Filed as internal issue #OSD-382 |
|
We have a few changes that we're going to be merging ahead of this PR and there may be some additional work (hopefully minor) that will be needed for this PR. We'll post additional notes here when the necessary changes are clear. Thanks! |
|
Thanks for the news David, I'll wait for your changes and adapt my PR when they arrived. |
|
This can be simplified since many of these are not primary artifacts and shouldn't be exported as config targets. Specifically, we should remove the changes in the examples, regression, tutorials, and opensubdiv/tools/stringify CMakeLists.txt files. Thanks! |
|
And there is a PR open to bump minimum CMake version, which sounds like it would aid in simplifying this PR. |
Downstream projects should not depend on these.
The cmake config export paths are now more consistent with other vfx platform projects. The target names are now more consistent with existing opensubdiv static and dynamic library names.
|
I've pushed some commits to remove the non-primary export targets and to update the config and target names to be more consistent with other packages in the vfx platform. |
|
Thank you @theblackunknown this is great to have! |
|
This is great ! |
I would love too as well, but I think it depends on the minimum CMake version Pixar - and anyone else consuming OpenSubdiv - requires which does not seems to be that recent ? |
Description
This PR adds a CMake Config to OpenSubdiv and expose preprocessor macros for OpenSubdiv enabled features for clients to depend on them.
This allow for a more robust and consistent way to pull
OpenSubdivas a dependency in other projects through CMakefind_packagecalls.Moreover, this enhance the experience when using package manager such as microsoft/vcpkg and alikes.