Fix ShapeResult::OffsetForPosition edge behavior

Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases to be better.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318
Review-Url: https://codereview.chromium.org/2924463002
Cr-Commit-Position: refs/heads/master@{#477263}
Committed: https://chromium.googlesource.com/chromium/src/+/1fb6007f25ce38b50bd6700f292e65ca8e501e7b

[kojii, 2017-06-03 14:04:31]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-03 14:04:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kojii, 2017-06-03 18:25:07]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-03 18:25:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-03 20:13:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-03 20:13:34]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[kojii, 2017-06-04 07:05:35]

Description was changed from

==========
hboffset

BUG=
==========

to

==========
Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318
==========

[kojii, 2017-06-04 07:07:30]

kojii@chromium.org changed reviewers:
+ eae@chromium.org

[kojii, 2017-06-04 07:07:31]

PTAL. A few more fixes are needed until we can enable shaper-driven-line
breaker, but with this and another WIP, I have only 2 more tests to analyze.

[kojii, 2017-06-04 07:08:57]

Description was changed from

==========
Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318
==========

to

==========
Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases to be better.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318
==========

[eae, 2017-06-06 00:03:54]

LGTM w/suggestion

https://codereview.chromium.org/2924463002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/fonts/shaping/ShapeResult.cpp (right):

https://codereview.chromium.org/2924463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/fonts/shaping/ShapeResult.cpp:136: if
(!Rtl()) {
How about

static bool targetPastEdge(bool rtl, float target_x, float next_x) {
  // In RTL, the edge belongs to the character on left.
  if (rtl)
    return target_x <= next_x;
  
  // In LTR, the edge belongs to the character on right.
  return target_x < next_x; 
}

if (targetPastEdge(Rtl(), target_x, next_x)
 return current_character_index;

[kojii, 2017-06-06 09:25:02]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-06 09:25:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 11:09:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-06 11:09:35]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-06-06 11:34:51]

Thank you for great advice as always, I like it. Took in PS6.

[kojii, 2017-06-06 11:34:55]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-06-06 11:34:55]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2924463002/#ps100001
(title: "eae review suggestion")

[commit-bot: I haz the power, 2017-06-06 11:35:01]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 12:02:28]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496748895430040,
"parent_rev": "d9ba02556dbc40c2059778aaab0f3e1dabc1acd0", "commit_rev":
"1fb6007f25ce38b50bd6700f292e65ca8e501e7b"}

[commit-bot: I haz the power, 2017-06-06 12:02:38]

Description was changed from

==========
Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases to be better.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318
==========

to

==========
Fix ShapeResult::OffsetForPosition edge behavior

This patch fixes ShapeResult::OffsetForPosition, when the position is at
exact character boundaries and include_partial_glyphs is false, to
return the next character.

Before this change, boundaries were part of previous characters. This
function is used to identify how many characters can fit in a space,
such as ShapingLineBreaker, ellipsis, and break-all, and thus the old
behavior failed to fit a character exactly equal to the available space.
Existing tests were rebaselined for such cases to be better.

This fix also make OffsetForPosition matches to PositionForOffset.

BUG=729318

Review-Url: https://codereview.chromium.org/2924463002
Cr-Commit-Position: refs/heads/master@{#477263}
Committed:
https://chromium.googlesource.com/chromium/src/+/1fb6007f25ce38b50bd6700f292e...
==========

[commit-bot: I haz the power, 2017-06-06 12:02:40]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/1fb6007f25ce38b50bd6700f292e...