mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 01:53:24 +09:00
[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.
This commit is contained in:
@@ -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<std::mutex> 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<std::mutex> 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<std::mutex> 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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user