[webkit-reviews] review granted: [Bug 205990] A partially selected RTL text is placed at a wrong vertical position if it has a vertical initial advance : [Attachment 388209] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 20 10:25:14 PST 2020
Darin Adler <darin at apple.com> has granted Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 205990: A partially selected RTL text is placed at a wrong vertical
position if it has a vertical initial advance
https://bugs.webkit.org/show_bug.cgi?id=205990
Attachment 388209: Patch
https://bugs.webkit.org/attachment.cgi?id=388209&action=review
--- Comment #20 from Darin Adler <darin at apple.com> ---
Comment on attachment 388209
--> https://bugs.webkit.org/attachment.cgi?id=388209
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=388209&action=review
Looks like the Windows build failed — I couldn't find the error message in the
EWS results, though. I retried so maybe it will succeed without any further
changes.
> Source/WebCore/platform/graphics/ComplexTextController.h:68
> + float totalWidth() const { return m_totalAdvance.width(); }
Are there any uses of this outside the class? Can this be a private member
function instead? Or maybe we can just write m_totalAdvance.width() at each of
the two call sites. I’d like to cut down on proliferation of functions that do
nearly the same thing.
> Source/WebCore/platform/graphics/FontCascade.cpp:292
> + FloatPoint oldStartPoint(startPoint);
I suggest that instead of this, we write:
float oldStartX = startPoint.x();
> Source/WebCore/platform/graphics/FontCascade.cpp:1438
> + FloatPoint startPoint(point);
Style wise, I think this is nicer with an "=", but not sure everyone else will
agree.
> Source/WebCore/platform/graphics/GlyphBuffer.h:66
> + operator FloatSize() const { return FloatSize(width(), height()); }
I think it’s a little risky to add an operator that does a lossy conversion
from doubles to floats, implicitly and silently. Normally we would like
conversions that narrow to be named function calls.
More information about the webkit-reviews
mailing list