From 24239316a628ca82b5559ff9aecd7a403ef01aef Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Sat, 27 Feb 2021 14:57:53 -0800 Subject: [PATCH] Convert cursor rect to device coordinates on Win32 (flutter/engine#24672) The handlers for the TextInput.setMarkedTextRect and TextInput.setEditableSizeAndTransform in the win32 embedding deal in Flutter root view co-ordinates. These need to be converted to window co-ordinates before being passed to the TextInputManager, which deals in Win32 window co-ordinates. This fixes a bug wherein the IME candidates window for CJK input was incorrectly positioned at display scales other than 100% in the OS settings. Fixes: https://github.com/flutter/flutter/issues/76902 --- .../platform/windows/flutter_windows_view.cc | 2 +- .../testing/mock_window_binding_handler.h | 2 +- .../windows/text_input_plugin_delegate.h | 3 +- .../platform/windows/win32_flutter_window.cc | 8 +++- .../platform/windows/win32_flutter_window.h | 6 +-- .../windows/win32_flutter_window_unittests.cc | 37 ++++++++++++++++++- .../shell/platform/windows/win32_window.cc | 4 ++ .../shell/platform/windows/win32_window.h | 7 +++- .../platform/windows/window_binding_handler.h | 4 +- 9 files changed, 61 insertions(+), 12 deletions(-) diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc index a8916053f93..bda100930bb 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view.cc @@ -228,7 +228,7 @@ void FlutterWindowsView::OnScroll(double x, } void FlutterWindowsView::OnCursorRectUpdated(const Rect& rect) { - binding_handler_->UpdateCursorRect(rect); + binding_handler_->OnCursorRectUpdated(rect); } // Sends new size information to FlutterEngine. diff --git a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h index 50786a4b5da..84501be318b 100644 --- a/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h +++ b/engine/src/flutter/shell/platform/windows/testing/mock_window_binding_handler.h @@ -29,7 +29,7 @@ class MockWindowBindingHandler : public WindowBindingHandler { MOCK_METHOD0(OnWindowResized, void()); MOCK_METHOD0(GetPhysicalWindowBounds, PhysicalWindowBounds()); MOCK_METHOD1(UpdateFlutterCursor, void(const std::string& cursor_name)); - MOCK_METHOD1(UpdateCursorRect, void(const Rect& rect)); + MOCK_METHOD1(OnCursorRectUpdated, void(const Rect& rect)); }; } // namespace testing diff --git a/engine/src/flutter/shell/platform/windows/text_input_plugin_delegate.h b/engine/src/flutter/shell/platform/windows/text_input_plugin_delegate.h index e2c28a63a66..3975f3a0f34 100644 --- a/engine/src/flutter/shell/platform/windows/text_input_plugin_delegate.h +++ b/engine/src/flutter/shell/platform/windows/text_input_plugin_delegate.h @@ -12,7 +12,8 @@ namespace flutter { class TextInputPluginDelegate { public: - // Notifies delegate that the cursor position has changed. + // Notifies the delegate of the updated the cursor rect in Flutter root view + // coordinates. virtual void OnCursorRectUpdated(const Rect& rect) = 0; }; diff --git a/engine/src/flutter/shell/platform/windows/win32_flutter_window.cc b/engine/src/flutter/shell/platform/windows/win32_flutter_window.cc index 727b2dd154a..41080639117 100644 --- a/engine/src/flutter/shell/platform/windows/win32_flutter_window.cc +++ b/engine/src/flutter/shell/platform/windows/win32_flutter_window.cc @@ -194,8 +194,12 @@ void Win32FlutterWindow::OnScroll(double delta_x, double delta_y) { kScrollOffsetMultiplier); } -void Win32FlutterWindow::UpdateCursorRect(const Rect& rect) { - text_input_manager_.UpdateCaretRect(rect); +void Win32FlutterWindow::OnCursorRectUpdated(const Rect& rect) { + // Convert the rect from Flutter logical coordinates to device coordinates. + auto scale = GetDpiScale(); + Point origin(rect.left() * scale, rect.top() * scale); + Size size(rect.width() * scale, rect.height() * scale); + UpdateCursorRect(Rect(origin, size)); } } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/win32_flutter_window.h b/engine/src/flutter/shell/platform/windows/win32_flutter_window.h index b6456fe6ca9..f2138740856 100644 --- a/engine/src/flutter/shell/platform/windows/win32_flutter_window.h +++ b/engine/src/flutter/shell/platform/windows/win32_flutter_window.h @@ -71,6 +71,9 @@ class Win32FlutterWindow : public Win32Window, public WindowBindingHandler { // |Win32Window| void OnComposeChange(const std::u16string& text, int cursor_pos) override; + // |FlutterWindowBindingHandler| + void OnCursorRectUpdated(const Rect& rect) override; + // |Win32Window| void OnScroll(double delta_x, double delta_y) override; @@ -89,9 +92,6 @@ class Win32FlutterWindow : public Win32Window, public WindowBindingHandler { // |FlutterWindowBindingHandler| void UpdateFlutterCursor(const std::string& cursor_name) override; - // |FlutterWindowBindingHandler| - void UpdateCursorRect(const Rect& rect) override; - // |FlutterWindowBindingHandler| void OnWindowResized() override; diff --git a/engine/src/flutter/shell/platform/windows/win32_flutter_window_unittests.cc b/engine/src/flutter/shell/platform/windows/win32_flutter_window_unittests.cc index 39ed090049c..6f31d5ebe30 100644 --- a/engine/src/flutter/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/win32_flutter_window_unittests.cc @@ -21,6 +21,7 @@ using testing::_; using testing::Invoke; +using testing::Return; namespace flutter { namespace testing { @@ -123,7 +124,10 @@ class SpyTextInputPlugin : public KeyboardHandlerBase, class MockWin32FlutterWindow : public Win32FlutterWindow { public: - MockWin32FlutterWindow() : Win32FlutterWindow(800, 600) {} + MockWin32FlutterWindow() : Win32FlutterWindow(800, 600) { + ON_CALL(*this, GetDpiScale()) + .WillByDefault(Return(this->Win32FlutterWindow::GetDpiScale())); + } virtual ~MockWin32FlutterWindow() {} // Prevent copying. @@ -149,6 +153,8 @@ class MockWin32FlutterWindow : public Win32FlutterWindow { MOCK_METHOD0(OnSetCursor, void()); MOCK_METHOD2(OnScroll, void(double, double)); MOCK_METHOD4(DefaultWindowProc, LRESULT(HWND, UINT, WPARAM, LPARAM)); + MOCK_METHOD0(GetDpiScale, float()); + MOCK_METHOD1(UpdateCursorRect, void(const Rect&)); }; // A FlutterWindowsView that overrides the RegisterKeyboardHandlers function @@ -472,5 +478,34 @@ TEST(Win32FlutterWindowTest, ModifierKeyDownPropagation) { } } +// Tests that composing rect updates are transformed from Flutter logical +// coordinates to device coordinates and passed to the text input manager +// when the DPI scale is 100% (96 DPI). +TEST(Win32FlutterWindowTest, OnCursorRectUpdatedRegularDPI) { + MockWin32FlutterWindow win32window; + ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.0)); + EXPECT_CALL(win32window, GetDpiScale()).Times(1); + + Rect cursor_rect(Point(10, 20), Size(30, 40)); + EXPECT_CALL(win32window, UpdateCursorRect(cursor_rect)).Times(1); + + win32window.OnCursorRectUpdated(cursor_rect); +} + +// Tests that composing rect updates are transformed from Flutter logical +// coordinates to device coordinates and passed to the text input manager +// when the DPI scale is 150% (144 DPI). +TEST(Win32FlutterWindowTest, OnCursorRectUpdatedHighDPI) { + MockWin32FlutterWindow win32window; + ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.5)); + EXPECT_CALL(win32window, GetDpiScale()).Times(1); + + Rect expected_cursor_rect(Point(15, 30), Size(45, 60)); + EXPECT_CALL(win32window, UpdateCursorRect(expected_cursor_rect)).Times(1); + + Rect cursor_rect(Point(10, 20), Size(30, 40)); + win32window.OnCursorRectUpdated(cursor_rect); +} + } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/win32_window.cc b/engine/src/flutter/shell/platform/windows/win32_window.cc index a8957b98197..d9b4391effc 100644 --- a/engine/src/flutter/shell/platform/windows/win32_window.cc +++ b/engine/src/flutter/shell/platform/windows/win32_window.cc @@ -173,6 +173,10 @@ void Win32Window::OnImeRequest(UINT const message, // https://github.com/flutter/flutter/issues/74547 } +void Win32Window::UpdateCursorRect(const Rect& rect) { + text_input_manager_.UpdateCaretRect(rect); +} + LRESULT Win32Window::HandleMessage(UINT const message, WPARAM const wparam, diff --git a/engine/src/flutter/shell/platform/windows/win32_window.h b/engine/src/flutter/shell/platform/windows/win32_window.h index 69d9137ded1..eecc08cb00c 100644 --- a/engine/src/flutter/shell/platform/windows/win32_window.h +++ b/engine/src/flutter/shell/platform/windows/win32_window.h @@ -141,6 +141,11 @@ class Win32Window { WPARAM const wparam, LPARAM const lparam); + // Called when the cursor rect has been updated. + // + // |rect| is in Win32 window coordinates. + virtual void UpdateCursorRect(const Rect& rect); + // Called when mouse scrollwheel input occurs. virtual void OnScroll(double delta_x, double delta_y) = 0; @@ -186,7 +191,7 @@ class Win32Window { // message. int keycode_for_char_message_ = 0; - protected: + // Manages IME state. TextInputManagerWin32 text_input_manager_; }; diff --git a/engine/src/flutter/shell/platform/windows/window_binding_handler.h b/engine/src/flutter/shell/platform/windows/window_binding_handler.h index 8ebe8fa39f2..0716b55c7c4 100644 --- a/engine/src/flutter/shell/platform/windows/window_binding_handler.h +++ b/engine/src/flutter/shell/platform/windows/window_binding_handler.h @@ -63,8 +63,8 @@ class WindowBindingHandler { // content. See mouse_cursor.dart for the values and meanings of cursor_name. virtual void UpdateFlutterCursor(const std::string& cursor_name) = 0; - // Sets the cursor rect in root view coordinates. - virtual void UpdateCursorRect(const Rect& rect) = 0; + // Invoked when the cursor/composing rect has been updated in the framework. + virtual void OnCursorRectUpdated(const Rect& rect) = 0; }; } // namespace flutter