[webkit-reviews] review denied: [Bug 103428] [WK2] TiledBackingStore: Frame view re-layouts with wrong Fixed Visible Content Rect. : [Attachment 176489] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 28 08:42:05 PST 2012


Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Mikhail Pozdnyakov
<mikhail.pozdnyakov at intel.com>'s request for review:
Bug 103428: [WK2] TiledBackingStore: Frame view re-layouts with wrong Fixed
Visible Content Rect.
https://bugs.webkit.org/show_bug.cgi?id=103428

Attachment 176489: patch
https://bugs.webkit.org/attachment.cgi?id=176489&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176489&action=review


> Source/WebKit2/UIProcess/DrawingAreaProxy.h:91
> +    virtual void setFixedLayoutSize(const WebCore::IntSize& /*layoutSize*/,
const WebCore::FloatRect& /*visibleContentsRect*/, float /*scale*/) { }

space after /* and before */

> Source/WebKit2/UIProcess/PageViewportController.cpp:223
> -    drawingArea->setVisibleContentsRect(visibleContentsRect,
m_effectiveScale, trajectoryVector);
> +    if (trajectoryVector == FloatPoint::zero()) {
> +	   IntSize fixedLayoutSize =
IntSize(static_cast<int>(m_rawAttributes.layoutSize.width()),
static_cast<int>(m_rawAttributes.layoutSize.height()));
> +	   drawingArea->setFixedLayoutSize(fixedLayoutSize,
visibleContentsRect, m_effectiveScale);
> +    } else
> +	   drawingArea->setVisibleContentsRect(visibleContentsRect,
m_effectiveScale, trajectoryVector);
>  

It is zero in other cases than when setting new fixed layout size. For instance
when panning ends.

Why not replace syncVisibleContents() inside didChangeViewportAttributes
instead?

IntSize fixedLayoutSize =
IntSize(static_cast<int>(m_rawAttributes.layoutSize.width()),
static_cast<int>(m_rawAttributes.layoutSize.height()));
drawingArea->setFixedLayoutSize(fixedLayoutSize, visibleContentsRect,
m_effectiveScale);

Also why not

IntSize fixedLayoutSize = roundedIntRect(m_rawAttributes.layoutSize);

Or

drawingArea->setFixedLayoutSize(roundedIntRect(m_rawAttributes.layoutSize),
visibleContentsRect, m_effectiveScale);


Notice that in the code above m_contentsSize cannot be trusted, it is from the
old content (old layout sizes)

I guess you should use:

FloatRect visibleContentsRect(FloatPoint(),
viewportSizeInContentsCoordinates());
drawingArea->setFixedLayoutSize(roundedIntRect(m_rawAttributes.layoutSize),
visibleContentsRect, m_effectiveScale);


More information about the webkit-reviews mailing list