Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to exclude Khronos platform code #211

Closed
ghost opened this issue May 23, 2019 · 11 comments
Closed

Add option to exclude Khronos platform code #211

ghost opened this issue May 23, 2019 · 11 comments

Comments

@ghost
Copy link

ghost commented May 23, 2019

In generated Glad 2 headers, there are a few hundred lines of code prefixed with khronos_ or KHRONOS_, none of which I use in my projects. Some of the khronos_ types directly translate to GL types and could be replaced with standard C/C++ types instead.

I recall that in the past, Glad had an option to exclude the Khronos code. It would be nice to have that feature.

@Dav1dde
Copy link
Owner

Dav1dde commented May 23, 2019

This is not that easy anymore, previously only GLES types and functions depended on khronos types, now more or less everything references the khronos types:

typedef unsigned int GLenum;
typedef unsigned char GLboolean;
typedef unsigned int GLbitfield;
typedef void GLvoid;
typedef khronos_int8_t GLbyte;
typedef khronos_uint8_t GLubyte;
typedef khronos_int16_t GLshort;
typedef khronos_uint16_t GLushort;
typedef int GLint;
typedef unsigned int GLuint;
typedef khronos_int32_t GLclampx;
typedef int GLsizei;
typedef khronos_float_t GLfloat;
typedef khronos_float_t GLclampf;
typedef double GLdouble;
typedef double GLclampd;
typedef void *GLeglClientBufferEXT;
typedef void *GLeglImageOES;
typedef char GLchar;
typedef char GLcharARB;
#ifdef __APPLE__
typedef void *GLhandleARB;
#else
typedef unsigned int GLhandleARB;
#endif
typedef khronos_uint16_t GLhalf;
typedef khronos_uint16_t GLhalfARB;
typedef khronos_int32_t GLfixed;
#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > 1060)
typedef khronos_intptr_t GLintptr;
#else
typedef khronos_intptr_t GLintptr;
#endif
#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > 1060)
typedef khronos_intptr_t GLintptrARB;
#else
typedef khronos_intptr_t GLintptrARB;
#endif
#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > 1060)
typedef khronos_ssize_t GLsizeiptr;
#else
typedef khronos_ssize_t GLsizeiptr;
#endif
#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && (__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ > 1060)
typedef khronos_ssize_t GLsizeiptrARB;
#else
typedef khronos_ssize_t GLsizeiptrARB;
#endif
typedef khronos_int64_t GLint64;
typedef khronos_int64_t GLint64EXT;
typedef khronos_uint64_t GLuint64;
typedef khronos_uint64_t GLuint64EXT;
typedef struct __GLsync *GLsync;

It's an enhancement but nothing with high priority right now, especially since you can just generate a header only version now which includes all the definitions of khronos types within the header.

@ghost
Copy link
Author

ghost commented May 23, 2019 via email

@Dav1dde
Copy link
Owner

Dav1dde commented May 24, 2019

You can edit the specification and remove all links (require instructions) to khrplatform. Alternatively to do it in glad, you'd probably need a pre processor that does the same, go through all types remove the khrplatform requirement and replace the types.

@ghost
Copy link
Author

ghost commented May 24, 2019

It may not be as simple as a dictionary lookup with this:

#ifdef _WIN64
typedef signed   long long int khronos_intptr_t;
typedef unsigned long long int khronos_uintptr_t;
typedef signed   long long int khronos_ssize_t;
typedef unsigned long long int khronos_usize_t;
#else
typedef signed   long  int     khronos_intptr_t;
typedef unsigned long  int     khronos_uintptr_t;
typedef signed   long  int     khronos_ssize_t;
typedef unsigned long  int     khronos_usize_t;
#endif

I'm open to suggestions.

EDIT: There are generic fallback types for everything except the above. OpenGL and EGL use khronos_Xptr_t and khronos_Xsize_t. Vulkan's way of handling types just confuses me. Generally I would use standard types and just ignore that conditional.

_KHR_TYPE_MAPPING = {
	'khronos_int8_t':    'signed char',
	'khronos_uint8_t':   'unsigned char',
	'khronos_int16_t':   'signed short int',
	'khronos_uint16_t':  'unsigned short int',
	'khronos_int32_t':   'int32_t',
	'khronos_uint32_t':  'uint32_t',
	'khronos_int64_t':   'int64_t',
	'khronos_uint64_t':  'uint64_t',
	'khronos_intptr_t':  'intptr_t',
	'khronos_uintptr_t': 'uintptr_t',
	'khronos_ssize_t':   'intptr_t',
	'khronos_usize_t':   'uintptr_t',
	'khronos_float_t':   'float',
	'khronos_stime_nanoseconds_t': 'int64_t',
	'khronos_utime_nanoseconds_t': 'uint64_t'
}

