Merge pull request #264 from winnerspiros/copilot/fix-logging-issues-and-prioritize-fork

Android: fix S Pen/mouse/keyboard input, hardware Back, import errors, sync upstream, pin Oboe

Author: winnerspiros

README.md

@@ -179,17 +179,14 @@ Settings → Graphics → Renderer now exposes the full set of fork-added option
 
 ---
 
-### 🛡️ Stability improvements
+### 🛡️ Robustness improvements
 
-This fork includes several hardening fixes on top of upstream:
+A few hardening fixes on top of upstream that are not directly performance-related but keep the app behaving correctly across edge cases:
 
-- **Multi-threaded execution lock-in (v145+)** — the framework's `ExecutionMode = SingleThread` is force-set to `MultiThreaded` on every startup and the threading-mode toggle is removed from Settings → Graphics → Renderer. SingleThread on Android collapsed the SDL/Vulkan thread onto the same thread that delivers the `SurfaceHolder.Callback`, so `VeldridDevice`'s 5-second `SurfaceHandle` poll deadlocked and Vulkan device creation crashed on a null function pointer (`SDLThread` `SI_TKILL` ~5 s into launch). The same risk applies to iOS Metal drawable attach; on desktop SingleThread is strictly slower with no UX benefit, so the lock-in is unconditional across all platforms.
-- **Sentry-safe init** — the app gracefully handles a missing/placeholder Sentry DSN instead of failing on startup
-- **Graceful native library loading** — if the Oboe or Vulkan native libraries are missing, the app continues without them
-- **JNI surface safety** — proper lifecycle management with atomic swaps and timeouts to prevent race conditions between Android surface creation and destruction
-- **Trimmer-safe builds** — critical reflection-heavy assemblies are protected from .NET IL trimming so release builds behave the same as debug
-- **Architecture-correct native libraries (v144+)** — `osu.Android.props` strips desktop runtime `.so` files (`runtimes/{linux,osx,ios,maccatalyst,win,…}-*/native/`) from the Android publish set and only marks Android-RID assets as `AssetType=native`, so the proper Android arm64 BASS libraries from `ppy.osu.Framework.Android`'s AAR (`jni/arm64-v8a/`) always win over the desktop `.so` files transitively pulled in by `ppy.osu.Framework.NativeLibs`. The release workflow scans every shipped `libbass*.so` for `GLIBC_*` versioned symbols (only present in glibc-linked Linux ELFs) and fails the build if any are found, so an architecture mismatch can never reach a release.
-- **IPC / WebSocket hardening (v144+)** — desktop external-integrations server (env-var gated, `localhost`-only) tightened on top of upstream: `WebSocketChannel` now uses a strict `UTF8Encoding(throwOnInvalidBytes: true)` decoder so malformed payloads are rejected with `InvalidPayloadData` instead of being silently replaced with `U+FFFD`, and the message-size guard accepts payloads of exactly `max_message_size` bytes (was off-by-one); `WebSocketServer.Dispose()` now cancels the request loop and waits briefly for it to exit before tearing down the cancellation/reset-event handles to avoid an `ObjectDisposedException` race on shutdown; `OsuWebSocketProvider.Dispose()` swaps the server reference under a local, properly disposes the bounded `CancellationTokenSource` via `using`, and always disposes the `WebSocketServer` in a `finally` so listener handles can't leak across screen transitions.
+- **Multi-threaded execution lock-in (v145+)** — the framework's `ExecutionMode = SingleThread` is force-set to `MultiThreaded` on every startup and the threading-mode toggle is removed from Settings → Graphics → Renderer. SingleThread is strictly slower than MultiThreaded with no UX benefit, so the lock-in is unconditional across all platforms.
+- **Sentry-safe init** — the app gracefully handles a missing/placeholder Sentry DSN.
+- **Architecture-correct native libraries (v144+)** — `osu.Android.props` strips desktop runtime `.so` files (`runtimes/{linux,osx,ios,maccatalyst,win,…}-*/native/`) from the Android publish set and only marks Android-RID assets as `AssetType=native`, so the proper Android arm64 BASS libraries from `ppy.osu.Framework.Android`'s AAR (`jni/arm64-v8a/`) always win over the desktop `.so` files transitively pulled in by `ppy.osu.Framework.NativeLibs`. The release workflow scans every shipped `libbass*.so` for `GLIBC_*` versioned symbols (only present in glibc-linked Linux ELFs) and fails the build if any are found.
+- **IPC / WebSocket polish (v144+)** — desktop external-integrations server (env-var gated, `localhost`-only) tightened on top of upstream: `WebSocketChannel` now uses a strict `UTF8Encoding(throwOnInvalidBytes: true)` decoder so malformed payloads are rejected with `InvalidPayloadData` instead of being silently replaced with `U+FFFD`, and the message-size guard accepts payloads of exactly `max_message_size` bytes (was off-by-one); `WebSocketServer.Dispose()` cancels the request loop and waits briefly for it to exit before tearing down the cancellation/reset-event handles; `OsuWebSocketProvider.Dispose()` swaps the server reference under a local, properly disposes the bounded `CancellationTokenSource` via `using`, and always disposes the `WebSocketServer` in a `finally` so listener handles can't leak across screen transitions.
 - **Ranked-play song-preview playback (v144+)** — restored the `Enabled`/`CardHovered` → `PreviewTrack.Start()/Stop()` wiring on `RankedPlayCard.SongPreviewContainer` that was lost in upstream's "playback rewrite" merge. The bind now happens in the `LoadComponentAsync` continuation (so previews never race the track's async load), with both bindables driving a single `updatePlaybackState()` callback.
 
 ---
