Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The dlwrap thunks define the symbol they wrap, so a signature that disagrees with the installed SDL_ttf.h is a hard conflicting-types error rather than a warning. SDL_ttf carried a const font argument on TTF_GlyphIsProvided until 2.0.18 dropped it, and the thunk here declares it non-const unconditionally, so any host still on the const spelling fails to build the wrapper at all: src/wrapper/sdl-ttf-wrapper.c:60:15: error: conflicting types for 'TTF_GlyphIsProvided' /usr/include/SDL2/SDL_ttf.h:160:29: note: previous declaration ... (const TTF_Font *font, Uint16 ch) That is the differential host, which ships 2.0.15, so its compat-side build could not start. Upstream headers put the boundary at 2.0.18: 2.0.12, 2.0.14 and 2.0.15 all prototype const TTF_Font *, while 2.0.18 and 2.24 do not. Split on that single upper bound. TTF_GlyphIsProvided32 arrived in 2.0.18, after the revert, so it is non-const for its whole life and needs no gate.
destroyWindow leaves a destroyed child in its parent's array typed CLOSED_WINDOW with a NULL struct, and the screen teardown relies on that: destroyScreenWindowImpl passes freeParentData=False so the ids linger until the whole array is freed. Walkers over that array are therefore expected to filter on the type slot, which hasPendingWindowPresent (src/drawing.c) and repaintTopLevelsOverlappingRect (src/window.c) already do. Two walkers were missing the filter and dereference the struct: postVisibilityForWindowAndSiblings crashes when a client calls XCloseDisplay while a popup is still mapped. Destroying the popup's parent drains the anchored popup, whose unmap posts VisibilityNotify to every sibling, and postEvent then dereferences a window that is already gone: postEvent postVisibilityForWindowAndSiblings unmapWindowInternal drainAnchoredPopups destroyWindow destroyScreenWindowImpl XCloseDisplay The faulting address is the struct field offset off a NULL base. Whether it fires depends on which child is destroyed first, so it needs only one more window ahead of the client's own in the root's list. enqueueResetExposures has the same shape: a render-targets or render-device reset arriving mid-teardown reads ->sdlWindow off a CLOSED_WINDOW slot.
destroyScreenWindowImpl frees the root WindowStruct directly instead of going through destroyWindow, and it released only the children array and the pixman regions. Everything else destroyWindow frees for an ordinary window leaked once per XOpenDisplay/XCloseDisplay cycle: the property list (the root already carries one, since compatPublishHiDpiScaleProperty writes the HiDPI scale there), a name if a client ever called XStoreName on the root, the colormap-window list, the present readback scratch and the GLX buffers. A process that cycles displays (the Motif and Xt probes, the check suite, an embedding host) grew its heap monotonically with no way to reclaim it, and a leak checker that runs a single cycle cannot see it.
An EWMH-aware toolkit (GTK, Qt, recent Xt) probes the root for _NET_SUPPORTING_WM_CHECK before it speaks EWMH at all. We run an in-process WM and already act on _NET_WM_STATE add/remove/toggle, _NET_CLOSE_WINDOW and _NET_WM_WINDOW_TYPE, but the root advertised nothing, so those clients concluded no compliant manager was running and fell back to self-decorating, skipping the very requests we honor. XOpenDisplay now publishes the two-property handshake: an internal, never-mapped InputOnly child of the root carries _NET_SUPPORTING_WM_CHECK pointing at itself plus a UTF8_STRING _NET_WM_NAME, and the root points at the same window. createInternalWindow marks it internal, which keeps it out of CreateNotify, XQueryTree and the state snapshot, and it is unmapped so pointer routing skips it. The root property is the only record of it, so a second display adopts the existing window instead of stacking up one per open. The four writes are one short-circuit chain with the root _NET_SUPPORTING_WM_CHECK last as the commit point; any failure destroys the check window and clears both root properties. _NET_SUPPORTED lists only what we act on, and rather than hand-maintain that list beside the handlers, the honored atoms are extracted into tables the handlers themselves read: netWmStateHonored pairs each substate with the NetWmStateSet field it sets via offsetof (replacing an if-else chain, and letting both MAXIMIZED axes share one field), and netWmWindowTypeHonored holds the borderless types. Advertised and honored cannot drift. Three atoms are deliberately absent. _NET_WM_ICON_NAME, because nothing reads it. _NET_ACTIVE_WINDOW, because EWMH defines it as a root property the manager keeps current as well as a ClientMessage and only the message is handled here, so advertising it would leave a client reading the root property (gdk_x11_screen_get_active_window, xdotool getactivewindow) seeing no active window at all. _NET_WM_STATE_ABOVE below SDL 2.0.16, where its only consumer is compiled out; the table entry carries the same gate as the apply. _NET_CLOSE_WINDOW has no property side, so it stays.
EWMH has clients set the type before the map request, which is the order GTK, Qt and Motif all use. The SDL window does not exist yet at that point, so XChangeProperty handler bailed on IS_MAPPED_TOP_LEVEL_WINDOW and nothing replayed the property afterwards: realizeTopLevelWindow derives borderless only from overrideRedirect, and replayDeferredWmProperties covered _MOTIF_WM_HINTS, WM_NORMAL_HINTS, _NET_WM_STATE and WM_TRANSIENT_FOR but not this one. A splash or menu that set its type the standard way came up fully decorated, while the root _NET_SUPPORTED told the client the type was honored, so the client had no reason to fall back to _MOTIF_WM_HINTS or override-redirect. Move the decode into applyNetWmWindowTypeFromProperty next to the other appliers and call it from both the property write and the realize replay, so the write order stops mattering. The write handler now just calls it, which also drops the duplicated mapped-state check and the open-coded data cast. Scope: only the eight honored types change behavior, and only for windows that previously kept a border they asked not to have. A window whose type is NORMAL, DIALOG, UTILITY, TOOLBAR or DESKTOP decodes as not-borderless and is untouched, so ordinary top-levels are unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Announces EWMH support on the root by creating a supporting-WM-check window and publishing an exact _NET_SUPPORTED list, so EWMH-aware toolkits send the _NET_WM_STATE and _NET_WM_WINDOW_TYPE requests we already handle. Centralizes honored atom tables, hides internal windows, honors pre-map _NET_WM_WINDOW_TYPE, and adds tests to pin the contract.
New Features
_NET_WM_STATE_ABOVEgated to SDL >= 2.0.16) and netWmWindowTypeHonored (borderless types). Deliberately exclude _NET_ACTIVE_WINDOW until the root property is maintained.Bug Fixes
SDL_ttfthunk for TTF_GlyphIsProvided: use const for versions < 2.0.18, avoiding conflicting types on hosts like 2.0.14/2.0.15.Written for commit 47f9227. Summary will update on new commits.