Make wavy underlines go all the way to the end of the inline.

Make wavy underlines go all the way to the end of the inline.

Previously, the wavy underline code only knew how to draw a complete
period.

This change adds the ability to draw a partial period of the wave.

Previously, the code tried, but failed, for reasons I cannot
determine, to fit the width of the inline by extending the period to
be an integral factor of the width.

This is bad because as you type text, the underline wave of earlier
characters would jiggle.

This fixes that by not changing the period of the wave when the length changes.

It's interesting to compare this to the previous code. Here's a test
that shows this in Safari/Chrome:

   http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3543

Notice how as you type "i"s, the underline changes how far it extends
(obviously a bug) but also what the period of the wave is (which looks
especially ugly when typing).

Firefox, on the other hand, renders something that isn't actually a
wave; it's a saw-tooth pattern with straight lines between each
direction change. This works for small font sizes at low densities,
but is not great on modern screens.

This CL addresses this issue by computing the bezier curve control
points for the curve that consists of just the remaining fraction of
the period. To do this, however, it allocates an object and solves a
bezier for x (which includes a numerical integration), which is
certainly a performance concern. Apps that try to wavy-underline an
entire UI are going to maybe suffer. Since this is typically going to
be used for spelling checkers, it means people who spell better will
have better performance.

(I haven't tested this to see what the perf impact actually is. If
it's actually bad, we can probably cache the results of the
integration pretty trivially.)

R=eseidel@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/141b69fe583acb20592ec700c137578ca9f27438

[abarth-chromium, 2015-06-20 06:00:08]

abarth@chromium.org changed reviewers:
+ abarth@chromium.org

[abarth-chromium, 2015-06-20 06:00:09]

Sorry, don't see anything obvious that might crash.

[Hixie, 2015-06-22 21:59:36]

On 2015/06/20 at 06:00:09, abarth wrote:
> Sorry, don't see anything obvious that might crash.

The bug was that I was doing "/ 2 * step" instead of "/ (2 * step)".

[eseidel, 2015-06-22 22:10:02]

eseidel@chromium.org changed reviewers:
+ eseidel@chromium.org

[eseidel, 2015-06-22 22:10:03]

Ojan/eae FYI.

https://codereview.chromium.org/1189403005/diff/20001/sky/engine/core/renderi...
File sky/engine/core/rendering/InlineTextBox.cpp (right):

https://codereview.chromium.org/1189403005/diff/20001/sky/engine/core/renderi...
sky/engine/core/rendering/InlineTextBox.cpp:871: OwnPtr<UnitBezier> bezier =
adoptPtr(new UnitBezier((Curve::x(controlPoint1) - x) * xScale,
How hot is this?  Do we care that you're mallocing here?

https://codereview.chromium.org/1189403005/diff/20001/sky/engine/core/renderi...
sky/engine/core/rendering/InlineTextBox.cpp:878: float qxb =  x*u1*u1 +
Curve::x(controlPoint1)*2*t*u1 + Curve::x(controlPoint2)*t*t;
Spaces?

[eae, 2015-06-22 22:15:44]

Thanks Eric!

[eseidel, 2015-06-22 22:22:37]

We checked and Blink and Safari both have identical code here.  It's guarded
behind a runtime flag in Blink, and on in SAfari using a
-webkit-text-decoration-style -webkit prefix.

So this is a bug in both Blink and WebKit which Ian is fixing here.  One might
argue about how he chose to fix it, but regardless this is a fix.

[eseidel, 2015-06-22 22:36:51]

lgtm

[eseidel, 2015-06-22 22:37:25]

Please update your lines to have spaces around your operators, but otherwise
lgtm.

Can we pixel test this?

[Hixie, 2015-06-22 22:43:10]

Committed patchset #2 (id:20001) manually as
141b69fe583acb20592ec700c137578ca9f27438 (presubmit successful).