@@ -251,11 +248,11 @@ The [winnerspiros/osu-framework](https://github.com/winnerspiros/osu-framework)
 **Platform targeting:**
 - Full `osu.Framework.Android` / `osu.Framework.iOS` implementations.
 - Android minimum bumped to **API 33** (matches app manifest), target API 36.
-- Android release config: profiled AOT (`AndroidEnableProfiledAot`), partial trimming, `AndroidStripILAfterAOT=false` (avoids `plt_entry` crashes), no LLVM (incompatible with profiled AOT).
+- Android release config: profiled AOT (`AndroidEnableProfiledAot`), partial trimming, `AndroidStripILAfterAOT=false`, no LLVM (incompatible with profiled AOT).
 - iOS: `SupportedOSPlatformVersion` 13.4, trim-analysis warnings suppressed with `[DynamicallyAccessedMembers]` and `[UnconditionalSuppressMessage]`.
 
-**Stability fixes consumed by the Android build:**
-- Null `ANativeWindow` guard in `VkSurfaceUtil` prevents SIGSEGV at `pc=0x0` from invalid Vulkan function pointers.
+**Android-specific framework polish:**
+- Null `ANativeWindow` guard in `VkSurfaceUtil`.
 - `VeldridDevice` polls `SurfaceHandle` for up to 5 s when the Android surface is not yet ready.
 - `DrawThread.OnInitialize()` wraps the initial `BeginFrame` in try-catch for graceful handling before surface readiness.
 - NRE fix in `GraphicsPipeline.cs` (null-conditional `ResourceLayouts?.Length`).

osu.Android/AndroidImportTask.cs

@@ -1,12 +1,14 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System;
 using System.IO;
 using System.Threading.Tasks;
 using Android.Content;
-using Android.Net;
 using Android.Provider;
+using osu.Framework.Logging;
 using osu.Game.Database;
+using Uri = Android.Net.Uri;
 
 namespace osu.Android
 {
@@ -25,7 +27,24 @@ private AndroidImportTask(Stream stream, string filename, ContentResolver conten
 
         public override void DeleteFile()
         {
-            contentResolver.Delete(uri, null, null);
+            try
+            {
+                contentResolver.Delete(uri, null, null);
+            }
+            catch (Java.Lang.SecurityException e)
+            {
+                // Some third-party file managers (notably MIUI's `com.android.fileexplorer`)
+                // share content URIs without granting the receiving app write permission, so
+                // the post-import source-file delete throws SecurityException. The import
+                // itself has already succeeded by this point; failing to delete the original
+                // is a best-effort cleanup, so swallow the exception and log at info level
+                // instead of letting it surface as a user-facing red error notification.
+                Logger.Log($"Skipped deleting imported source file (provider denied write access): {e.Message}", LoggingTarget.Database);
+            }
+            catch (Exception e)
+            {
+                Logger.Log($"Failed to delete imported source file: {e.Message}", LoggingTarget.Database);
+            }
         }
 
         public static async Task<AndroidImportTask?> Create(ContentResolver contentResolver, Uri uri)

osu.Android/Input/AndroidKeyboardHandler.cs

@@ -60,7 +60,13 @@ public AndroidKeyboardHandler()
             Enabled.Value = true;
         }
 
-        public override bool Initialize(GameHost host) => true;
+        public override bool Initialize(GameHost host)
+        {
+            if (!base.Initialize(host))
+                return false;
+
+            return true;
+        }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool HandleKeyEvent(KeyEvent e)

osu.Android/Input/AndroidMouseHandler.cs

@@ -28,7 +28,13 @@ public AndroidMouseHandler()
             Enabled.Value = true;
         }
 
-        public override bool Initialize(GameHost host) => true;
+        public override bool Initialize(GameHost host)
+        {
+            if (!base.Initialize(host))
+                return false;
+
+            return true;
+        }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool HandleMotionEvent(MotionEvent e)
@@ -64,15 +70,28 @@ private void handlePointer(MotionEvent e, int historyIndex)
 
             PendingInputs.Enqueue(new MousePositionAbsoluteInput { Position = new Vector2(x, y) });
 
+            // Drive button state purely from MotionEvent.ButtonState — the bitmask is
+            // already authoritative for which physical buttons are currently held, across
+            // every action type (Move, ButtonPress, ButtonRelease, Down, Up). The previous
+            // implementation also force-set `left = true` on any `Down`/`ButtonPress` and
+            // `false` on any `Up`/`ButtonRelease`, which collapsed every right-click /
+            // middle-click / forward-click into a spurious left-click (an osu! hit) — the
+            // exact opposite of the README's "all 5 buttons" promise. ButtonState alone
+            // already covers the touchscreen-Down case for trackpads (Primary bit set on
+            // tap) so no override is needed.
+            //
+            // Mouse back/forward are exposed as MouseButton.Button1/Button2 (the README's
+            // "all 5 buttons"). On devices that *additionally* synthesise a Keycode.Back
+            // for the back button, OsuGameActivity.DispatchKeyEvent also translates that
+            // into Escape for menu navigation (README: "Mouse back button = Escape"); the
+            // small overlap on those devices is harmless because Button1 has no default
+            // binding in osu! and so cannot trigger an unintended gameplay hit.
             bool left = (e.ButtonState & MotionEventButtonState.Primary) != 0;
             bool right = (e.ButtonState & MotionEventButtonState.Secondary) != 0;
             bool middle = (e.ButtonState & MotionEventButtonState.Tertiary) != 0;
             bool back = (e.ButtonState & MotionEventButtonState.Back) != 0;
             bool forward = (e.ButtonState & MotionEventButtonState.Forward) != 0;
 
-            if (e.ActionMasked == MotionEventActions.Down || e.ActionMasked == MotionEventActions.ButtonPress) left = true;
-            if (e.ActionMasked == MotionEventActions.Up || e.ActionMasked == MotionEventActions.ButtonRelease) left = false;
-
             if (left != lastLeft) { PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, left)); lastLeft = left; }
             if (right != lastRight) { PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Right, right)); lastRight = right; }
             if (middle != lastMiddle) { PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Middle, middle)); lastMiddle = middle; }

osu.Android/Input/AndroidStylusHandler.cs

@@ -47,6 +47,14 @@ public class AndroidStylusHandler : InputHandler, ITabletHandler
         private float areaLeft, areaTop, areaWidth, areaHeight;
         private float outLeft, outTop, outWidth, outHeight;
         private float rotSin, rotCos;
+        private bool useRotation;
+        private float cachedPressureThreshold;
+
+        // Cached tablet bounds — updated whenever `tablet.Value` is reassigned. Avoids
+        // three bindable reads + property accesses per historical pointer sample in the
+        // hot path. A local-field comparison is a single un-locked memory read.
+        private float cachedTabletSizeX = 1920;
+        private float cachedTabletSizeY = 1080;
 
         private const float deg_to_rad = MathF.PI / 180f;
 
@@ -66,6 +74,7 @@ public override bool Initialize(GameHost host)
             OutputAreaSize.BindValueChanged(_ => updateCachedTransform());
             OutputAreaOffset.BindValueChanged(_ => updateCachedTransform());
             Rotation.BindValueChanged(_ => updateCachedTransform());
+            PressureThreshold.BindValueChanged(v => cachedPressureThreshold = v.NewValue, true);
 
             return base.Initialize(host);
         }
@@ -78,6 +87,8 @@ public void SetDisplaySize(int width, int height)
         {
             var size = new Vector2(width, height);
             tablet.Value = new TabletInfo("S Pen", size);
+            cachedTabletSizeX = width;
+            cachedTabletSizeY = height;
 
             // Default: full digitizer area mapped to full screen (1:1 passthrough).
             AreaSize.Default = size;
@@ -117,7 +128,9 @@ private void updateCachedTransform()
             outWidth = oSize.X;
             outHeight = oSize.Y;
 
-            float radians = deg_to_rad * Rotation.Value;
+            float rotation = Rotation.Value;
+            useRotation = rotation != 0;
+            float radians = deg_to_rad * rotation;
             rotSin = MathF.Sin(radians);
             rotCos = MathF.Cos(radians);
         }
