search

widberg / bgfx.cmake

https://github.com/bkaradzic/bgfx.cmake. Independently maintained CMake build scripts for bgfx. Released under public domain.
https://github.com/bkaradzic/bgfx.cmake
Creative Commons Zero v1.0 Universal
286 stars 252 forks source link

Support granular selection of tools to build #60

Closed Qix- closed 4 years ago

Qix- commented 4 years ago

Ref #35.

This is one step closer to solving the issue; it allows you to disable/enable tools granularly.

By disabling geometryc and texturev and only building shaderc and texturec, I can now successfully pull in an external meshoptimizer, stb, freetype2 and harfbuzz without any issues.

The reason for moving cmake/tools.cmake's contents into the root was that if I didn't, I'd have to check each of the tool's options in an if() statement, which would be prone to error if a tool was added later on (adding an option, putting it in tools.cmake but not including it in the if statement). It felt more correct.

The edit at the bottom of https://github.com/JoshuaBrookover/bgfx.cmake/issues/35#issuecomment-532594750 explains what needs to be done to have geometryc and texturev play nicely with parent projects, but those aren't covered by this PR.

JonnyPtn commented 4 years ago

Just for clarity this is almost completely unrelated to #35.

I don't see what this fixes or what the point of it is, but It also won't really break anything, so I don't feel strongly either way

JoshuaBrookover commented 4 years ago

I agree, it has nothing to do with the original issue in #35. It does relate to the discussion taking place in there. It's a good change, especially since texturev forces you to pull in example's dependencies.

JoshuaBrookover commented 4 years ago

Sorry, did not mean to close.

JonnyPtn commented 4 years ago

Probably worth adding a warning/error when trying to build texturev without the examples too so it’s clear?

Qix- commented 4 years ago

@widberg why was this closed?

widberg commented 4 years ago

It is unrelated to the issue you referenced. You seem to be the only person that needs this. You changed the submodule remotes. If you want I’ll leave it open for others to comment, but the problem you are trying to solve does not appear to exist. I have no intention of merging this.

Qix- commented 4 years ago

but the problem you are trying to solve does not appear to exist

Is nobody else using this in their project? Shaderc recompiles almost every time we do an incremental rebuild. Relinking it takes almost 20 seconds.

There's something very wrong with the cmake config as-is.

bwrsandman commented 4 years ago

It wasn't going to be merged with the changes to .gitmodules pointing to a different branch than upstream. Not to mention suggestions to fix the PR being ignored for months. Your quick reply suggests that you are active on github. I don't mind the issue you're trying to fix and if the PR is cleaned, then I'd very much like to see it merged.

As for your rebuild issue, have you tried setting the target property EXCLUDE_FROM_ALL from your host projet? https://cmake.org/cmake/help/latest/prop_tgt/EXCLUDE_FROM_ALL.html#prop_tgt:EXCLUDE_FROM_ALL

Qix- commented 4 years ago

It wasn't going to be merged with the changes to .gitmodules

Guys, I have already mentioned I understood this. This isn't what I'm arguing for.

As for your rebuild issue, have you tried setting the target property EXCLUDE_FROM_ALL from your host projet?

No, shaderc is not part of my own project. It's part of this project.

I'm not going to respond to this, this is an innane discussion. I'll fork and maintain my own.