Use cached composition bounds for firstRectForCharacterRange

content/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 
 #include <QuartzCore/QuartzCore.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 629 matching lines @@
 void RenderWidgetHostViewMac::ImeCancelComposition() {
   [cocoa_view_ cancelComposition];
 }
 
 void RenderWidgetHostViewMac::ImeCompositionRangeChanged(
     const ui::Range& range,
     const std::vector<gfx::Rect>& character_bounds) {
   // The RangeChanged message is only sent with valid values. The current
   // caret position (start == end) will be sent if there is no IME range.
   [cocoa_view_ setMarkedRange:range.ToNSRange()];
+  composition_range_ = range;
+  composition_bounds_ = character_bounds;
 }
 
 void RenderWidgetHostViewMac::DidUpdateBackingStore(
     const gfx::Rect& scroll_rect, int scroll_dx, int scroll_dy,
     const std::vector<gfx::Rect>& copy_rects) {
   GotSoftwareFrame();
 
   if (!is_hidden_) {
     std::vector<gfx::Rect> rects(copy_rects);
 
@@ skipping 372 matching lines @@
           pending_swap_buffers_acks_.front().first,
           pending_swap_buffers_acks_.front().second,
           0);
       if (render_widget_host_)
         render_widget_host_->AcknowledgeSwapBuffersToRenderer();
     }
     pending_swap_buffers_acks_.erase(pending_swap_buffers_acks_.begin());
   }
 }
 
+bool RenderWidgetHostViewMac::GetLineBreakIdx(
+    const std::vector<gfx::Rect>& bounds,
+    const ui::Range& range,
+    size_t* line_break_point) {
+  DCHECK(line_break_point);
+  if (range.start() >= bounds.size() || range.is_reversed() || range.is_empty())
+    return false;
+
+  // Detect line break by comparing x value because we can't assume the
+  // each character's height is fixed.
+  const size_t loop_end_idx = std::min(bounds.size(), range.end());
+  for (size_t idx = range.start(); idx < loop_end_idx - 1; ++idx) {
+    if (bounds[idx].x() > bounds[idx + 1].x()) {

[James Su, 2012/07/04 01:44:04]

I'm wondering if it works for text with complex layout characters, in which the
characters visual position maybe swapped.

[Seigo Nonaka, 2012/07/04 06:56:58]

Yes, this logic does not work for bidi characters.
However at this moment, this function is used only for composition rectangle.
So if there is some tool which composes bidi characters, this should be fixed,
but I think it's okay to leave TODO.
What do you think?

And I'm not sure about the correct behavior of firstRectForCharRange for bidi
mixed character.


On 2012/07/04 01:44:04, James Su wrote:

+      *line_break_point = idx + 1;
+      return true;
+    }
+  }
+  return false;
+}
+
+gfx::Rect RenderWidgetHostViewMac::GetFirstRectForCompositionRange(
+    const ui::Range& range) {
+  DCHECK(range.start() < composition_bounds_.size());
+  DCHECK(range.end() < composition_bounds_.size());

[James Su, 2012/07/04 01:44:04]

start and end may equal to composition_bounds_.size().

[Seigo Nonaka, 2012/07/04 06:56:58]

Done for range.end().
start point is inclusive, so we can't accept it.
On 2012/07/04 01:44:04, James Su wrote:

+  int right = 0;
+  if (range.is_empty()) {
+    right = composition_bounds_[range.start()].x();
+  } else {
+    size_t line_break_idx;
+    if (GetLineBreakIdx(composition_bounds_, range, &line_break_idx)) {
+      right = composition_bounds_[line_break_idx - 1].right();
+    } else {
+      right = composition_bounds_[range.end() - 1].right();
+    }
+  }
+  return gfx::Rect(
+      composition_bounds_[range.start()].x(),
+      composition_bounds_[range.start()].y(),
+      right - composition_bounds_[range.start()].x(),
+      composition_bounds_[range.start()].height());

[James Su, 2012/07/04 01:44:04]

I think we should return the union rect of all rects within [start, end),
instead of just a rect calculated by start and end.

[Seigo Nonaka, 2012/07/04 06:56:58]

Done.

+}
+
+ui::Range RenderWidgetHostViewMac::ConvertCharacterRangeToCompositionRange(
+    const ui::Range& request_range) {
+  if (composition_range_.is_empty())

[James Su, 2012/07/04 01:44:04]

I think we probably should handle the empty case, as long as it's inside the
composition_range_. Maybe we should return a zero width rect for an empty range.

[Seigo Nonaka, 2012/07/04 06:56:58]

This check means that we don't have any cached composition boundaries. If you
worry about request_range empty case, it's okay because we already handled.

On 2012/07/04 01:44:04, James Su wrote:

+    return ui::Range::InvalidRange();
+
+  if (request_range.is_reversed())
+    return ui::Range::InvalidRange();
+
+  if (request_range.start() < composition_range_.start() ||
+      request_range.start() >= composition_range_.end())

[James Su, 2012/07/04 01:44:04]

request_rang.end() > composition_range_.end() ?

[Seigo Nonaka, 2012/07/04 06:56:58]

The end is exclusive, so treating out of range is safer I think.
On 2012/07/04 01:44:04, James Su wrote:

+    return ui::Range::InvalidRange();
+
+  return ui::Range(
+      request_range.start() - composition_range_.start(),
+      request_range.end() - composition_range_.start());
+}
+
+bool RenderWidgetHostViewMac::GetCachedFirstRectForCharacterRange(
+  NSRange range,
+  NSRect* rect) {
+  TRACE_EVENT0("browser",
+               "RenderWidgetHostViewMac::GetFirstRectForCharacterRange");
+  const ui::Range request_range_in_composition =
+    ConvertCharacterRangeToCompositionRange(ui::Range(range));
+  if (request_range_in_composition == ui::Range::InvalidRange())
+    return false;
+  *rect = NSRectFromCGRect(
+      GetFirstRectForCompositionRange(request_range_in_composition).ToCGRect());
+  return true;
+}
+
 void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
     const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params,
     int gpu_host_id) {
   TRACE_EVENT0("browser",
       "RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped");
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   pending_swap_buffers_acks_.push_back(std::make_pair(params.route_id,
                                                       gpu_host_id));
 
@@ skipping 1625 matching lines @@
           renderWidgetHostView_->render_widget_host_,
           gfx::Point(thePoint.x, thePoint.y));
   return index;
 }
 
 - (NSRect)firstRectForCharacterRange:(NSRange)theRange
                          actualRange:(NSRangePointer)actualRange {
   // TODO(thakis): Pipe |actualRange| through TextInputClientMac machinery.
   if (actualRange)
     *actualRange = theRange;
-  NSRect rect = TextInputClientMac::GetInstance()->GetFirstRectForRange(
-      renderWidgetHostView_->render_widget_host_, theRange);
+  NSRect rect;
+  if (!renderWidgetHostView_->GetCachedFirstRectForCharacterRange(theRange,
+                                                                  &rect)) {
+    rect = TextInputClientMac::GetInstance()->GetFirstRectForRange(
+        renderWidgetHostView_->render_widget_host_, theRange);
+  }
 
   // The returned rectangle is in WebKit coordinates (upper left origin), so
   // flip the coordinate system and then convert it into screen coordinates for
   // return.
   NSRect viewFrame = [self frame];
   rect.origin.y = NSHeight(viewFrame) - rect.origin.y;
   rect.origin.y -= rect.size.height;
   rect = [self convertRectToBase:rect];
   rect.origin = [[self window] convertBaseToScreen:rect.origin];
   return rect;
@@ skipping 392 matching lines @@
   if (!string) return NO;
 
   // If the user is currently using an IME, confirm the IME input,
   // and then insert the text from the service, the same as TextEdit and Safari.
   [self confirmComposition];
   [self insertText:string];
   return YES;
 }
 
 @end