@@ -156,11 +169,15 @@ private void handlePointer(MotionEvent e, int historyIndex)
             float pressure = historyIndex < 0 ? e.GetPressure(pointer_index) : e.GetHistoricalPressure(pointer_index, historyIndex);
 
             // Auto-expand tablet size if the digitizer reports coordinates beyond current bounds.
-            if (tablet.Value == null || rawX > tablet.Value.Size.X || rawY > tablet.Value.Size.Y)
+            // Compares against cached field values to avoid the bindable read + property access on
+            // every historical sample (which can fire 5-20× per MotionEvent on busy stylus drags).
+            if (rawX > cachedTabletSizeX || rawY > cachedTabletSizeY)
             {
-                var currentSize = tablet.Value?.Size ?? Vector2.Zero;
-                var newSize = new Vector2(Math.Max(rawX + 1, currentSize.X), Math.Max(rawY + 1, currentSize.Y));
-                tablet.Value = new TabletInfo("S Pen", newSize);
+                float newW = MathF.Max(rawX + 1, cachedTabletSizeX);
+                float newH = MathF.Max(rawY + 1, cachedTabletSizeY);
+                cachedTabletSizeX = newW;
+                cachedTabletSizeY = newH;
+                tablet.Value = new TabletInfo("S Pen", new Vector2(newW, newH));
             }
 
             // Apply tablet area → output area coordinate mapping.
@@ -173,7 +190,7 @@ private void handlePointer(MotionEvent e, int historyIndex)
                 float normY = (rawY - areaTop) / areaHeight;
 
                 // Apply rotation around center of normalized space.
-                if (Rotation.Value != 0)
+                if (useRotation)
                 {
                     float cx = normX - 0.5f;
                     float cy = normY - 0.5f;
@@ -195,10 +212,14 @@ private void handlePointer(MotionEvent e, int historyIndex)
             PendingInputs.Enqueue(new MousePositionAbsoluteInput { Position = new Vector2(mappedX, mappedY) });
 
             // Button state: pressure-based click (primary) with action overrides.
-            bool isLeftDown = pressure >= PressureThreshold.Value;
-            if (e.ActionMasked == MotionEventActions.Down || e.ActionMasked == MotionEventActions.ButtonPress) isLeftDown = true;
-            else if (e.ActionMasked == MotionEventActions.Up || e.ActionMasked == MotionEventActions.ButtonRelease || e.ActionMasked == MotionEventActions.Cancel) isLeftDown = false;
-            else if (e.ActionMasked == MotionEventActions.Move && (e.ButtonState & MotionEventButtonState.Primary) != 0) isLeftDown = true;
+            // Uses the cached threshold field rather than `PressureThreshold.Value` to skip the
+            // per-event bindable read.
+            var actionMasked = e.ActionMasked;
+            var buttonState = e.ButtonState;
+            bool isLeftDown = pressure >= cachedPressureThreshold;
+            if (actionMasked == MotionEventActions.Down || actionMasked == MotionEventActions.ButtonPress) isLeftDown = true;
+            else if (actionMasked == MotionEventActions.Up || actionMasked == MotionEventActions.ButtonRelease || actionMasked == MotionEventActions.Cancel) isLeftDown = false;
+            else if (actionMasked == MotionEventActions.Move && (buttonState & MotionEventButtonState.Primary) != 0) isLeftDown = true;
 
             if (isLeftDown != lastLeftDown)
             {
@@ -207,15 +228,15 @@ private void handlePointer(MotionEvent e, int historyIndex)
             }
 
             // S Pen button → right click.
-            bool isRightDown = (e.ButtonState & MotionEventButtonState.StylusPrimary) != 0;
+            bool isRightDown = (buttonState & MotionEventButtonState.StylusPrimary) != 0;
             if (isRightDown != lastRightDown)
             {
                 PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Right, isRightDown));
                 lastRightDown = isRightDown;
             }
 
             // Eraser → middle click.
-            bool isEraserDown = (e.ButtonState & MotionEventButtonState.StylusSecondary) != 0 || e.GetToolType(pointer_index) == MotionEventToolType.Eraser;
+            bool isEraserDown = (buttonState & MotionEventButtonState.StylusSecondary) != 0 || e.GetToolType(pointer_index) == MotionEventToolType.Eraser;
             if (isEraserDown != lastEraserDown)
             {
                 PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Middle, isEraserDown));