Skip to content

SDLWindow: MSAA fallback retry + null-safe CreateWindow #2059

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.

Sign up for GitHub

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

Open
AxGord wants to merge 5 commits into
base: develop
Choose a base branch
from
+34 −15
Open

SDLWindow: MSAA fallback retry + null-safe CreateWindow #2059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6370d46
SDLWindow: MSAA fallback retry + null-safe CreateWindow
AxGord Mar 20, 2026
cbc833f
SDLWindow: nullptr + ASCII comment; NativeWindow: null-safe create
AxGord May 17, 2026
1da6efe
Merge upstream/develop into bugfix/sdlwindow-msaa-fallback
AxGord May 17, 2026
16a34b6
SDLWindow: pass NULL for unused SDL_GetWindowSize out-param
AxGord May 17, 2026
c668007
NativeWindow: set frameRate before null-handle early return
AxGord May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions project/src/backend/sdl/SDLWindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,15 @@ namespace lime {
}
#endif

if (!sdlWindow && (flags & (WINDOW_FLAG_HW_AA | WINDOW_FLAG_HW_AA_HIRES))) {

// Retry without antialiasing - emulators and some devices don't support MSAA
SDL_GL_SetAttribute (SDL_GL_MULTISAMPLEBUFFERS, 0);
SDL_GL_SetAttribute (SDL_GL_MULTISAMPLESAMPLES, 0);
sdlWindow = SDL_CreateWindow (title, SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, width, height, sdlWindowFlags);

}

if (!sdlWindow) {

printf ("Could not create SDL window: %s.\n", SDL_GetError ());
Expand Down Expand Up @@ -797,10 +806,9 @@ namespace lime {

int SDLWindow::GetHeight () {

int width;
int height;
int height = 0;
Copy link
Copy Markdown
Member

@tobil4sk tobil4sk May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can SDL_GetWindowSize actually exit without writing these values? Sdl2 docs don't mention that case: https://wiki.libsdl.org/SDL2/SDL_GetWindowSize

Copy link
Copy Markdown
Author

@AxGord AxGord May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can, via CHECK_WINDOW_MAGIC. In SDL2 (src/video/SDL_video.c) SDL_GetWindowSize starts with CHECK_WINDOW_MAGIC(window, );, which early-returns before writing *w/*h when the video subsystem is uninitialized, window is NULL, or window->magic doesn't match (a destroyed/stale window — the teardown race this guards). The wiki doesn't mention it but the source confirms it, so the = 0 init is defensive against exactly that path; the happy path is unaffected.

Copy link
Copy Markdown
Member

@tobil4sk tobil4sk May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to provide the source:
https://github.com/libsdl-org/SDL/blob/f0d1101920864995c0c265128cfd7b96eb959ac5/src/video/SDL_video.c#L2399

https://github.com/libsdl-org/SDL/blob/f0d1101920864995c0c265128cfd7b96eb959ac5/src/video/SDL_video.c#L161

The docs also say:

NULL can safely be passed as the w or h parameter if the width or height value is not desired.

Which can make the code cleaner and avoid setting the one we don't need to 0.

Also as an aside, it looks like in SDL3 we will be able to check the return value of SDL_GetWindowSize to see if it failed: https://wiki.libsdl.org/SDL3/SDL_GetWindowSize

Copy link
Copy Markdown
Author

@AxGord AxGord May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — adopted in 16a34b6. GetWidth/GetHeight now pass NULL for the dimension they don't return and only init the one they do (still = 0 so the CHECK_WINDOW_MAGIC early-return path stays defensive). Thanks for the SDL source links and the SDL3 return-value note.


SDL_GetWindowSize (sdlWindow, &width, &height);
SDL_GetWindowSize (sdlWindow, NULL, &height);

return height;

Expand Down Expand Up @@ -880,10 +888,9 @@ namespace lime {

int SDLWindow::GetWidth () {

int width;
int height;
int width = 0;

SDL_GetWindowSize (sdlWindow, &width, &height);
SDL_GetWindowSize (sdlWindow, &width, NULL);

return width;

Expand Down Expand Up @@ -1439,7 +1446,16 @@ namespace lime {

Window* CreateWindow (Application* application, int width, int height, int flags, const char* title) {

return new SDLWindow (application, width, height, flags, title);
SDLWindow* window = new SDLWindow (application, width, height, flags, title);

if (!window->sdlWindow) {

delete window;
return nullptr;

}

return window;
Copy link
Copy Markdown
Member

@tobil4sk tobil4sk May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to make sure all callers of CreateWindow check for null with this change. We should also return nullptr rather than NULL since it is c++ code.

Copy link
Copy Markdown
Author

@AxGord AxGord May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — nullptr in cbc833f. On the caller side: both C++ callers (lime_window_create / hl_window_create in ExternalInterface.cpp) pass the result straight into CFFIPointer/HLCFFIPointer, which already null-check (CFFIPointer.cpp returns alloc_null() for a null pointer; HLCFFIPointer returns 0), so the Haxe side gets a real null. NativeWindow.create() did guard the width/height/x/y block with if (handle != null), but then called lime_window_get_scale(handle) and lime_window_get_context_type(handle) unconditionally — those deref a null handle. Fixed in the same commit by replacing the partial guard with an early return right after lime_window_create.


}

Expand Down
19 changes: 11 additions & 8 deletions src/lime/_internal/backend/native/NativeWindow.hx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,20 @@ class NativeWindow
#if (!macro && lime_cffi)
handle = NativeCFFI.lime_window_create(parent.application.__backend.handle, width, height, flags, title);

if (handle != null)
if (handle == null)
{
parent.__width = NativeCFFI.lime_window_get_width(handle);
frameRate = resolvedFrameRate;
return;
Copy link
Copy Markdown
Member

@tobil4sk tobil4sk May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this early return causes frameRate to not be set. Not sure what the expected state is for a window that failed to launch.

Copy link
Copy Markdown
Author

@AxGord AxGord May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c668007 — frameRate = resolvedFrameRate; now runs before the early return, so a window that failed to launch still has frameRate populated for getFrameRate()'s field fallback. It's a plain field write (no handle deref) and resolvedFrameRate is already computed above, so it's safe on the failure tail. setTextInputEnabled(false) is still skipped, but that one only configures SDL state via the (now null) handle and is internally handle-guarded, so there's nothing meaningful to set for a window that never opened.

}

parent.__height = NativeCFFI.lime_window_get_height(handle);
parent.__x = NativeCFFI.lime_window_get_x(handle);
parent.__y = NativeCFFI.lime_window_get_y(handle);
parent.__hidden = (Reflect.hasField(attributes, "hidden") && attributes.hidden);
parent.__width = NativeCFFI.lime_window_get_width(handle);

parent.id = NativeCFFI.lime_window_get_id(handle);
}
parent.__height = NativeCFFI.lime_window_get_height(handle);
parent.__x = NativeCFFI.lime_window_get_x(handle);
parent.__y = NativeCFFI.lime_window_get_y(handle);
parent.__hidden = (Reflect.hasField(attributes, "hidden") && attributes.hidden);

parent.id = NativeCFFI.lime_window_get_id(handle);
parent.__scale = NativeCFFI.lime_window_get_scale(handle);

var context = new RenderContext();
Expand Down