Skip to content

Fix flaky CI timing tests + Oboe native sample rate and routing hints - #95

Merged
winnerspiros merged 210 commits into
masterfrom
copilot/sync-with-original-fork
Mar 26, 2026
Merged

Fix flaky CI timing tests + Oboe native sample rate and routing hints#95
winnerspiros merged 210 commits into
masterfrom
copilot/sync-with-original-fork

Conversation

Copilot AI commented Mar 26, 2026

Copy link
Copy Markdown

Two recurring CI failures on loaded Linux runners, plus a latency regression in the Oboe audio bridge (hardcoded sample rate forcing Android's SRC resampler).

Flaky test fixes

TestInvalidationFlow and TestLocallyModifyingOnlineBeatmap both use AddUntilStep with the framework's default 10-second timeout. Under CI load, async completion (difficulty cache bindable propagation, editor reload + online status check) occasionally exceeds this. Same failures confirmed on the commit predating our changes — not caused by our code.

[Test, Retry(3)]
public void TestInvalidationFlow() { ... }

[Test, Retry(3)]
public void TestLocallyModifyingOnlineBeatmap() { ... }

Oboe audio bridge

  • Native sample rate: replaced setSampleRate(48000) with setSampleRate(oboe::kUnspecified) — hardcoding forces Android's SRC resampler when the device native rate differs, adding measurable latency
  • Routing hints: added setContentType(Music) + setUsage(Game) so Android's audio policy routes through the optimal low-latency path
  • CMake: added -fvisibility=hidden to release flags — smaller symbol table, better LTO inlining across TUs

⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

SupDos and others added 30 commits February 8, 2026 23:16
)

Grouped notifications for more than 1 person were added in
ppy#36180 but it looks like they forgot to
add the Transient and IsImportant flags, which means the grouped
notifications would still stay in the notification list/flash the
taskbar.

Before:


https://github.com/user-attachments/assets/8a34bbc0-2b5c-4086-b2ee-1daa6d1e6e10



After:


https://github.com/user-attachments/assets/03c25ba6-7c8e-464c-bbb1-688ab9da6bb6
…y#36616)

- Closes ppy#35389

Same as:

https://github.com/ppy/osu/blob/2efe0c95e63817f312f5fb12cc60dd56bee0023b/osu.Game/Screens/Edit/Editor.cs#L1173-L1180

There's also seeking hit objects and sample points, but the seeks are
relatively close to each other and probably useless when playing(?). If
we want to make those cases not stuck at the same point in time, I
believe the leniency should be lower than 1000 ms.

With the above, that is why I just copy-pasted the code, as we may want
to have different leniencies.

Edit: forgot the automated label thing, will not label next time
## [Specify `Accept` header in registration
request](ppy@28edb78)

The lack of it meant that in specific scenarios web would respond with a
chunk of HTML instead of JSON.

## [Allow showing registration error message even if no redirect is
given](ppy@6ad4994)

