Bug 51456 – Introduce FontMetrics abstraction

• Component: Text
• Status: RESOLVED FIXED

[Nikolas Zimmermann]

In order to fix several SVG bugs, regarding very small font sizes, I'd ideally have access to floating-point ascent/descent information. SimpleFontData stores lineGap/lineHeight/ascent/descent as integers at the moment. For CG the ascent/descent is actually a floating-point number that (integerAscent scaled according to emPerUnits, multiplied with pointSize == floatAscent) gets rounded using lroundf and then stored as integer.

Just changing the floats to integers, and storing accurate ascent/descent/.. information is not easily doable, as the int ascent() accessors would need to round the float m_ascent, every time on access.
That means for HTML layout (where we only need integers atm) we'd round much more often than necessary.
Storing both int and float variants of the metrics in Font, seems also redundant and costly.

I'm proposing a new FontMetrics class, that holds floating-point values for lineGap/lineHeight/ascent/descent, plus another IntegerFontMetrics class, that holds rounded integers.
The IntegerFontMetrics class is dynamically created and stored as OwnPtr in FontMetrics. Whenever someone accesses the int versions of ascent/descent/etc.. that datastructure is created
and stored in FontMetrics. Follow-up calls to ascent/descent, just return the cached integer, no need to round again.

Of course that will double the amount of memory to hold font metrics, in the worst-case, but gives us the ability to expose accurate ascent/descent/lineHeight/lineGap values for users that need it -> SVG.
I've worked on this the last weeks, and could fix all bugs related to small-font usages & SVG with this approach.

Stripped down my current work, to a patch which just introduces FontMetrics, w/o any functional changes.
I'm open for all ideas, on how to make it better... uploading the patch to discuss soon.

[Nikolas Zimmermann]

Created attachment 77220 [details]
Patch

[WebKit Review Bot]

Attachment 77220 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7231100

[WebKit Review Bot]

Attachment 77220 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7333056

[Early Warning System Bot]

Attachment 77220 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7284072

