Bug 176497 - [GTK] Bring back line height rounding when computing font metrics
Summary: [GTK] Bring back line height rounding when computing font metrics
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2017-09-07 01:20 PDT by Carlos Garcia Campos
Modified: 2017-09-07 20:52 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.63 KB, patch)
2017-09-07 01:22 PDT, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-09-07 01:20:40 PDT
In r221670 we changed the way we get the metrics to avoid rounding that was causing a negative line gap to be computed. Since the font metrics value is indeed a float value, we also removed the rounding when setting the line height. However, this caused some test failures because now we report non integer line heights, see for example:

https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20(Tests)/r221719%20(3018)/fast/css/font-calculated-value-pretty-diff.html
https://build.webkit.org/results/GTK%20Linux%2064-bit%20Release%20(Tests)/r221719%20(3018)/fast/ruby/ruby-line-height-pretty-diff.html

I think it would be better to bring back the rounding for consistency with all other ports, but also with other browsers. This will require another rebaseline, but a lot smaller :-)
Comment 1 Carlos Garcia Campos 2017-09-07 01:22:33 PDT
Created attachment 320103 [details]
Patch
Comment 2 Carlos Garcia Campos 2017-09-07 01:52:49 PDT
Committed r221724: <http://trac.webkit.org/changeset/221724>
Comment 3 Michael Catanzaro 2017-09-07 08:58:08 PDT
Tom, did this reintroduce the bug with Evolution? Is it OK?
Comment 4 Tomas Popela 2017-09-07 20:52:27 PDT
(In reply to Michael Catanzaro from comment #3)
> Tom, did this reintroduce the bug with Evolution? Is it OK?

I don't know about Evolution (I don't work on it, nor use it anymore), but the testcase that made for bug 149703 was running fine when I tested it yesterday. It was running fine with and without this change. I will probably add it as a layout test, so we are sure, that it works in the future.