Because khronos_Xptr_t is always equal to khronos_Xsize_t, it seems that Khronos APIs are not designed for platforms with segmented memory, but they are designed for platforms that don't support 64-bit, so I'm going with intptr_t and uintptr_t.

@Dav1dde
Copy link
Owner

Dav1dde commented Jun 1, 2019

The problem is, if you put the mapping that is in khrplatform.h into glad, you'll either end up with the same code or an inferior version (e.g. not supporting some platforms).

And the mapping needs to be maintained, this already broke once.

What is the usecase (with a header only build) that you would not want to have khrplatform.h included?

@ghost
Copy link
Author

ghost commented Jun 1, 2019

the same code

Not true. khrplatform.h does a song and dance where it tries to find the "best" int32/uint32/int64/uint64 types for the platform, while my substitutions just use the fixed-size fallbacks, which I consider to be more correct.

inferior version (e.g. not supporting some platforms)

True, but they would be platforms that don't support fixed-size integers, which are IMHO not worthwhile platforms anyways. 32-bit APIs (slightly older OpenGL) simply won't have 64-bit types to resolve, so compatibility would not change for them.

And the mapping needs to be maintained, this already broke once.

Please elaborate.

What is the usecase (with a header only build) that you would not want to have khrplatform.h included?

It's just a nice-to-have. It's easy to forget about the khrplatform code in there, which is subject to license terms that are easy to ignore. I'd rather have slightly smaller headers that make more sense for my way of thinking.

@Dav1dde
Copy link
Owner

Dav1dde commented Jun 1, 2019

And the mapping needs to be maintained, this already broke once.

Please elaborate.

Every time new types are introduced or the names change, the "internal" mapping is going to be out of sync. This affects all non-C generators a lot more than the C generator since most of the time it's just new C types, but it happened with C before: KhronosGroup/OpenGL-Registry@d146746

It's just a nice-to-have. It's easy to forget about the khrplatform code in there, which is subject to license terms that are easy to ignore. I'd rather have slightly smaller headers that make more sense for my way of thinking.

That's the thing, I do agree with you here, but at the end of the day it's "just" generated code.

@ghost
Copy link
Author

ghost commented Jun 1, 2019

The scope of this proposed feature is C, C++, and languages that can use C such as D. It will not be available for OpenGL SC or Vulkan SC as that could raise compliance issues. In other words, this is a feature for game engines only. There are plenty of engines that copy the functions into their own code (such as Lobster) but I don't want to do that because Glad feels better.

I don't see a problem with intptr_t this time because Khronos is using the name khronos_intptr_t, which is basically a compatibility solution for the language standard intptr_t. As for khronos_ssize_t, I don't know of a language standard equivalent, but it is currently the same type as khronos_intptr_t. This feature could be marked (unstable).

That's the thing, I do agree with you here, but at the end of the day it's "just" generated code.

If it's in my source repo, it's my generated code, and I love it like the rest of my code.

@Dav1dde
Copy link
Owner

Dav1dde commented Jun 1, 2019

The link to the Khronos commit was an example of what broke glad. It is possible to workaround it

This feature could be marked (unstable)

Yeah, I think having it as an option doesn't hurt. I'll have to look into a proper way how to implement it, since this kind of breaks the requirement system of the specifications. The types "require" khrplatform and not khronos_* types.

One option would be to simply replace khrplatform with a custom khrplatform that exposes the same types but is "simpler", but that feels like a half-assed solution, I'd prefer something where only the required types end up in the header (without khronos_ types at all).

If it's in my source repo, it's my generated code, and I love it like the rest of my code.

Agreed.

@ghost
Copy link
Author

ghost commented Jun 1, 2019

The link to the Khronos commit was an example of what broke glad. It is possible to workaround it

I know. Not sure how a workaround would work, but I'm content with the idea of assuming things won't break in the future after substituting types.

One option would be to simply replace khrplatform with a custom khrplatform that exposes the same types but is "simpler", but that feels like a half-assed solution, I'd prefer something where only the required types end up in the header (without khronos_ types at all).

Agreed.

@Dav1dde
Copy link
Owner

Dav1dde commented Oct 21, 2022

I dont think this is feasible anymore and already is broken in glad 1, glad 2 includes a header only option which embedds the external headers.

@Dav1dde Dav1dde closed this as completed Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant