Skip to content

Commit f29f6a4

Browse files
authored
Merge pull request #270 from winnerspiros/copilot/fix-s-pen-issues-and-optimizations
Fix S Pen stuck top-left, rewrite "Treat as touch", exit on Back, default perf toggles on
2 parents 730b624 + cebdc51 commit f29f6a4

8 files changed

Lines changed: 250 additions & 51 deletions

File tree

osu.Android/Input/AndroidStylusHandler.cs

Lines changed: 109 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Runtime.CompilerServices;
66
using Android.Views;
77
using osu.Framework.Bindables;
8+
using osu.Framework.Input;
89
using osu.Framework.Input.Handlers;
910
using osu.Framework.Input.Handlers.Tablet;
1011
using osu.Framework.Input.StateChanges;
@@ -40,6 +41,16 @@ public class AndroidStylusHandler : InputHandler, ITabletHandler
4041
private readonly Bindable<TabletInfo?> tablet = new Bindable<TabletInfo?>();
4142

4243
private bool lastLeftDown;
44+
private bool lastTouchActive;
45+
46+
/// <summary>
47+
/// Mirrored from <see cref="osu.Game.Configuration.OsuSetting.AndroidStylusAsTouch"/>.
48+
/// When true, stylus events are enqueued as <see cref="TouchInput"/> (TouchSource.Touch1)
49+
/// instead of <see cref="MousePositionAbsoluteInput"/> + <see cref="MouseButtonInput"/>.
50+
/// Held as a volatile field so the OS dispatch thread can read it without
51+
/// crossing the managed-config bindable lock on every motion event.
52+
/// </summary>
53+
public volatile bool TreatAsTouch;
4354

4455
// Cached area values for hot path (avoids bindable access per event).
4556
private float areaLeft, areaTop, areaWidth, areaHeight;
@@ -147,11 +158,20 @@ public bool HandleMotionEvent(MotionEvent e)
147158

148159
if (actionMasked == MotionEventActions.HoverExit || actionMasked == MotionEventActions.Up || actionMasked == MotionEventActions.Cancel)
149160
{
150-
if (lastLeftDown) { PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, false)); lastLeftDown = false; }
161+
releaseAllButtons();
151162

152163
if (actionMasked != MotionEventActions.HoverExit)
153164
return true;
154165
}
166+
else if (actionMasked == MotionEventActions.HoverEnter)
167+
{
168+
// Reset stale button/touch state across sleep / focus-regain cycles. The
169+
// previous hover session may have ended without a clean Up if the OS
170+
// dropped the activity; without this reset the next first sample can
171+
// strand `lastLeftDown=true` (or `lastTouchActive=true`) and produce a
172+
// phantom hold from wherever the cursor last was.
173+
releaseAllButtons();
174+
}
155175

156176
// Process all batched historical events for maximum accuracy.
157177
int historySize = e.HistorySize;
@@ -163,6 +183,24 @@ public bool HandleMotionEvent(MotionEvent e)
163183
return true;
164184
}
165185

186+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
187+
private void releaseAllButtons()
188+
{
189+
if (lastLeftDown)
190+
{
191+
PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, false));
192+
lastLeftDown = false;
193+
}
194+
195+
if (lastTouchActive)
196+
{
197+
PendingInputs.Enqueue(new TouchInput(new[] { new Touch(TouchSource.Touch1, lastTouchPosition) }, false));
198+
lastTouchActive = false;
199+
}
200+
}
201+
202+
private Vector2 lastTouchPosition;
203+
166204
[MethodImpl(MethodImplOptions.AggressiveInlining)]
167205
private void handlePointer(MotionEvent e, int historyIndex, MotionEventActions actionMasked)
168206
{
@@ -173,6 +211,19 @@ private void handlePointer(MotionEvent e, int historyIndex, MotionEventActions a
173211
float rawY = historyIndex < 0 ? e.GetY(pointer_index) : e.GetHistoricalY(pointer_index, historyIndex);
174212
float pressure = historyIndex < 0 ? e.GetPressure(pointer_index) : e.GetHistoricalPressure(pointer_index, historyIndex);
175213

214+
// Drop (0, 0, 0) garbage samples. The Samsung digitizer occasionally emits a
215+
// single (rawX=0, rawY=0, pressure=0) sample when the pen wakes up after sleep,
216+
// when the activity regains focus, or as the very first HoverEnter sample
217+
// before the real coordinate is latched. Mapping that sample produces a snap
218+
// to the top-left of the screen — the long-standing "S Pen stuck top-left"
219+
// bug. A real pen sample would always have *some* coordinate (the pen is
220+
// physically *somewhere* on the digitizer to have triggered an event), so a
221+
// strict triple-zero match is a safe filter that doesn't drop legitimate
222+
// edge-of-digitizer samples (which would have pressure > 0 on contact, or
223+
// non-zero hover Y/X off the screen origin).
224+
if (rawX == 0f && rawY == 0f && pressure == 0f)
225+
return;
226+
176227
// Auto-expand tablet size if the digitizer reports coordinates beyond current bounds.
177228
// Compares against cached field values to avoid the bindable read + property access on
178229
// every historical sample (which can fire 5-20× per MotionEvent on busy stylus drags).
@@ -214,7 +265,18 @@ private void handlePointer(MotionEvent e, int historyIndex, MotionEventActions a
214265
mappedY = rawY;
215266
}
216267

217-
PendingInputs.Enqueue(new MousePositionAbsoluteInput { Position = new Vector2(mappedX, mappedY) });
268+
var mappedPos = new Vector2(mappedX, mappedY);
269+
270+
// Belt-and-braces: drop pathologically out-of-bounds mapped samples. A
271+
// half-initialised digitizer or a device-specific firmware glitch can emit
272+
// raw coordinates a few orders of magnitude beyond the actual screen — those
273+
// map to coordinates several screens away and visibly fling the cursor.
274+
// The ±2x output-area window is generous enough to keep legitimate
275+
// off-area samples (hover near the screen edge, area-rotation overshoot)
276+
// while rejecting the obvious garbage.
277+
if (mappedX < outLeft - 2f * outWidth || mappedX > outLeft + 3f * outWidth
278+
|| mappedY < outTop - 2f * outHeight || mappedY > outTop + 3f * outHeight)
279+
return;
218280

219281
// Button state: pressure-based click (primary) with action overrides.
220282
// Uses the cached threshold field rather than `PressureThreshold.Value` to skip the
@@ -230,10 +292,52 @@ private void handlePointer(MotionEvent e, int historyIndex, MotionEventActions a
230292
else if (actionMasked == MotionEventActions.Up || actionMasked == MotionEventActions.ButtonRelease || actionMasked == MotionEventActions.Cancel) isLeftDown = false;
231293
else if (actionMasked == MotionEventActions.Move && (buttonState & MotionEventButtonState.Primary) != 0) isLeftDown = true;
232294

233-
if (isLeftDown != lastLeftDown)
295+
if (TreatAsTouch)
234296
{
235-
PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, isLeftDown));
236-
lastLeftDown = isLeftDown;
297+
// Route as a Touch1 event so the gameplay paths that only fire on real
298+
// touch input (osu! relax/touch-device mod, mania touch columns, mobile
299+
// tap suppression toggles, etc.) treat the S Pen as a finger.
300+
//
301+
// Two queue items per state change:
302+
// - Position update (always, so hover-only motion still moves the touch
303+
// point — needed for slider drawing in the editor and for the
304+
// OsuTouchInputMapper to track the active touch).
305+
// - Activate/deactivate when contact state changes.
306+
//
307+
// The companion mouse-pipeline state is force-released so a runtime toggle
308+
// of the setting doesn't strand a phantom MouseButton.Left=true.
309+
if (lastLeftDown)
310+
{
311+
PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, false));
312+
lastLeftDown = false;
313+
}
314+
315+
lastTouchPosition = mappedPos;
316+
317+
// Position update (always emitted while the touch is active or starting).
318+
if (isLeftDown || lastTouchActive)
319+
PendingInputs.Enqueue(new TouchInput(new[] { new Touch(TouchSource.Touch1, mappedPos) }, isLeftDown));
320+
321+
if (isLeftDown != lastTouchActive)
322+
lastTouchActive = isLeftDown;
323+
}
324+
else
325+
{
326+
// Mouse-pipeline path. Position is published as MousePositionAbsoluteInput
327+
// so the desktop-style cursor tracks the pen tip even when not in contact.
328+
PendingInputs.Enqueue(new MousePositionAbsoluteInput { Position = mappedPos });
329+
330+
if (lastTouchActive)
331+
{
332+
PendingInputs.Enqueue(new TouchInput(new[] { new Touch(TouchSource.Touch1, lastTouchPosition) }, false));
333+
lastTouchActive = false;
334+
}
335+
336+
if (isLeftDown != lastLeftDown)
337+
{
338+
PendingInputs.Enqueue(new MouseButtonInput(MouseButton.Left, isLeftDown));
339+
lastLeftDown = isLeftDown;
340+
}
237341
}
238342

239343
// S Pen side button and eraser tip are intentionally NOT mapped to right/middle
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
2+
// See the LICENCE file in the repository root for full licence text.
3+
4+
using System.Collections.Generic;
5+
using System.Linq;
6+
using osu.Framework.Allocation;
7+
using osu.Framework.Localisation;
8+
using osu.Game.Configuration;
9+
using osu.Game.Graphics.UserInterfaceV2;
10+
using osu.Game.Overlays.Settings;
11+
using osu.Game.Overlays.Settings.Sections.Input;
12+
13+
namespace osu.Android.Input
14+
{
15+
/// <summary>
16+
/// Android-specific stylus / S Pen settings subsection. Reuses the standard
17+
/// <see cref="TabletSettings"/> area-mapping UI and adds the Android-only
18+
/// "Treat S Pen as touch" toggle so it lives next to the related stylus settings
19+
/// (rather than buried inside the Android Performance graphics subsection).
20+
/// </summary>
21+
public partial class AndroidStylusSettings : TabletSettings
22+
{
23+
public AndroidStylusSettings(AndroidStylusHandler handler)
24+
: base(handler)
25+
{
26+
}
27+
28+
[BackgroundDependencyLoader]
29+
private void load(OsuConfigManager osuConfig)
30+
{
31+
// Appended after the base TabletSettings.AddRange (the area-selection UI),
32+
// so the toggle appears at the bottom of the section. Settings search
33+
// (FilterTerms below) still surfaces it under "s pen" / "stylus" / "touch".
34+
Add(new SettingsItemV2(new FormCheckBox
35+
{
36+
Caption = "Treat S Pen as touch",
37+
HintText = "When enabled, S Pen / stylus input is enqueued as touch events (TouchSource.Touch1) rather than mouse events. Useful if a touch-only ruleset (e.g. mania touch columns, osu! touch-device mod) should treat the pen as a finger, or if the stylus pipeline misbehaves on your device.",
38+
Current = osuConfig.GetBindable<bool>(OsuSetting.AndroidStylusAsTouch),
39+
}));
40+
}
41+
42+
public override IEnumerable<LocalisableString> FilterTerms => base.FilterTerms.Concat(new LocalisableString[]
43+
{
44+
@"s pen", @"spen", @"stylus", @"pen", @"touch", @"samsung",
45+
});
46+
}
47+
}

osu.Android/OsuGameActivity.cs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -394,26 +394,17 @@ public override bool OnGenericMotionEvent(MotionEvent? e)
394394
return base.OnGenericMotionEvent(e);
395395
}
396396

397-
/// <summary>
398-
/// When true, S Pen / stylus events are routed through the standard touch dispatch
399-
/// pipeline (i.e. treated like a finger) instead of through <see cref="AndroidStylusHandler"/>.
400-
/// Mirrored from <see cref="osu.Game.Configuration.OsuSetting.AndroidStylusAsTouch"/>
401-
/// by <see cref="OsuGameAndroid"/>. Held as a volatile static so the per-event
402-
/// dispatch hot path on the OS dispatch thread can read it without crossing the
403-
/// managed-config bindable lock.
404-
/// </summary>
405-
internal static volatile bool StylusAsTouch;
406-
407397
[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
408398
private bool isStylusEvent(MotionEvent e)
409399
{
410-
// User opted to treat S Pen as plain touch input — short-circuit so the
411-
// event flows to base.DispatchTouchEvent (and the framework's SDL touch
412-
// handler) instead of to AndroidStylusHandler.
413-
if (StylusAsTouch)
414-
return false;
415-
416400
// Source flag check is cheapest and short-circuits for the common case.
401+
// Note: the "Treat S Pen as touch" toggle is intentionally NOT consulted here.
402+
// We always route stylus events through AndroidStylusHandler — that handler
403+
// internally branches between MousePositionAbsoluteInput and TouchInput based
404+
// on the toggle (see AndroidStylusHandler.TreatAsTouch). Letting events fall
405+
// through to the framework's SDL touch dispatch (the previous implementation)
406+
// dropped them entirely on phones (we strip SDL's PenHandler) and on the
407+
// secondary DeX display (different Window token).
417408
if ((e.Source & InputSourceType.Stylus) == InputSourceType.Stylus)
418409
return true;
419410

osu.Android/OsuGameAndroid.cs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,16 @@ private void load(FrameworkConfigManager frameworkConfig)
217217
LocalConfig.BindWith(OsuSetting.AndroidHardwareAudioOffsetEnabled, hardwareAudioOffsetEnabled);
218218

219219
// Mirror the stylus-as-touch toggle into the volatile flag the OS-thread
220-
// dispatch hot path reads. Subscribed (not just set once) so toggling at
221-
// runtime takes effect on the very next motion event.
222-
stylusAsTouch.BindValueChanged(e => OsuGameActivity.StylusAsTouch = e.NewValue, true);
220+
// dispatch hot path reads on AndroidStylusHandler. Subscribed (not just
221+
// set once) so toggling at runtime takes effect on the very next motion
222+
// event. The handler instance may not yet exist at this point — the
223+
// value is also re-applied at the bottom of registerInputHandlers() once
224+
// the handler is constructed, so the initial value is never lost.
225+
stylusAsTouch.BindValueChanged(e =>
226+
{
227+
if (stylusHandler != null)
228+
stylusHandler.TreatAsTouch = e.NewValue;
229+
}, true);
223230

224231
// sentinelOnDisable=true → presence ⇒ "feature disabled". The
225232
// safety nets default to ON, so the sentinel is created only
@@ -1276,6 +1283,20 @@ public override void PerformPlatformExit()
12761283

12771284
public double GetMeasuredAudioLatencyMs() => getMeasuredAudioLatencyFromBridge();
12781285

1286+
/// <summary>
1287+
/// On Android, "Back at the top of the navigation stack" should fully exit the
1288+
/// process rather than the framework default of <c>MoveTaskToBack</c>. The default
1289+
/// leaves the audio thread mixing, the GC scheduling work, and the Vulkan swapchain
1290+
/// pinned — perceived by the user as "I closed the app, why is it still draining
1291+
/// battery?". Routing to <see cref="PerformPlatformExit"/> reuses the documented
1292+
/// hard-exit dance (MoveTaskToBack + Activity.Finish + Process.KillProcess(MyPid)).
1293+
/// </summary>
1294+
public override bool SuspendToBackground()
1295+
{
1296+
PerformPlatformExit();
1297+
return true;
1298+
}
1299+
12791300
// ------------------------------------------------------------------
12801301
// Layer 3 helpers — extracted Oboe / Vulkan-probe BindValueChanged
12811302
// bodies so the initial fire can be EITHER synchronous (the original
@@ -1639,6 +1660,12 @@ private void registerAndroidInputHandlers(GameHost host)
16391660
mouseHandler = new AndroidMouseHandler();
16401661
keyboardHandler = new AndroidKeyboardHandler();
16411662

1663+
// Apply the persisted "Treat S Pen as touch" preference now that the
1664+
// handler instance exists. The BindValueChanged subscription installed
1665+
// in load() may have fired before this point (when stylusHandler was
1666+
// still null) — re-applying the current value here closes that race.
1667+
stylusHandler.TreatAsTouch = stylusAsTouch.Value;
1668+
16421669
gameActivity.StylusHandler = stylusHandler;
16431670
gameActivity.MouseHandler = mouseHandler;
16441671
gameActivity.KeyboardHandler = keyboardHandler;
@@ -1834,7 +1861,7 @@ protected override void Dispose(bool isDisposing)
18341861
public override osu.Game.Overlays.Settings.SettingsSubsection CreateSettingsSubsectionFor(osu.Framework.Input.Handlers.InputHandler handler)
18351862
{
18361863
if (handler is AndroidStylusHandler stylus)
1837-
return new osu.Game.Overlays.Settings.Sections.Input.TabletSettings(stylus);
1864+
return new AndroidStylusSettings(stylus);
18381865

18391866
return base.CreateSettingsSubsectionFor(handler);
18401867
}

osu.Game/Configuration/OsuConfigManager.cs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,32 @@ protected override void InitialiseDefaults()
239239

240240
SetDefault(OsuSetting.DashboardSortMode, UserSortCriteria.LastVisit);
241241
SetDefault(OsuSetting.DashboardDisplayStyle, OverlayPanelDisplayStyle.Card);
242-
SetDefault(OsuSetting.AndroidPerformanceMode, false);
243-
SetDefault(OsuSetting.AndroidLowLatencyAudio, false);
242+
// Android performance defaults — both ON for fresh installs.
243+
//
244+
// - AndroidPerformanceMode = true:
245+
// Begins a high-performance GC session (GCLatencyMode.SustainedLowLatency
246+
// while in foreground) for the entire app lifetime, eliminating the
247+
// Gen1/Gen2 STW pauses that are the largest source of >1ms frame spikes
248+
// on the Update/Draw threads. Sustained Performance Mode + immersive
249+
// fullscreen + highest-refresh-rate are layered on top from
250+
// OsuGameAndroid. The setting was historically default-off because the
251+
// mitigations layered on it (refresh-rate switch, surface mode change)
252+
// could race the cold-start swapchain bring-up; the deferred-by-5s
253+
// scheduler in OsuGameAndroid.LoadComplete now handles that, and
254+
// AndroidStartupSafeMode catches any device-specific regression on the
255+
// next launch — so default-on is finally safe.
256+
// - AndroidLowLatencyAudio = true:
257+
// Routes BASS through Oboe (AAudio Exclusive + MMAP + StabilizedCallback)
258+
// instead of BASS's default OpenSL ES path. Cuts output latency from
259+
// ~80-120ms to ~5-15ms on devices that support MMAP, and makes the audio
260+
// callback eligible for ADPF priority hints. Initial bind is also
261+
// deferred under AndroidDeferStartupNativeInit, so default-on does not
262+
// widen the cold-start window.
263+
// - AndroidVulkanProbe stays default-off — it's a pure diagnostic that
264+
// constructs a transient VkInstance and adds startup time without
265+
// affecting steady-state performance. Power users opt in.
266+
SetDefault(OsuSetting.AndroidPerformanceMode, true);
267+
SetDefault(OsuSetting.AndroidLowLatencyAudio, true);
244268
SetDefault(OsuSetting.AndroidVulkanProbe, false);
245269
SetDefault(OsuSetting.AndroidStartupFrameSyncMigrationApplied, false);
246270

0 commit comments

Comments
 (0)