[webkit-reviews] review granted: [Bug 132164] Subpixel rendering: Inline text selection painting should not snap to integral CSS pixel position. : [Attachment 230131] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Apr 29 09:02:23 PDT 2014
Darin Adler <darin at apple.com> has granted Zalan Bujtas <zalan at apple.com>'s
request for review:
Bug 132164: Subpixel rendering: Inline text selection painting should not snap
to integral CSS pixel position.
https://bugs.webkit.org/show_bug.cgi?id=132164
Attachment 230131: Patch
https://bugs.webkit.org/attachment.cgi?id=230131&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230131&action=review
I wish I understood the difference between “rounding” and “pixel snapping”.
> Source/WebCore/rendering/EllipsisBox.cpp:140
> + FloatRect clipRect = pixelSnappedForPainting(x() + paintOffset.x(), top
+ paintOffset.y(), m_logicalWidth, h,
renderer().document().deviceScaleFactor());
> +
> context->clip(clipRect);
Not sure we need a local variable for this.
> Source/WebCore/rendering/InlineTextBox.cpp:760
> + LayoutUnit selHeight = std::max<LayoutUnit>(0, selectionBottom -
selectionTop);
Might be nice to use the word “selection” instead of the abbreviation “sel”
here.
> Source/WebCore/rendering/InlineTextBox.cpp:765
> + FloatRect clipRect =
pixelSnappedForPainting(LayoutRect(LayoutPoint(localOrigin),
LayoutSize(m_logicalWidth, selHeight)), deviceScaleFactor);
> context->clip(clipRect);
Not sure we need a local variable for this.
> Source/WebCore/rendering/InlineTextBox.h:199
> INLINE_BOX_OBJECT_TYPE_CASTS(InlineTextBox, isInlineTextBox())
> -
> -void alignSelectionRectToDevicePixels(FloatRect&);
> -
> } // namespace WebCore
Should leave a blank line here.
More information about the webkit-reviews
mailing list