From 277940f39f1da0c30588bc3571dfcf02972d78cd Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 24 Sep 2026 14:03:46 +0000 Subject: [PATCH] [MG_Remote, MG_Test] (P12 review): a session asking for the window's own size after a fixed-size session waits for the layout's size instead of leasing the stale fixed one AcquireFor(0, 0) granted the lease at once, at whatever the window reported - and right after a session that fixed the size, that is still the fixed size: setSizeFromLayout answers asynchronously, like setFixedSize. ServerDisplay now remembers the geometry last requested and the layout's last reported extent; a 0/0 request that follows a fixed one waits (within the geometry grace) for the layout's extent, or for any report after the request that is not the old fixed size. ServerDisplayTest.TheWindowsOwnSizeAfterAFixedSizeIsTheLayoutsNotTheStaleFixedOne. --- MobileGL/MG_Remote/Server/ServerDisplay.cpp | 44 +++++++++++++++++++-- MobileGL/MG_Remote/Server/ServerDisplay.h | 12 ++++++ MobileGL/MG_Test/Wire/ServerDisplayTest.cpp | 42 ++++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Remote/Server/ServerDisplay.cpp b/MobileGL/MG_Remote/Server/ServerDisplay.cpp index 1cc526e2..0ce67084 100644 --- a/MobileGL/MG_Remote/Server/ServerDisplay.cpp +++ b/MobileGL/MG_Remote/Server/ServerDisplay.cpp @@ -56,6 +56,15 @@ namespace MobileGL::MG_Remote::Server { if (window != nullptr && hooks.release != nullptr) hooks.release(hooks.user, window); } + void ServerDisplay::NoteExtentReportLocked(Uint32 width, Uint32 height) { + ++m_extentReports; + // Only a report made while the layout owns the size says what the layout's size is. + if (m_requestedWidth == 0 && m_requestedHeight == 0) { + m_layoutWidth = width; + m_layoutHeight = height; + } + } + void ServerDisplay::Install(const ServerDisplayHooks& hooks) { { const std::lock_guard lock(m_mutex); @@ -93,6 +102,7 @@ namespace MobileGL::MG_Remote::Server { // is answered, so a waiting AcquireFor is woken. m_width = width; m_height = height; + NoteExtentReportLocked(width, height); lock.unlock(); m_cv.notify_all(); MGLOG_I("MG_Remote server: server window %p is now %ux%u", window, width, height); @@ -112,6 +122,7 @@ namespace MobileGL::MG_Remote::Server { m_window = window; m_width = width; m_height = height; + NoteExtentReportLocked(width, height); ++m_generation; const Uint64 generation = m_generation; lock.unlock(); @@ -166,17 +177,39 @@ namespace MobileGL::MG_Remote::Server { void* cancelUser, ServerWindowLease* out) { ServerDisplayHooks hooks{}; Uint64 interrupts = 0; + const Bool sizeRequested = width != 0 && height != 0; + // P12 review fix (stale size): a 0/0 request after a fixed one hands the size BACK to the + // layout, and until the platform has done so the window still reports the fixed size. + Bool backToLayout = false; + Uint32 previousWidth = 0; + Uint32 previousHeight = 0; + Uint64 reportsAtRequest = 0; { const std::lock_guard lock(m_mutex); if (!m_installed) return ServerWindowAcquire::NoDisplay; if (m_leaseHolder != nullptr && m_leaseHolder != holder) return ServerWindowAcquire::LeasedElsewhere; hooks = m_hooks; interrupts = m_interrupts; + previousWidth = m_requestedWidth; + previousHeight = m_requestedHeight; + backToLayout = !sizeRequested && (previousWidth != 0 || previousHeight != 0); + m_requestedWidth = sizeRequested ? width : 0u; + m_requestedHeight = sizeRequested ? height : 0u; + reportsAtRequest = m_extentReports; } // The geometry request is an up-call into the platform (Java), made without the lock: its // answer comes back as an Attach() of the same window, which needs the lock. if (hooks.requestGeometry != nullptr) hooks.requestGeometry(hooks.user, width, height); - const Bool sizeRequested = width != 0 && height != 0; + // What "the size asked for" means on each path. A fixed size: the window reports it. The + // layout's, after a fixed one: the window reports the layout's extent it reported before, or + // - the layout moved meanwhile (a rotation), or was never seen - reports anything but the old + // fixed size after this request. The layout's, with no fixed size in effect: whatever it is. + const auto sizeReached = [&] { + if (sizeRequested) return m_width == width && m_height == height; + if (!backToLayout) return true; + if (m_layoutWidth != 0 && m_width == m_layoutWidth && m_height == m_layoutHeight) return true; + return m_extentReports != reportsAtRequest && (m_width != previousWidth || m_height != previousHeight); + }; std::unique_lock lock(m_mutex); const auto deadline = Clock::now() + std::chrono::milliseconds(timeoutMs); @@ -193,7 +226,7 @@ namespace MobileGL::MG_Remote::Server { attachedSeen = true; geometryDeadline = std::min(deadline, now + std::chrono::milliseconds(kGeometryGraceMs)); } - const Bool sizeOk = !sizeRequested || (m_width == width && m_height == height); + const Bool sizeOk = sizeReached(); if (sizeOk || now >= geometryDeadline) { m_leaseHolder = holder; m_onLost = onLost; @@ -205,11 +238,16 @@ namespace MobileGL::MG_Remote::Server { out->generation = m_generation; out->sizeAsRequested = sizeOk; } - if (!sizeOk) { + if (!sizeOk && sizeRequested) { MGLOG_W("MG_Remote server: the server window is %ux%u, not the %ux%u requested, after " "%u ms of geometry grace; the surface is created at the window's size and the " "client is told so", m_width, m_height, width, height, kGeometryGraceMs); + } else if (!sizeOk) { + MGLOG_W("MG_Remote server: the server window still reports %ux%u - the previous fixed " + "size - %u ms after its size was handed back to the layout; the surface is created " + "at that size and the client is told so", + m_width, m_height, kGeometryGraceMs); } return ServerWindowAcquire::Acquired; } diff --git a/MobileGL/MG_Remote/Server/ServerDisplay.h b/MobileGL/MG_Remote/Server/ServerDisplay.h index f761db61..e195f7f1 100644 --- a/MobileGL/MG_Remote/Server/ServerDisplay.h +++ b/MobileGL/MG_Remote/Server/ServerDisplay.h @@ -145,6 +145,8 @@ namespace MobileGL::MG_Remote::Server { private: void ReleaseOutsideLock(const ServerDisplayHooks& hooks, void* window) const; + // m_mutex held: an Attach reported `width`x`height` (see m_extentReports). + void NoteExtentReportLocked(Uint32 width, Uint32 height); mutable std::mutex m_mutex; std::condition_variable m_cv; @@ -155,6 +157,16 @@ namespace MobileGL::MG_Remote::Server { Uint32 m_height = 0; Uint64 m_generation = 0; Uint64 m_interrupts = 0; + // P12 review fix (stale size). The geometry the last AcquireFor asked the platform for (0/0 = + // the layout's), the extent the window last reported while the layout owned its size (0/0 = + // not seen yet), and a count of every extent report. A 0/0 request that FOLLOWS a fixed one + // is a change of size too - setSizeFromLayout answers asynchronously, like setFixedSize - so + // it waits for the layout's extent instead of leasing the previous session's fixed one. + Uint32 m_requestedWidth = 0; + Uint32 m_requestedHeight = 0; + Uint32 m_layoutWidth = 0; + Uint32 m_layoutHeight = 0; + Uint64 m_extentReports = 0; void* m_leaseHolder = nullptr; ServerWindowLostHook m_onLost = nullptr; Bool m_lostRequested = false; diff --git a/MobileGL/MG_Test/Wire/ServerDisplayTest.cpp b/MobileGL/MG_Test/Wire/ServerDisplayTest.cpp index 967e2376..9c84ac4c 100644 --- a/MobileGL/MG_Test/Wire/ServerDisplayTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerDisplayTest.cpp @@ -201,6 +201,48 @@ TEST(ServerDisplayTest, ARequestedSizeIsWaitedForUntilTheWindowReportsIt) { display.EndLease(this); } +// P12 review fix (stale size). A session that asks for the window's OWN size after a session that fixed +// it must get the layout's size back, not the fixed one the window still reports until setSizeFromLayout +// lands. Red with AcquireFor granting 0/0 at once (the first version): the lease is 640x480 and comes +// back before the layout's surfaceChanged. +TEST(ServerDisplayTest, TheWindowsOwnSizeAfterAFixedSizeIsTheLayoutsNotTheStaleFixedOne) { + FakePlatform platform; + ServerDisplay display; + display.Install(HooksFor(platform)); + display.Attach(FakeWindow(0), 1080, 2400); // surfaceCreated at the layout's size + // Session 1 fixes 640x480 (setFixedSize answers with surfaceChanged) and ends. + std::thread fixer([&] { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + display.Attach(FakeWindow(0), 640, 480); + }); + ServerWindowLease lease; + ASSERT_EQ(display.AcquireFor(640, 480, 5000, this, nullptr, nullptr, nullptr, &lease), ServerWindowAcquire::Acquired); + fixer.join(); + display.EndLease(this); + // Session 2 asks for the window's own size: setSizeFromLayout answers later, at the layout's size. + std::thread layout([&] { + std::this_thread::sleep_for(std::chrono::milliseconds(150)); + display.Attach(FakeWindow(0), 1080, 2400); + }); + const auto started = std::chrono::steady_clock::now(); + ASSERT_EQ(display.AcquireFor(0, 0, 5000, this, nullptr, nullptr, nullptr, &lease), ServerWindowAcquire::Acquired); + const auto waited = std::chrono::steady_clock::now() - started; + layout.join(); + EXPECT_EQ(platform.lastWidth, 0u) << "0/0 is the setSizeFromLayout request"; + EXPECT_EQ(lease.width, 1080u) << "the lease took the previous session's fixed size"; + EXPECT_EQ(lease.height, 2400u); + EXPECT_TRUE(lease.sizeAsRequested); + EXPECT_GE(waited, std::chrono::milliseconds(100)) << "granted before the layout's size came back"; + EXPECT_LT(waited, std::chrono::milliseconds(ServerDisplay::kGeometryGraceMs)) << "waited out the whole grace"; + display.EndLease(this); + // And a 0/0 request with the layout already owning the size is granted at once, as before. + const auto again = std::chrono::steady_clock::now(); + ASSERT_EQ(display.AcquireFor(0, 0, 5000, this, nullptr, nullptr, nullptr, &lease), ServerWindowAcquire::Acquired); + EXPECT_LT(std::chrono::steady_clock::now() - again, std::chrono::milliseconds(100)); + EXPECT_EQ(lease.width, 1080u); + display.EndLease(this); +} + // D6, THE CONTRACT: Detach of a window a session holds asks the holder to let go, WAITS until it did, // and only then releases the window's reference. Red with Detach's wait deleted: the release is // recorded before the session released its surface (and Detach answers Released, not