FIX: Add hopeful fix for mouse polling causing frame-rate issues on windows.#2441
Open
Darren-Kelly-Unity wants to merge 2 commits into
Open
FIX: Add hopeful fix for mouse polling causing frame-rate issues on windows.#2441Darren-Kelly-Unity wants to merge 2 commits into
Darren-Kelly-Unity wants to merge 2 commits into
Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
|
|
||
| var currentEvent = StateEvent.FromUnchecked(currentEventPtr); | ||
| var nextEvent = StateEvent.FromUnchecked(nextEventPtr); | ||
| MouseState* currentState; |
Contributor
There was a problem hiding this comment.
Have you considered centralizing the logic for extracting a MouseState* pointer from an InputEventPtr into a private helper method?
Currently, both OnStateEvent and MergeForward contain highly repetitive conditional checks (StateEvent vs DeltaStateEvent), format/size validations, and unsafe pointer casting. Centralizing this significantly improves readability and maintainability without introducing any performance or GC overhead (since it deals entirely with unmanaged pointers).
For example, we could define the helper:
private static unsafe MouseState* TryGetMouseState(InputEventPtr eventPtr)
{
if (eventPtr.type == StateEvent.Type)
{
var stateEvent = StateEvent.FromUnchecked(eventPtr);
if (stateEvent->stateFormat == MouseState.Format)
return (MouseState*)stateEvent->state;
}
else if (eventPtr.type == DeltaStateEvent.Type)
{
var deltaEvent = DeltaStateEvent.FromUnchecked(eventPtr);
if (IsFullMouseStateDeltaEvent(deltaEvent))
return (MouseState*)deltaEvent->deltaState;
}
return null;
}With this helper, MergeForward simplifies dramatically to:
internal static unsafe bool MergeForward(InputEventPtr currentEventPtr, InputEventPtr nextEventPtr)
{
var currentState = TryGetMouseState(currentEventPtr);
if (currentState == null)
return false;
var nextState = TryGetMouseState(nextEventPtr);
if (nextState == null)
return false;
if (currentState->buttons != nextState->buttons || currentState->clickCount != nextState->clickCount)
return false;
nextState->delta += currentState->delta;
nextState->scroll += currentState->scroll;
return true;
}And OnStateEvent becomes:
protected new unsafe void OnStateEvent(InputEventPtr eventPtr)
{
var newStatePtr = TryGetMouseState(eventPtr);
if (newStatePtr == null)
{
base.OnStateEvent(eventPtr);
return;
}
var newState = *newStatePtr;
var stateFromDevice = (MouseState*)((byte*)currentStatePtr + m_StateBlock.byteOffset);
newState.delta += stateFromDevice->delta;
newState.scroll += stateFromDevice->scroll;
InputState.Change(this, ref newState, InputState.currentUpdateType, eventPtr: eventPtr);
}🤖 Helpful? 👍/👎 by quality_agent
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2441 +/- ##
============================================
+ Coverage 58.58% 79.05% +20.47%
============================================
Files 738 766 +28
Lines 135831 140558 +4727
============================================
+ Hits 79570 111121 +31551
+ Misses 56261 29437 -26824 Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
DISCLAIMER:
This was very heavily done by overconfident Claude and may be the completely wrong place to do this. He seemed to know what he was talking about..
Fixes a performance regression (UUM-142550) where high-polling-rate mice (1000–8000 Hz) cause severe FPS drops on Windows. The
FastMouseevent merger only handledStateEvent; this PR extends it to also coalesceDeltaStateEventmessages that carry a fullMouseStatepayload, closing the path by which Windows Raw Input events could flood the managed event loop unmerged.Release Notes
FastMouseevent merger now handlesDeltaStateEventwith fullMouseStatepayload, preventing high-polling-rate mice from flooding the event loop on Windows. (UUM-142550)Functional Testing status
CoreTests_FastMouseMerger.cscovering:MergeForwardaccepts/rejectsDeltaStateEventby type and sizeStateEvent+DeltaStateEventmergeProcessEventBufferpipelinePerformance Testing Status
This change reduces the number of events processed per frame for high-polling-rate mice on Windows from O(polling rate / fps) to O(1) for pure movement. No performance regression expected on other platforms or lower polling rates.
Overall Product Risks
FastMouse.partial.cs; only affects devices instantiated asFastMouse(layout"Mouse"). All fourMergeForwardtype combinations are guarded by format and size checks; non-qualifying events fall through to the existing base class path unchanged.IsFullMouseStateDeltaEventgate (stateOffset=0, size≥sizeof(MouseState)) is conservative and only activates on full-device delta events, so partial control delta events are unaffected.Class diagram
Class diagram for
origin/develop...HEADClass diagram
classDiagram IInputStateCallbackReceiver <|.. FastMouse IEventMerger <|.. FastMouse class FastMouse { +OnStateEvent(InputEventPtr eventPtr) void +MergeForward(InputEventPtr currentEventPtr, InputEventPtr nextEventPtr) bool -IsFullMouseStateDeltaEvent(DeltaStateEvent* deltaEvent) bool } class CoreTests { +FastMouseMerger_Unit_MergesDeltaStateEventsWithSameButtonState() +FastMouseMerger_Unit_DoesNotMergeDeltaStateEventsWithDifferentButtonState() +FastMouseMerger_Unit_RejectsPartialDeltaStateEvent() +FastMouseMerger_Unit_MergesMixedStateAndDeltaStateEvent() +FastMouseMerger_Integration_ManyDeltaStateEvents_CoalesceToOne() +FastMouseMerger_Integration_ButtonPressMidStream_PreservesButtonEventAndMergesSurroundingMoves() +FastMouseMerger_Integration_MixedStateAndDeltaStateEvents_Merge() +FastMouseMerger_Integration_PartialDeltaStateEvent_IsAppliedButNotMerged() +FastMouseMerger_Integration_HighPollingRateSimulation(bool mergeEvents) }