[webkit-reviews] review denied: [Bug 50621] Implement text-combine rendering code : [Attachment 79565] Fixed a conflict in WebCore.xcodeproj.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 20 15:01:57 PST 2011


Dave Hyatt <hyatt at apple.com> has denied Takumi Takano <takano at apple.com>'s
request for review:
Bug 50621: Implement text-combine rendering code
https://bugs.webkit.org/show_bug.cgi?id=50621

Attachment 79565: Fixed a conflict in WebCore.xcodeproj.
https://bugs.webkit.org/attachment.cgi?id=79565&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=79565&action=review

widthFromCache can be de-virtualized, so do that also.	You can get rid of
rotateWhenVertical completely with the changes I mention following this
comment.

> Source/WebCore/rendering/InlineTextBox.cpp:492
> +    bool doRotation = !isHorizontal() &&
textRenderer()->rotateWhenVertical();
> +    if (doRotation) {

I'd suggest using styleToUse->hasTextCombine() to store RenderCombineText*
combinedText in a local variable.  Then you can just make bool doRotation =
!isHorizontal && combinedText;

> Source/WebCore/rendering/InlineTextBox.cpp:508
> +    if (styleToUse->hasTextCombine()) {

Can use the local I mentioned above:

if (combinedText)
    combinedText->adjustTextOrigin(textOrigin, boxRect();

> Source/WebCore/rendering/InlineTextBox.cpp:618
> +    if (!styleToUse->hasTextCombine())
> +	   characters = textRenderer()->text()->characters() + m_start;
> +    else {
> +	   RenderCombineText* t = toRenderCombineText(textRenderer());
> +	   characters = t->charactersToRender(m_start, length);

Can use the local from above:

const UChar* characters = !combinedText ? textRenderer()->text()->characters()
+ m_start : combinedText->charactersToRender(m_start, length);


More information about the webkit-reviews mailing list