[windows] Restore framework cursor when mouse re-enters client area#189568
[windows] Restore framework cursor when mouse re-enters client area#189568sero583 wants to merge 2 commits into
Conversation
The cursor set while the mouse is over the non-client area (e.g. a resize arrow over the window border) persisted after the mouse re-entered the client area: the WM_SETCURSOR handler halts processing to protect against DefWindowProc resetting the class cursor, but that also prevents the stale non-client cursor from being replaced until the framework happens to push a new cursor update. Remember the last cursor set by the framework in FlutterWindowsEngine and restore it from the WM_SETCURSOR handler when the hit test result is HTCLIENT.
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the Windows platform shell to restore the cursor requested by the Flutter framework when the mouse re-enters the window's client area during WM_SETCURSOR events. The engine now tracks the current cursor, and a new GetFlutterCursor method has been introduced to retrieve it. The review feedback suggests declaring this new GetFlutterCursor method as const across the interface, implementation, and mock classes to adhere to C++ const-correctness guidelines.
| // framework's cursor when the mouse re-enters the client area, as the | ||
| // cursor may have been changed while over the non-client area (e.g. a | ||
| // resize arrow over the window border). | ||
| virtual HCURSOR GetFlutterCursor() = 0; |
There was a problem hiding this comment.
The GetFlutterCursor method is a getter that does not modify any state. To adhere to C++ const-correctness best practices (as recommended by the Google C++ Style Guide), this virtual method should be declared as const.
| virtual HCURSOR GetFlutterCursor() = 0; | |
| virtual HCURSOR GetFlutterCursor() const = 0; |
References
- Google C++ Style Guide recommends using const on member functions that do not modify the state of the object. (link)
| void OnHighContrastChanged() override; | ||
|
|
||
| // |WindowBindingHandlerDelegate| | ||
| HCURSOR GetFlutterCursor() override; |
There was a problem hiding this comment.
Since GetFlutterCursor in the base interface WindowBindingHandlerDelegate should be const, this override in FlutterWindowsView should also be declared as const.
| HCURSOR GetFlutterCursor() override; | |
| HCURSOR GetFlutterCursor() const override; |
References
- Google C++ Style Guide recommends using const on member functions that do not modify the state of the object. (link)
| HCURSOR FlutterWindowsView::GetFlutterCursor() { | ||
| return engine_->GetFlutterCursor(); | ||
| } |
There was a problem hiding this comment.
Update the implementation of GetFlutterCursor to be const to match the updated declaration in the header file.
| HCURSOR FlutterWindowsView::GetFlutterCursor() { | |
| return engine_->GetFlutterCursor(); | |
| } | |
| HCURSOR FlutterWindowsView::GetFlutterCursor() const { | |
| return engine_->GetFlutterCursor(); | |
| } |
References
- Google C++ Style Guide recommends using const on member functions that do not modify the state of the object. (link)
| (), | ||
| (override)); | ||
|
|
||
| MOCK_METHOD(HCURSOR, GetFlutterCursor, (), (override)); |
There was a problem hiding this comment.
Update the mock method definition to include the const qualifier to match the updated interface.
| MOCK_METHOD(HCURSOR, GetFlutterCursor, (), (override)); | |
| MOCK_METHOD(HCURSOR, GetFlutterCursor, (), (const, override)); |
References
- Google C++ Style Guide recommends using const on member functions that do not modify the state of the object. (link)
1 participant
On Windows, a cursor set while the pointer is over the non-client area (for example the resize arrow from a window border) sticks around after the pointer moves back into the Flutter content whenever the UI thread is busy at that moment. The cause is in
FlutterWindow'sWM_SETCURSORhandler: forHTCLIENThits it returnsTRUEwithout setting any cursor. That return is intentional (it stopsDefWindowProcfrom resetting the cursor to the window class cursor on every mouse move), but it means the visible cursor is only corrected by the framework's nextmouse_cursorchannel update. On a responsive app that round trip completes within milliseconds; under UI-thread latency (app startup, shader-compilation jank, heavy frames, debug builds) the stale cursor stays visible for the whole stall. #189563 documents the analysis, measurements, and a deterministic repro.This change makes the engine remember the cursor most recently requested by the framework (
FlutterWindowsEngine::SetFlutterCursoralready funnels every cursor update, both system and custom cursors) and makes theWM_SETCURSORhandler restore that cursor before halting processing, the way native Win32 apps answer the message. The cursor is then correct as soon as the window processes the message, without waiting for a channel round trip and without depending on the framework's cursor cache. The window pulls the cursor through a newWindowBindingHandlerDelegate::GetFlutterCursor(), so the engine remains the single source of truth and windows created at any time behave consistently.Changes:
FlutterWindowsEngine: remember the last cursor set by the framework, expose it viaGetFlutterCursor().UpdateFlutterCursor/SetFlutterCursorlose theirconstsince they now record state.WindowBindingHandlerDelegate: newGetFlutterCursor()pure virtual, implemented byFlutterWindowsView(forwards to the engine) and mocked for tests.FlutterWindow:WM_SETCURSOR/HTCLIENTnow restores the framework cursor before returningTRUE.Tests:
FlutterWindowTest.OnSetCursorRestoresFlutterCursor:WM_SETCURSORover the client area queries the delegate and activates the returned cursor.FlutterWindowTest.OnSetCursorNonClientAreaLeavesCursorAlone: non-client hits do not touch the cursor.FlutterWindowsEngineTest.SetFlutterCursorStoresCursor: the engine records the cursor and still forwards it to::SetCursorviaWindowsProcTable.Part of #189563
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.