KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
861 stars 225 forks source link

Do not redefine NOMINMAX #801

Closed corporateshark closed 9 months ago

CLAassistant commented 10 months ago

CLA assistant check
All committers have signed the CLA.

MarkCallow commented 9 months ago

What is the build environment in which you encountered NOMINMAX being set and a compiler that complained? Since this is a plain #define I don't see how it could be redefining NOMINMAX. Compilers usually only complain if there is a redefine not if a previous definition is repeated.

corporateshark commented 9 months ago

@MarkCallow There's yet another header which defines NOMINMAX in a similar way. Every library thinks it is the only library and mess up somebody else's macros in its headers ;)

MarkCallow commented 9 months ago

The file you've changed is internal to libktx so should not be affected and should not affect other headers your application may be using. And, as I said, compilers usually only complain when an item is being defined to a different value than it's current value. Even then they don't complain if the current value is undefined. So I'm curious which compiler you are using.

corporateshark commented 9 months ago

So I'm curious which compiler you are using.

Visual Studio 2022.

I'm including that file directly to access OpenGL texture format definitions without including full-sized actual OpenGL headers (of which I have none, because the app is using Vulkan for rendering and needs to render KTX1 textures as well as KTX2).

MarkCallow commented 9 months ago

I'm including that file directly to access OpenGL texture format definitions

Got it. So you are writing your own KTX texture loader, correct? May I ask why you chose not to use the Vulkan loader in libktx?

I expect to remove gl_format.h from the project eventually.

corporateshark commented 9 months ago

May I ask why you chose not to use the Vulkan loader in libktx?

I have a multi-API renderer. Loaded textures are agnostic of any specific API.

MarkCallow commented 9 months ago

I have a multi-API renderer. Loaded textures are agnostic of any specific API.

Great. I would happily accept PR(s) for D3D12 and Metal loaders if you were so inclined.

Thanks for this fix.