search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.99k stars 1.84k forks source link

SDL_x11opengles: cast to smaller integer type #6348

Closed madebr closed 1 month ago

madebr commented 2 years ago

In #6336, I'm combing though all warnings to be able so we can introduce -Werror on CI.

I noticed the following warning with autotools on Macos:

libtool: compile:  gcc -g -O3 -DUSING_GENERATED_CONFIG_H -Iinclude -I/Users/runner/work/SDL/SDL/include -idirafter /Users/runner/work/SDL/SDL/src/video/khronos -DSDL_BUILD_MAJOR_VERSION=2 -DSDL_BUILD_MINOR_VERSION=25 -DSDL_BUILD_MICRO_VERSION=0 -mmmx -m3dnow -msse -msse2 -msse3 -Wall -fno-strict-aliasing -DTARGET_API_MAC_CARBON -DTARGET_API_MAC_OSX -fobjc-arc -fvisibility=hidden -Werror -Wno-error=deprecated-declarations -Wdeclaration-after-statement -Werror=declaration-after-statement -D_THREAD_SAFE -fobjc-weak -Wno-unused-command-line-argument -MMD -MT build/SDL_x11shape.lo -c /Users/runner/work/SDL/SDL/src/video/x11/SDL_x11shape.c  -fno-common -DPIC -o build/.libs/SDL_x11shape.o
/Users/runner/work/SDL/SDL/src/video/x11/SDL_x11opengles.c:57:45: error: cast to smaller integer type 'NativeDisplayType' (aka 'int') from 'Display *' (aka 'struct _XDisplay *') [-Werror,-Wpointer-to-int-cast]
    return SDL_EGL_LoadLibrary(_this, path, (NativeDisplayType) data->display, 0);
                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

I'm unsure how to fix this issue as on 64-bit Macos, this casts a (64-bit) pointer to an integer.

Warning location: https://github.com/libsdl-org/SDL/blob/b4aba10154f901fc36af0fb94742201c21ce7f74/src/video/x11/SDL_x11opengles.c#L55-L58

data->display has type Data *: https://github.com/libsdl-org/SDL/blob/b4aba10154f901fc36af0fb94742201c21ce7f74/src/video/x11/SDL_x11video.h#L71

SDL_EGL_LoadLibrary is declared here: https://github.com/libsdl-org/SDL/blob/b4aba10154f901fc36af0fb94742201c21ce7f74/src/video/SDL_egl.c#L501-L502

The system EGL headers on my (Linux) machine contain the following:

typedef EGLNativeDisplayType NativeDisplayType;

// ...

#elif defined(__APPLE__)

typedef int   EGLNativeDisplayType;
typedef void *EGLNativePixmapType;
typedef void *EGLNativeWindowType;

#elif defined(__HAIKU__)

// ...
slouken commented 2 years ago

We should just disable X11 on macOS, that hasn't been supported for years.

madebr commented 2 years ago

That explains why I didn't get this error with CMake.

sezero commented 2 years ago

For X11, it seems like USE_X11 needs to be defined.

madebr commented 2 years ago

Not an actual fix, but in #6336 I added a commit that changes the default for --enable-x11 for Macos.

sezero commented 2 years ago

Not an actual fix, but in #6336 I added a commit that changes the default for --enable-x11 for Macos.

No, not an actual fix. SDL_x11opengles.c (I think) should define USE_X11 before anything else.

madebr commented 2 years ago

Not an actual fix, but in #6336 I added a commit that changes the default for --enable-x11 for Macos.

No, not an actual fix. SDL_x11opengles.c (I think) should define USE_X11 before anything else.

I'm testing this out on my fork. I will create a pr with a fix if it works out.

madebr commented 2 years ago

I"m getting this error now:

libtool: compile:  gcc -g -O3 -DUSING_GENERATED_CONFIG_H -Iinclude -I/Users/runner/work/SDL/SDL/include -idirafter /Users/runner/work/SDL/SDL/src/video/khronos -DSDL_BUILD_MAJOR_VERSION=2 -DSDL_BUILD_MINOR_VERSION=25 -DSDL_BUILD_MICRO_VERSION=0 -mmmx -m3dnow -msse -msse2 -msse3 -Wall -fno-strict-aliasing -DTARGET_API_MAC_CARBON -DTARGET_API_MAC_OSX -fobjc-arc -fvisibility=hidden -Werror -Wno-error=deprecated-declarations -Wdeclaration-after-statement -Werror=declaration-after-statement -DUSE_X11 -D_THREAD_SAFE -fobjc-weak -Wno-unused-command-line-argument -MMD -MT build/SDL_cocoaopengles.lo -c /Users/runner/work/SDL/SDL/src/video/cocoa/SDL_cocoaopengles.m  -fno-common -DPIC -o build/.libs/SDL_cocoaopengles.o
/Users/runner/work/SDL/SDL/src/video/cocoa/SDL_cocoaopengles.m:139:59: error: incompatible types casting 'API_AVAILABLE(macos(10.5)) CALayer *' to 'NativeWindowType' (aka 'unsigned long') with a __bridge cast
    windowdata.egl_surface = SDL_EGL_CreateSurface(_this, (__bridge NativeWindowType)[v layer]);

https://github.com/madebr/SDL/actions/runs/3200779876/jobs/5228076093#step:13:446

Would adding it to SDL_config.h be appropriate?

sezero commented 2 years ago

ObjC and ARC stuff are beyond my knowledge. @slime73 or @slouken can probably help.

slouken commented 1 year ago

Is this still active?