Fix input method bugs on macos.

There are a lot of problems about input methods and we will definitely spend more on this. And the bug made me awkward on today's interview when showing the demo.
crupest · Oct 18, 2023 · e8abfdb · e8abfdb
1 parent 0a67b1c
commit e8abfdb
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 64 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -4,6 +4,10 @@ root = true
 end_of_line = lf
 insert_final_newline = true
 
+[*.mm]
+indent_style = space
+indent_size = 2
+
 [*.cpp]
 indent_style = space
 indent_size = 2

diff --git a/cru-words.txt b/cru-words.txt
@@ -1,4 +1,5 @@
 emscripten
+clangd
 
 # cmake
 endfunction

diff --git a/demos/InputMethod/main.cpp b/demos/InputMethod/main.cpp
@@ -1,4 +1,5 @@
 #include "cru/platform/Color.h"
+#include "cru/platform/GraphicsBase.h"
 #include "cru/platform/bootstrap/Bootstrap.h"
 #include "cru/platform/graphics/Factory.h"
 #include "cru/platform/graphics/Font.h"
@@ -7,12 +8,20 @@
 #include "cru/platform/gui/UiApplication.h"
 #include "cru/platform/gui/Window.h"
 
-int main() {
-  using namespace cru;
-  using namespace cru::platform;
-  using namespace cru::platform::graphics;
-  using namespace cru::platform::gui;
+#include <optional>
+
+using namespace cru;
+using namespace cru::platform;
+using namespace cru::platform::graphics;
+using namespace cru::platform::gui;
+
+struct InputMethodState {
+  CompositionText composition_text;
+  Rect cursor_rect;
+  TextRange colored_text_range;
+};
 
+int main() {
   IUiApplication* application = bootstrap::CreateUiApplication();
 
   auto graphics_factory = application->GetGraphicsFactory();
@@ -33,22 +42,30 @@ int main() {
 
   std::shared_ptr<IFont> font = graphics_factory->CreateFont(String{}, 30);
 
-  float window_width = 10000;
-
   auto prompt_text_layout =
       graphics_factory->CreateTextLayout(font,
                                          u"Ctrl+1: Enable IME\n"
                                          u"Ctrl+2: Disable IME\n"
                                          u"Ctrl+3: Complete composition.\n"
                                          u"Ctrl+4: Cancel composition.");
+  float anchor_y;
 
-  std::optional<CompositionText> optional_composition_text;
   String committed_text;
+  auto text_layout = graphics_factory->CreateTextLayout(font, u"");
+  std::optional<InputMethodState> state;
+
+  auto update_text_layout_width = [&prompt_text_layout, &anchor_y,
+                                   &text_layout](float width) {
+    prompt_text_layout->SetMaxWidth(width);
+    text_layout->SetMaxWidth(width);
+    anchor_y = prompt_text_layout->GetTextBounds().height;
+  };
+
+  update_text_layout_width(window->GetClientSize().width);
 
   window->ResizeEvent()->AddHandler(
-      [&prompt_text_layout, &window_width](const Size& size) {
-        prompt_text_layout->SetMaxWidth(size.width);
-        window_width = size.width;
+      [&update_text_layout_width](const Size& size) {
+        update_text_layout_width(size.width);
       });
 
   window->PaintEvent()->AddHandler([&](auto) {
@@ -57,16 +74,8 @@ int main() {
 
     painter->DrawText(Point{}, prompt_text_layout.get(), brush.get());
 
-    const auto anchor_y = prompt_text_layout->GetTextBounds().height;
-
-    auto text_layout = graphics_factory->CreateTextLayout(
-        font, committed_text + (optional_composition_text
-                                    ? optional_composition_text->text
-                                    : u""));
-    text_layout->SetMaxWidth(window_width);
-
-    if (optional_composition_text) {
-      const auto& composition_text = *optional_composition_text;
+    if (state) {
+      const auto& composition_text = state->composition_text;
 
       for (int i = 0; i < static_cast<int>(composition_text.clauses.size());
            i++) {
@@ -85,19 +94,13 @@ int main() {
 
     painter->DrawText(Point{0, anchor_y}, text_layout.get(), brush.get());
 
-    if (optional_composition_text) {
-      const auto& composition_text = *optional_composition_text;
+    if (state) {
+      const auto& composition_text = state->composition_text;
+      const auto& cursor_rect = state->cursor_rect;
 
-      const auto cursor_pos = composition_text.selection.position +
-                              gsl::narrow_cast<int>(committed_text.size());
-
-      const auto cursor_lefttop =
-          text_layout->TextSinglePoint(cursor_pos, false);
-
-      painter->FillRectangle(
-          Rect{cursor_lefttop.left, cursor_lefttop.top + anchor_y, 3,
-               cursor_lefttop.height},
-          brush.get());
+      painter->FillRectangle(Rect{cursor_rect.left, cursor_rect.top + anchor_y,
+                                  3, cursor_rect.height},
+                             brush.get());
     }
 
     painter->EndDraw();
@@ -131,15 +134,30 @@ int main() {
         window->RequestRepaint();
       });
 
-  input_method_context->CompositionEvent()->AddHandler(
-      [window, &input_method_context, &optional_composition_text](auto) {
-        optional_composition_text = input_method_context->GetCompositionText();
-        window->RequestRepaint();
-      });
+  input_method_context->CompositionEvent()->AddHandler([window,
+                                                        &input_method_context,
+                                                        &committed_text,
+                                                        &anchor_y, &state,
+                                                        &text_layout](auto) {
+    const auto composition_text = input_method_context->GetCompositionText();
+    state.emplace();
+    state->composition_text = input_method_context->GetCompositionText();
+
+    text_layout->SetText(committed_text + composition_text.text);
+
+    const auto cursor_pos = composition_text.selection.position +
+                            gsl::narrow_cast<int>(committed_text.size());
+    state->cursor_rect = text_layout->TextSinglePoint(cursor_pos, false);
+
+    input_method_context->SetCandidateWindowPosition(
+        {state->cursor_rect.left, anchor_y + state->cursor_rect.GetBottom()});
+
+    window->RequestRepaint();
+  });
 
   input_method_context->CompositionEndEvent()->AddHandler(
-      [window, &optional_composition_text](auto) {
-        optional_composition_text = std::nullopt;
+      [window, &state](auto) {
+        state = std::nullopt;
         window->RequestRepaint();
       });
 

diff --git a/include/cru/platform/gui/InputMethod.h b/include/cru/platform/gui/InputMethod.h
@@ -21,6 +21,24 @@ struct CompositionText {
   TextRange selection;
 };
 
+/**
+ * \remarks I think it's time to standatdize this. The most important thing is
+ * the events.
+ *
+ * The events hould be triggered in this way.
+ * 1. Any time the IME begins to work, CompositionStartEvent is fired. Only
+ * once. Not triggerred again until CompositionEndEvent is fired.
+ * 2. Any time composition state changed, maybe user typed more characters, or
+ * user commit part of composition, CompositionEvent is fired.
+ * 3. TextEvent is fired when user commit part or whole of the composition. And
+ * you can use the args to get what characters are committed. So it is where you
+ * get the real text user want to give you.
+ * 4. Whenever a commit happens, TextEvent first, followed by CompositionEvent.
+ * Each for once. So use the TextEvent to get real input and use
+ * CompositionEvent to update UI.
+ * 5. When composition stops, a final CompositionEndEvent is fired. Also only
+ * once.
+ */
 struct IInputMethodContext : virtual IPlatformResource {
   // Return true if you should draw composition text manually. Return false if
   // system will take care of that for you.
@@ -36,20 +54,13 @@ struct IInputMethodContext : virtual IPlatformResource {
 
   virtual CompositionText GetCompositionText() = 0;
 
-  // Set the candidate window lefttop. Relative to window lefttop. Use this
+  // Set the candidate window left-top. Relative to window left-top. Use this
   // method to prepare typing.
   virtual void SetCandidateWindowPosition(const Point& point) = 0;
 
-  // Triggered when user starts composition.
   virtual IEvent<std::nullptr_t>* CompositionStartEvent() = 0;
-
-  // Triggered when user stops composition.
   virtual IEvent<std::nullptr_t>* CompositionEndEvent() = 0;
-
-  // Triggered every time composition text changes.
   virtual IEvent<std::nullptr_t>* CompositionEvent() = 0;
-
   virtual IEvent<StringView>* TextEvent() = 0;
 };
 }  // namespace cru::platform::gui
-
diff --git a/src/platform/gui/osx/Window.mm b/src/platform/gui/osx/Window.mm
@@ -5,17 +5,17 @@
 #include "InputMethodPrivate.h"
 #include "cru/common/Range.h"
 #include "cru/common/log/Logger.h"
-#include "cru/platform/osx/Convert.h"
+#include "cru/platform/Check.h"
+#include "cru/platform/graphics/NullPainter.h"
 #include "cru/platform/graphics/quartz/Convert.h"
 #include "cru/platform/graphics/quartz/Painter.h"
+#include "cru/platform/gui/TimerHelper.h"
 #include "cru/platform/gui/osx/Cursor.h"
 #include "cru/platform/gui/osx/InputMethod.h"
 #include "cru/platform/gui/osx/Keyboard.h"
 #include "cru/platform/gui/osx/Resource.h"
 #include "cru/platform/gui/osx/UiApplication.h"
-#include "cru/platform/Check.h"
-#include "cru/platform/graphics/NullPainter.h"
-#include "cru/platform/gui/TimerHelper.h"
+#include "cru/platform/osx/Convert.h"
 
 #include <AppKit/AppKit.h>
 #include <Foundation/Foundation.h>
@@ -28,8 +28,8 @@
 constexpr int key_down_debug = 0;
 }
 
-using cru::platform::osx::Convert;
 using cru::platform::graphics::quartz::Convert;
+using cru::platform::osx::Convert;
 
 namespace cru::platform::gui::osx {
 namespace {
@@ -39,7 +39,7 @@ inline NSWindowStyleMask CalcWindowStyleMask(WindowStyleFlag flag) {
              : NSWindowStyleMaskTitled | NSWindowStyleMaskClosable |
                    NSWindowStyleMaskMiniaturizable | NSWindowStyleMaskResizable;
 }
-}
+}  // namespace
 
 namespace details {
 OsxWindowPrivate::OsxWindowPrivate(OsxWindow* osx_window) : osx_window_(osx_window) {
@@ -213,7 +213,7 @@ inline NSWindowStyleMask CalcWindowStyleMask(WindowStyleFlag flag) {
   return cru::platform::graphics::quartz::Convert(rect);
 }
 
-}
+}  // namespace details
 
 OsxWindow::OsxWindow(OsxUiApplication* ui_application)
     : OsxGuiResource(ui_application), p_(new details::OsxWindowPrivate(this)) {}
@@ -400,7 +400,7 @@ inline NSWindowStyleMask CalcWindowStyleMask(WindowStyleFlag flag) {
 IEvent<NativeKeyEventArgs>* OsxWindow::KeyUpEvent() { return &p_->key_up_event_; }
 
 IInputMethodContext* OsxWindow::GetInputMethodContext() { return p_->input_method_context_.get(); }
-}
+}  // namespace cru::platform::gui::osx
 
 namespace {
 cru::platform::gui::KeyModifier GetKeyModifier(NSEvent* event) {
@@ -415,7 +415,7 @@ inline NSWindowStyleMask CalcWindowStyleMask(WindowStyleFlag flag) {
     key_modifier |= cru::platform::gui::KeyModifiers::command;
   return key_modifier;
 }
-}
+}  // namespace
 
 @implementation CruWindow {
   cru::platform::gui::osx::details::OsxWindowPrivate* _p;
@@ -597,7 +597,7 @@ - (void)scrollWheel:(NSEvent*)event {
     KeyCode::Equal,
     KeyCode::GraveAccent,
 };
-}
+}  // namespace
 
 const std::unordered_set<KeyCode> input_context_handle_codes_when_has_text{
     KeyCode::Backspace, KeyCode::Space, KeyCode::Return, KeyCode::Left,
@@ -738,20 +738,28 @@ - (void)insertText:(id)string replacementRange:(NSRange)replacementRange {
   // cru::CRU_LOG_DEBUG(u"CruView", u"Finish composition: {}, replacement range: ({}, {})", ss,
   //                    replacementRange.location, replacementRange.length);
 
+  _input_context_p->RaiseTextEvent(ss);
   _input_context_p->RaiseCompositionEvent();
   _input_context_p->RaiseCompositionEndEvent();
-  _input_context_p->RaiseTextEvent(ss);
 }
 
 - (NSUInteger)characterIndexForPoint:(NSPoint)point {
   return NSNotFound;
 }
 
+// The key to composition window. It is in screen coordinate.
 - (NSRect)firstRectForCharacterRange:(NSRange)range actualRange:(NSRangePointer)actualRange {
+  const auto window_rect = _p->GetWindow()->GetClientRect();
+  auto position = _input_context_p->GetCandidateWindowPosition();
+
+  position.x += window_rect.left;
+  position.y += window_rect.top;
+  position.y = _p->GetScreenSize().height - position.y;
+
   NSRect result;
-  result.origin.x = _input_context_p->GetCandidateWindowPosition().x;
-  result.origin.y = _input_context_p->GetCandidateWindowPosition().y;
-  result.size.height = 16;
+  result.origin.x = position.x;
+  result.origin.y = position.y;
+  result.size.height = 0;
   result.size.width = 0;
   return result;
 }

diff --git a/src/platform/gui/osx/WindowPrivate.h b/src/platform/gui/osx/WindowPrivate.h
@@ -2,9 +2,9 @@
 #include "cru/platform/gui/osx/Window.h"
 
 #include "cru/common/Event.h"
-#include "cru/platform/gui/osx/Cursor.h"
 #include "cru/platform/gui/TimerHelper.h"
 #include "cru/platform/gui/Window.h"
+#include "cru/platform/gui/osx/Cursor.h"
 
 #import <AppKit/AppKit.h>
 
@@ -64,9 +64,9 @@ class OsxWindowPrivate {
   OsxWindow* GetWindow() { return osx_window_; }
   NSWindow* GetNSWindow() { return window_; }
 
- private:
   Size GetScreenSize();
 
+ private:
   void CreateWindow();
 
   void UpdateCursor();