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

Wrong EGL native types used for QNX #191

Closed
jmcdonnell-blackberry opened this issue Nov 27, 2023 · 6 comments
Closed

Wrong EGL native types used for QNX #191

jmcdonnell-blackberry opened this issue Nov 27, 2023 · 6 comments
Labels

Comments

@jmcdonnell-blackberry
Copy link
Contributor

On QNX, __unix__ is also defined. Since the __unix__ block appears before the __QNX__ block, the Unix native types are chosen rather than the QNX native types. The __QNX__ block needs to precede the __unix__ block.

The __QNX__ block also needs to precede the WL_EGL_PLATFORM block. The existence of WL_EGL_PLATFORM doesn't necessarily indicate that Wayland is the native window platform. It only indicates that Wayland is a supported platform. For QNX, QNX Screen is currently the native window platform but Wayland may also be a window platform (accessed via eglGetPlatformDisplay, et al).

Really, it would be better if the native window platform was explicitly defined rather than being guessed. But that's probably something that can't be changed at this point.

@stonesthrow
Copy link
Contributor

Agreed, unix should be last in the if-else sequence. Sorry we missed this.

It can be conflated, eglPlatform.h should be only indicating the default. That reordering seems OK.

I think it was supposed that the compiler would know the platform and and the macro set. But with multiple support there maybe many combinations of which goes first.

Do you want to propose a change to eglPlatform.h? The MR should be limited to just this change.

@jmcdonnell-blackberry
Copy link
Contributor Author

Yes, I can propose a change.

@stonesthrow
Copy link
Contributor

stonesthrow commented Nov 27, 2023

The "__unix__" clause seems like its a catch all for any undefined window system on a linux/unix platform. Jon may remember some history on this.

@oddhack
Copy link
Contributor

oddhack commented Nov 28, 2023

I have a vague recollection of initially using some comparable set of ifdefs from Mesa headers. That was a long time ago.

@stonesthrow
Copy link
Contributor

I think we'll need a couple of reviewers, so to check that we haven't inadvertently broke some platform. Especially around QNX, Wayland, Unix. where the change will touch.

jmcdonnell-blackberry pushed a commit to jmcdonnell-blackberry/EGL-Registry that referenced this issue Nov 28, 2023
oddhack pushed a commit that referenced this issue Dec 1, 2023
Co-authored-by: James McDonnell <jmcdonnell@qnx.com>
@stonesthrow
Copy link
Contributor

#192 is merged.
Closing issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants