Skip to content

Force MultiThreaded execution mode and remove SingleThread option - #237

Merged
winnerspiros merged 2 commits into
masterfrom
copilot/analyze-android-crash-logs
Apr 22, 2026
Merged

Force MultiThreaded execution mode and remove SingleThread option#237
winnerspiros merged 2 commits into
masterfrom
copilot/analyze-android-crash-logs

Conversation

Copilot AI commented Apr 22, 2026

Copy link
Copy Markdown
  • Force ExecutionMode = MultiThreaded on game load in OsuGameBase (clears any persisted SingleThread value, all OSes)
  • Remove Threading Mode dropdown from RendererSettings
  • Remove unused ThreadingMode localisation string
  • Remove TestSwitchThreadExecutionMode test that exercised the removed UI path
  • Drop SingleThread entry from the CI test matrix (no longer a user-reachable configuration; halves CI test time)
  • README: remove stale Threading mode renderer option row; add stability bullet documenting the MultiThreaded lock-in and Android Vulkan-surface root cause
  • Reviewed for "unlocked" optimizations — none are safe to apply: scheduler hops between Update/Draw/Audio/Input threads still exist in MultiThreaded, and the two single-threaded execution workaround comments in PlayButton files remain harmless under MultiThreaded. Removing them would be a behaviour change with no clear win.

SingleThread is incompatible with the Android Vulkan surface-readiness
path (VeldridDevice polls SurfaceHandle for up to 5s on the same thread
that publishes it in SingleThread mode -> deadlock -> null Vulkan func
ptr -> SDLThread SI_TKILL crash within ~5s of launch, which matches the
tablet crash report).

The same pattern would also bite iOS Metal drawable attach. On desktop
SingleThread is just slower with no UX benefit. Upstream ppy/osu defaults
to MultiThreaded; we now enforce it on every startup so a stale
framework.ini cannot pin a user into the broken mode.

- OsuGameBase.load: force ExecutionMode = MultiThreaded once on load.
- RendererSettings: drop the Threading Mode dropdown.
- GraphicsSettingsStrings: drop the now-unused ThreadingMode string.
- TestSceneOsuGame: drop TestSwitchThreadExecutionMode (covered behaviour
  is no longer user-reachable) and prune the now-unused usings.

Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/cccaf70f-1049-4fde-a21c-cd61a27bf8b2

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
- ci.yml: matrix.threadingMode reduced to ['MultiThreaded']. SingleThread
  is no longer a user-reachable mode (force-overridden in OsuGameBase),
  so testing under it just races the override and burns CI for an
  unsupported configuration.
- README.md: drop the stale "Threading mode" row from the renderer
  options table; add a stability bullet documenting the MultiThreaded
  lock-in and the Android Vulkan-surface root cause.
- No changes to runtime/src/test code beyond the previous PR; no
  optimizations are *unlocked* purely by the lock-in (Update / Draw /
  Audio / Input threads were already separate in MultiThreaded), so
  nothing else is touched.

Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/89c57795-45d9-40ed-a99e-dd17caa8767d

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
@winnerspiros
winnerspiros marked this pull request as ready for review April 22, 2026 12:14
Copilot AI review requested due to automatic review settings April 22, 2026 12:14
@winnerspiros
winnerspiros merged commit b807c1e into master Apr 22, 2026
13 of 15 checks passed
@gitar-bot

gitar-bot Bot commented Apr 22, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

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.

Pull request overview

Locks the game/framework into multi-threaded execution by removing the user-facing SingleThread selection and ensuring stale config can’t persist a broken mode, while also reducing CI runtime by no longer testing an unreachable configuration.

Changes:

  • Remove the Threading Mode (ExecutionMode) dropdown from Graphics → Renderer settings and delete its localisation string.
  • Force FrameworkSetting.ExecutionMode to ExecutionMode.MultiThreaded on startup.
  • Remove the UI-path test and drop SingleThread from the CI test matrix; update README accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
osu.Game/Overlays/Settings/Sections/Graphics/RendererSettings.cs Removes the settings UI control for choosing execution mode.
osu.Game/OsuGameBase.cs Forces MultiThreaded execution on startup to prevent persisted SingleThread usage.
osu.Game/Localisation/GraphicsSettingsStrings.cs Removes the now-unused “Threading mode” localisation string.
osu.Game.Tests/Visual/Navigation/TestSceneOsuGame.cs Removes a test that exercised switching execution modes via the removed UI/config path.
README.md Updates documented renderer settings and adds a stability note explaining the lock-in rationale.
.github/workflows/ci.yml Removes SingleThread from the CI test matrix to reduce runtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread osu.Game/OsuGameBase.cs
Comment on lines +332 to +345
// Force the framework execution mode to MultiThreaded on every startup.
//
// SingleThread used to be exposed in the graphics settings, but it is incompatible
// with our Android Vulkan-surface readiness path: VeldridDevice / AndroidGameHost
// poll AndroidGameWindow.SurfaceHandle for up to 5s, and in SingleThread mode that
// poll runs on the same thread that publishes the surface handle, deadlocking
// initialisation until the timeout fires and Vulkan device creation crashes with a
// null function pointer (see PR history around the SDLThread/SI_TKILL crash).
//
// The same risk applies to iOS Metal layer attach (drawable surfaces are also
// delivered via the main loop), and on desktop SingleThread is strictly slower with
// no UX benefit. Upstream ppy/osu defaults to MultiThreaded; we now enforce it
// unconditionally so a stale `framework.ini` cannot pin a user into the broken mode.
frameworkConfig.SetValue(FrameworkSetting.ExecutionMode, ExecutionMode.MultiThreaded);
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.

3 participants