Bug 122367 – [MathML] Use of floating point floor/ceil operations on LayoutUnits seems wrong

• Component: MathML
• Status: RESOLVED FIXED

[Brent Fulgham]

While investigating another problem, I noticed the following code in RenderMathMLOperator::paintCharacter:

    switch (trim) {
    case TrimTop:
        glyphPaintRect.shiftYEdgeTo(ceil(glyphPaintRect.y()) + 1);
        clipBounds.shiftYEdgeTo(glyphPaintRect.y());
        break;
    case TrimBottom:
        glyphPaintRect.shiftMaxYEdgeTo(floor(glyphPaintRect.maxY()) - 1);
        clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
        break;
    case TrimTopAndBottom:
        glyphPaintRect.shiftYEdgeTo(ceil(glyphPaintRect.y() + 1));
        glyphPaintRect.shiftMaxYEdgeTo(floor(glyphPaintRect.maxY()) - 1);
        clipBounds.shiftYEdgeTo(glyphPaintRect.y());
        clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
        break;
    }

The various paint rects in this code are LayoutRects, and the y()/maxY() methods return LayoutUnits.

The LayoutRect and LayoutUnit wrap an 'int' type.  So the calls to the ceil/floor C API results in an implicit cast from LayoutUnit->float, then a call to floor/ceil.

Since we are already working with integer types, these calls seem at best wasteful.

Given that the layout units do some magic internally for dealing with sub-pixel layout for their ceil() and floor() methods, I feel like this code should be revised as outlined in the attached patch.

[Brent Fulgham]

Created attachment 213428 [details]
Patch

[Martin Robinson]

landing I would like to verify that this fits not regress rendering quality on GTK+, if you don't mind.

[Martin Robinson]

This looks good on GTK+ so feel free to land this when it looks like the the patch for 12146 sticks.

[Radar WebKit Bug Importer]

<rdar://problem/15176294>

[Brent Fulgham]

Committed r157135: <http://trac.webkit.org/changeset/157135>