Optimize complex text shaping

Optimize complex text shaping

Optimize the performance of the complex text shaper by avoiding creating
an extra copy of the text and by using a simplified collection mechanism
for 8-bit text runs. This moves the white-space normalization stage from
setup-time to pre-shaping time and changes the normalization pass to use
the harfbuzz buffer instead of a dedicated copy. This in turn allows the
removal of the normalized copy and further avoids up-converting 8-bit to
16-bit runs.

R=behdad@chromium.org, skobes@chromium.org
BUG=462643, 446011
TEST=PerformanceTests/Layout/ArabicLineLayout.html,
     PerformanceTests/Layout/hindi-line-layout.html,
     PerformanceTests/Layout/latin-complex-text.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191227

[eae, 2015-02-25 00:21:30]

Patchset #2 (id:20001) has been deleted

[eae, 2015-02-27 18:09:03]

eae@chromium.org changed reviewers:
+ behdad@google.com, skobes@chromium.org

[behdad_google, 2015-02-27 18:18:30]

lgtm

This is great.  Could you measure any perf improvements?  Also interesting to
see what kind of effects it has on ASCII text going down this path.

[eae, 2015-02-27 18:19:11]

On 2015/02/27 18:18:30, behdad_google wrote:
> lgtm
> 
> This is great.  Could you measure any perf improvements?  Also interesting to
> see what kind of effects it has on ASCII text going down this path.

Running perf tests as we speak and have added a dedicated perf test for complex
8bit text.

[skobes, 2015-02-27 19:20:03]

lgtm % nits

How did you get the right edge of your change description aligned so well? 
That's fairly impressive. :)

https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
File Source/platform/fonts/shaping/HarfBuzzShaper.cpp (right):

https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
Source/platform/fonts/shaping/HarfBuzzShaper.cpp:859: // FIXME: Remove this hack
once all supported distros have HB >= 0.9.38.
Is there a bug # to associate this FIXME with?

https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
Source/platform/fonts/shaping/HarfBuzzShaper.cpp:1028: int currentCharacterIndex
= currentRun->startIndex() + glyphInfos[i].cluster;
Why is the index signed here, but unsigned in adjustSpacing?  Seems like we
should be consistent.

[eae, 2015-02-27 19:29:56]

On 2015/02/27 19:20:03, skobes wrote:
> lgtm % nits
> 
> How did you get the right edge of your change description aligned so well? 
> That's fairly impressive. :)

Practice... :)
https://blink.lc/blink/log/?qt=author&q=eae&showmsg=1


> 
>
https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
> File Source/platform/fonts/shaping/HarfBuzzShaper.cpp (right):
> 
>
https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
> Source/platform/fonts/shaping/HarfBuzzShaper.cpp:859: // FIXME: Remove this
hack
> once all supported distros have HB >= 0.9.38.
> Is there a bug # to associate this FIXME with?

Good point, I'll file one.

https://codereview.chromium.org/957473003/diff/80001/Source/platform/fonts/sh...
> Source/platform/fonts/shaping/HarfBuzzShaper.cpp:1028: int
currentCharacterIndex
> = currentRun->startIndex() + glyphInfos[i].cluster;
> Why is the index signed here, but unsigned in adjustSpacing?  Seems like we
> should be consistent.

TextRun uses signed, I intend to change it in a separate change.

[eae, 2015-02-27 20:20:40]

Patchset #5 (id:100001) has been deleted

[eae, 2015-03-04 00:10:02]

The CQ bit was checked by eae@chromium.org

[eae, 2015-03-04 00:10:02]

The patchset sent to the CQ was uploaded after l-g-t-m from behdad@google.com,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/957473003/#ps120001
(title: " ")

[commit-bot: I haz the power, 2015-03-04 00:10:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/957473003/120001

[eae, 2015-03-04 01:12:22]

The CQ bit was checked by eae@chromium.org

[eae, 2015-03-04 01:12:22]

The patchset sent to the CQ was uploaded after l-g-t-m from behdad@google.com,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/957473003/#ps140001
(title: "Rebase w/HEAD")

[commit-bot: I haz the power, 2015-03-04 01:12:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/957473003/140001

[commit-bot: I haz the power, 2015-03-04 01:25:37]

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

[commit-bot: I haz the power, 2015-03-04 01:25:38]

Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/45813)

[eae, 2015-03-04 02:09:44]

The CQ bit was checked by eae@chromium.org

[eae, 2015-03-04 02:09:44]

The patchset sent to the CQ was uploaded after l-g-t-m from behdad@google.com,
skobes@chromium.org
Link to the patchset: https://codereview.chromium.org/957473003/#ps160001
(title: "Rebase again")

[commit-bot: I haz the power, 2015-03-04 02:11:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/957473003/160001

[commit-bot: I haz the power, 2015-03-04 04:02:30]

Committed patchset #7 (id:160001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191227