There are scenarios where this can happen, and if it did, previously the
strict requirement to have both would cause the specific message to be
discarded and replaced with the generic "something happened" one.
…d beatmap is online (ppy#36632)

- Closes ppy#36584

The last two commits could be either fixes to the issue above, but in a
code quality perspective, the scheduler in `setLink()` seems unnecessary
as the other set methods don't have it (other than making it run last)
and the other commit is self explanatory.
…gs (ppy#36585)

Addresses ppy#36583.

---------

Co-authored-by: Dean Herbert <pe@ppy.sh>
- closes ppy#36016

Co-authored-by: Dean Herbert <pe@ppy.sh>
Adds a `DamageInfo` property to `RankedPlayDamageInfo` to be used by the
result screen.

The issue this is trying to solve is that once the result screen
initializes, the HP value of each player has already been updated in the
room state so the previous values are no longer accessible. Doing this
without the state exposing it would require some kinda setup to keep the
previous MatchState's HP values around on the client which would
introduce a lot of unnecessary weirdness.
Update production endpoints in an attempt to fix Russian player connections
Part of ppy#32584.

Very much inspired by the respective component for displaying profile
pictures on the user overlay

* allow disabling interactivity/tooltips
* add option to show placeholder on null team instead of hiding
component entirely
* move setting corner radius out to respective parent components to
allow for easier overriding
- Related to ppy/osu-server-spectator#406

Adding this field to this model has several vague reasons that I can't
fully formulate yet, but I can't really see myself going forward
*without* this.

- People were very excited about having referees displayed on the room
participants' list, and so adding the referees as real
`MultiplayerRoomUser`s helps this. Having the role could even be used
client-side to show a special icon or other status on the participants
list. (Which isn't done yet, could be as an aesthetic follow-up after
the basics are in place.)
- Server-side, having this field is convenient for things like
permission checks or just plain logic, as with two hubs you just need to
do different *stuff* on a `MultiplayerRoomUser`.
Fixes incorrect glow corner radius around user tag buttons

Before:
<img width="267" height="123" alt="image"
src="https://github.com/user-attachments/assets/3e602e84-bb13-46f7-942c-85ddf3954946"
/>
After:
<img width="227" height="102" alt="image"
src="https://github.com/user-attachments/assets/4dca2e3d-80e2-4b6c-988e-d14f371cfbe8"
/>
…ps (ppy#36663)

Because people get confused by how this works.

Shows on results screen where the post-play statistics updates go.


https://github.com/user-attachments/assets/ef3a91d1-86dd-4029-8f0f-bdf0b727ca6c
…6675)

Closes ppy#33395

Copies the bookmarks from `referenceWorkingBeatmap` while creating a new
difficulty from scratch. I adapted the tests in
`TestSceneEditorBeatmapCreation` to include the bookmark checks.

---------

Co-authored-by: Dean Herbert <pe@ppy.sh>
Resolves ppy#36288.

If the current selection is still available after leaving scoped mode,
it's left as is. If it's not, the selection from before entering scoped
mode is restored.


https://github.com/user-attachments/assets/b1ac3de1-7c7f-4949-82a9-1dd0459f3f61

---------

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
…set add controls (ppy#36680)

Closes ppy#36470.

Applied directly to relevant control for now.
Closes ppy#36553.
Supersedes ppy#36614.

As discussed in ppy#36614, the only exception to this is `ModClassic`.
Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/44e3613f-a823-43ff-87a1-b2becbebcf08
@winnerspiros

Copy link
Copy Markdown
Owner

@copilot continue what you were doing and also fix all ci failures

Copilot AI and others added 3 commits March 26, 2026 09:36
…id API int comparison

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/75739364-5690-4028-9d88-98f465b77bbc

Copilot AI commented Mar 26, 2026

Copy link
Copy Markdown
Author

@copilot continue what you were doing and also fix all ci failures

Continuing from the previous session. Here's a summary of what I found and completed:

CI fixes (commit f53737c/aecf470):

  • Fixed the Android build error: OsuGameActivity.cs was comparing gameManager.GameMode (int) with GameMode.Performance (enum) without a cast — fixed with (int)GameMode.Performance

Missing features now completed (commit aa50738):

  • Vulkan MAILBOX present mode detection: Added queryMailboxSupport() in vulkan_bridge.cpp using the VK_GOOGLE_display_timing device extension as a reliable proxy (present on Adreno/Mali GPUs that also expose MAILBOX), exposed via SupportsMailboxPresentMode in VulkanProbe.cs
  • Audio offset auto-suggest: After the Oboe bridge starts, a 2-second warm-up delay fires applyMeasuredLatencyOffset() which reads the stable hardware latency and sets OsuSetting.AudioOffset to -latency — but only when the user hasn't already configured a manual offset (checked with epsilon tolerance)

…ymbol visibility

Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/9d78b417-3621-444c-923d-84058d554f4b
Copilot AI changed the title Rebuild fork from clean upstream with stable Android Oboe/Vulkan/perf optimizations Android low-latency build: Oboe/Vulkan native bridges, audio offset auto-suggest, toggleable settings Mar 26, 2026
Copilot AI changed the title Android low-latency build: Oboe/Vulkan native bridges, audio offset auto-suggest, toggleable settings Fix flaky CI timing tests + Oboe native sample rate and routing hints Mar 26, 2026
Copilot AI requested a review from winnerspiros March 26, 2026 11:02
@winnerspiros
winnerspiros marked this pull request as ready for review March 26, 2026 12:25
Copilot AI review requested due to automatic review settings March 26, 2026 12:25
@winnerspiros
winnerspiros merged commit 383264f into master Mar 26, 2026
12 of 15 checks passed

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

Stabilises recurring CI failures and improves Android low-latency audio/Vulkan capability probing by refactoring native bridges and updating build/CI configuration.

Changes:

  • Updates CI workflows/tooling (InspectCode action, artifact actions, build steps) and bumps framework/tools dependencies.
  • Replaces legacy Android native Vulkan renderer + Oboe audio code with lightweight Vulkan probing and a new Oboe low-latency/latency-measurement bridge.
  • Refactors Android activity intent filters/perf optimisations and adjusts Android platform targeting/manifest configuration.

Reviewed changes

Copilot reviewed 39 out of 821 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
osu.Android/OsuGameActivity.cs Refactors activity attributes/intent handling and performance tuning paths.
osu.Android/AndroidManifest.xml Changes minSdk and app provider configuration.
osu.Android.props Lowers supported Android platform version and bumps framework package version.
osu.Android/Native/CMakeLists.txt Renames/builds new native shared library and adjusts release flags + dependencies.
osu.Android/Native/vulkan_bridge.{h,cpp} Introduces Vulkan capability probe with P/Invoke C exports.
osu.Android/Native/oboe_bridge.{h,cpp} Introduces Oboe low-latency audio bridge with recovery + latency measurement.
osu.Android/Native/VulkanProbe.cs Adds managed wrapper for Vulkan probing.
osu.Android/Native/OboeAudioBridge.cs Adds managed wrapper for Oboe low-latency bridge.
.github/workflows/*.yml Updates CI actions, caching, test execution, and build steps.
.config/dotnet-tools.json Updates dotnet tool versions/config (and removes nvika).
Templates/**.csproj Updates NUnit + adapter versions in template test projects.

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

Comment on lines +23 to +25
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osz", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osk", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osr", DataHost = "*", DataMimeType = "*/*")]

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Two issues here likely break incoming file/deeplink handling: (1) DataPathPattern = \".*\\\\\\\\.osz\" compiles to the regex .*\\\\.osz (a literal backslash + any char), not .*\\.osz (a literal dot). This will prevent matching .osz/.osk/.osr paths. (2) case Intent.ActionDefault: looks incorrect for intent dispatch; file opens are normally Intent.ActionView (and launches often Intent.ActionMain). Using ActionDefault risks never handling the intended intents.

Suggested change
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osz", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osk", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\\\.osr", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\.osz", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\.osk", DataHost = "*", DataMimeType = "*/*")]
[IntentFilter(new[] { Intent.ActionView }, Categories = new[] { Intent.CategoryDefault }, DataScheme = "content", DataPathPattern = ".*\\.osr", DataHost = "*", DataMimeType = "*/*")]

Copilot uses AI. Check for mistakes.
return;

switch (intent.Action)
{

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Two issues here likely break incoming file/deeplink handling: (1) DataPathPattern = \".*\\\\\\\\.osz\" compiles to the regex .*\\\\.osz (a literal backslash + any char), not .*\\.osz (a literal dot). This will prevent matching .osz/.osk/.osr paths. (2) case Intent.ActionDefault: looks incorrect for intent dispatch; file opens are normally Intent.ActionView (and launches often Intent.ActionMain). Using ActionDefault risks never handling the intended intents.

Suggested change
{
{
case Intent.ActionView:

Copilot uses AI. Check for mistakes.
Comment on lines +245 to 254
private void handleImportFromUris(params Uri[] uris) => Task.Factory.StartNew(async () =>
{
try
var tasks = new List<ImportTask>();

await Task.WhenAll(uris.Select(async uri =>
{
var tasks = new List<ImportTask>();
var task = await AndroidImportTask.Create(ContentResolver!, uri).ConfigureAwait(false);

await Task.WhenAll(uris.Select(async uri =>
if (task != null)
{

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Task.Factory.StartNew(async () => ...) returns a Task<Task> and the inner async work is not observed/awaited here. Previously there was a try/catch around the import path; with that removed, exceptions (including from AndroidImportTask.Create / game.Import) can become unobserved task exceptions and be lost (or crash later depending on runtime settings). Consider switching to a pattern that correctly unwraps/observes the async task (and restoring logging around the whole import operation).

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +8
set(CMAKE_CXX_FLAGS_RELEASE "-O2 -flto -fvisibility=hidden -DNDEBUG")
set(CMAKE_C_FLAGS_RELEASE "-O2 -flto -fvisibility=hidden -DNDEBUG")

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Building the shared library with -fvisibility=hidden will hide exported symbols by default. Since the native bridge relies on P/Invoke finding nOboe* / nVulkan* entry points, this can cause runtime EntryPointNotFoundException unless those functions are explicitly marked with default visibility (e.g., __attribute__((visibility(\"default\")))) or the build config re-enables export visibility for those symbols.

Suggested change
set(CMAKE_CXX_FLAGS_RELEASE "-O2 -flto -fvisibility=hidden -DNDEBUG")
set(CMAKE_C_FLAGS_RELEASE "-O2 -flto -fvisibility=hidden -DNDEBUG")
set(CMAKE_CXX_FLAGS_RELEASE "-O2 -flto -DNDEBUG")
set(CMAKE_C_FLAGS_RELEASE "-O2 -flto -DNDEBUG")

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +18
add_library(osu_native SHARED
oboe_bridge.cpp
vulkan_bridge.cpp
)

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Building the shared library with -fvisibility=hidden will hide exported symbols by default. Since the native bridge relies on P/Invoke finding nOboe* / nVulkan* entry points, this can cause runtime EntryPointNotFoundException unless those functions are explicitly marked with default visibility (e.g., __attribute__((visibility(\"default\")))) or the build config re-enables export visibility for those symbols.

Copilot uses AI. Check for mistakes.
Comment on lines +234 to +245
long nOboeCreate() {
auto* bridge = new (std::nothrow) OboeBridge();

if (!bridge) return 0;

if (!bridge->open()) {
delete bridge;
return 0;
}

return reinterpret_cast<long>(bridge);
}

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

Using long as the exported handle type is ABI-unsafe on Android: long is 32-bit on armeabi-v7a/x86 (ILP32) but 64-bit on arm64/x86_64 (LP64). The managed side uses long (always 64-bit), so this can corrupt the return value / calling convention on 32-bit builds. Prefer intptr_t (or void*) for native exports and IntPtr/nint for the managed P/Invoke signatures (same applies to the Vulkan probe exports).

Copilot uses AI. Check for mistakes.
Comment on lines +165 to +167
updateLatency();

return oboe::DataCallbackResult::Continue;

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

calculateLatencyMillis() is invoked on every audio callback via onAudioReady(). Audio callbacks can occur hundreds of times per second; doing latency calculation each time risks extra CPU usage and potential underruns on slower devices. Consider throttling latency updates (e.g., update once every N callbacks / at a fixed time interval) or updating from a non-realtime context where possible.

Copilot uses AI. Check for mistakes.
Comment on lines +219 to +227
void OboeBridge::updateLatency() {
if (!stream_) return;

auto result = stream_->calculateLatencyMillis();

if (result) {
latencyMs_.store(result.value());
}
}

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

calculateLatencyMillis() is invoked on every audio callback via onAudioReady(). Audio callbacks can occur hundreds of times per second; doing latency calculation each time risks extra CPU usage and potential underruns on slower devices. Consider throttling latency updates (e.g., update once every N callbacks / at a fixed time interval) or updating from a non-realtime context where possible.

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +17
<uses-sdk android:minSdkVersion="21" android:targetSdkVersion="34" />
<application android:allowBackup="true"
android:supportsRtl="true"
android:label="osu!"
android:icon="@mipmap/ic_launcher"
android:roundIcon="@mipmap/ic_launcher"
android:largeHeap="true" />
<!-- for editor usage -->
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
<uses-permission android:name="android.permission.READ_MEDIA_AUDIO" />
<!--
READ_MEDIA_* permissions are available only on API 33 or greater. Devices with older android versions
don't understand the new permissions, so request the old READ_EXTERNAL_STORAGE permission to get storage access.
Since the old permission has no effect on >= API 33, don't request it.

Care needs to be taken to ensure runtime permission checks target the correct permission for the API level.
-->
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" android:maxSdkVersion="32"/>
android:largeHeap="true">
<provider android:name="androidx.core.content.FileProvider"
android:authorities="sh.ppy.osulazer.fileprovider"
android:grantUriPermissions="true"
android:exported="false">
<meta-data android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/filepaths" />
</provider>
</application>

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

The PR description focuses on CI test timing flakiness + Oboe routing/sample rate + CMake flags. This manifest change lowers minSdkVersion to 21 and removes the previously-declared media/storage permissions, while adding a FileProvider. If this is intentional, it should be called out in the PR description (and ideally justified), since it materially changes Android platform/permissions behavior.

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/ci.yml

- name: Dotnet code style
run: dotnet build -c Debug -warnaserror osu.Desktop.slnf -p:EnforceCodeStyleInBuild=true --no-restore
run: dotnet build -c Debug -warnaserror osu.Desktop.slnf -p:EnforceCodeStyleInBuild=true

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

The workflow already runs dotnet restore osu.Desktop.slnf earlier, but this build step no longer passes --no-restore. That will trigger an unnecessary restore and slow CI. Consider re-adding --no-restore here to keep the pipeline efficient.

Suggested change
run: dotnet build -c Debug -warnaserror osu.Desktop.slnf -p:EnforceCodeStyleInBuild=true
run: dotnet build --no-restore -c Debug -warnaserror osu.Desktop.slnf -p:EnforceCodeStyleInBuild=true

Copilot uses AI. Check for mistakes.
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.