H-uru / Plasma

Cyan Worlds's Plasma game engine
http://h-uru.github.io/Plasma/
GNU General Public License v3.0
205 stars 81 forks source link

Move shaders into shader library target #1581

Closed colincornaby closed 4 months ago

colincornaby commented 5 months ago

This PR does several things:

I'd like some feedback on the implementation in the CMake - and feedback on this change requiring CMake 3.28.

colincornaby commented 5 months ago

You probably need to assert in the metal pipeline that the CMake version is at the minimum required for this feature to work.

What would be the best way to do that? A different cmake_minimum_required or some sort of different check?

Hoikas commented 5 months ago

What would be the best way to do that? A different cmake_minimum_required or some sort of different check?

We may not want to use cmake_ minimum_required() because this has side effects related to setting policies, and we may not want to have different policy scopes playing with our mind. It may be better to test and error if the version is too low:

if(${CMAKE_VERSION} VERSION_LESS 3.9)
    message(FATAL_ERROR "boo")
endif()
colincornaby commented 5 months ago

Just checking in on this PR. I made some changes a few weeks back but haven't seen any feedback yet.

Hoikas commented 5 months ago

Looks like this PR needs to be rebased to build.

colincornaby commented 5 months ago

I went ahead and rebased pulling in @dpogue's latest Mac fixes. Waiting to see if the build is green.

colincornaby commented 5 months ago

It looks like builds are passing again on this PR.