[webkit-reviews] review granted: [Bug 98317] Only measure text once instead of twice when performing line layout : [Attachment 167493] Patch2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 9 11:17:44 PDT 2012
mitz at webkit.org <mitz at webkit.org> has granted Enrica Casucci
<enrica at apple.com>'s request for review:
Bug 98317: Only measure text once instead of twice when performing line layout
https://bugs.webkit.org/show_bug.cgi?id=98317
Attachment 167493: Patch2
https://bugs.webkit.org/attachment.cgi?id=167493&action=review
------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167493&action=review
r=me but please try to address my comments before landing
> Source/WebCore/ChangeLog:15
> + Each entry in the vector is a Measure object that contains
information
You ended up calling this Measurement, not Measure.
> Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:538
> + if (fallbackFonts && complexTextRun.fontData() !=
m_font.primaryFont())
> + fallbackFonts->add(complexTextRun.fontData());
Doing this here may be too late! There are return statements in the while()
loop above.
> Source/WebCore/rendering/RenderBlock.h:56
> class LineInfo;
> class RenderRubyRun;
> class TextLayout;
> +class Measurement;
These should be kept sorted.
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2489
> + float wordSpacingForTextMeasure = 0;
ForTextMeasurement?
> Source/WebCore/rendering/RenderBlockLineLayout.cpp:2570
> + measurements.append(Measurement());
You want to avoid creating a temporary Measurement object and then copying it
into the Vector. I don’t know if the above code does this. Instead, you can
just use Vector’s grow() function.
More information about the webkit-reviews
mailing list