[LayoutNG] Misc fixes in ShapingLineBreaker

third_party/WebKit/Source/platform/fonts/shaping/ShapeResult.h

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 49 matching lines @@
   static PassRefPtr<ShapeResult> CreateForTabulationCharacters(
       const Font*,
       const TextRun&,
       float position_offset,
       unsigned count);
   ~ShapeResult();
 
   float Width() const { return width_; }
   LayoutUnit SnappedWidth() const { return LayoutUnit::FromFloatCeil(width_); }
   const FloatRect& Bounds() const { return glyph_bounding_box_; }
+  // The character start index of a range shape result.
+  unsigned CharacterStartIndex() const;

[eae, 2017/05/12 18:45:49]

CharacterOffset perhaps and CharacterLength for (NumCharacters) to be more
consistent?

or CharacterStartIndex and CharacterEndIndex

[kojii, 2017/05/12 19:08:13]

Existing functions such as OffsetForPosition() use "Offset" meaning offset from
the first shape range, so I avoided "Offset". ...or should all these function
changed to use offset from the beginning of a full string?

If not, I might add CharacterEndIndex(). Or if you get better suggestions after
reading my reply.

[eae, 2017/05/12 21:24:05]

Ah, fair enough. Perhaps we could be even more explicit and call it
StartIndexForResult and EndIndexForResult?

Either way, as long as the comment is accurate (which it is) the name is fine as
is. Thanks for the explanation.

   unsigned NumCharacters() const { return num_characters_; }
   void FallbackFonts(HashSet<const SimpleFontData*>*) const;
   TextDirection Direction() const {
     return static_cast<TextDirection>(direction_);
   }
   bool Rtl() const { return Direction() == TextDirection::kRtl; }
   bool HasVerticalOffsets() const { return has_vertical_offsets_; }
 
   // For memory reporting.
   size_t ByteSize() const;
 
+  enum OffsetOptions {
+    // If the position is outside of this result, returns the start or the end
+    // offset depends on the position.
+    kOutsideAsStartOrEnd,
+    // If the position is outside of this result, returns the most right offset.
+    kOutsideAsRight,
+  };
+  unsigned OffsetForPosition(float target_x, OffsetOptions) const;

[eae, 2017/05/12 18:45:49]

Isn't kOutsideAsStartOrEnd always the desirable behavior?

[kojii, 2017/05/12 19:08:13]

I think so, but there are bunch of existing callers and hard to determine if
anyone is relying on kOutsideAsRight behavior.

I'll try to change and see if any tests break.

   unsigned OffsetForPosition(float target_x, bool include_partial_glyphs) const;
   float PositionForOffset(unsigned offset) const;
   LayoutUnit SnappedStartPositionForOffset(unsigned offset) const {
     return LayoutUnit::FromFloatFloor(PositionForOffset(offset));
   }
   LayoutUnit SnappedEndPositionForOffset(unsigned offset) const {
     return LayoutUnit::FromFloatCeil(PositionForOffset(offset));
   }
 
   PassRefPtr<ShapeResult> ApplySpacingToCopy(ShapeResultSpacing&,
@@ skipping 34 matching lines @@
   unsigned has_vertical_offsets_ : 1;
 
   friend class HarfBuzzShaper;
   friend class ShapeResultBuffer;
   friend class ShapeResultBloberizer;
 };
 
 }  // namespace blink
 
 #endif  // ShapeResult_h