Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(225)

Side by Side Diff: ui/gfx/render_text_harfbuzz.cc

Issue 351963002: RenderTextHarfBuzz: Allow mid-glyph cursors in multi-grapheme clusters (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: reuse breakiterator Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "ui/gfx/render_text_harfbuzz.h" 5 #include "ui/gfx/render_text_harfbuzz.h"
6 6
7 #include <map> 7 #include <map>
8 8
9 #include "base/debug/leak_annotations.h" 9 #include "base/debug/leak_annotations.h"
10 #include "base/i18n/bidi_line_iterator.h" 10 #include "base/i18n/bidi_line_iterator.h"
(...skipping 381 matching lines...) Expand 10 before | Expand all | Expand 10 after
392 } 392 }
393 393
394 // A port of hb_icu_script_to_script because harfbuzz on CrOS is built without 394 // A port of hb_icu_script_to_script because harfbuzz on CrOS is built without
395 // hb-icu. See http://crbug.com/356929 395 // hb-icu. See http://crbug.com/356929
396 inline hb_script_t ICUScriptToHBScript(UScriptCode script) { 396 inline hb_script_t ICUScriptToHBScript(UScriptCode script) {
397 if (script == USCRIPT_INVALID_CODE) 397 if (script == USCRIPT_INVALID_CODE)
398 return HB_SCRIPT_INVALID; 398 return HB_SCRIPT_INVALID;
399 return hb_script_from_string(uscript_getShortName(script), -1); 399 return hb_script_from_string(uscript_getShortName(script), -1);
400 } 400 }
401 401
402 template <class Iterator>
403 void GetClusterAtImpl(size_t pos,
404 Range range,
405 Iterator elements_begin,
406 Iterator elements_end,
407 bool reversed,
408 Range* chars,
409 Range* glyphs) {
410 Iterator element = std::upper_bound(elements_begin, elements_end, pos);
411 chars->set_end(element == elements_end ? range.end() : *element);
412 glyphs->set_end(reversed ? elements_end - element : element - elements_begin);
413
414 DCHECK(element != elements_begin);
msw 2014/06/29 22:29:09 nit: can you DCHECK_NE?
ckocagil 2014/07/24 21:46:56 It doesn't compile with DCHECK_NE due to some temp
msw 2014/07/25 00:36:59 Acknowledged.
415 do {
416 --element;
msw 2014/06/29 22:29:09 nit: maybe just make this a one-liner "while(--ele
ckocagil 2014/07/24 21:46:56 Done.
417 } while (element != elements_begin && *element == element[-1]);
msw 2014/06/29 22:29:09 nit: Checking for |element[-1]| is a bit odd, can
ckocagil 2014/07/24 21:46:55 Done.
418 chars->set_start(*element);
419 glyphs->set_start(reversed ?
msw 2014/06/29 22:29:10 nit: move "reversed ?" to the next line.
ckocagil 2014/07/24 21:46:55 Done.
420 elements_end - element : element - elements_begin);
421 if (reversed)
422 *glyphs = Range(glyphs->end(), glyphs->start());
423
424 DCHECK(!chars->is_reversed());
425 DCHECK(!chars->is_empty());
426 DCHECK(!glyphs->is_reversed());
427 DCHECK(!glyphs->is_empty());
428 }
429
402 } // namespace 430 } // namespace
403 431
404 namespace internal { 432 namespace internal {
405 433
406 TextRunHarfBuzz::TextRunHarfBuzz() 434 TextRunHarfBuzz::TextRunHarfBuzz()
407 : width(0), 435 : width(0),
408 preceding_run_widths(0), 436 preceding_run_widths(0),
409 is_rtl(false), 437 is_rtl(false),
410 level(0), 438 level(0),
411 script(USCRIPT_INVALID_CODE), 439 script(USCRIPT_INVALID_CODE),
412 glyph_count(-1), 440 glyph_count(-1),
413 font_size(0), 441 font_size(0),
414 font_style(0), 442 font_style(0),
415 strike(false), 443 strike(false),
416 diagonal_strike(false), 444 diagonal_strike(false),
417 underline(false) {} 445 underline(false) {}
418 446
419 TextRunHarfBuzz::~TextRunHarfBuzz() {} 447 TextRunHarfBuzz::~TextRunHarfBuzz() {}
420 448
421 size_t TextRunHarfBuzz::CharToGlyph(size_t pos) const { 449 void TextRunHarfBuzz::GetClusterAt(size_t pos,
msw 2014/06/29 22:29:09 Hmm, It might make more sense to just offer GetCha
ckocagil 2014/07/24 21:46:55 But in the cases where we need both, we'd have to
msw 2014/07/25 00:37:00 That's fair; it's a careful balance between perf a
422 DCHECK(range.start() <= pos && pos < range.end()); 450 Range* chars,
451 Range* glyphs) const {
452 DCHECK(range.Contains(Range(pos, pos + 1)));
423 453
424 if (!is_rtl) { 454 scoped_ptr<Range> temp_chars;
425 size_t cluster_start = 0; 455 scoped_ptr<Range> temp_glyphs;
426 for (size_t i = 1; i < glyph_count && pos >= glyph_to_char[i]; ++i) 456 if (!chars) {
427 if (glyph_to_char[i] != glyph_to_char[i - 1]) 457 temp_chars.reset(new Range);
428 cluster_start = i; 458 chars = temp_chars.get();
429 return cluster_start; 459 }
460 if (!glyphs) {
461 temp_glyphs.reset(new Range);
462 glyphs = temp_glyphs.get();
430 } 463 }
431 464
432 for (size_t i = 0; i < glyph_count; ++i) { 465 if (is_rtl) {
433 if (pos >= glyph_to_char[i]) 466 GetClusterAtImpl(pos, range, glyph_to_char.rbegin(), glyph_to_char.rend(),
434 return i; 467 true, chars, glyphs);
468 return;
435 } 469 }
436 NOTREACHED(); 470
437 return 0; 471 GetClusterAtImpl(pos, range, glyph_to_char.begin(), glyph_to_char.end(),
472 false, chars, glyphs);
438 } 473 }
439 474
440 Range TextRunHarfBuzz::CharRangeToGlyphRange(const Range& char_range) const { 475 Range TextRunHarfBuzz::CharRangeToGlyphRange(const Range& char_range) const {
441 DCHECK(range.Contains(char_range)); 476 DCHECK(range.Contains(char_range));
442 DCHECK(!char_range.is_reversed()); 477 DCHECK(!char_range.is_reversed());
443 DCHECK(!char_range.is_empty()); 478 DCHECK(!char_range.is_empty());
444 479
445 size_t first = 0; 480 Range start_glyphs;
446 size_t last = 0; 481 Range end_glyphs;
482 GetClusterAt(char_range.start(), NULL, &start_glyphs);
483 GetClusterAt(char_range.end() - 1, NULL, &end_glyphs);
447 484
448 if (is_rtl) { 485 return is_rtl ? Range(end_glyphs.start(), start_glyphs.end()) :
449 // For RTL runs, we subtract 1 from |char_range| to get the leading edges. 486 Range(start_glyphs.start(), end_glyphs.end());
450 last = CharToGlyph(char_range.end() - 1);
451 // Loop until we find a non-empty glyph range. For multi-character clusters,
452 // the loop is needed to find the cluster end. Do the same for LTR below.
453 for (size_t i = char_range.start(); i > range.start(); --i) {
454 first = CharToGlyph(i - 1);
455 if (first != last)
456 return Range(last, first);
457 }
458 return Range(last, glyph_count);
459 }
460
461 first = CharToGlyph(char_range.start());
462 for (size_t i = char_range.end(); i < range.end(); ++i) {
463 last = CharToGlyph(i);
464 if (first != last)
465 return Range(first, last);
466 }
467 return Range(first, glyph_count);
468 } 487 }
469 488
470 // Returns whether the given shaped run contains any missing glyphs. 489 // Returns whether the given shaped run contains any missing glyphs.
471 bool TextRunHarfBuzz::HasMissingGlyphs() const { 490 bool TextRunHarfBuzz::HasMissingGlyphs() const {
472 static const int kMissingGlyphId = 0; 491 static const int kMissingGlyphId = 0;
473 for (size_t i = 0; i < glyph_count; ++i) { 492 for (size_t i = 0; i < glyph_count; ++i) {
474 if (glyphs[i] == kMissingGlyphId) 493 if (glyphs[i] == kMissingGlyphId)
475 return true; 494 return true;
476 } 495 }
477 return false; 496 return false;
478 } 497 }
479 498
480 int TextRunHarfBuzz::GetGlyphXBoundary(size_t text_index, bool trailing) const { 499 Range TextRunHarfBuzz::GetGraphemeBounds(const base::string16& text,
msw 2014/06/29 22:29:09 Perhaps GetGraphemeBounds could also be a RenderTe
ckocagil 2014/07/24 21:46:56 I actually want to move more logic to TextRunHarfB
msw 2014/07/25 00:37:00 Acknowledged.
481 if (text_index == range.end()) { 500 size_t text_index) {
482 trailing = true; 501 DCHECK_LT(text_index, range.end());
483 --text_index; 502
503 Range chars;
504 Range glyphs;
505 GetClusterAt(text_index, &chars, &glyphs);
506 const int cluster_begin_x = SkScalarRoundToInt(positions[glyphs.start()].x());
507 const int cluster_end_x = glyphs.end() < glyph_count ?
508 SkScalarRoundToInt(positions[glyphs.end()].x()) : width;
509
510 if (chars.length() != 1) {
msw 2014/06/29 22:29:09 This block needs a comment about the bounds treatm
ckocagil 2014/07/24 21:46:56 Done.
511 base::i18n::BreakIterator* grapheme_iterator = GetGraphemeIterator(text);
512 if (grapheme_iterator) {
513 int before = 0;
514 int total = 0;
515 for (size_t i = chars.start(); i < chars.end(); ++i) {
msw 2014/06/29 22:29:09 Should we instead calculate and store all the grap
ckocagil 2014/07/24 21:46:56 It seems like ICU is caching bounds internally. Ev
msw 2014/07/25 00:36:59 Acknowledged.
516 if (grapheme_iterator->IsGraphemeBoundary(i - range.start())) {
517 if (i < text_index)
518 ++before;
519 ++total;
520 }
521 }
522
523 if (is_rtl)
524 before = total - before - 1;
525 if (total <= 1) {
msw 2014/06/29 22:29:10 Could total ever really be 0? (DCHECK against that
ckocagil 2014/07/24 21:46:56 You're right, I added a DCHECK.
526 return Range(preceding_run_widths + cluster_begin_x,
msw 2014/06/29 22:29:09 Optionally reverse the conditional to execute the
ckocagil 2014/07/24 21:46:56 Done.
527 preceding_run_widths + cluster_end_x);
528 }
529 DCHECK_GE(before, 0);
msw 2014/06/29 22:29:09 Wouldn't this be false if the index was the first
ckocagil 2014/07/24 21:46:56 I don't understand the question. Perhaps you read
msw 2014/07/25 00:37:00 Yeah, I think you're right, ignore that question.
530 DCHECK_LE(before, total);
531
532 const int cluster_width = cluster_end_x - cluster_begin_x;
533 int grapheme_begin_x = cluster_begin_x;
534 if (before > 0) {
535 grapheme_begin_x += static_cast<int>(0.5f +
536 cluster_width * before / static_cast<float>(total));
537 }
538 int grapheme_end_x = cluster_end_x;
539 if (before != total - 1) {
540 grapheme_end_x = cluster_begin_x + static_cast<int>(0.5f +
541 cluster_width * (before + 1) / static_cast<float>(total));
542 }
543 return Range(preceding_run_widths + grapheme_begin_x,
544 preceding_run_widths + grapheme_end_x);
545 }
484 } 546 }
485 Range glyph_range = CharRangeToGlyphRange(Range(text_index, text_index + 1)); 547
486 const size_t glyph_pos = (is_rtl == trailing) ? 548 return Range(preceding_run_widths + cluster_begin_x,
487 glyph_range.start() : glyph_range.end(); 549 preceding_run_widths + cluster_end_x);
488 const int x = glyph_pos < glyph_count ? 550 }
489 SkScalarRoundToInt(positions[glyph_pos].x()) : width; 551
490 return preceding_run_widths + x; 552 base::i18n::BreakIterator* TextRunHarfBuzz::GetGraphemeIterator(
msw 2014/06/29 22:29:09 It seems like the RenderText should have an iterat
ckocagil 2014/07/24 21:46:55 Done.
553 const base::string16& text) {
554 if (!grapheme_iterator.get()) {
555 grapheme_iterator.reset(new base::i18n::BreakIterator(
556 base::string16(), base::i18n::BreakIterator::BREAK_CHARACTER));
557 if (!grapheme_iterator->Init() || !grapheme_iterator->SetText(
558 text.c_str() + range.start(), range.length())) {
559 NOTREACHED();
560 return NULL;
561 }
562 }
563 return grapheme_iterator.get();
491 } 564 }
492 565
493 } // namespace internal 566 } // namespace internal
494 567
495 RenderTextHarfBuzz::RenderTextHarfBuzz() 568 RenderTextHarfBuzz::RenderTextHarfBuzz()
496 : RenderText(), 569 : RenderText(),
497 needs_layout_(false) {} 570 needs_layout_(false) {}
498 571
499 RenderTextHarfBuzz::~RenderTextHarfBuzz() {} 572 RenderTextHarfBuzz::~RenderTextHarfBuzz() {}
500 573
(...skipping 125 matching lines...) Expand 10 before | Expand all | Expand 10 after
626 break; 699 break;
627 } 700 }
628 pos = iter.pos() - iter.GetString().length(); 701 pos = iter.pos() - iter.GetString().length();
629 } 702 }
630 } 703 }
631 } 704 }
632 return SelectionModel(pos, CURSOR_FORWARD); 705 return SelectionModel(pos, CURSOR_FORWARD);
633 } 706 }
634 707
635 Range RenderTextHarfBuzz::GetGlyphBounds(size_t index) { 708 Range RenderTextHarfBuzz::GetGlyphBounds(size_t index) {
709 EnsureLayout();
636 const size_t run_index = 710 const size_t run_index =
637 GetRunContainingCaret(SelectionModel(index, CURSOR_FORWARD)); 711 GetRunContainingCaret(SelectionModel(index, CURSOR_FORWARD));
638 // Return edge bounds if the index is invalid or beyond the layout text size. 712 // Return edge bounds if the index is invalid or beyond the layout text size.
639 if (run_index >= runs_.size()) 713 if (run_index >= runs_.size())
640 return Range(GetStringSize().width()); 714 return Range(GetStringSize().width());
641 const size_t layout_index = TextIndexToLayoutIndex(index); 715 const size_t layout_index = TextIndexToLayoutIndex(index);
642 return Range(runs_[run_index]->GetGlyphXBoundary(layout_index, false), 716 internal::TextRunHarfBuzz* run = runs_[run_index];
643 runs_[run_index]->GetGlyphXBoundary(layout_index, true)); 717 Range bounds = run->GetGraphemeBounds(GetLayoutText(), layout_index);
718 return run->is_rtl ? Range(bounds.end(), bounds.start()) : bounds;
644 } 719 }
645 720
646 std::vector<Rect> RenderTextHarfBuzz::GetSubstringBounds(const Range& range) { 721 std::vector<Rect> RenderTextHarfBuzz::GetSubstringBounds(const Range& range) {
647 DCHECK(!needs_layout_); 722 DCHECK(!needs_layout_);
648 DCHECK(Range(0, text().length()).Contains(range)); 723 DCHECK(Range(0, text().length()).Contains(range));
649 Range layout_range(TextIndexToLayoutIndex(range.start()), 724 Range layout_range(TextIndexToLayoutIndex(range.start()),
650 TextIndexToLayoutIndex(range.end())); 725 TextIndexToLayoutIndex(range.end()));
651 DCHECK(Range(0, GetLayoutText().length()).Contains(layout_range)); 726 DCHECK(Range(0, GetLayoutText().length()).Contains(layout_range));
652 727
653 std::vector<Rect> rects; 728 std::vector<Rect> rects;
654 if (layout_range.is_empty()) 729 if (layout_range.is_empty())
655 return rects; 730 return rects;
656 std::vector<Range> bounds; 731 std::vector<Range> bounds;
657 732
658 // Add a Range for each run/selection intersection. 733 // Add a Range for each run/selection intersection.
659 // TODO(msw): The bounds should probably not always be leading the range ends. 734 // TODO(msw): The bounds should probably not always be leading the range ends.
660 for (size_t i = 0; i < runs_.size(); ++i) { 735 for (size_t i = 0; i < runs_.size(); ++i) {
661 const internal::TextRunHarfBuzz* run = runs_[visual_to_logical_[i]]; 736 internal::TextRunHarfBuzz* run = runs_[visual_to_logical_[i]];
662 Range intersection = run->range.Intersect(layout_range); 737 Range intersection = run->range.Intersect(layout_range);
663 if (intersection.IsValid()) { 738 if (intersection.IsValid()) {
664 DCHECK(!intersection.is_reversed()); 739 DCHECK(!intersection.is_reversed());
665 Range range_x(run->GetGlyphXBoundary(intersection.start(), false), 740 Range range_x(
666 run->GetGlyphXBoundary(intersection.end(), false)); 741 run->GetGraphemeBounds(GetLayoutText(), run->is_rtl ?
742 intersection.end() - 1 : intersection.start()).start(),
743 run->GetGraphemeBounds(GetLayoutText(), run->is_rtl ?
744 intersection.start() : intersection.end() - 1).end());
667 if (range_x.is_empty()) 745 if (range_x.is_empty())
668 continue; 746 continue;
669 range_x = Range(range_x.GetMin(), range_x.GetMax()); 747 range_x = Range(range_x.GetMin(), range_x.GetMax());
670 // Union this with the last range if they're adjacent. 748 // Union this with the last range if they're adjacent.
671 DCHECK(bounds.empty() || bounds.back().GetMax() <= range_x.GetMin()); 749 DCHECK(bounds.empty() || bounds.back().GetMax() <= range_x.GetMin());
672 if (!bounds.empty() && bounds.back().GetMax() == range_x.GetMin()) { 750 if (!bounds.empty() && bounds.back().GetMax() == range_x.GetMin()) {
673 range_x = Range(bounds.back().GetMin(), range_x.GetMax()); 751 range_x = Range(bounds.back().GetMin(), range_x.GetMax());
674 bounds.pop_back(); 752 bounds.pop_back();
675 } 753 }
676 bounds.push_back(range_x); 754 bounds.push_back(range_x);
(...skipping 23 matching lines...) Expand all
700 DCHECK_LE(text_index, text().length()); 778 DCHECK_LE(text_index, text().length());
701 return text_index; 779 return text_index;
702 } 780 }
703 781
704 bool RenderTextHarfBuzz::IsValidCursorIndex(size_t index) { 782 bool RenderTextHarfBuzz::IsValidCursorIndex(size_t index) {
705 if (index == 0 || index == text().length()) 783 if (index == 0 || index == text().length())
706 return true; 784 return true;
707 if (!IsValidLogicalIndex(index)) 785 if (!IsValidLogicalIndex(index))
708 return false; 786 return false;
709 EnsureLayout(); 787 EnsureLayout();
710 // Disallow indices amid multi-character graphemes by checking glyph bounds. 788 internal::TextRunHarfBuzz* run =
msw 2014/06/29 22:29:09 Please verify that these examples and Issue 327903
ckocagil 2014/07/24 21:46:56 Done. Added a test too.
711 // These characters are not surrogate-pairs, but may yield a single glyph: 789 runs_[GetRunContainingCaret(SelectionModel(index, CURSOR_BACKWARD))];
712 // \x0915\x093f - (ki) - one of many Devanagari biconsonantal conjuncts. 790 base::i18n::BreakIterator* grapheme_iterator =
msw 2014/06/29 22:29:09 nit: Name this |iter| for a one-liner and below "r
ckocagil 2014/07/24 21:46:56 Done. (I made it "return !iter || iter->...") sinc
msw 2014/07/25 00:36:59 Acknowledged.
713 // \x0e08\x0e33 - (cho chan + sara am) - a Thai consonant and vowel pair. 791 run->GetGraphemeIterator(GetLayoutText());
714 return GetGlyphBounds(index) != GetGlyphBounds(index - 1); 792 if (!grapheme_iterator)
793 return true;
794 return grapheme_iterator->IsGraphemeBoundary(index - run->range.start());
715 } 795 }
716 796
717 void RenderTextHarfBuzz::ResetLayout() { 797 void RenderTextHarfBuzz::ResetLayout() {
718 needs_layout_ = true; 798 needs_layout_ = true;
719 } 799 }
720 800
721 void RenderTextHarfBuzz::EnsureLayout() { 801 void RenderTextHarfBuzz::EnsureLayout() {
722 if (needs_layout_) { 802 if (needs_layout_) {
723 runs_.clear(); 803 runs_.clear();
724 804
(...skipping 257 matching lines...) Expand 10 before | Expand all | Expand 10 after
982 // Shape the text. 1062 // Shape the text.
983 hb_shape(harfbuzz_font, buffer, NULL, 0); 1063 hb_shape(harfbuzz_font, buffer, NULL, 0);
984 1064
985 // Populate the run fields with the resulting glyph data in the buffer. 1065 // Populate the run fields with the resulting glyph data in the buffer.
986 unsigned int glyph_count = 0; 1066 unsigned int glyph_count = 0;
987 hb_glyph_info_t* infos = hb_buffer_get_glyph_infos(buffer, &glyph_count); 1067 hb_glyph_info_t* infos = hb_buffer_get_glyph_infos(buffer, &glyph_count);
988 hb_glyph_position_t* hb_positions = hb_buffer_get_glyph_positions(buffer, 1068 hb_glyph_position_t* hb_positions = hb_buffer_get_glyph_positions(buffer,
989 NULL); 1069 NULL);
990 run->glyph_count = glyph_count; 1070 run->glyph_count = glyph_count;
991 run->glyphs.reset(new uint16[run->glyph_count]); 1071 run->glyphs.reset(new uint16[run->glyph_count]);
992 run->glyph_to_char.reset(new uint32[run->glyph_count]); 1072 run->glyph_to_char.resize(run->glyph_count);
993 run->positions.reset(new SkPoint[run->glyph_count]); 1073 run->positions.reset(new SkPoint[run->glyph_count]);
994 for (size_t i = 0; i < run->glyph_count; ++i) { 1074 for (size_t i = 0; i < run->glyph_count; ++i) {
995 run->glyphs[i] = infos[i].codepoint; 1075 run->glyphs[i] = infos[i].codepoint;
996 run->glyph_to_char[i] = infos[i].cluster; 1076 run->glyph_to_char[i] = infos[i].cluster;
997 const int x_offset = 1077 const int x_offset =
998 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].x_offset)); 1078 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].x_offset));
999 const int y_offset = 1079 const int y_offset =
1000 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].y_offset)); 1080 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].y_offset));
1001 run->positions[i].set(run->width + x_offset, y_offset); 1081 run->positions[i].set(run->width + x_offset, y_offset);
1002 run->width += 1082 run->width +=
1003 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].x_advance)); 1083 SkScalarRoundToInt(SkFixedToScalar(hb_positions[i].x_advance));
1004 } 1084 }
1005 1085
1006 hb_buffer_destroy(buffer); 1086 hb_buffer_destroy(buffer);
1007 hb_font_destroy(harfbuzz_font); 1087 hb_font_destroy(harfbuzz_font);
1008 } 1088 }
1009 1089
1010 } // namespace gfx 1090 } // namespace gfx
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698