Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new CMakeLists.txt to provide PhysXConfig.cmake #222

Open
wants to merge 17 commits into
base: 4.1
Choose a base branch
from

Conversation

phcerdan
Copy link

This allows to find_package(PhysX) and facilitate integration
in third party CMake projects.

  • Provide PhysXConfig.cmake and exported targets file for build and install tree.
  • Other projects can use find_package(PhysX) where PhysX_DIR can be a build tree or an install tree.
  • The implementation only adds two new CMakeLists.txt that do not collide with
    the existing build procedure of Nvidia. Instead of using python and the generate_projects scripts, now it solely uses CMake in a standard way.
  • This allows PhysX to be used with modern standards of CMake, making it compatible
    with FetchContent (add_subdirectory) and ExternalProject (find_package) commands.
  • Snippets and Samples have not been integrated into the new build procedure.
  • But added a simpler project to show find_package(PhysX) usage.
  • The original build procedure is maintained compatible for easier integration with future upstream changes from NVidia.

How to build:

mkdir PhysX;
git clone https://github.com/phcerdan/PhysX src
mkdir build; cd build;
cmake -GNinja -DCMAKE_BUILD_TYPE:STRING=release -DCMAKE_INSTALL_PREFIX:PATH=/tmp/physx ../src
ninja install

Example project using PhysX (example added)

find_package(PhysX REQUIRED)
add_executable(main main.cpp)
target_link_libraries(main PRIVATE PhysX::PhysXCommon PhysX::PhysXExtensions)

You can also use FetchContent, or ExternalProjects for handling PhysX automatically.

When building your project, just provide PhysX_DIR to where the PhysXConfig.cmake is located (it could be from a build or an install tree)

cmake -GNinja -DCMAKE_BUILD_TYPE:STRING=RelWithDebInfo -DPhysX_DIR:PATH=/tmp/physx/PhysX/bin/cmake/physx ../src

Fix #208

The idea is to make integrating this project independent of python
and the pretty Nvidia specific `generate_project.py`.

The final goal is for other projects to FetchContent and add_subdirectory
or use ExternalProject in CMake and FindPhysX as any other standard
CMake project. But right now it only focus on building and installing
without needing python and with sane defaults with tweakable options.

The user just needs to run `CMake` in a `build-folder` not inside the
source tree (as usual for CMake projects), the top folder of the CMake
tree is `physx`.

This commit does not deal with the snippets or samples projects.

This commits only deals with new CMakeLists files added (heavily relying
on existing Nvidia CMake files).
The next commit modifies existing CMake files.
The split is just for clarity for external review.
Write version and install targets for build and install tree.

Also fix:
- Set CMAKE_BUILD_TYPE to `release` by default, and only allow the build
types defined in CMAKE_CONFIGURATION_TYPES: release, profile, checked, debug
The header typeinfo.h was removed in msvc 16.3
https://docs.microsoft.com/en-us/cpp/overview/cpp-conformance-improvements?view=vs-2019#standard-library-improvements-1

<typeinfo> is standardized since forever, expect compatibility with old compilers.
Tested with project using:
`find_package(PhysX REQUIRED)`

for the build and install tree
WIP: Should be renamed/reallocated
@phcerdan
Copy link
Author

If anybody wants to use CMake FetchContent or external projects, I have set the default branch in my fork to be this PR: https://github.com/phcerdan/PhysX

Copy link

@diegoferigo diegoferigo left a comment

Choose a reason for hiding this comment

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

@phcerdan thanks for taking care of this, amazing work! I still have to test this PR, I just had a quick look and I left some minor comments. I will let know if I have any problem as soon as I test it.

The only thing I haven't understood (even before this PR) is the libPhysXGPU status, and I'm not a PhysX expert since I still have to start using it. If I got it right, the only way to have a proper GPU / CUDA support is using the closed-source libraries? When I first tried to compile the project, I think that that library was compiled. Can you please provide more details about it?

message(STATUS "Setting build type to '${default_build_type}' as none was specified.")
set(CMAKE_BUILD_TYPE ${default_build_type} CACHE STRING "Choose the type of build.")
set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
"release" "profile" "checked" "debug")

Choose a reason for hiding this comment

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

I'm not aware of what "checked" and ''profile" are. If they are not standard, is there any documentation?

I would also add RelWithDebInfo and MinSizeRel https://cmake.org/cmake/help/v3.0/variable/CMAKE_BUILD_TYPE.html.

Copy link
Author

Choose a reason for hiding this comment

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

The only thing I haven't understood (even before this PR) is the libPhysXGPU status, and I'm not a PhysX expert since I still have to start using it. If I got it right, the only way to have a proper GPU / CUDA support is using the closed-source libraries? When I first tried to compile the project, I think that that library was compiled. Can you please provide more details about it?

I am not from NVidia, but from what I understand yes, libPhysXGpu (PhysXDevice in Windows...) are indeed close source and distributed by Nvidia in a binary form with each update of this repository. Only these GPU kernels are closed source, the rest is open source and is compiled when you build the project.

Following up, these libraries are distributed for the 4 non-standard configurations of Nvidia: release, profile, checked, debug. Each one having their own compiler flags.
Your request of adding the standard RelWithDebInfo etc, will have the problem of not having a matching GPU library with the same configuration. So, I would discard it for now.

Choose a reason for hiding this comment

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

Your comment makes totally sense, thanks for the clarification.

set(default_build_type "Release")
if(EXISTS "${CMAKE_SOURCE_DIR}/.git")
set(default_build_type "Debug")
endif()

Choose a reason for hiding this comment

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

I suspect that this check could get a strange configuration in the case the project is gathered with FetchContent. Projects should inherit these properties from the caller, and I think that this would override the user's downstream selection.

Copy link
Author

Choose a reason for hiding this comment

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

Uhh thanks, this is a left over, I would remove it completely. The right handling of the default build type is a few lines above:

# Change options for CMAKE_BUILD_TYPE and set default.
# Same values than CMAKE_CONFIGURATION_TYPES
# From https://gitlab.kitware.com/cmake/cmake/issues/19401
get_property(multiConfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
if(NOT multiConfig AND NOT DEFINED CMAKE_BUILD_TYPE)
    set(default_build_type "release")
    message(STATUS "Setting build type to '${default_build_type}' as none was specified.")
    set(CMAKE_BUILD_TYPE ${default_build_type} CACHE STRING "Choose the type of build.")
    set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
        "release" "profile" "checked" "debug")
endif()

Comment on lines +63 to +73
if(NOT DEFINED BUILD_SHARED_LIBS)
if(PX_GENERATE_STATIC_LIBRARIES)
set(BUILD_SHARED_LIBS OFF)
else()
set(BUILD_SHARED_LIBS ON)
endif()
else()
if(BUILD_SHARED_LIBS EQUAL PX_GENERATE_STATIC_LIBRARIES)
message(FATAL_ERROR "Contradictory options: BUILD_SHARED_LIBS and PX_GENERATE_STATIC_LIBRARIES have both the same value: ${BUILD_SHARED_LIBS}")
endif()
endif()

Choose a reason for hiding this comment

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

What about adding this variable in CACHE so that downstream users can easily switch them with ccmake or cmake gui?

Copy link
Author

Choose a reason for hiding this comment

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

Users can do that with the PX_GENERATE_STATIC_LIBRARIES option (stored in the CACHE already). I would prefer to keep changes from current NVidia setup at a minimum to facilitate future integration.

Choose a reason for hiding this comment

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

Ow that's right, I missed it!

@diegoferigo
Copy link

Another thing, is PhysX_DIR required also to find the project from the install tree? If yes, probable the most idiomatic way to handle this case would be adding the install prefix to CMAKE_INSTALL_PREFIX.

Copy link
Author

@phcerdan phcerdan left a comment

Choose a reason for hiding this comment

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

Thanks for the review, really useful, here some comments.

@phcerdan
Copy link
Author

phcerdan commented Nov 13, 2019

Another thing, is PhysX_DIR required also to find the project from the install tree? If yes, probable the most idiomatic way to handle this case would be adding the install prefix to CMAKE_INSTALL_PREFIX.

I don't think that's the CMake idiomatic way. PhysX_DIR is set by the consumer to point wherever PhysXConfig.cmake is, in the build or install tree.

  • In the install tree is in: PhysX/bin/cmake/physx/PhysXConfig.cmake.
    Having the install config file in the bin/cmake (or lib/cmake) folder is idiomatic in CMake.

  • In the build folder is in the top source folder named: sdk_source_bin (name is maintained from upstream).

The right handling is a few lines above:
```cmake
get_property(multiConfig GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
if(NOT multiConfig AND NOT DEFINED CMAKE_BUILD_TYPE)
    set(default_build_type "release")
    message(STATUS "Setting build type to '${default_build_type}' as none was specified.")
    set(CMAKE_BUILD_TYPE ${default_build_type} CACHE STRING "Choose the type of build.")
    set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
        "release" "profile" "checked" "debug")
endif()
```

