search

KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
874 stars 229 forks source link

KTX_FEATURE_GL_UPLOAD can't be turned off when using libktx as a cmake submodule #523

Closed MarkCallow closed 2 years ago

MarkCallow commented 2 years ago

First reported by @jansol as part of #504. Transferring here because the issues and fixes are completely unrelated. @jansol subscribe to this issue for updates.

Also, disabling the gl loader when using libktx as a cmake submodule does not seem to work. Might be related to the cmake settings being implemented as variables set(...) rather than options option(...)?

See #504 for discussion and solutions proposed by @atteneder.

We will use his recommendation

I'd say we expose those settings as set+CACHE now and with option once we're certain all combinations work as expected.

The code would be:

set( KTX_FEATURE_KTX1 ON  "" ON CACHE BOOL "Enable KTX 1 support" )
set( KTX_FEATURE_KTX2 ON  "" ON CACHE BOOL "Enable KTX 2 support" )
set( KTX_FEATURE_VULKAN ON  "" ON CACHE BOOL "Enable Vulkan texture upload" )
set( KTX_FEATURE_GL_UPLOAD "" ON CACHE BOOL "Enable OpenGL texture upload" )

Perhaps we can use cmake_dependent_option, if there are dependencies between these features. That will have to wait until we have time to explore the combinations.

MarkCallow commented 2 years ago

What is the "" ON in each of these the for? And note there is no first ON in the GL_UPLOAD item.

Shouldn't they be

set( VARIABLE ON CACHE BOOL "doc string" )

?

I tried this and it is working. All are now appearing in the CMake GUI so I can't seen any difference between this and option(). Is there some difference I am missing or should I just use option()?

atteneder commented 2 years ago

You're right, the code I suggested is wrong (probably copy paste or editing errors).

I presumed cached variables wouldn't show up in UI, but apparently I was wrong.

Sure, you can use option. It's a bit easier to read, but I don't see a difference otherwise.