(In reply to comment #0)
> In order to fix several SVG bugs, regarding very small font sizes, I'd ideally have access to floating-point ascent/descent information. SimpleFontData stores lineGap/lineHeight/ascent/descent as integers at the moment. For CG the ascent/descent is actually a floating-point number that (integerAscent scaled according to emPerUnits, multiplied with pointSize == floatAscent) gets rounded using lroundf and then stored as integer.
> 
> Just changing the floats to integers, and storing accurate ascent/descent/.. information is not easily doable, as the int ascent() accessors would need to round the float m_ascent, every time on access.

I am not sure I understand why that would not be easily doable. Can you explain?

> That means for HTML layout (where we only need integers atm) we'd round much more often than necessary.

How much more often?

> Storing both int and float variants of the metrics in Font, seems also redundant and costly.

Okay…

> 
> I'm proposing a new FontMetrics class, that holds floating-point values for lineGap/lineHeight/ascent/descent, plus another IntegerFontMetrics class, that holds rounded integers.
> The IntegerFontMetrics class is dynamically created and stored as OwnPtr in FontMetrics. Whenever someone accesses the int versions of ascent/descent/etc.. that datastructure is created
> and stored in FontMetrics. Follow-up calls to ascent/descent, just return the cached integer, no need to round again.

So, if I understand correctly, with the rare exception of fonts used only in SVG, all fonts will store both the floating point and integer values. That seems to be at least as “redundant and costly” as just adding the floating point data members alongside the integer ones.

> Of course that will double the amount of memory to hold font metrics, in the worst-case, but gives us the ability to expose accurate ascent/descent/lineHeight/lineGap values for users that need it -> SVG.
> I've worked on this the last weeks, and could fix all bugs related to small-font usages & SVG with this approach.
> 
> Stripped down my current work, to a patch which just introduces FontMetrics, w/o any functional changes.
> I'm open for all ideas, on how to make it better... uploading the patch to discuss soon.

I am not convinced that this is the best approach to take.

[Nikolas Zimmermann]

(In reply to comment #5)
> (In reply to comment #0)
> > In order to fix several SVG bugs, regarding very small font sizes, I'd ideally have access to floating-point ascent/descent information. SimpleFontData stores lineGap/lineHeight/ascent/descent as integers at the moment. For CG the ascent/descent is actually a floating-point number that (integerAscent scaled according to emPerUnits, multiplied with pointSize == floatAscent) gets rounded using lroundf and then stored as integer.
> > 
> > Just changing the floats to integers, and storing accurate ascent/descent/.. information is not easily doable, as the int ascent() accessors would need to round the float m_ascent, every time on access.
> 
> I am not sure I understand why that would not be easily doable. Can you explain?
It's easy doable, sorry, but I think we'd round a lot more than now..

> 
> > That means for HTML layout (where we only need integers atm) we'd round much more often than necessary.
> 
> How much more often?
On every access of any font metrics like ascent/descent. During each layout call..

> 
> > Storing both int and float variants of the metrics in Font, seems also redundant and costly.
> 
> Okay…
> 
> > 
> > I'm proposing a new FontMetrics class, that holds floating-point values for lineGap/lineHeight/ascent/descent, plus another IntegerFontMetrics class, that holds rounded integers.
> > The IntegerFontMetrics class is dynamically created and stored as OwnPtr in FontMetrics. Whenever someone accesses the int versions of ascent/descent/etc.. that datastructure is created
> > and stored in FontMetrics. Follow-up calls to ascent/descent, just return the cached integer, no need to round again.
> 
> So, if I understand correctly, with the rare exception of fonts used only in SVG, all fonts will store both the floating point and integer values. That seems to be at least as “redundant and costly” as just adding the floating point data members alongside the integer ones.
Yeah it's not a good approach, it only avoids the cost of storing both integer & float for SVG... for HTML it doubles the cost, so it's not good.
 
> I am not convinced that this is the best approach to take.
I agree, I mostly uploaded it to discuss the issue.
Another option would be to use a union { float f; int i; } and to decide upon creation of the FontMetrics object, wheter floating point or integer accurancy is desired. That would only require rounding the incoming float values once, if integers are wanted, but it makes it impossible to reconstruct floating-point ascent/descent values, for the Font object, once constructed with IntegerMetrics.

The most easy approach would be to use int ascent() const { return lroundf(m_ascent); } of course, but I was worried that it would be a performance decrease, so I thought that maybe it's a better trade-off to use more memory for the Font object, rather than decreasing performance.
Note, I did not benchmark this at all, it's just a feeling.

If I upload another, simpler version, which just does the rounding on every invocation, would you be able to run it through Apples PLT?

[Nikolas Zimmermann]

Created attachment 77235 [details]
Patch v2

The easier way: always round, don't cache an IntegerFontMetrics object.

[Simon Fraser (smfr)]

Comment on attachment 77235 [details]
Patch v2

There should be no need to add a new class and touch dozens of files for this. Why not just start by rounding in the accessors, and measuring performance?

[WebKit Review Bot]

Attachment 77235 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7252110

[WebKit Review Bot]

Attachment 77235 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7306109

[Nikolas Zimmermann]

(In reply to comment #8)
> (From update of attachment 77235 [details])
> There should be no need to add a new class and touch dozens of files for this. Why not just start by rounding in the accessors, and measuring performance?
Well, I still think introducing a FontMetrics class is cleaner, than duplicating each accessor in Font and SimpleFontData. Especially if I want to introduce floatAscent/floatDescent/floatLineGap/floatLineSpacing methods, I'd need to add them to both Font and SimpleFontData, which is a bad design. The metrics should go into their own class.

Of course, to just measure performance it's not needed, but I have already written it, so why not?

[Build Bot]

Attachment 77235 [details] did not build on win:
Build output: http://queues.webkit.org/results/7272105

[Nikolas Zimmermann]

Created attachment 77315 [details]
Patch v3

Patch v3: Attempt to make it build on more platform.
Is it really unacceptable to just apply this patch and try a PLT run with it?? It at least works fine on Mac, at the moment.

[WebKit Review Bot]

Attachment 77315 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7214141

[Nikolas Zimmermann]

Created attachment 77317 [details]
Patch v4

Uploading Patch v4, hopefully chromium build works now.

[Build Bot]

Attachment 77317 [details] did not build on win:
Build output: http://queues.webkit.org/results/7211127

[Nikolas Zimmermann]

I performance tested this for a while (without PLT, of course) and didn't notice any measurable slowdown.

I created an artifical testcase, compiled w/o optimizations:
#include <math.h>

class FooInt {
public:
    FooInt(float bar)
        : m_bar(lroundf(bar))
    {
    }

    int bar() const { return m_bar; }

private:
    int m_bar;
};


class FooFloat {
public:
    FooFloat(float bar)
        : m_bar(bar)
    {
    }

    int bar() const { return lroundf(m_bar); }

private:
    float m_bar;
};

int main()
{
    int result = 0;
    FooInt foo(1.7f);
    for (long i = 0; i < 1e8; ++i) {
        result = foo.bar();
    }
}

Results:
using FooFloat(1.7f) and 1e8 calls: 1.531s (1e8 lroundf calls)
using FooInt(1.7f) and 1e8 calls: 0.6754s (1 lroundf call)

This is artifical of course, but it gives a rough estimation of the slowdown, but of course 1e8 calls is _a lot_. The usual count is several orders of magnitudes lower. I suspect that's why I didn't notice any slowdown when runing DRT.

I tested 10 DRT runs w/o the patch and 10 with the patch, and there was no difference, except the usual deviations betweens runs.

I'll upload a new patch soon, fixing the win build, hopefully the approach is sane enough now to get it reviewed?

[Nikolas Zimmermann]

Created attachment 77452 [details]
Patch v5

[Build Bot]

Attachment 77452 [details] did not build on win:
Build output: http://queues.webkit.org/results/7292189

[Nikolas Zimmermann]

Created attachment 77453 [details]
Patch v6

Forgot to alter files in WebKit/win, uploading patch v6, let's see if win build is fixed.

[Build Bot]

Attachment 77453 [details] did not build on win:
Build output: http://queues.webkit.org/results/7274174

[Nikolas Zimmermann]

Created attachment 77491 [details]
Patch v7

Fixed another win problem. void FontMetrics(..) clashes with FontMetrics class, use WebCore:: prefix, hopefully it builds now.

[Build Bot]

Attachment 77491 [details] did not build on win:
Build output: http://queues.webkit.org/results/7295199

[Nikolas Zimmermann]

Created attachment 77501 [details]
Patch v8

Fix WebKit2/win build.

[Nikolas Zimmermann]

Comment on attachment 77501 [details]
Patch v8

Going to rebase to trunk first, the patch doesn't seem to apply on cr-mac-ews otherhwise, obsoleting the patch in the meanwhile.

[Nikolas Zimmermann]

Created attachment 77502 [details]
Patch v9

Rebased against trunk, hopefully patch v9 now builds everywhere.

[Nikolas Zimmermann]

I don't understand why mac-ews and cr-mac-ews reject this patch. It tells me:

patching file WebCore/WebCore.vcproj/WebCore.vcproj
Hunk #1 FAILED at 26381.
1 out of 1 hunk FAILED -- saving rejects to file WebCore/WebCore.vcproj/WebCore.vcproj.rej

I'm on trunk, and the patch just works fine on the other ews bots:

nikolaszimmermann ~/Coding/WebKit > svn diff WebCore/WebCore.vcproj
Index: WebCore/WebCore.vcproj/WebCore.vcproj
===================================================================
--- WebCore/WebCore.vcproj/WebCore.vcproj	(revision 74683)
+++ WebCore/WebCore.vcproj/WebCore.vcproj	(working copy)
@@ -26381,6 +26381,10 @@
 					>
 				</File>
 				<File
+					RelativePath="..\platform\graphics\FontMetrics.h"
+					>
+				</File>
+				<File
 					RelativePath="..\platform\graphics\FontRenderingMode.h"
 					>
 				</File>
nikolaszimmermann ~/Coding/WebKit > svn up WebCore/WebCore.vcproj
At revision 74683.

Any idea?

[Dirk Schulze]

(In reply to comment #27)
> I don't understand why mac-ews and cr-mac-ews reject this patch. It tells me:
> 
> patching file WebCore/WebCore.vcproj/WebCore.vcproj
> Hunk #1 FAILED at 26381.
> 1 out of 1 hunk FAILED -- saving rejects to file WebCore/WebCore.vcproj/WebCore.vcproj.rej
> 
> I'm on trunk, and the patch just works fine on the other ews bots:
> 
> nikolaszimmermann ~/Coding/WebKit > svn diff WebCore/WebCore.vcproj
> Index: WebCore/WebCore.vcproj/WebCore.vcproj
> ===================================================================
> --- WebCore/WebCore.vcproj/WebCore.vcproj    (revision 74683)
> +++ WebCore/WebCore.vcproj/WebCore.vcproj    (working copy)
> @@ -26381,6 +26381,10 @@
>                      >
>                  </File>
>                  <File
> +                    RelativePath="..\platform\graphics\FontMetrics.h"
> +                    >
> +                </File>
> +                <File
>                      RelativePath="..\platform\graphics\FontRenderingMode.h"
>                      >
>                  </File>
> nikolaszimmermann ~/Coding/WebKit > svn up WebCore/WebCore.vcproj
> At revision 74683.
> 
> Any idea?

Not sure if you hit the same problem like me, but I had this with some editors (and not always, just sometimes). It helped to use another editor. I think I took vi.

[Nikolas Zimmermann]

(In reply to comment #28)
> > 
> > Any idea?
> 
> Not sure if you hit the same problem like me, but I had this with some editors (and not always, just sometimes). It helped to use another editor. I think I took vi.

It bothers me, that only the mac bots can't process it. Anyway, I'm going to revert the file and reedit, then resubmit. Let's see..

[Nikolas Zimmermann]

Created attachment 77512 [details]
Patch v10

[Dirk Schulze]

(In reply to comment #30)
> Created an attachment (id=77512) [details]
> Patch v10

You're right, just the two bots can't apply the patch. Are they not up to trunk?

[Nikolas Zimmermann]

Is anyone available for a real review? Simon? David? Dan?

[Simon Fraser (smfr)]

Comment on attachment 77512 [details]
Patch v10

I don't think FontMetrics warrants its own class and header. I think it should just be a struct in Font.h.

[Nikolas Zimmermann]

(In reply to comment #33)
> (From update of attachment 77512 [details])
> I don't think FontMetrics warrants its own class and header. I think it should just be a struct in Font.h.
I'm a huge fan of our one-class-one-file policy. So even though a struct in Font.h might sound convienent, it's violating our style guidelines.


nikolaszimmermann ~/Coding/WebKit/WebCore/platform/graphics > ls Font*.h
Font.h              FontCache.h         FontDescription.h   FontFamily.h        FontOrientation.h   FontSelector.h      FontTraitsMask.h
FontBaseline.h      FontData.h          FontFallbackList.h       FontRenderingMode.h FontSmoothingMode.h

Why should adding FontMetrics.h be any problem?

[Nikolas Zimmermann]

Simon, did I convince you?

[Dirk Schulze]

Comment on attachment 77512 [details]
Patch v10

View in context: https://bugs.webkit.org/attachment.cgi?id=77512&action=review

I'm not sure why a struct is violating the style guidelines. Sometime a struct is the better choice. But in this case I agree to Niko, because this code makes use of various functions (with more than one line) and this should really be in a header.

I'd like to see another run r-, but not because of the header itself.

> WebCore/platform/graphics/FontMetrics.h:19
> +/*
> +    Copyright (C) Research In Motion Limited 2010. All rights reserved.
> +
> +    This library is free software; you can redistribute it and/or
> +    modify it under the terms of the GNU Library General Public
> +    License as published by the Free Software Foundation; either
> +    version 2 of the License, or (at your option) any later version.
> +
> +    This library is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +    Library General Public License for more details.
> +
> +    You should have received a copy of the GNU Library General Public License
> +    aint with this library; see the file COPYING.LIB.  If not, write to
> +    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> +    Boston, MA 02110-1301, USA.
> +*/
> +

Can you start every line with a space and a star? Also why do you copy&paste wrong licenses? aint? What is aint? Do you want to save memory? ;-)

> WebCore/platform/graphics/FontMetrics.h:112
> +private:

Do you think this 'private:' is helpful? I don't.

> WebCore/platform/graphics/SimpleFontData.cpp:85
> +    int xHeight = static_cast<int>(svgFontFaceElement->xHeight() * scale);
> +    int ascent = static_cast<int>(svgFontFaceElement->ascent() * scale);
> +    int descent = static_cast<int>(svgFontFaceElement->descent() * scale);

why not floorf?

> WebCore/platform/graphics/SimpleFontData.cpp:86
> +    int lineGap = 0.1f * size;

This doesn't cause a compiler warning?

[Nikolas Zimmermann]

(In reply to comment #36)
> (From update of attachment 77512 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77512&action=review
> 
> I'm not sure why a struct is violating the style guidelines. Sometime a struct is the better choice. But in this case I agree to Niko, because this code makes use of various functions (with more than one line) and this should really be in a header.
Okay.
 
> I'd like to see another run r-, but not because of the header itself.
> 
> Can you start every line with a space and a star? Also why do you copy&paste wrong licenses? aint? What is aint? Do you want to save memory? ;-)
Fixed.

> 
> > WebCore/platform/graphics/FontMetrics.h:112
> > +private:
> 
> Do you think this 'private:' is helpful? I don't.
Removed.

> > WebCore/platform/graphics/SimpleFontData.cpp:85
> > +    int xHeight = static_cast<int>(svgFontFaceElement->xHeight() * scale);
> > +    int ascent = static_cast<int>(svgFontFaceElement->ascent() * scale);
> > +    int descent = static_cast<int>(svgFontFaceElement->descent() * scale);
> why not floorf?
There's a FIXME just before the lines to use floating-point FontMetrics, in a follow-up patch
Note the static_cast<int>(..) is existing code - changing this causes multiple SVGFonts DRT differences (progressions, but I want to land them seperated).

> 
> > WebCore/platform/graphics/SimpleFontData.cpp:86
> > +    int lineGap = 0.1f * size;
> 
> This doesn't cause a compiler warning?
Wrapped in the same static_cast<int>, with the same warning, that this is gonne change soon to use floating-point metrics, fixing bugs with small font sizes.

[Nikolas Zimmermann]

Created attachment 79730 [details]
Patch v11

Updated patch v11 to trunk. Save memory in FontMetrics by not storing but computing the lineSpacing() from the lineGap / ascent / descent, remaining compatibility with existing DRT expectations.
Current assumption across most ports (mac/win/chromium/gtk...) is "lineSpacing = lroundf(ascent) + lroundf(descent) + lroundf(lineGap)". In my previous patch I stored m_lineSpacing as "lineSpacing = lroundf(floatAscent + floatDescent + floatLineGap)" which lead to several changes in layout tests, which might be correct, but I don't want to cause any layout test changes with this patch.

[Dirk Schulze]

Comment on attachment 79730 [details]
Patch v11

r=me

[Nikolas Zimmermann]

Committed r76442: <http://trac.webkit.org/changeset/76442>

[Nikolas Zimmermann]

Reopening bug.
Landed cr-win build fix in r76443.

I broke several tests on the bot, due the last-minute changes regarding lineSpacing rounding.
I'm going to rechek and post a follow-up patch, if that doesn't work out I'm going to roll it out.

[WebKit Review Bot]

http://trac.webkit.org/changeset/76442 might have broken Qt Linux Release
The following tests are not passing:
animations/missing-values-first-keyframe.html
animations/missing-values-last-keyframe.html
animations/state-at-end-event.html
css1/basic/class_as_selector.html
css1/basic/comments.html
css1/basic/containment.html
css1/basic/contextual_selectors.html
css1/basic/grouping.html
css1/basic/id_as_selector.html
css1/basic/inheritance.html
css1/box_properties/border.html
css1/box_properties/border_bottom.html
css1/box_properties/border_bottom_inline.html
css1/box_properties/border_bottom_width.html
css1/box_properties/border_bottom_width_inline.html
css1/box_properties/border_color.html
css1/box_properties/border_color_inline.html
css1/box_properties/border_inline.html
css1/box_properties/border_left.html
css1/box_properties/border_left_inline.html
css1/box_properties/border_left_width.html
css1/box_properties/border_left_width_inline.html
css1/box_properties/border_right.html
css1/box_properties/border_right_inline.html
css1/box_properties/border_right_width.html
css1/box_properties/border_right_width_inline.html
css1/box_properties/border_style.html
css1/box_properties/border_style_inline.html
css1/box_properties/border_top.html
css1/box_properties/border_top_inline.html
css1/box_properties/border_top_width.html
css1/box_properties/border_top_width_inline.html
css1/box_properties/border_width.html
css1/box_properties/border_width_inline.html
css1/box_properties/clear.html
css1/box_properties/clear_float.html
css1/box_properties/float.html
css1/box_properties/float_elements_in_series.html
css1/box_properties/float_margin.html
css1/box_properties/float_on_text_elements.html
css1/box_properties/height.html
css1/box_properties/margin.html
css1/box_properties/margin_bottom.html
css1/box_properties/margin_bottom_inline.html
css1/box_properties/margin_inline.html
css1/box_properties/margin_left.html
css1/box_properties/margin_left_inline.html
css1/box_properties/margin_right.html
css1/box_properties/margin_right_inline.html
css1/box_properties/margin_top.html
css1/box_properties/margin_top_inline.html
css1/box_properties/padding.html
css1/box_properties/padding_bottom.html
css1/box_properties/padding_bottom_inline.html
css1/box_properties/padding_inline.html
css1/box_properties/padding_left.html
css1/box_properties/padding_left_inline.html
css1/box_properties/padding_right.html
css1/box_properties/padding_right_inline.html
css1/box_properties/padding_top.html
css1/box_properties/padding_top_inline.html
css1/box_properties/width.html
css1/cascade/cascade_order.html
css1/cascade/important.html
css1/classification/display.html
css1/classification/list_style.html
css1/classification/list_style_image.html
css1/classification/list_style_position.html
css1/classification/list_style_type.html
css1/classification/white_space.html
css1/color_and_background/background.html
css1/color_and_background/background_attachment.html
css1/color_and_background/background_color.html
css1/color_and_background/background_image.html
css1/color_and_background/background_position.html
css1/color_and_background/background_repeat.html
css1/color_and_background/color.html
css1/conformance/forward_compatible_parsing.html
css1/font_properties/font.html
css1/font_properties/font_family.html
css1/font_properties/font_size.html
css1/font_properties/font_style.html
css1/font_properties/font_variant.html
css1/font_properties/font_weight.html
css1/formatting_model/canvas.html
css1/formatting_model/floating_elements.html
css1/formatting_model/height_of_lines.html
css1/formatting_model/horizontal_formatting.html
css1/formatting_model/inline_elements.html
css1/formatting_model/replaced_elements.html
css1/formatting_model/vertical_formatting.html
css1/pseudo/anchor.html
css1/pseudo/firstletter.html
css1/pseudo/firstline.html
css1/pseudo/multiple_pseudo_elements.html
css1/pseudo/pseudo_elements_in_selectors.html
css1/text_properties/letter_spacing.html
css1/text_properties/line_height.html
css1/text_properties/text_align.html
css1/text_properties/text_decoration.html
css1/text_properties/text_indent.html
css1/text_properties/text_transform.html
css1/text_properties/vertical_align.html
css1/text_properties/word_spacing.html
css1/units/color_units.html
css1/units/length_units.html
css1/units/percentage_units.html
css1/units/urls.html
css2.1/t010403-shand-border-00-c.html
css2.1/t010403-shand-font-00-b.html
css2.1/t010403-shand-font-01-b.html
css2.1/t040102-keywords-00-b.html
css2.1/t040102-keywords-01-b.html
css2.1/t040103-case-00-b.html
css2.1/t040103-case-01-c.html
css2.1/t040103-escapes-00-b.html
css2.1/t040103-escapes-01-b.html
css2.1/t040103-escapes-02-d.html
css2.1/t040103-escapes-03-b.html
css2.1/t040103-escapes-04-b.html
css2.1/t040103-escapes-05-c.html
css2.1/t040103-escapes-06-b.html
css2.1/t040103-escapes-07-b.html
css2.1/t040103-escapes-08-b.html
css2.1/t040103-ident-00-c.html
css2.1/t040103-ident-01-c.html
css2.1/t040103-ident-02-c.html
css2.1/t040103-ident-03-c.html
css2.1/t040103-ident-04-c.html
css2.1/t040103-ident-05-c.html
css2.1/t040103-ident-06-c.html
css2.1/t040103-ident-07-c.html
css2.1/t040103-ident-08-c.html
css2.1/t040103-ident-09-c.html
css2.1/t040103-ident-10-c.html
css2.1/t040103-ident-11-c.html
css2.1/t040103-ident-12-c.html
css2.1/t040103-ident-13-c.html
css2.1/t040105-atkeyw-00-b.html
css2.1/t040105-atkeyw-01-b.html
css2.1/t040105-atkeyw-02-b.html
css2.1/t040105-atrule-00-b.html
css2.1/t040105-atrule-01-b.html
css2.1/t040105-atrule-02-b.html
css2.1/t040105-atrule-03-b.html
css2.1/t040105-atrule-04-b.html
css2.1/t040105-import-00-b.html
css2.1/t040105-import-01-b.html
css2.1/t040105-import-10-b.html
css2.1/t040109-c17-comments-00-b.html
css2.1/t040109-c17-comments-01-b.html
css2.1/t0402-c71-fwd-parsing-00-f.html
css2.1/t0402-c71-fwd-parsing-01-f.html
css2.1/t0402-c71-fwd-parsing-02-f.html
css2.1/t0402-c71-fwd-parsing-03-f.html
css2.1/t0402-c71-fwd-parsing-04-f.html
css2.1/t0402-syntax-01-f.html
css2.1/t0402-syntax-02-f.html
css2.1/t0402-syntax-03-f.html
css2.1/t0402-syntax-04-f.html
css2.1/t0402-syntax-05-f.html
css2.1/t0402-syntax-06-f.html
css2.1/t040302-c61-ex-len-00-b-a.html
css2.1/t040302-c61-phys-len-00-b.html
css2.1/t040302-c61-rel-len-00-b-ag.html
css2.1/t040303-c62-percent-00-b-ag.html
css2.1/t040304-c64-uri-00-a-g.html
css2.1/t040306-c63-color-00-b-ag.html
css2.1/t040306-syntax-01-f.html
css2.1/t040307-syntax-01-b.html
css2.1/t050201-c12-grouping-00-b.html
css2.1/t0505-c16-descendant-00-e.html
css2.1/t0505-c16-descendant-01-e.html
css2.1/t0505-c16-descendant-02-e.html
css2.1/t050803-c14-classes-00-e.html
css2.1/t0509-c15-ids-00-a.html
css2.1/t0509-c15-ids-01-e.html
css2.1/t0509-id-sel-syntax-01-f.html
css2.1/t0509-id-sel-syntax-02-b.html
css2.1/t0510-c25-pseudo-elmnt-00-c.html
css2.1/t0511-c21-pseud-anch-00-e-i.html
css2.1/t0511-c21-pseud-link-00-e.html
css2.1/t0511-c21-pseud-link-01-e.html
css2.1/t051103-c21-activ-ln-00-e-i.html
css2.1/t051103-c21-focus-ln-00-e-i.html
css2.1/t051103-c21-hover-ln-00-e-i.html
css2.1/t051103-dom-hover-01-c-io.html
css2.1/t051103-dom-hover-02-c-io.html
css2.1/t0602-c13-inheritance-00-e.html
css2.1/t0602-inherit-bdr-pad-b-00.html
css2.1/t0603-c11-import-00-b.html
css2.1/t060401-c32-cascading-00-b.html
css2.1/t060402-c31-important-00-b.html
css2.1/t060403-c21-pseu-cls-00-e-i.html
css2.1/t060403-c21-pseu-id-00-e-i.html
css2.1/t0801-c412-hz-box-00-b-a.html
css2.1/t0803-c5501-imrgn-t-00-b-ag.html
css2.1/t0803-c5501-mrgn-t-00-b-a.html
css2.1/t0803-c5502-imrgn-r-00-b-ag.html
css2.1/t0803-c5502-imrgn-r-01-b-ag.html
css2.1/t0803-c5502-imrgn-r-02-b-a.html
css2.1/t0803-c5502-imrgn-r-03-b-a.html
css2.1/t0803-c5502-imrgn-r-04-b-ag.html
css2.1/t0803-c5502-imrgn-r-05-b-ag.html
css2.1/t0803-c5502-imrgn-r-06-b-ag.html
css2.1/t0803-c5502-mrgn-r-00-c-ag.html
css2.1/t0803-c5502-mrgn-r-01-c-a.html
css2.1/t0803-c5502-mrgn-r-02-c.html
css2.1/t0803-c5502-mrgn-r-03-c.html
css2.1/t0803-c5503-imrgn-b-00-b-a.html
css2.1/t0803-c5503-mrgn-b-00-b-a.html
css2.1/t0803-c5504-imrgn-l-00-b-ag.html
css2.1/t0803-c5504-imrgn-l-01-b-ag.html
css2.1/t0803-c5504-imrgn-l-02-b-ag.html
css2.1/t0803-c5504-imrgn-l-03-b-a.html
css2.1/t0803-c5504-imrgn-l-04-b-ag.html
css2.1/t0803-c5504-imrgn-l-05-b-ag.html
css2.1/t0803-c5504-imrgn-l-06-b-ag.html
css2.1/t0803-c5504-mrgn-l-00-c-ag.html
css2.1/t0803-c5504-mrgn-l-01-c-a.html
css2.1/t0803-c5504-mrgn-l-02-c.html
css2.1/t0803-c5504-mrgn-l-03-c.html
css2.1/t0803-c5505-imrgn-00-a-ag.html
css2.1/t0803-c5505-mrgn-00-b-ag.html
css2.1/t0803-c5505-mrgn-01-e-a.html
css2.1/t0803-c5505-mrgn-02-c.html
css2.1/t0803-c5505-mrgn-03-c-ag.html
css2.1/t080301-c411-vt-mrgn-00-b.html
css2.1/t0804-c5506-ipadn-t-00-b-a.html
css2.1/t0804-c5506-ipadn-t-01-b-a.html
css2.1/t0804-c5506-ipadn-t-02-b-a.html
css2.1/t0804-c5506-padn-t-00-b-a.html
css2.1/t0804-c5507-ipadn-r-00-b-ag.html
css2.1/t0804-c5507-ipadn-r-01-b-ag.html
css2.1/t0804-c5507-ipadn-r-03-b-a.html
css2.1/t0804-c5507-ipadn-r-04-b-ag.html
css2.1/t0804-c5507-padn-r-00-c-ag.html
css2.1/t0804-c5507-padn-r-01-c-a.html
css2.1/t0804-c5507-padn-r-02-f.html
css2.1/t0804-c5507-padn-r-03-f.html
css2.1/t0804-c5508-ipadn-b-00-b-a.html
css2.1/t0804-c5508-ipadn-b-01-f-a.html
css2.1/t0804-c5508-ipadn-b-02-b-a.html
css2.1/t0804-c5508-ipadn-b-03-b-a.html
css2.1/t0804-c5509-ipadn-l-00-b-ag.html
css2.1/t0804-c5509-ipadn-l-01-b-ag.html
css2.1/t0804-c5509-ipadn-l-02-b-ag.html
css2.1/t0804-c5509-ipadn-l-03-b-a.html
css2.1/t0804-c5509-ipadn-l-04-f-ag.html
css2.1/t0804-c5509-padn-l-00-b-ag.html
css2.1/t0804-c5509-padn-l-01-b-a.html
css2.1/t0804-c5509-padn-l-02-f.html
css2.1/t0804-c5509-padn-l-03-f-g.html
css2.1/t0804-c5510-ipadn-00-b-ag.html
css2.1/t0804-c5510-padn-00-b-ag.html
css2.1/t0804-c5510-padn-01-e-a.html
css2.1/t0804-c5510-padn-02-f.html
css2.1/t0805-c5511-brdr-tw-00-b.html
css2.1/t0805-c5511-brdr-tw-01-b-g.html
css2.1/t0805-c5511-brdr-tw-02-b.html
css2.1/t0805-c5511-brdr-tw-03-b.html
css2.1/t0805-c5511-ibrdr-tw-00-a.html
css2.1/t0805-c5512-brdr-rw-00-b.html
css2.1/t0805-c5512-brdr-rw-01-b-g.html
css2.1/t0805-c5512-brdr-rw-02-b.html
css2.1/t0805-c5512-brdr-rw-03-b.html
css2.1/t0805-c5512-ibrdr-rw-00-a.html
css2.1/t0805-c5513-brdr-bw-00-b.html
css2.1/t0805-c5513-brdr-bw-01-b-g.html
css2.1/t0805-c5513-brdr-bw-02-b.html
css2.1/t0805-c5513-brdr-bw-03-b.html
css2.1/t0805-c5513-ibrdr-bw-00-a.html
css2.1/t0805-c5514-brdr-lw-00-b.html
css2.1/t0805-c5514-brdr-lw-01-b-g.html
css2.1/t0805-c5514-brdr-lw-02-b.html
css2.1/t0805-c5514-brdr-lw-03-b.html
css2.1/t0805-c5514-ibrdr-lw-00-a.html
css2.1/t0805-c5515-brdr-w-00-a.html
css2.1/t0805-c5515-brdr-w-01-b-g.html
css2.1/t0805-c5515-brdr-w-02-b.html
css2.1/t0805-c5515-ibrdr-00-b.html
css2.1/t0805-c5516-brdr-c-00-a.html
css2.1/t0805-c5516-ibrdr-c-00-a.html
css2.1/t0805-c5517-brdr-s-00-c.html
css2.1/t0805-c5517-ibrdr-s-00-a.html
css2.1/t0805-c5518-brdr-t-00-a.html
css2.1/t0805-c5518-brdr-t-01-e.html
css2.1/t0805-c5518-ibrdr-t-00-a.html
css2.1/t0805-c5519-brdr-r-00-a.html
css2.1/t0805-c5519-brdr-r-01-e.html
css2.1/t0805-c5519-brdr-r-02-e.html
css2.1/t0805-c5519-ibrdr-r-00-a.html
css2.1/t0805-c5520-brdr-b-00-a.html
css2.1/t0805-c5520-brdr-b-01-e.html
css2.1/t0805-c5520-ibrdr-b-00-a.html
css2.1/t0805-c5521-brdr-l-00-a.html
css2.1/t0805-c5521-brdr-l-01-e.html
css2.1/t0805-c5521-brdr-l-02-e.html
css2.1/t0805-c5521-ibrdr-l-00-a.html
css2.1/t0805-c5522-brdr-00-b.html
css2.1/t0805-c5522-brdr-01-b-g.html
css2.1/t0805-c5522-ibrdr-00-a.html
css2.1/t090204-display-change-01-b-ao.html
css2.1/t090402-c42-ibx-pad-00-d-ag.html
css2.1/t0905-c414-flt-00-d.html
css2.1/t0905-c414-flt-01-d-g.html
css2.1/t0905-c414-flt-02-c.html
css2.1/t0905-c414-flt-03-c.html
css2.1/t0905-c414-flt-04-c.html
css2.1/t0905-c414-flt-fit-00-d.html
css2.1/t0905-c414-flt-wrap-00-e.html
css2.1/t0905-c414-flt-wrap-01-d-g.html
css2.1/t0905-c5525-fltblck-00-d-ag.html
css2.1/t0905-c5525-fltblck-01-d.html
css2.1/t0905-c5525-fltclr-00-c-ag.html
css2.1/t0905-c5525-fltcont-00-d-g.html
css2.1/t0905-c5525-fltinln-00-c-ag.html
css2.1/t0905-c5525-fltmrgn-00-c-ag.html
css2.1/t0905-c5525-fltmult-00-d-g.html
css2.1/t0905-c5525-fltwidth-00-c-g.html
css2.1/t0905-c5525-fltwidth-01-c-g.html
css2.1/t0905-c5525-fltwidth-02-c-g.html
css2.1/t0905-c5525-fltwidth-03-c-g.html
css2.1/t0905-c5525-fltwrap-00-b.html
css2.1/t0905-c5526-fltclr-00-c-ag.html
css2.1/t090501-c414-flt-00-d.html
css2.1/t090501-c414-flt-01-b.html
css2.1/t090501-c414-flt-02-d-g.html
css2.1/t090501-c414-flt-03-b-g.html
css2.1/t090501-c414-flt-ln-00-d.html
css2.1/t090501-c414-flt-ln-01-d-g.html
css2.1/t090501-c414-flt-ln-02-d.html
css2.1/t090501-c414-flt-ln-03-d.html
css2.1/t090501-c5525-flt-l-00-b-g.html
css2.1/t090501-c5525-flt-r-00-b-g.html
css2.1/t1002-c5523-width-00-b-g.html
css2.1/t1002-c5523-width-01-b-g.html
css2.1/t1002-c5523-width-02-b-g.html
css2.1/t100303-c412-blockw-00-d-ag.html
css2.1/t100304-c43-rpl-bbx-00-d-g.html
css2.1/t100304-c43-rpl-bbx-01-d-g.html
css2.1/t1004-c43-rpl-bbx-00-d-ag.html
css2.1/t1004-c5524-width-00-b-g.html
css2.1/t1005-c5524-width-00-b-g.html
css2.1/t1005-c5524-width-01-b-g.html
css2.1/t1008-c44-ln-box-03-d-ag.html
css2.1/t100801-c544-valgn-00-a-ag.html
css2.1/t100801-c544-valgn-02-d-agi.html
css2.1/t100801-c544-valgn-03-d-agi.html
css2.1/t100801-c544-valgn-04-d-agi.html
css2.1/t100801-c548-leadin-00-d-a.html
css2.1/t100801-c548-ln-ht-00-c-a.html
css2.1/t100801-c548-ln-ht-01-b-ag.html
css2.1/t100801-c548-ln-ht-03-d-ag.html
css2.1/t100801-c548-ln-ht-04-d-ag.html
css2.1/t1202-counter-00-b.html
css2.1/t1202-counter-01-b.html
css2.1/t1202-counter-02-b.html
css2.1/t1202-counter-03-b.html
css2.1/t1202-counter-04-b.html
css2.1/t1202-counter-05-b.html
css2.1/t1202-counter-06-b.html
css2.1/t1202-counter-07-b.html
css2.1/t1202-counter-08-b.html
css2.1/t1202-counter-11-b.html
css2.1/t1202-counter-12-b.html
css2.1/t1202-counter-13-b.html
css2.1/t1202-counter-14-b.html
css2.1/t1202-counter-16-f.html
css2.1/t1202-counters-00-b.html
css2.1/t1202-counters-01-b.html
css2.1/t1202-counters-02-b.html
css2.1/t1202-counters-03-b.html
css2.1/t1202-counters-05-b.html
css2.1/t1202-counters-06-b.html
css2.1/t1202-counters-07-b.html
css2.1/t1202-counters-08-b.html
css2.1/t1202-counters-11-b.html
css2.1/t1202-counters-12-b.html
css2.1/t1202-counters-13-b.html
css2.1/t1202-counters-14-b.html
css2.1/t1202-counters-16-c.html
css2.1/t1202-counters-17-d.html
css2.1/t1202-counters-18-f.html
css2.1/t1204-implied-00-b.html
css2.1/t1204-implied-01-c.html
css2.1/t1204-implied-02-d.html
css2.1/t1204-multiple-00-c.html
css2.1/t1204-multiple-01-c.html
css2.1/t1204-order-00-c.html
css2.1/t1204-order-01-d.html
css2.1/t1204-root-e.html
css2.1/t120401-scope-00-b.html
css2.1/t120401-scope-01-c.html
css2.1/t120401-scope-02-c.html
css2.1/t120401-scope-03-c.html
css2.1/t120401-scope-04-d.html
css2.1/t120403-content-none-00-c.html
css2.1/t120403-display-none-00-c.html
css2.1/t120403-visibility-00-c.html
css2.1/t1205-c561-list-displ-00-b.html
css2.1/t1205-c563-list-type-00-b.html
css2.1/t1205-c563-list-type-01-b.html
css2.1/t1205-c564-list-img-00-b-g.html
css2.1/t1205-c565-list-pos-00-b.html
css2.1/t1205-c566-list-stl-00-e-ag.html
css2.1/t1401-c531-color-00-a.html
css2.1/t1402-c45-bg-canvas-00-b.html
css2.1/t140201-c532-bgcolor-00-a.html
css2.1/t140201-c532-bgcolor-01-b.html
css2.1/t140201-c533-bgimage-00-a.html
css2.1/t140201-c533-bgimage-01-b-g.html
css2.1/t140201-c534-bgre-00-b-ag.html
css2.1/t140201-c534-bgre-01-b-ag.html
css2.1/t140201-c535-bg-fixd-00-b-g.html
css2.1/t140201-c536-bgpos-00-b-ag.html
css2.1/t140201-c536-bgpos-01-b-ag.html
css2.1/t1503-c522-font-family-00-b.html
css2.1/t1504-c523-font-style-00-b.html
css2.1/t1504-c543-txt-decor-00-d-g.html
css2.1/t1505-c524-font-var-00-b.html
css2.1/t1506-c525-font-wt-00-b.html
css2.1/t1507-c526-font-sz-00-b.html
css2.1/t1507-c526-font-sz-01-b-a.html
css2.1/t1507-c526-font-sz-02-b-a.html
css2.1/t1507-c526-font-sz-03-f-a.html
css2.1/t1508-c527-font-00-b.html
css2.1/t1601-c547-indent-01-d.html
css2.1/t1602-c546-txt-align-00-b.html
css2.1/t1606-c562-white-sp-00-b-ag.html
css2.1/t170602-bdr-conflct-w-00-d.html
css2.1/t170602-bdr-conflct-w-01-d.html
css2.1/t170602-bdr-conflct-w-02-d.html
css2.1/t170602-bdr-conflct-w-03-d.html
css2.1/t170602-bdr-conflct-w-04-d.html
css2.1/t170602-bdr-conflct-w-05-d.html
css2.1/t170602-bdr-conflct-w-06-d.html
css2.1/t170602-bdr-conflct-w-07-d.html
css2.1/t170602-bdr-conflct-w-08-d.html
css2.1/t170602-bdr-conflct-w-09-d.html
css2.1/t170602-bdr-conflct-w-10-d.html
css2.1/t170602-bdr-conflct-w-11-d.html
css2.1/t170602-bdr-conflct-w-12-d.html
css2.1/t170602-bdr-conflct-w-13-d.html
css2.1/t170602-bdr-conflct-w-14-d.html
css2.1/t170602-bdr-conflct-w-15-d.html
css2.1/t170602-bdr-conflct-w-16-d.html
css2.1/t170602-bdr-conflct-w-17-d.html
css2.1/t170602-bdr-conflct-w-18-d.html
css2.1/t170602-bdr-conflct-w-19-d.html
css2.1/t170602-bdr-conflct-w-20-d.html
css2.1/t170602-bdr-conflct-w-21-d.html
css2.1/t170602-bdr-conflct-w-22-d.html
css2.1/t170602-bdr-conflct-w-23-d.html
css2.1/t170602-bdr-conflct-w-24-d.html
css2.1/t170602-bdr-conflct-w-25-d.html
css2.1/t170602-bdr-conflct-w-26-d.html
css2.1/t170602-bdr-conflct-w-27-d.html
css2.1/t170602-bdr-conflct-w-28-d.html
css2.1/t170602-bdr-conflct-w-29-d.html
css2.1/t170602-bdr-conflct-w-30-d.html
css2.1/t170602-bdr-conflct-w-31-d.html
css2.1/t170602-bdr-conflct-w-32-d.html
css2.1/t170602-bdr-conflct-w-33-d.html
css2.1/t170602-bdr-conflct-w-34-d.html
css2.1/t170602-bdr-conflct-w-35-d.html
css2.1/t170602-bdr-conflct-w-36-d.html
css2.1/t170602-bdr-conflct-w-37-d.html
css2.1/t170602-bdr-conflct-w-38-d.html
css2.1/t170602-bdr-conflct-w-39-d.html
css2.1/t170602-bdr-conflct-w-40-d.html
css2.1/t170602-bdr-conflct-w-41-d.html
css2.1/t170602-bdr-conflct-w-42-d.html
css2.1/t170602-bdr-conflct-w-43-d.html
css2.1/t170602-bdr-conflct-w-44-d.html
css2.1/t170602-bdr-conflct-w-45-d.html
css2.1/t170602-bdr-conflct-w-46-d.html
css2.1/t170602-bdr-conflct-w-47-d.html
css2.1/t170602-bdr-conflct-w-48-d.html
css2.1/t170602-bdr-conflct-w-49-d.html
css2.1/t170602-bdr-conflct-w-50-d.html
css2.1/t170602-bdr-conflct-w-51-d.html
css2.1/t170602-bdr-conflct-w-52-d.html
css2.1/t170602-bdr-conflct-w-53-d.html
css2.1/t170602-bdr-conflct-w-54-d.html
css2.1/t170602-bdr-conflct-w-55-d.html
css2.1/t170602-bdr-conflct-w-56-d.html
css2.1/t170602-bdr-conflct-w-57-d.html
css2.1/t170602-bdr-conflct-w-58-d.html
css2.1/t170602-bdr-conflct-w-59-d.html
css2.1/t170602-bdr-conflct-w-60-d.html
css2.1/t170602-bdr-conflct-w-61-d.html
css2.1/t170602-bdr-conflct-w-62-d.html
css2.1/t170602-bdr-conflct-w-63-d.html
css2.1/t170602-bdr-conflct-w-64-d.html
css2.1/t170602-bdr-conflct-w-65-d.html
css2.1/t170602-bdr-conflct-w-66-d.html
css2.1/t170602-bdr-conflct-w-67-d.html
css2.1/t170602-bdr-conflct-w-68-d.html
css2.1/t170602-bdr-conflct-w-69-d.html

[Nikolas Zimmermann]

Bug 52960 covers fixing the regression. Closing this bug, continuing the work there.

[Martin Robinson]

This patch seems to have introduced some rounding error between 64-bit and 32-bit platforms. You can see that both 64-bit GTK+ bots now fail this set of tests, even after your rebaselines:

svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
svg/text/text-hkern-on-vertical-text.svg
svg/text/text-hkern.svg	expecte
svg/text/text-vkern-on-horizontal-text.svg
svg/text/text-vkern.svg

[Csaba Osztrogonác]

(In reply to comment #44)
> This patch seems to have introduced some rounding error between 64-bit and 32-bit platforms. You can see that both 64-bit GTK+ bots now fail this set of tests, even after your rebaselines:
> 
> svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
> svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
> svg/text/text-hkern-on-vertical-text.svg
> svg/text/text-hkern.svg    expecte
> svg/text/text-vkern-on-horizontal-text.svg
> svg/text/text-vkern.svg

Similar problem on Qt: https://bugs.webkit.org/show_bug.cgi?id=52812

[Nikolas Zimmermann]

(In reply to comment #45)
> (In reply to comment #44)
> > This patch seems to have introduced some rounding error between 64-bit and 32-bit platforms. You can see that both 64-bit GTK+ bots now fail this set of tests, even after your rebaselines:
> > 
> > svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
> > svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
> > svg/text/text-hkern-on-vertical-text.svg
> > svg/text/text-hkern.svg    expecte
> > svg/text/text-vkern-on-horizontal-text.svg
> > svg/text/text-vkern.svg
> 
> Similar problem on Qt: https://bugs.webkit.org/show_bug.cgi?id=52812

Ok, I'm not sure whether I can manage to investigate today.
Does anyone have an idea? Maybe the lroundf usage causes differences across 32/64 bit boundaries?

If we can't find a solution in the next few hours, it should probably be rolled out.
Sorry for the trouble, but that was really unexpected to me...

[Jessie Berlin]

(In reply to comment #46)
> (In reply to comment #45)
> > (In reply to comment #44)
> > > This patch seems to have introduced some rounding error between 64-bit and 32-bit platforms. You can see that both 64-bit GTK+ bots now fail this set of tests, even after your rebaselines:
> > > 
> > > svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
> > > svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
> > > svg/text/text-hkern-on-vertical-text.svg
> > > svg/text/text-hkern.svg    expecte
> > > svg/text/text-vkern-on-horizontal-text.svg
> > > svg/text/text-vkern.svg
> > 
> > Similar problem on Qt: https://bugs.webkit.org/show_bug.cgi?id=52812
> 
> Ok, I'm not sure whether I can manage to investigate today.
> Does anyone have an idea? Maybe the lroundf usage causes differences across 32/64 bit boundaries?
> 
> If we can't find a solution in the next few hours, it should probably be rolled out.
> Sorry for the trouble, but that was really unexpected to me...

This patch is causing ~25 tests to fail on the Windows 7 Release Tests bots:

http://build.webkit.org/builders/Windows%207%20Release%20%28Tests%29/builds/8525
http://build.webkit.org/builders/Windows%207%20Release%20%28Tests%29/builds/8526
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r76442%20(8526)/results.html

[Nikolas Zimmermann]

I'm aware of the breakage, and working on a fix...

[Nikolas Zimmermann]

Created attachment 80041 [details]
Fixup patch (after patch v11 landed)

Potential fix, let's see whether it builds.

[Dirk Schulze]

Comment on attachment 80041 [details]
Fixup patch (after patch v11 landed)

r=me. It may had been better to fix this all at once :-/

[Nikolas Zimmermann]

Created attachment 80043 [details]
Fixup patch v2 (new approach, only SVGFonts changes)

Another attempt, which does not involve changing the lroundf() rounding for lineSpacing, I'm worried about the number of rebaselines, that would need discussion first, as it changes the visual appearance of lots of tests.
I'll try again with just fixing SVGFonts alone, let ossy try on Qt/64 and see if it helps :-)

[Nikolas Zimmermann]

Created attachment 80044 [details]
Fixup patch v3

Fix ChangeLog, included mac layout test changes, for a full review.

[Dirk Schulze]

Comment on attachment 80044 [details]
Fixup patch v3

LGTM. r=me.

[WebKit Review Bot]

http://trac.webkit.org/changeset/76586 might have broken Qt Linux Release
The following tests are not passing:
svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
svg/W3C-SVG-1.1-SE/pservers-grad-17-b.svg
svg/W3C-SVG-1.1-SE/pservers-grad-20-b.svg
svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
svg/custom/svg-fonts-with-no-element-reference.html
svg/custom/svg-fonts-without-missing-glyph.xhtml
svg/text/text-hkern-on-vertical-text.svg
svg/text/text-hkern.svg
svg/text/text-vkern-on-horizontal-text.svg
svg/text/text-vkern.svg
svg/transforms/text-with-mask-with-svg-transform.svg

[Csaba Osztrogonác]

(In reply to comment #54)
> http://trac.webkit.org/changeset/76586 might have broken Qt Linux Release
> The following tests are not passing:
> svg/W3C-SVG-1.1-SE/filters-image-03-f.svg
> svg/W3C-SVG-1.1-SE/pservers-grad-17-b.svg
> svg/W3C-SVG-1.1-SE/pservers-grad-20-b.svg
> svg/W3C-SVG-1.1-SE/pservers-pattern-03-f.svg
> svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
> svg/custom/svg-fonts-with-no-element-reference.html
> svg/custom/svg-fonts-without-missing-glyph.xhtml
> svg/text/text-hkern-on-vertical-text.svg
> svg/text/text-hkern.svg
> svg/text/text-vkern-on-horizontal-text.svg
> svg/text/text-vkern.svg
> svg/transforms/text-with-mask-with-svg-transform.svg

Fixed by http://trac.webkit.org/changeset/76588. Thx.

[Nikolas Zimmermann]

Everything is fine again after r76588, no diffs between 32/64 bit bots anymore, closing bug.