qt/wayland: gate present path on m_hidden to kill tab-switch ghost race
Detaching the subsurface buffer on QEvent::Hide (commit
52d4ee413) wasn't enough on its own: libghostty's renderer
thread can produce one more frame *after* set_occlusion(false)
returns (the occlusion request is async; the renderer thread
may already be partway through a draw). That stray frame went
through presentVulkanDmabuf → drainVulkan → presentDmabuf +
forceParentCommit, re-attaching a buffer immediately after we'd
just detached one. The detached-then-reattached sequence
collapsed into "just attached" from the compositor's view and
the now-inactive tab ghosted on top of whoever was active.
Add an explicit m_hidden std::atomic<bool> that:
- is set true (release) BEFORE calling presenter.hide() in the
Hide handler, so any concurrent renderer-thread call to
presentVulkanDmabuf either sees the flag and bails, or
parks its frame (which drainVulkan then sees and bails on)
- is cleared (release) in the Show handler so the next
occupant of the tab can present normally
- gates presentVulkanDmabuf (renderer thread), drainVulkan
(GUI thread Vulkan path), and renderTerminal (GUI thread,
GL path) — three different code paths that all could
otherwise sneak a frame past the hide
ghostty_surface_set_occlusion still throttles the renderer
thread idle-wise (it's the cheap path); m_hidden is the
synchronous correctness backstop.
Co-Authored-By: claude-flow <ruv@ruv.net>
pull/12846/head
parent
e0b90b3bbf
commit
f686fa50e8
|
|
@ -428,10 +428,9 @@ bool GhosttySurface::event(QEvent *e) {
|
|||
if (m_surface) {
|
||||
if (e->type() == QEvent::Show) {
|
||||
ghostty_surface_set_occlusion(m_surface, true);
|
||||
std::fprintf(stderr,
|
||||
"[ghastty] Show surface=%p presenter=%p\n",
|
||||
static_cast<void *>(this),
|
||||
static_cast<void *>(m_subsurfacePresenter.get()));
|
||||
// Clear the present-gate latch: subsequent frames go through
|
||||
// the subsurface as normal.
|
||||
m_hidden.store(false, std::memory_order_release);
|
||||
// First successful Show is also when our native QWindow exists
|
||||
// and we can safely look up the Wayland parent wl_surface.
|
||||
// Lazy-init the subsurface presenter once and keep it for the
|
||||
|
|
@ -477,21 +476,20 @@ bool GhosttySurface::event(QEvent *e) {
|
|||
}
|
||||
}
|
||||
} else if (e->type() == QEvent::Hide) {
|
||||
// Set the present-gate FIRST so any racing renderer frame
|
||||
// (libghostty's render thread may produce one more after
|
||||
// set_occlusion returns) is blocked from re-attaching a
|
||||
// buffer in presentVulkanDmabuf / drainVulkan / renderTerminal.
|
||||
m_hidden.store(true, std::memory_order_release);
|
||||
ghostty_surface_set_occlusion(m_surface, false);
|
||||
// Detach the subsurface buffer so this pane's last frame
|
||||
// doesn't ghost on top of whatever the now-active tab is
|
||||
// showing. The next Show + render reattaches a buffer and
|
||||
// makes it visible again.
|
||||
bool fpc = false;
|
||||
if (m_subsurfacePresenter) {
|
||||
m_subsurfacePresenter->hide();
|
||||
fpc = forceParentCommit();
|
||||
forceParentCommit();
|
||||
}
|
||||
std::fprintf(stderr,
|
||||
"[ghastty] Hide surface=%p presenter=%p fpc=%d\n",
|
||||
static_cast<void *>(this),
|
||||
static_cast<void *>(m_subsurfacePresenter.get()),
|
||||
fpc ? 1 : 0);
|
||||
}
|
||||
}
|
||||
return QWidget::event(e);
|
||||
|
|
@ -554,6 +552,11 @@ void GhosttySurface::flashScrollbar() {
|
|||
void GhosttySurface::renderTerminal() {
|
||||
if (!m_surface) return;
|
||||
|
||||
// Don't render / present while hidden — the subsurface is already
|
||||
// detached from a buffer by Hide; doing more work here would just
|
||||
// race a stale frame back into view on the next compositor cycle.
|
||||
if (m_hidden.load(std::memory_order_acquire)) return;
|
||||
|
||||
// Vulkan path: libghostty owns its target VkImage; it renders into
|
||||
// it directly and presents via the apprt dmabuf callback. No GL
|
||||
// context, no FBO, no readback — just kick the draw and let the
|
||||
|
|
@ -1620,6 +1623,11 @@ void GhosttySurface::presentVulkanDmabuf(
|
|||
if (dmabuf_fd < 0 || width == 0 || height == 0 || stride < width * 4)
|
||||
return;
|
||||
|
||||
// Don't park / dispatch frames while we're hidden — racing the
|
||||
// renderer's final post-Hide frame past presenter.hide() is what
|
||||
// restores the ghost on tab switch.
|
||||
if (m_hidden.load(std::memory_order_acquire)) return;
|
||||
|
||||
if (useSubsurface) {
|
||||
// Subsurface path. Park the descriptor under the mutex (so
|
||||
// a concurrent drainVulkan sees a consistent snapshot) and
|
||||
|
|
@ -1671,6 +1679,7 @@ void GhosttySurface::drainVulkan() {
|
|||
// under the mutex, then dispatch it to the presenter outside the
|
||||
// lock so a renderer-thread `presentVulkanDmabuf` parking the
|
||||
// next frame doesn't block on wl_display_flush.
|
||||
if (m_hidden.load(std::memory_order_acquire)) return;
|
||||
if (m_useSubsurface.load(std::memory_order_acquire) &&
|
||||
m_subsurfacePresenter) {
|
||||
PendingDmabuf frame;
|
||||
|
|
|
|||
|
|
@ -372,4 +372,13 @@ private:
|
|||
// Per-surface latch for the first-dmabuf log breadcrumb so each
|
||||
// pane / split prints its own line on first frame.
|
||||
bool m_loggedFirstFrame = false;
|
||||
// Set true on QEvent::Hide, false on QEvent::Show. Guards the
|
||||
// present path against a race where libghostty's renderer thread
|
||||
// fires one more frame after we've detached the subsurface
|
||||
// buffer on Hide — without this gate, that stray frame re-
|
||||
// attaches a buffer and the now-inactive tab ghosts on top of
|
||||
// whatever tab the user just switched to. `std::atomic` because
|
||||
// the renderer thread reads it in `presentVulkanDmabuf` /
|
||||
// `drainVulkan` while the GUI thread writes from event().
|
||||
std::atomic<bool> m_hidden{false};
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue