Skip to content

fix: prevent black screen by setting surface event before format change - #354

Closed
winnerspiros wants to merge 1 commit into
masterfrom
fix/black-screen-surface-teardown
Closed

fix: prevent black screen by setting surface event before format change#354
winnerspiros wants to merge 1 commit into
masterfrom
fix/black-screen-surface-teardown

Conversation

@winnerspiros

@winnerspiros winnerspiros commented May 23, 2026

Copy link
Copy Markdown
Owner

See commit message for details. Fixes black screen on launch caused by surface format change blocking the draw thread.

Summary by CodeRabbit

  • Bug Fixes
    • Optimized graphics rendering synchronization on Android to prevent frame processing delays during color format transitions. Addressed a timing issue that could cause temporary performance stalls on certain devices.

Review Change Stack

When the surface is born as RGB565 on Vulkan, the RGB565 guard in
SurfaceChanged calls SetFormat(RGBA8888) to trigger a surface recreate.
Previously, this reset surfaceEvent and returned early, blocking the
draw thread on surfaceEvent.Wait(5000) while the UI thread was stuck
in the synchronous SetFormat call. This blocked the entire managed
runtime from sending heartbeats, causing the native watchdog to fire
at 10s and producing a black screen.

The fix sets surfaceEvent BEFORE calling SetFormat, so the draw thread
can proceed with the current (soon-to-be-recreated) surface. The new
surface triggers another SurfaceChanged which sets surfaceEvent again
with the correct RGBA8888 format.

Fixes the issue where the APK shows a black screen after the last build.
Copilot AI review requested due to automatic review settings May 23, 2026 18:38
@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c1da771-ae75-4a4f-be42-5263048b7952

📥 Commits

Reviewing files that changed from the base of the PR and between 59300b0 and 90dc644.

📒 Files selected for processing (1)
  • osu.Android/OsuGameActivity.cs

📝 Walkthrough

Walkthrough

This PR adjusts the Vulkan RGB565 recovery synchronization in OsuGameActivity.SurfaceChanged. When RGB565 is detected under Vulkan, surfaceEvent is now signaled before calling holder.SetFormat(Rgba8888) to prevent the draw thread from blocking during the synchronous surface teardown, followed by dedicated logging and removal of an immediate reset.

Changes

Vulkan RGB565 Recovery Synchronization

Layer / File(s) Summary
Vulkan RGB565 recovery synchronization
osu.Android/OsuGameActivity.cs
When RGB565 is detected under Vulkan, surfaceEvent is set before requesting the format change (guarded by valid width/height), a dedicated debug log message is added after SetFormat, and the previous immediate surfaceEvent.Reset() is removed to allow the draw thread to progress during synchronous teardown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

The change involves careful synchronization logic with threading implications. Understanding the interaction between surfaceEvent, SetFormat teardown behavior, and draw-thread timing requires examining the complete control flow and potential race conditions. The logic is concentrated in one method but demands careful review of synchronization guarantees and state transitions.

Poem

🐰 A surface needs sync when RGB565 shows,
Set the signal first, before teardown flows,
Draw-thread keeps dancing while formats align,
No blocking here—just perfect timing design! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: prevent black screen by setting surface event before format change' directly and clearly describes the main change: preventing a black screen by adjusting when the surface event is set relative to the format change call, which is exactly what the changeset accomplishes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/black-screen-surface-teardown

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@winnerspiros

Copy link
Copy Markdown
Owner Author

Closing — the fix introduces a data race between the draw thread reading surfaceGlobalRef and SurfaceDestroyed freeing it. The correct fix needs to handle the surface lifecycle more carefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants