Skip to content

[opensubdiv] Add features#20895

Merged
dan-shaw merged 11 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/20871
Oct 28, 2021
Merged

[opensubdiv] Add features#20895
dan-shaw merged 11 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/20871

Conversation

@JackBoosY
Copy link
Copy Markdown
Contributor

@JackBoosY JackBoosY commented Oct 21, 2021 •
edited
Loading

Add features:

  • cuda
  • dx
  • examples
  • glew
  • glfw
  • omp
  • opencl
  • ptex
  • tbb
  • true-deriv-eval
  • tutorials

Fixes #20871.

Already tested core / all features on x86-windows, x64-windows-static, x64-linux and x64-osx.

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal labels Oct 21, 2021
github-actions[bot]
github-actions bot requested changes Oct 21, 2021
View reviewed changes
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated 
git add -u && git commit
git checkout 4474aba1e77be2d643eb684298b7d5479cad9a1f -- versions
./vcpkg x-add-version --all
Diff 
diff --git a/versions/o-/opensubdiv.json b/versions/o-/opensubdiv.json
index b8e49b0..fd69682 100644
--- a/versions/o-/opensubdiv.json
+++ b/versions/o-/opensubdiv.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "028271fea29de8189bc5fb7a048467dcfaea4d32",
+      "git-tree": "827dc996587bf5d4343bce5b5f11fafb49444a5b",
       "version-semver": "3.4.3",
       "port-version": 3
     },

JackBoosY
JackBoosY commented Oct 21, 2021
View reviewed changes
ports/opensubdiv/fix-dependencies.patch

+if (OPENCL_FOUND)
+ list(APPEND PLATFORM_LIBRARIES
+ Opengl32
Copy link
Copy Markdown
Contributor Author

@JackBoosY JackBoosY Oct 21, 2021

Choose a reason for hiding this comment

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

It's strange here, opencl should not use the opengl function when the feature opengl is closed, but it is included in the upstream code.

@JackBoosY JackBoosY marked this pull request as ready for review October 21, 2021 09:48
@JackBoosY
Copy link
Copy Markdown
Contributor Author

JackBoosY commented Oct 21, 2021

@thomthom Please test these features.

Thanks.

@JackBoosY JackBoosY requested a review from NancyLi1013 October 21, 2021 10:11
@thomthom
Copy link
Copy Markdown

thomthom commented Oct 21, 2021

Wow - that was quick response!

I'm trying this out now, but I'm running into some issues. (Which is probably related to now new I am to vcpkg)

It appear that vcpkg builds OSD, but I'm having trouble with find_package in my test project:

[cmake] CMake Error at vcpkg/scripts/buildsystems/vcpkg.cmake:788 (_find_package):
[cmake]   By not providing "Findopensubdiv.cmake" in CMAKE_MODULE_PATH this project
[cmake]   has asked CMake to find a package configuration file provided by
[cmake]   "opensubdiv", but CMake did not find one.
[cmake] 
[cmake]   Could not find a package configuration file provided by "opensubdiv" with
[cmake]   any of the following names:
[cmake] 
[cmake]     opensubdivConfig.cmake
[cmake]     opensubdiv-config.cmake
[cmake] 
[cmake]   Add the installation prefix of "opensubdiv" to CMAKE_PREFIX_PATH or set
[cmake]   "opensubdiv_DIR" to a directory containing one of the above files.  If
[cmake]   "opensubdiv" provides a separate development package or SDK, be sure it has
[cmake]   been installed.
[cmake] Call Stack (most recent call first):
[cmake]   CMakeLists.txt:40 (find_package)

I've pushed the test project to GitHub: https://github.com/thomthom/osd-vcpkg-test

My vcpkg.json file: https://github.com/thomthom/osd-vcpkg-test/blob/main/vcpkg.json

{
  "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json",
  "name": "osd-vcpkg-test",
  "version": "0.1.0",
  "dependencies": [
    {
      "name": "opensubdiv",
      "default-features": false,
      "features": []
    }
  ]
}

My CMakeLists.txt: https://github.com/thomthom/osd-vcpkg-test/blob/main/CMakeLists.txt

set(CMAKE_TOOLCHAIN_FILE ${CMAKE_CURRENT_SOURCE_DIR}/vcpkg/scripts/buildsystems/vcpkg.cmake
  CACHE STRING "Vcpkg toolchain file")

project(OSDVCPKGTest
  VERSION 1.0.0
  DESCRIPTION "OpenSubdiv vcpkg Test"
  LANGUAGES C CXX
)

find_package(opensubdiv REQUIRED)

add_executable(testapp main.cpp)
target_link_libraries(testapp PUBLIC osd_static_cpu)

How do I figure out the name to use along with find_package?

@thomthom
Copy link
Copy Markdown

thomthom commented Oct 21, 2021

Oh, this package doesn't have a CMake package defined? I compared to another project where I use vcpkg and I see that all those packages have share/*-config.cmake files. This means I need to provide my own FindModule for OpenSubdiv? (Previously, before vcpkg, I used OSD as a submodule and used add_subdirectory and thus no need for a find module.)

@thomthom
Copy link
Copy Markdown

thomthom commented Oct 21, 2021

I created a minimal find module: https://github.com/thomthom/osd-vcpkg-test/tree/dev/find-module

Tested on Windows 10 and macOS, in both cases they built minimal OpenSubdiv with CPU only library.

For CMake convenience, could this post be also provide a CMake config file? Should I log a new issue for that? Or is that out of the scope for this repo?

@JackBoosY
Copy link
Copy Markdown
Contributor Author

JackBoosY commented Oct 22, 2021 •
edited
Loading

@thomthom That's out of scope of this PR, please open a new issue to ask for it.
And I will add this in the next PR.

Related issue: #20932.

@NancyLi1013 NancyLi1013 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 22, 2021
@JackBoosY JackBoosY removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 22, 2021
@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 25, 2021
@dan-shaw dan-shaw merged commit 7c10f98 into microsoft:master Oct 28, 2021
@JackBoosY JackBoosY deleted the dev/jack/20871 branch October 28, 2021 06:08
@theblackunknown theblackunknown mentioned this pull request Apr 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

+1 more reviewer

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@NancyLi1013 NancyLi1013

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[opensubdiv] Expose build options via vcpkg package

4 participants

@JackBoosY @thomthom @NancyLi1013 @dan-shaw