Pointed by @diegoferigo under a review.
Allowing FetchContent (add_subdirectory) to work.
Use: `if(DEFINED var)` instead of `if(DEFINED ${var})`
Copy link

@diegoferigo diegoferigo left a comment

Choose a reason for hiding this comment

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

I just tried your PR. I left few more comments after a quick hands on.

You can find hereafter the installed tree in debug configuration. Now I understand your comment about the CMAKE_INSTALL_PREFIX, I originally thought that the install tree was not prefixed by the PhysX|PxShared directories. Is there any reason to do it? I can understand the need of using the PhysX_DIR when loading the targets from the build tree, not much instead from the install tree.

The libraries are installed in <install_prefix>/PhysX/bin/<build_mode>, which is a bit odd. Is there any rationale of not using <install_prefix>/${CMAKE_INSTALL_LIBDIR}? (requires GNUInstallDirs)

Similarly, the headers get installed in <install_prefix>/PhysX/include. It makes sense to scope the installed headers in a subfolder, though I think is more common using <install_prefix>/${CMAKE_INSTALL_INCLUDEDIR}/PhysX.

CMake install output
-- Install configuration: "debug"
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/PsUnixAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/PsUnixFPU.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/PsUnixInlineAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/PsUnixIntrinsics.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/PsUnixTrigConstants.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/neon/PsUnixNeonAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/neon/PsUnixNeonInlineAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/sse2/PsUnixSse2AoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/unix/sse2/PsUnixSse2InlineAoS.h
-- Installing: /tmp/physx/PxShared/include/foundation/unix/PxUnixIntrinsics.h
-- Installing: /tmp/physx/PhysX/include/PxFoundation.h
-- Installing: /tmp/physx/PhysX/include/foundation/PxAssert.h
-- Installing: /tmp/physx/PhysX/include/foundation/PxFoundationConfig.h
-- Installing: /tmp/physx/PhysX/include/foundation/PxMathUtils.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/Ps.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsAlignedMalloc.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsAlloca.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsAllocator.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsArray.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsAtomic.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsBasicTemplates.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsBitUtils.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsBroadcast.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsCpu.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsFoundation.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsFPU.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsHash.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsHashInternals.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsHashMap.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsHashSet.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsInlineAllocator.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsInlineAoS.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsInlineArray.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsIntrinsics.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsMathUtils.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsMutex.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsPool.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsSList.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsSocket.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsSort.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsSortInternals.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsString.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsSync.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsTempAllocator.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsThread.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsTime.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsUserAllocated.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsUtilities.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecMath.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecMathAoSScalar.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecMathAoSScalarInline.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecMathSSE.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecMathUtilities.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecQuat.h
-- Installing: /tmp/physx/PhysX/source/foundation/include/PsVecTransform.h
-- Installing: /tmp/physx/PxShared/include/foundation/Px.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxAllocatorCallback.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxProfiler.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxSharedAssert.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxBitAndData.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxBounds3.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxErrorCallback.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxErrors.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxFlags.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxIntrinsics.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxIO.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxMat33.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxMat44.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxMath.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxMemory.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxPlane.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxPreprocessor.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxQuat.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxSimpleTypes.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxStrideIterator.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxTransform.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxUnionCast.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxVec2.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxVec3.h
-- Installing: /tmp/physx/PxShared/include/foundation/PxVec4.h
-- Installing: /tmp/physx/PhysX/include/gpu/PxGpu.h
-- Installing: /tmp/physx/PhysX/include/cudamanager/PxCudaContextManager.h
-- Installing: /tmp/physx/PhysX/include/cudamanager/PxCudaMemoryManager.h
-- Installing: /tmp/physx/PhysX/include/PxActor.h
-- Installing: /tmp/physx/PhysX/include/PxAggregate.h
-- Installing: /tmp/physx/PhysX/include/PxArticulationReducedCoordinate.h
-- Installing: /tmp/physx/PhysX/include/PxArticulationBase.h
-- Installing: /tmp/physx/PhysX/include/PxArticulation.h
-- Installing: /tmp/physx/PhysX/include/PxArticulationJoint.h
-- Installing: /tmp/physx/PhysX/include/PxArticulationJointReducedCoordinate.h
-- Installing: /tmp/physx/PhysX/include/PxArticulationLink.h
-- Installing: /tmp/physx/PhysX/include/PxBatchQuery.h
-- Installing: /tmp/physx/PhysX/include/PxBatchQueryDesc.h
-- Installing: /tmp/physx/PhysX/include/PxBroadPhase.h
-- Installing: /tmp/physx/PhysX/include/PxClient.h
-- Installing: /tmp/physx/PhysX/include/PxConstraint.h
-- Installing: /tmp/physx/PhysX/include/PxConstraintDesc.h
-- Installing: /tmp/physx/PhysX/include/PxContact.h
-- Installing: /tmp/physx/PhysX/include/PxContactModifyCallback.h
-- Installing: /tmp/physx/PhysX/include/PxDeletionListener.h
-- Installing: /tmp/physx/PhysX/include/PxFiltering.h
-- Installing: /tmp/physx/PhysX/include/PxForceMode.h
-- Installing: /tmp/physx/PhysX/include/PxImmediateMode.h
-- Installing: /tmp/physx/PhysX/include/PxLockedData.h
-- Installing: /tmp/physx/PhysX/include/PxMaterial.h
-- Installing: /tmp/physx/PhysX/include/PxPhysics.h
-- Installing: /tmp/physx/PhysX/include/PxPhysicsAPI.h
-- Installing: /tmp/physx/PhysX/include/PxPhysicsSerialization.h
-- Installing: /tmp/physx/PhysX/include/PxPhysicsVersion.h
-- Installing: /tmp/physx/PhysX/include/PxPhysXConfig.h
-- Installing: /tmp/physx/PhysX/include/PxPruningStructure.h
-- Installing: /tmp/physx/PhysX/include/PxQueryFiltering.h
-- Installing: /tmp/physx/PhysX/include/PxQueryReport.h
-- Installing: /tmp/physx/PhysX/include/PxRigidActor.h
-- Installing: /tmp/physx/PhysX/include/PxRigidBody.h
-- Installing: /tmp/physx/PhysX/include/PxRigidDynamic.h
-- Installing: /tmp/physx/PhysX/include/PxRigidStatic.h
-- Installing: /tmp/physx/PhysX/include/PxScene.h
-- Installing: /tmp/physx/PhysX/include/PxSceneDesc.h
-- Installing: /tmp/physx/PhysX/include/PxSceneLock.h
-- Installing: /tmp/physx/PhysX/include/PxShape.h
-- Installing: /tmp/physx/PhysX/include/PxSimulationEventCallback.h
-- Installing: /tmp/physx/PhysX/include/PxSimulationStatistics.h
-- Installing: /tmp/physx/PhysX/include/PxVisualizationParameter.h
-- Installing: /tmp/physx/PhysX/include/common/PxBase.h
-- Installing: /tmp/physx/PhysX/include/common/PxCollection.h
-- Installing: /tmp/physx/PhysX/include/common/PxCoreUtilityTypes.h
-- Installing: /tmp/physx/PhysX/include/common/PxMetaData.h
-- Installing: /tmp/physx/PhysX/include/common/PxMetaDataFlags.h
-- Installing: /tmp/physx/PhysX/include/common/PxPhysicsInsertionCallback.h
-- Installing: /tmp/physx/PhysX/include/common/PxPhysXCommonConfig.h
-- Installing: /tmp/physx/PhysX/include/common/PxRenderBuffer.h
-- Installing: /tmp/physx/PhysX/include/common/PxSerialFramework.h
-- Installing: /tmp/physx/PhysX/include/common/PxSerializer.h
-- Installing: /tmp/physx/PhysX/include/common/PxStringTable.h
-- Installing: /tmp/physx/PhysX/include/common/PxTolerancesScale.h
-- Installing: /tmp/physx/PhysX/include/common/PxTypeInfo.h
-- Installing: /tmp/physx/PhysX/include/common/PxProfileZone.h
-- Installing: /tmp/physx/PhysX/include/pvd/PxPvdSceneClient.h
-- Installing: /tmp/physx/PhysX/include/pvd/PxPvd.h
-- Installing: /tmp/physx/PhysX/include/pvd/PxPvdTransport.h
-- Installing: /tmp/physx/PhysX/include/collision/PxCollisionDefs.h
-- Installing: /tmp/physx/PhysX/include/solver/PxSolverDefs.h
-- Installing: /tmp/physx/PhysX/include/PxConfig.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxBoxController.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxCapsuleController.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxController.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxControllerBehavior.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxControllerManager.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxControllerObstacles.h
-- Installing: /tmp/physx/PhysX/include/characterkinematic/PxExtended.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxBoxGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxCapsuleGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxConvexMesh.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxConvexMeshGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxGeometryHelpers.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxGeometryQuery.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxHeightField.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxHeightFieldDesc.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxHeightFieldFlag.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxHeightFieldGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxHeightFieldSample.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxMeshQuery.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxMeshScale.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxPlaneGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxSimpleTriangleMesh.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxSphereGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxTriangle.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxTriangleMesh.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxTriangleMeshGeometry.h
-- Installing: /tmp/physx/PhysX/include/geometry/PxBVHStructure.h
-- Installing: /tmp/physx/PhysX/include/geomutils/GuContactBuffer.h
-- Installing: /tmp/physx/PhysX/include/geomutils/GuContactPoint.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxBVH33MidphaseDesc.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxBVH34MidphaseDesc.h
-- Installing: /tmp/physx/PhysX/include/cooking/Pxc.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxConvexMeshDesc.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxCooking.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxMidphaseDesc.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxTriangleMeshDesc.h
-- Installing: /tmp/physx/PhysX/include/cooking/PxBVHStructureDesc.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxBinaryConverter.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxBroadPhaseExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxCollectionExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxConstraintExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxContactJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxConvexMeshExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxD6Joint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxD6JointCreate.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDefaultAllocator.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDefaultCpuDispatcher.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDefaultErrorCallback.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDefaultSimulationFilterShader.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDefaultStreams.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxDistanceJoint.h
-- Up-to-date: /tmp/physx/PhysX/include/extensions/PxContactJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxExtensionsAPI.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxFixedJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxJointLimit.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxMassProperties.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxPrismaticJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRaycastCCD.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRepXSerializer.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRepXSimpleType.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRevoluteJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRigidActorExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxRigidBodyExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxSceneQueryExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxSerialization.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxShapeExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxSimpleFactory.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxSmoothNormals.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxSphericalJoint.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxStringTableExt.h
-- Installing: /tmp/physx/PhysX/include/extensions/PxTriangleMeshExt.h
-- Installing: /tmp/physx/PhysX/include/filebuf/PxFileBuf.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleComponents.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleDrive.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleDrive4W.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleDriveNW.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleDriveTank.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleNoDrive.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleSDK.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleShaders.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleTireFriction.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleUpdate.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleUtil.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleUtilControl.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleUtilSetup.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleUtilTelemetry.h
-- Installing: /tmp/physx/PhysX/include/vehicle/PxVehicleWheels.h
-- Installing: /tmp/physx/PhysX/source/fastxml/include/PsFastXml.h
-- Installing: /tmp/physx/PhysX/include/task/PxCpuDispatcher.h
-- Installing: /tmp/physx/PhysX/include/task/PxTask.h
-- Installing: /tmp/physx/PhysX/include/task/PxTaskDefine.h
-- Installing: /tmp/physx/PhysX/include/task/PxTaskManager.h
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXFoundation.so
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysX.so
-- Set runtime path of "/tmp/physx/PhysX/bin/debug/libPhysX.so" to ""
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXCharacterKinematic_static.a
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXPvdSDK_static.a
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXCommon.so
-- Set runtime path of "/tmp/physx/PhysX/bin/debug/libPhysXCommon.so" to ""
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXCooking.so
-- Set runtime path of "/tmp/physx/PhysX/bin/debug/libPhysXCooking.so" to ""
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXExtensions_static.a
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXVehicle_static.a
-- Up-to-date: /tmp/physx/PhysX/bin/debug
-- Installing: /tmp/physx/PhysX/bin/debug/libPhysXGpu_64.so
-- Installing: /tmp/physx/PhysX/bin/cmake/physx/PhysXTargets.cmake
-- Installing: /tmp/physx/PhysX/bin/cmake/physx/PhysXTargets-debug.cmake
-- Installing: /tmp/physx/PhysX/bin/cmake/physx/PhysXConfig.cmake
-- Installing: /tmp/physx/PhysX/bin/cmake/physx/PhysXConfigVersion.cmake

