Speed up rendering of the input method candidate window.

chrome/browser/chromeos/input_method/candidate_window.cc

Index: chrome/browser/chromeos/input_method/candidate_window.cc
diff --git a/chrome/browser/chromeos/input_method/candidate_window.cc b/chrome/browser/chromeos/input_method/candidate_window.cc
index 68f9c63b21336bde77f3bfcd561ef298f120a257..3d56e347b4ede8766dfafa55efd790873c8f2fdb 100644
--- a/chrome/browser/chromeos/input_method/candidate_window.cc
+++ b/chrome/browser/chromeos/input_method/candidate_window.cc
@@ -236,6 +236,15 @@ int ComputeShortcutColumnWidth(
   return shortcut_column_width;
 }
 
+// Computes the page index. For instance, if the page size is 9, and the
+// cursor is pointing to 13th candidate, the page index will be 1 (2nd
+// page, as the index is zero-origin). Returns -1 on error.
+int ComputePageIndex(const chromeos::InputMethodLookupTable& lookup_table) {
+  if (lookup_table.page_size > 0)
+    return lookup_table.cursor_absolute_index / lookup_table.page_size;
+  return -1;
+}
+
 // Computes candidate column width.
 int ComputeCandidateColumnWidth(
     const chromeos::InputMethodLookupTable& lookup_table) {
@@ -244,8 +253,9 @@ int ComputeCandidateColumnWidth(
       CreateCandidateLabel(lookup_table.orientation));
 
   // Compute the start index of |lookup_table_|.
-  const int current_page_index =
-      lookup_table.cursor_absolute_index / lookup_table.page_size;
+  const int current_page_index = ComputePageIndex(lookup_table);
+  if (current_page_index < 0)
+    return 0;
   const size_t start_from = current_page_index * lookup_table.page_size;
 
   // Compute the max width in candidate labels.
@@ -271,8 +281,9 @@ int ComputeAnnotationColumnWidth(
       CreateAnnotationLabel(lookup_table.orientation));
 
   // Compute the start index of |lookup_table_|.
-  const int current_page_index =
-      lookup_table.cursor_absolute_index / lookup_table.page_size;
+  const int current_page_index = ComputePageIndex(lookup_table);
+  if (current_page_index < 0)
+    return 0;
   const size_t start_from = current_page_index * lookup_table.page_size;
 
   // Compute max width in annotation labels.
@@ -345,6 +356,14 @@ class CandidateWindowView : public views::View {
   // Updates the auxiliary text.
   void UpdateAuxiliaryText(const std::string& utf8_text);
 
+  // Returns true if we should update candidate views in the window.  For
+  // instance, if we are going to show the same candidates as before, we
+  // don't have to update candidate views. This happens when the user just
+  // moves the cursor in the same page in the candidate window.
+  bool ShouldUpdateCandidateViews(
+      const InputMethodLookupTable& previous_table,
+      const InputMethodLookupTable& current_table);
+
   // Updates candidates of the candidate window from |lookup_table|.
   // Candidates are arranged per |orientation|.
   void UpdateCandidates(const InputMethodLookupTable& lookup_table);
@@ -382,10 +401,6 @@ class CandidateWindowView : public views::View {
   // The lookup table (candidates).
   InputMethodLookupTable lookup_table_;
 
-  // Zero-origin index of the current page. If the cursor is on the first
-  // page, the value will be 0.
-  int current_page_index_;
-
   // The index in the current page of the candidate currently being selected.
   int selected_candidate_index_in_page_;
 
@@ -696,8 +711,7 @@ void CandidateView::OnMouseReleased(const views::MouseEvent& event,
 
 CandidateWindowView::CandidateWindowView(
     views::Widget* parent_frame)
-    : current_page_index_(0),
-      selected_candidate_index_in_page_(0),
+    : selected_candidate_index_in_page_(0),
       parent_frame_(parent_frame),
       candidate_area_(NULL),
       footer_area_(NULL),
@@ -777,64 +791,86 @@ void CandidateWindowView::UpdateAuxiliaryText(const std::string& utf8_text) {
   target_label->SetText(UTF8ToWide(utf8_text));
 }
 
+bool CandidateWindowView::ShouldUpdateCandidateViews(
+    const InputMethodLookupTable& previous_table,
+    const InputMethodLookupTable& current_table) {
+  // Check if most table contents are identical.
+  if (previous_table.page_size == current_table.page_size &&
+      previous_table.orientation == current_table.orientation &&
+      previous_table.candidates == current_table.candidates &&
+      previous_table.labels == current_table.labels &&
+      previous_table.annotations == current_table.annotations &&
+      // Check if the page indexes are identical.
+      ComputePageIndex(previous_table) == ComputePageIndex(current_table)) {
+    // If all of the conditions are met, we don't have to update candidate
+    // views.
+    return false;
+  }
+  return true;
+}
+
 void CandidateWindowView::UpdateCandidates(
     const InputMethodLookupTable& lookup_table) {

[Yusuke Sato, 2010/12/01 22:39:04]

Using lookup_table_ and lookup_table at the same time looks a bit confusing to
me. How about renaming the parameter to new_lookup_table or something similar?

[satorux1, 2010/12/02 02:13:07]

Agreed. Done.

-  // Initialize candidate views if necessary.
-  MaybeInitializeCandidateViews(lookup_table);
-
-  // In MaybeInitializeCandidateViews(),
-  // |lookup_table| values and |lookup_table_| values are compared,
-  // so this substitution is needed after the function.
-  lookup_table_ = lookup_table;
-
-  // Compute the index of the current page.
-  current_page_index_ =
-      lookup_table.cursor_absolute_index / lookup_table.page_size;
-
-  // Update the candidates in the current page.
-  const size_t start_from = current_page_index_ * lookup_table.page_size;
-
-  // In some cases, engines send empty shortcut labels. For instance,
-  // ibus-mozc sends empty labels when they show suggestions. In this
-  // case, we should not show shortcut labels.
-  const bool no_shortcut_mode = (start_from < lookup_table_.labels.size() &&
-                                 lookup_table_.labels[start_from] == "");
-  for (size_t i = 0; i < candidate_views_.size(); ++i) {
-    const size_t index_in_page = i;
-    const size_t candidate_index = start_from + index_in_page;
-    CandidateView* candidate_view = candidate_views_[index_in_page];
-    // Set the shortcut text.
-    if (no_shortcut_mode) {
-      candidate_view->SetShortcutText(L"");
-    } else {
-      // At this moment, we don't use labels sent from engines for UX
-      // reasons. First, we want to show shortcut labels in empty rows
-      // (ex. show 6, 7, 8, ... in empty rows when the number of
-      // candidates is 5). Second, we want to add a period after each
-      // shortcut label when the candidate window is horizontal.
-      candidate_view->SetShortcutText(
-          CreateShortcutText(i, lookup_table_.orientation));
+  const bool should_update = ShouldUpdateCandidateViews(lookup_table_,
+                                                        lookup_table);
+  // Updating the candidate views is expensive. We'll skip this if possible.
+  if (should_update) {
+    // Initialize candidate views if necessary.
+    MaybeInitializeCandidateViews(lookup_table);
+
+    // Compute the index of the current page.
+    const int current_page_index = ComputePageIndex(lookup_table);
+    if (current_page_index < 0) {
+      LOG(ERROR) << "Invalid lookup_table: " << lookup_table.ToString();
+      return;
     }
-    // Set the candidate text.
-    if (candidate_index < lookup_table_.candidates.size() &&
-        candidate_index < lookup_table_.annotations.size()) {
-      candidate_view->SetCandidateText(
-          UTF8ToWide(lookup_table_.candidates[candidate_index]));
-      candidate_view->SetAnnotationText(
-          UTF8ToWide(lookup_table_.annotations[candidate_index]));
-      candidate_view->SetRowEnabled(true);
-    } else {
-      // Disable the empty row.
-      candidate_view->SetCandidateText(L"");
-      candidate_view->SetAnnotationText(L"");
-      candidate_view->SetRowEnabled(false);
+
+    // Update the candidates in the current page.
+    const size_t start_from = current_page_index * lookup_table.page_size;
+
+    // In some cases, engines send empty shortcut labels. For instance,
+    // ibus-mozc sends empty labels when they show suggestions. In this
+    // case, we should not show shortcut labels.
+    const bool no_shortcut_mode = (start_from < lookup_table.labels.size() &&
+                                   lookup_table.labels[start_from] == "");
+    for (size_t i = 0; i < candidate_views_.size(); ++i) {
+      const size_t index_in_page = i;
+      const size_t candidate_index = start_from + index_in_page;
+      CandidateView* candidate_view = candidate_views_[index_in_page];
+      // Set the shortcut text.
+      if (no_shortcut_mode) {
+        candidate_view->SetShortcutText(L"");
+      } else {
+        // At this moment, we don't use labels sent from engines for UX
+        // reasons. First, we want to show shortcut labels in empty rows
+        // (ex. show 6, 7, 8, ... in empty rows when the number of
+        // candidates is 5). Second, we want to add a period after each
+        // shortcut label when the candidate window is horizontal.
+        candidate_view->SetShortcutText(
+            CreateShortcutText(i, lookup_table.orientation));
+      }
+      // Set the candidate text.
+      if (candidate_index < lookup_table.candidates.size() &&
+          candidate_index < lookup_table.annotations.size()) {
+        candidate_view->SetCandidateText(
+            UTF8ToWide(lookup_table.candidates[candidate_index]));
+        candidate_view->SetAnnotationText(
+            UTF8ToWide(lookup_table.annotations[candidate_index]));
+        candidate_view->SetRowEnabled(true);
+      } else {
+        // Disable the empty row.
+        candidate_view->SetCandidateText(L"");
+        candidate_view->SetAnnotationText(L"");
+        candidate_view->SetRowEnabled(false);
+      }
     }
   }
+  lookup_table_ = lookup_table;
 
-  // Select the first candidate candidate in the page.
-  const int first_candidate_in_page =
+  // Select the current candidate in the page.
+  const int current_candidate_in_page =
       lookup_table.cursor_absolute_index % lookup_table.page_size;
-  SelectCandidateAt(first_candidate_in_page);
+  SelectCandidateAt(current_candidate_in_page);
 }
 
 void CandidateWindowView::MaybeInitializeCandidateViews(
@@ -858,6 +894,13 @@ void CandidateWindowView::MaybeInitializeCandidateViews(
 
   // If the requested number of views matches the number of current views, and
   // previous and current column width are same, just reuse these.
+  //
+  // Note that the early exit logic is not only useful for improving
+  // performance, but also necessary for the horizontal candidate window
+  // to be redrawn properly. If we get rid of the logic, the horizontal
+  // candidate window won't get redrawn properly for some reason when
+  // there is no size change. You can test this by removing "return" here
+  // and type "ni" with Pinyin input method.
   if (static_cast<int>(candidate_views_.size()) == page_size &&
       lookup_table_.orientation == orientation &&
       previous_shortcut_column_width_ == shortcut_column_width &&
@@ -996,8 +1039,14 @@ views::View* CandidateWindowView::CreateFooterArea() {
 }
 
 void CandidateWindowView::SelectCandidateAt(int index_in_page) {
-  int cursor_absolute_index =
-      lookup_table_.page_size * current_page_index_ + index_in_page;
+  const int current_page_index = ComputePageIndex(lookup_table_);
+  if (current_page_index < 0) {
+    LOG(ERROR) << "Invalid lookup_table: " << lookup_table_.ToString();
+    return;
+  }
+
+  const int cursor_absolute_index =
+      lookup_table_.page_size * current_page_index + index_in_page;
   // Ignore click on out of range views.
   if (cursor_absolute_index < 0 ||
       cursor_absolute_index >=
@@ -1211,7 +1260,7 @@ void CandidateWindowController::Impl::OnSetCursorLocation(
   controller->MoveCandidateWindow(
       controller->cursor_location(),
       controller->candidate_window_->GetHorizontalOffset());
-  // The call is needed to ensure that the candidate window is redrawed
+  // The call is needed to ensure that the candidate window is redrawn
   // properly after the cursor location is changed.
   controller->candidate_window_->ResizeAndSchedulePaint();
 }