Skip to content

Commit 16cec58

Browse files
Fix Vulkan surface teardown loop causing 9-screen tiling and bad FPS on Android
The DecorView.Post lambda was calling holder.SetFormat(Rgba8888) unconditionally every startup, even when the surface was already born with the correct format via Window.SetFormat(Rgba8888) before base.OnCreate(). On Samsung/Qualcomm devices, any SurfaceHolder.setFormat() call — even a no-op — triggers SurfaceDestroyed+ SurfaceCreated. During the draw-thread stall Veldrid called vkGetPhysicalDeviceSurfaceCapabilitiesKHR on a mid-transition ANativeWindow that reported dp-scaled dimensions (1029x480) instead of physical pixels (3088x1440), permanently baking a 1/9-scale swapchain: 9-screen tiling, blurry text, animated UI flashes, and bad FPS throughout the session. Additionally, AddCallback(this) was called AFTER SetFormat, so the synchronous SurfaceChanged fired with the pre-change format, triggering the reactive RGB565 guard to call SetFormat a second time — chaining two back-to-back teardowns. Fix: - Move holder.AddCallback(this) BEFORE the SetFormat decision so SurfaceChanged fires synchronously and populates lastSurfaceFormat first - Add lastSurfaceFormat (volatile int): tracks format from last SurfaceChanged - Add setFormatPending (volatile bool): debounce flag preventing chained SetFormats - SurfaceCreated: reset both flags for new surface lifecycle - SurfaceChanged: record lastSurfaceFormat; gate RGB565 guard on !setFormatPending - DecorView.Post: only call SetFormat when lastSurfaceFormat is non-zero and non-Rgba8888 and !setFormatPending — skipping the no-op call entirely when Window.SetFormat already gave us the correct surface format Normal path (Window.SetFormat worked): zero teardowns, correct swapchain from frame 1 Fallback path (surface born RGB565): one teardown via reactive guard, no chaining Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/5c98b728-804c-45e7-b271-05e8ea071ff3 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
1 parent 0273184 commit 16cec58

1 file changed

Lines changed: 80 additions & 18 deletions

File tree

osu.Android/OsuGameActivity.cs

Lines changed: 80 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -276,27 +276,51 @@ protected override void OnCreate(Bundle? savedInstanceState)
276276

277277
if (holder != null)
278278
{
279-
// Only request RGBA8888 on the SurfaceHolder when the configured
280-
// renderer is Vulkan. SDL3 already calls setFormat(RGBA8888) for
281-
// OpenGL/GLES from its own EGL surface initialization, and calling
282-
// setFormat() a second time AFTER SDL has bound its EGL surface
283-
// forces Android to recreate the surface (surfaceDestroyed →
284-
// surfaceCreated → surfaceChanged) mid-frame. That:
285-
// 1. Trips the 250ms drawThreadAcknowledgedTeardown wait in
286-
// AndroidGameSurface.SurfaceDestroyed (visible warning in HUD).
287-
// 2. Leaves SDL's EGL surface bound to a destroyed ANativeWindow,
288-
// causing eglSwapBuffers to silently no-op → permanent black
289-
// screen while Update/Audio/Input keep running.
290-
// SDL3 does NOT call setFormat on the Vulkan path, so Vulkan still
291-
// needs this stamping to avoid the RGB565 default that crashes Adreno.
279+
// Register our lifecycle callback FIRST so that if the surface is
280+
// already alive, SurfaceChanged fires synchronously here and populates
281+
// lastSurfaceFormat before we decide whether a proactive SetFormat is
282+
// needed. This is critical: calling SetFormat(Rgba8888) when the surface
283+
// already has the correct format triggers SurfaceDestroyed+SurfaceCreated
284+
// on Samsung/Qualcomm devices even for a no-op change. During the
285+
// resulting draw-thread stall, Veldrid calls
286+
// vkGetPhysicalDeviceSurfaceCapabilitiesKHR on a mid-transition
287+
// ANativeWindow that reports dp-scaled dimensions (e.g. 1029×480 on a
288+
// 3088×1440 3×-density panel) instead of physical pixels. That
289+
// permanently bakes a 1/9-scale swapchain — producing 9-screen tiling,
290+
// blurry text (layout calculated at dp-scale), animated flashes, and
291+
// bad FPS throughout the session.
292+
holder.AddCallback(this);
293+
294+
// After AddCallback, lastSurfaceFormat reflects what SurfaceChanged
295+
// reported synchronously (if the surface was already alive), or 0 if
296+
// the surface has not been created yet.
297+
//
298+
// Only stamp RGBA8888 on the SurfaceHolder when Vulkan is configured
299+
// AND the surface exists with a wrong format AND the reactive guard
300+
// in SurfaceChanged has not already queued a SetFormat for this cycle.
301+
//
302+
// - lastSurfaceFormat == Rgba8888 : surface was born correct (typical
303+
// when Window.SetFormat ran before base.OnCreate); no action needed.
304+
// - lastSurfaceFormat == 0 : surface not yet alive; the reactive
305+
// guard will stamp it via SurfaceChanged when it arrives.
306+
// - setFormatPending == true : SurfaceChanged reactive guard already
307+
// called SetFormat; issuing a second call would chain teardowns.
308+
// - any other format : surface exists but is wrong; stamp it.
292309
bool isVulkan = false;
293310
try { isVulkan = LogManagement.IsVulkanConfigured(); }
294311
catch (Exception e) { Debug.WriteLine($"[osu!] SurfaceHolder format gate: IsVulkanConfigured failed, defaulting to skip SetFormat: {e.Message}"); }
295312

296-
if (isVulkan)
313+
// lastSurfaceFormat and setFormatPending are only written by
314+
// SurfaceCreated/SurfaceChanged, which (like this Post lambda) run
315+
// on the main UI thread — no concurrent access is possible here.
316+
if (isVulkan
317+
&& lastSurfaceFormat != 0
318+
&& lastSurfaceFormat != (int)global::Android.Graphics.Format.Rgba8888
319+
&& !setFormatPending)
297320
{
298321
try
299322
{
323+
setFormatPending = true;
300324
holder.SetFormat(global::Android.Graphics.Format.Rgba8888);
301325
Logger.Log("[osu!] SurfaceHolder.SetFormat(Rgba8888) applied (Vulkan renderer).", LoggingTarget.Runtime, LogLevel.Important);
302326
}
@@ -305,12 +329,10 @@ protected override void OnCreate(Bundle? savedInstanceState)
305329
Debug.WriteLine($"[osu!] Failed to request RGBA8888 surface format: {e.Message}");
306330
}
307331
}
308-
else
332+
else if (!isVulkan)
309333
{
310334
Logger.Log("[osu!] SurfaceHolder.SetFormat skipped (OpenGL/Auto renderer — SDL3 handles format).", LoggingTarget.Runtime, LogLevel.Debug);
311335
}
312-
313-
holder.AddCallback(this);
314336
}
315337

316338
// Also hide the pointer icon on the SurfaceView itself.
@@ -605,6 +627,28 @@ await Task.WhenAll(uris.Select(async uri =>
605627
private global::Android.Views.Surface? heldSurface;
606628
private IntPtr surfaceGlobalRef;
607629

630+
// Pixel format (as an int cast of Android.Graphics.Format) from the most recent
631+
// SurfaceChanged callback for this surface lifetime, or 0 if SurfaceChanged has not
632+
// yet fired.
633+
//
634+
// Threading: ISurfaceHolderCallback methods (SurfaceCreated / SurfaceChanged /
635+
// SurfaceDestroyed) are guaranteed by Android to be called on the main UI thread.
636+
// The DecorView.Post lambda that reads this field also runs on the main UI thread.
637+
// Accesses are therefore single-threaded with no cross-thread races; volatile is
638+
// retained only as a compiler-reordering barrier.
639+
private volatile int lastSurfaceFormat;
640+
641+
// True between a SetFormat(Rgba8888) call (from the DecorView.Post lambda or the
642+
// SurfaceChanged reactive guard) and the SurfaceCreated that follows the resulting
643+
// surface recreate. Guards against issuing a second SetFormat before the first
644+
// teardown+recreate cycle has completed, which would chain two back-to-back teardowns
645+
// and leave the draw thread unable to acknowledge either within 250 ms.
646+
//
647+
// Threading: same UI-thread-only guarantee as lastSurfaceFormat above. The
648+
// check-then-set in SurfaceChanged (lines ~732-734) is not a concurrency concern
649+
// because no two SurfaceChanged calls can overlap on the single UI thread.
650+
private volatile bool setFormatPending;
651+
608652
public IntPtr GetSurfaceGlobalRef()
609653
{
610654
if (!surfaceEvent.Wait(5000))
@@ -638,6 +682,12 @@ public void SurfaceCreated(ISurfaceHolder holder)
638682
if (handle == IntPtr.Zero)
639683
return;
640684

685+
// Reset per-lifecycle flags. The new surface has not yet reported its format
686+
// (SurfaceChanged fires after SurfaceCreated), and any previous pending-format
687+
// stamp no longer applies to this new surface instance.
688+
lastSurfaceFormat = 0;
689+
setFormatPending = false;
690+
641691
IntPtr newRef = global::Android.Runtime.JNIEnv.NewGlobalRef(handle);
642692

643693
lock (surfaceLock)
@@ -664,6 +714,11 @@ public void SurfaceCreated(ISurfaceHolder holder)
664714

665715
public void SurfaceChanged(ISurfaceHolder holder, global::Android.Graphics.Format format, int width, int height)
666716
{
717+
// Record the current surface format so the DecorView.Post lambda can decide
718+
// whether a proactive SetFormat(Rgba8888) is needed without calling SetFormat
719+
// unconditionally (which always triggers a teardown on Samsung/Adreno devices).
720+
lastSurfaceFormat = (int)format;
721+
667722
// Guard: if the Android surface materialised with a 16-bit pixel format (RGB565)
668723
// while Vulkan is configured, request a format change to RGBA8888 immediately.
669724
//
@@ -681,8 +736,15 @@ public void SurfaceChanged(ISurfaceHolder holder, global::Android.Graphics.Forma
681736
// windows). Calling SetFormat here triggers SurfaceDestroyed + SurfaceCreated +
682737
// SurfaceChanged with the corrected format; Veldrid's VkSurfaceKHR-loss recovery
683738
// picks up the new ANativeWindow and negotiates a proper BGRA/RGBA 8-bit swapchain.
684-
if (format == global::Android.Graphics.Format.Rgb565 && LogManagement.IsVulkanConfigured())
739+
//
740+
// setFormatPending prevents this guard from issuing a second SetFormat call when
741+
// the surface that arrives after the first teardown also briefly reports RGB565
742+
// (e.g. during a compositor mode transition), which would chain teardowns and
743+
// prevent the draw thread from ever acknowledging either one within 250 ms.
744+
if (format == global::Android.Graphics.Format.Rgb565 && LogManagement.IsVulkanConfigured() && !setFormatPending)
685745
{
746+
setFormatPending = true;
747+
686748
// Log to Runtime so the mid-session RGB565 reset is visible in the main log
687749
// (and therefore in the notification overlay). Performance log gets the same
688750
// entry for correlation with display-mode and frame-timing data.

0 commit comments

Comments
 (0)