Comment on lines +63 to +73
if(NOT DEFINED BUILD_SHARED_LIBS)
if(PX_GENERATE_STATIC_LIBRARIES)
set(BUILD_SHARED_LIBS OFF)
else()
set(BUILD_SHARED_LIBS ON)
endif()
else()
if(BUILD_SHARED_LIBS EQUAL PX_GENERATE_STATIC_LIBRARIES)
message(FATAL_ERROR "Contradictory options: BUILD_SHARED_LIBS and PX_GENERATE_STATIC_LIBRARIES have both the same value: ${BUILD_SHARED_LIBS}")
endif()
endif()

Choose a reason for hiding this comment

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

Ow that's right, I missed it!

message(STATUS "Setting build type to '${default_build_type}' as none was specified.")
set(CMAKE_BUILD_TYPE ${default_build_type} CACHE STRING "Choose the type of build.")
set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
"release" "profile" "checked" "debug")

Choose a reason for hiding this comment

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

Your comment makes totally sense, thanks for the clarification.

COMMENT "Copying libPhysXGPU from the appropiate source folder to the build directory.")
# Install tree
install(FILES ${PROJECT_SOURCE_DIR}/bin/${PLATFORM_BIN_NAME}/$<CONFIG>
DESTINATION ${PHYSX_INSTALL_PREFIX}/bin/)

Choose a reason for hiding this comment

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

I am typically used to pass upper case build modes to CMake, e.g. -DCMAKE_BUILD_TYPE=Debug. Here you are using this variable as the user specifies it, and CMake in this case is not case sensitive. The folders to be copied are all lower case, maybe converting the variable to lower case before using it would be more robust?

@diegoferigo
Copy link

Another thing, is PhysX_DIR required also to find the project from the install tree? If yes, probable the most idiomatic way to handle this case would be adding the install prefix to CMAKE_INSTALL_PREFIX.

I don't think that's the CMake idiomatic way. PhysX_DIR is set by the consumer to point wherever PhysXConfig.cmake is, in the build or install tree.
- In the install tree is in: PhysX/bin/cmake/physx/PhysXConfig.cmake. Having the install config file in the bin/cmake (or lib/cmake) folder is idiomatic in CMake.
- In the build folder is in the top source folder named: sdk_source_bin (name is maintained from upstream).

Sorry, I didn't notice that the installed tree had the additional PhysX folder. Without this scope, my comment would apply and as you said, it would be the most idiomatic way to handle install trees.

Configuring and building PhysX at configure time from another project.
Copy link

@alanjfs alanjfs left a comment

Choose a reason for hiding this comment

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

This works well for me when building standalone, but I haven't yet been able to use add_subdirectory(Physx/physx).

When I do, I get the typical CMake warning..

...
-- PhysX VERSION: 4.1.1.27006925
-- BUILD_SHARED_LIBS: OFF
--   PX_GENERATE_STATIC_LIBRARIES: ON
-- PhysX Build Platform: windows
--   CXX Compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.27.29110/bin/Hostx64/x64/cl.exe
-- PLATFORM_BIN_NAME: win.x86_64.vc142.mt
-- PLATFORM_CMAKELISTS: C:/myproject/PhysX/physx/source/compiler/cmake/windows/CMakeLists.txt
CMake Error at src/CMakeLists.txt:15 (find_package):
  By not providing "FindPhysX.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "PhysX", but
  CMake did not find one.

  Could not find a package configuration file provided by "PhysX" with any of
  the following names:

    PhysXConfig.cmake
    physx-config.cmake

  Add the installation prefix of "PhysX" to CMAKE_PREFIX_PATH or set
  "PhysX_DIR" to a directory containing one of the above files.  If "PhysX"
  provides a separate development package or SDK, be sure it has been
  installed.


-- Configuring incomplete, errors occurred!

With a CMakeLists.txt file that looks something like this.

...
add_subdirectory(external/PhysX/physx EXCLUDE_FROM_ALL)
find_package(PhysX REQUIRED)
...
target_link_libraries(
    MyProject
    PRIVATE
        PhysX
)

Any idea what I might be doing wrong?

README.md Outdated
Comment on lines 18 to 22
mkdir PhysX;
git clone https://github.com/phcerdan/PhysX src
mkdir build; cd build;
cmake -GNinja -DCMAKE_BUILD_TYPE:STRING=release -DCMAKE_INSTALL_PREFIX:PATH=/tmp/physx ../src
ninja install
Copy link

Choose a reason for hiding this comment

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

Took this for a spin on Windows VS2019 but ran into a few issues.

  1. mkdir PhysX; doesn't seem necessary
  2. The source directory should be ../src/physx instead of ..src/ as that's where the CMakeLists.txt file is at

Other than that, this worked wonders!

Copy link
Author

Choose a reason for hiding this comment

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

You cannot mix add_subdirectory and find_package like that.
When you add_subdirectory, you are building PhysX, so no need to find anything, you already have the PhysX target in scope.

Try to remove find_package.

Thanks for pointing it out, there is a missing cd PhysX in the Readme.

It should be:

mkdir PhysX; cd PhysX
git clone https://github.com/phcerdan/PhysX src
mkdir build; cd build;
cmake -GNinja -DCMAKE_BUILD_TYPE:STRING=release -DCMAKE_INSTALL_PREFIX:PATH=/tmp/physx ../src
ninja install

Copy link

Choose a reason for hiding this comment

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

Thanks, I'm quite new at CMake so this doesn't surprise me.

However, I did try that and found myself facing this message:

CMake Error at src/CMakeLists.txt:23 (add_executable):
  Target "MyProject" links to target "PhysX::PhysXCommon" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?


CMake Error at src/CMakeLists.txt:23 (add_executable):
  Target "MyProject" links to target "PhysX::PhysXExtensions" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?

And wasn't able to figure out where ::PhysXCommon comes from. Removing those namespaces, and just calling..

target_link_libraries(
    MyProject
    PRIVATE
      PhysX
)

Got me past those errors, but then onto a..

C:\myproject\src\application.h(3): fatal error C1083: Cannot open include file: 'PxPhysicsAPI.h': No such file or directory
[414/420] Building CXX object build\win32\Release\lib\sdk_source_bin\CMakeFiles\SimulationController.dir\simulationcontroller\src\ScScene.cpp.obj

Given a mere..

#include "PxPhysicsAPI.h"

Which leads me to believe the include directory isn't being included, and sure enough, looking at the generated Ninja build file, I can see that it's missing from the main.cpp file that triggers this error.

Anything else I'm missing?

Copy link
Author

Choose a reason for hiding this comment

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

You have PxPhysicsAPI.h included in a header, have you tried linking publicly to PhysX?

target_link_libraries(
    MyProject
    PUBLIC
      PhysX
)

Copy link

Choose a reason for hiding this comment

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

I hadn't, but tried it just now. No change. :(

Anyway I can "force" it? I tried this...

target_include_directories(MyProject
    PRIVATE
        ${PROJECT_SOURCE_DIR}/external/PhysX/physx/include
)

..which brought new fire and fury.

..\..\external\PhysX\physx\include\PxPhysicsAPI.h(45): fatal error C1083: Cannot open include file: 'foundation/Px.h': No such file or directory

Copy link
Author

Choose a reason for hiding this comment

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

I forgot, you have to link to specific targets you want:

Try:

target_link_libraries(
    MyProject
    PRIVATE
      PhysX::PhysXCommon
)

Have a look at the sample project in this PR:
https://github.com/phcerdan/PhysX/blob/9789a7a8083d4a0a2e035adf53f8e7d0b14ad40b/project_using_physx/CMakeLists.txt

Copy link

Choose a reason for hiding this comment

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

Thanks, but I tried that earlier (see above) and it gets me this error here:

CMake Error at src/CMakeLists.txt:18 (add_executable):
  Target "MyProject" links to target "PhysX::PhysXCommon" but the target
  was not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?

It looks like those targets are only added with find_package? :S

I've (temporarily) made my (minimal) project public here, if you would happen to be able to have a look:

This was a tricky one!

@phcerdan
Copy link
Author

phcerdan commented Aug 7, 2020

The alias is missing then: try without the PhysX:: namespace? Sorry, I am away from the computer to check it properly.

EDIT: Your test project is untestable in Linux.

@alanjfs
Copy link

alanjfs commented Aug 8, 2020

The alias is missing then: try without the PhysX:: namespace?

That was it! Thanks for sticking with me on this one, this is great. :D

Moving forwards, is this an error in the docs, or the CMake files in this PR? I still can't figure out where the namespace is meant to come from, or even these names.

@alanjfs
Copy link

alanjfs commented Aug 8, 2020

There was one thing I ran into that maybe could be handled by this PR.

cmake .. -DCMAKE_BUILD_TYPE=Release
...
Error copying directory /PhysX/physx/bin/linux.clang/Release to /some/install/path

Presumably because I passed Release rather than release.

cmake .. -DCMAKE_BUILD_TYPE=release
...
Success

Bit of a tricky one to debug, and maybe not something handled here?

@alanjfs
Copy link

alanjfs commented Aug 8, 2020

Chatting with the author of another project providing a CMake module, he says missing namespaces is a common CMake-thing, and pointed at his workaround:

add_library(Magnum::MeshTools ALIAS MagnumMeshTools)

https://github.com/mosra/magnum/blob/bd44e2ab7cc0a00a4ad3d4fc0f7bcf24cd05c697/src/Magnum/MeshTools/CMakeLists.txt#L160-L161

I figure, it's either that, or update the documentation to not include the namespace?

This allow to refer to the target with the same name
`PhysX::PhysXCommon` when using either `add_subdirectory` or
`find_package`
@phcerdan
Copy link
Author

phcerdan commented Aug 8, 2020

I have added an alias for all the targets with the namespace PhysX:: on it.

@phcerdan
Copy link
Author

phcerdan commented Aug 8, 2020

Presumably because I passed Release rather than release.

I have commented on this up in this discussion: #222 (comment)

@alanjfs
Copy link

alanjfs commented Aug 9, 2020

I have commented on this up in this discussion: #222 (comment)

Thanks, that makes sense.

I wonder, would it not be possible to convert whatever was passed by the user to lowercase, such that at least Release and Debug would work as expected? It looks like the failure happens during a CMake copy.

@alanjfs
Copy link

alanjfs commented Aug 24, 2020

I'm back! :)

Regarding the debug, profile, checked, and release build configurations of PhysX, I can't figure out how to "map" PhysX's checked configuration to my CMake project's Release configuration.

From what I gather, the only way to influence how PhysX is build when add_subdirectory(dir/to/physx) is to set CMAKE_BUILD_TYPE=checked . That would get PhysX building, but then break the parent project which doesn't know about checked.

What I'd like to do is..

if(CMAKE_BUILD_TYPE STREQUAL "Release")
  set(PHYSX_BUILD_TYPE "checked")
else()
  set(PHYSX_BUILD_TYPE "profile")
endif()

Is that possible?

@phcerdan
Copy link
Author

@alanjfs I would love to, I asked a few months ago this question in the CMake discourse, but it wasn't clear to me. https://discourse.cmake.org/t/mapping-cmake-build-type-and-cmake-configuration-types-between-project-subdirectories/192

I hacked a way to restore the original CMAKE_BUILD_TYPE for your add_subdirectory case, please test.
Please remember that this is somehow of a quimera to bring CMake here. There are old CMakeLists.txt files (probably contributed by a third-party), that I am guessing are untouchable, and the new CMakeLists to bring all the goodies to find_package(physx) and easier interaction with the C++ ecosystem.

A complete new rewrite of all the CMakeLists.txt files would be way easier than the current mixing between new and old CMakeLists.txt. But, hey, let's try. No word of any maintainer, for good or bad so far.

I would be happy to modernize all the CMakeLists.txt, but I have no idea about NVIDIA constraints about not breaking code with 3rd parties. This PR tries to not break anything and bring utilities, but for example, the non-standard CMAKE_BUILD_TYPES are a pain.

Please try the last commit that tries to hack on it. Remember that using find_package should work (instead of add_subdirectory in your project).

@phcerdan phcerdan force-pushed the cmake_for_easier_integration branch from 1687e56 to 1db08c8 Compare August 24, 2020 14:56
@alanjfs
Copy link

alanjfs commented Aug 25, 2020

Thanks for this, I'll try!

I'm not sure how to actually determine whether the right config is built though, other than by looking at whether the library file sizes differ :S Do you have any idea?

Remember that using find_package should work (instead of add_subdirectory in your project).

find_package does work, but I haven't figured out how to build PhysX alongside my project using find_package. It seems like add_subdirectory is the only option?

@phcerdan
Copy link
Author

You can use ExternalProject_Add to build all your dependencies and your own project, it's what in CMake is called a Superbuild.
This Superbuild project builds all your dependencies, and lastly your project also with ExternalProject_Add.
Your project would need to worry on building only itself, using find_package to find the external dependencies (that you just built/managed in the Superproject). If you think about it, add_subdirectory is like your project is owning the dependencies, it doesn't allow to find libraries in your system.

ExternalProject_Add has the mapping ability with: CMAKE_MAP_IMPORTED_CONFIG_<CONFIG> that would solve this non-standard CMAKE_BUILD_TYPE of Physx.
Nevertheless, give the last commit a try and let me know.

@alanjfs
Copy link

alanjfs commented Aug 25, 2020

I hadn't heard of ExternalProject_Add before, that does sound like the right approach, I will experiment. Thanks a lot @phcerdan!

@xamado
Copy link

xamado commented Jul 8, 2022

I'm assuming this never got integrated ?

@phcerdan
Copy link
Author

phcerdan commented Jul 8, 2022

I'm assuming this never got integrated ?

You are right 👀

@xamado
Copy link

xamado commented Jul 9, 2022

I'm assuming this never got integrated ?

You are right 👀

😞

davidlatwe added a commit to davidlatwe/PhysX5 that referenced this pull request Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake targets are not exported
4 participants