powervr-graphics / Native_SDK

C++ cross-platform 3D graphics SDK. Includes demos & helper code (resource loading etc.) to speed up development of Vulkan, OpenGL ES 2.0 & 3.x applications
https://docs.imgtec.com/sdk-documentation/html/introduction.html
MIT License
702 stars 197 forks source link

Amaiorano/cmake keep source clean #52

Closed amaiorano closed 4 years ago

amaiorano commented 4 years ago

These changes make it so that the cmake build doesn't generate any files outside of the cmake build folder (the binary directory). This keeps the git status clean when generating and building the SDK.

We add this project as a submodule in SwiftShader, and when we build, git status reports that the submodule has changed, which it shouldn't do.

I've tried to minimize the changes here to simply point the download and output directories to fall under CMAKE_CURRENT_BINARY_DIR, which keeps things clean. I've tested this on Windows and Linux, and both generate and build properly.

EDIT: Also, opportunistically added a .gitignore to ignore the common build/ folder for out-of-source builds.

Thanks.

graptis commented 4 years ago

Hi Antonio, Thanks very much for the contribution! We will take this git pull, it may be a couple of days (or less hopefully), we're just making sure it does not break our internal testing either :). I am also considering building upon this and adding a CMAKE-cache variable with the download folder, just to allow people to reuse the downloads, but I am not yet convinced of its value so I might use it as-is. Thanks again! Gerry

graptis commented 4 years ago

Hi Antonio, tldr: Working to adapt this, as we bumped into a couple of issues.

Not very surprisingly :/ Android. Due to the way it uses CMake, Android necessarily uses a different mechanism for defining the projects (the dependencies are Gradle based, not CMake based, and dealt by Cmake as prebuilts, otherwise each example would rebuild the entire codebase from scratch, for about a GB each :) ). @omarzohdi is currently working on this with your solution as a baseline, and it appears to be close to working without changing the behaviour of your solution for non-android, but for android we need to customise it a bit further so that the downloaded packages are placed with the build artifacts there as well, but without being replicated per architecture. We'll keep you updated, sorry for having this on ice.

amaiorano commented 4 years ago

Hi @graptis, no worries about taking time here. This isn't a blocker or anything :) I totally appreciate the extra complexity with supporting other targets. Let me know if there's anything I can do to help. Cheers!

omarzohdi commented 4 years ago

Hi @amaiorano, thanks for the contribution and sorry for the delay. We've implemented your changes internally and due to the nature of the changes will be adding them to the next release of the PowerVR SDK. The release should be up in the coming weeks.

amaiorano commented 4 years ago

That's great news, thanks!

omarzohdi commented 4 years ago

Hi @amaiorano, we've implemented the changes in our latest release (R20.1-v5.5) that is now live. Thanks for the contribution.