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.

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

Jump to bottom

Call XInitThreads first; XCloseDisplay cleanup #1291

Merged
merged 6 commits into from
Oct 25, 2024
Merged

Call XInitThreads first; XCloseDisplay cleanup #1291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a4bd056
change(core/linux/x11): call to XInitThreads must be first libx11 call
RonaldZielaznicki Oct 21, 2024
d247fbb
remove(core/linux/X11): errorHandler in favor of the more verbose def…
RonaldZielaznicki Oct 21, 2024
653fbab
change(Core): deinit platform after GPU backend release
RonaldZielaznicki Oct 21, 2024
85c2c17
add(core/linux/X11): comment to explain why XInitThreads is called first
RonaldZielaznicki Oct 21, 2024
6ac4b93
chore(Core): oxford commas are important
RonaldZielaznicki Oct 21, 2024
a105e00
revise comments
slimsag Oct 25, 2024
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
7 changes: 6 additions & 1 deletion src/Core.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,18 @@ pub fn deinit(entities: *mach.Entities.Mod, core: *Mod) !void {
}
}

state.platform.deinit();
// GPU backend must be released BEFORE platform deinit, otherwise we may enter a race
// where the GPU might try to present to the window server.
state.swap_chain.release();
state.queue.release();
state.device.release();
state.surface.release();
state.adapter.release();
state.instance.release();

// Deinit the platform
state.platform.deinit();

state.events.deinit();
}

Expand Down
13 changes: 5 additions & 8 deletions src/core/linux/X11.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ pub fn init(
) !X11 {
// TODO(core): return errors.NotSupported if not supported
const libx11 = try LibX11.load();

// Xlibx11.XInitThreads must be called first
//
// if not, 'Unknown sequence number while processing queue' errors occur.
_ = libx11.XInitThreads();
Copy link
Member

Choose a reason for hiding this comment

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

I'd like us to remove this from the codebase ASAP, since we don't have any multi-threading yet this should be doing nothing :) so let's get it out and debug any issues that arise from removing it

const libgl: ?LibGL = LibGL.load() catch |err| switch (err) {
error.LibraryNotFound => null,
else => return err,
Expand Down Expand Up @@ -184,8 +189,6 @@ pub fn init(
.surface_descriptor = surface_descriptor,
.libxkbcommon = try LibXkbCommon.load(),
};
_ = libx11.XSetErrorHandler(errorHandler);
_ = libx11.XInitThreads();
_ = libx11.XrmInitialize();
defer _ = libx11.XFreeColormap(display, colormap);
for (0..2) |i| {
Expand Down Expand Up @@ -442,12 +445,6 @@ const LibXkbCommon = struct {
}
};

fn errorHandler(display: ?*c.Display, event: [*c]c.XErrorEvent) callconv(.C) c_int {
_ = display;
log.err("X11: error code {d}\n", .{event.*.error_code});
return 0;
}

fn createStandardCursor(x11: *X11, shape: CursorShape) !c.Cursor {
if (x11.libxcursor) |libxcursor| {
const theme = libxcursor.XcursorGetTheme(x11.display);
Expand Down
Loading