Skip to content

Commit

Permalink
Revert of Initial draft - modify ViewportAnchor to know about both in…
Browse files Browse the repository at this point in the history 
…ner and outer viewports. (patchset #11 id:200001 of https://codereview.chromium.org/556703005/)

Reason for revert:
The new test is failing on mac. Reverting for now so it can be investigated.

[  FAILED  ] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms)
[1471/1471] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms)
Retrying 2 tests (retry #3)
[ RUN      ] PinchViewportTest.TestResizeAfterVerticalScroll
../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure
Value of: (pinchViewport.visibleRect().size()).width()
Actual: 45.94595
Expected: (FloatSize(50, 25)).width()
Which is: 50
../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure
Value of: (pinchViewport.visibleRect().size()).height()
Actual: 22.972975
Expected: (FloatSize(50, 25)).height()
Which is: 25
../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:253: Failure
Value of: (frame()->view()->scrollPosition()).y()
Actual: 638
Expected: (IntPoint(0, 625)).y()
Which is: 625
../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:254: Failure
Value of: (pinchViewport.location()).y()
Actual: 62
Expected: (FloatPoint(0, 75)).y()
Which is: 75

Original issue's description:
> Fix pinch virtual viewport position after resize.
> 
> Associate the viewport anchor with the inner viewport.
> Adjust the inner and outer viewport positions after resize
> such that they both remain in their allowed range and the
> inner viewport origin scales proportionally within the
> outer viewport.
> 
> As a small cleanup, made the method ScrollView::scrollTo() protected.
> 
> BUG=364108
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182365

[email protected],[email protected],[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=364108

Review URL: https://codereview.chromium.org/597113002

git-svn-id: svn://svn.chromium.org/blink/trunk@182580 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
@madsager
madsager committed Sep 24, 2014
1 parent 79bab42 commit 2502bcc
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 310 deletions.
1 change: 0 additions & 1 deletion Source/core/frame/PinchViewport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class PinchViewport FINAL : public GraphicsLayerClient, public ScrollableArea {
// coordinates are in partial CSS pixels.
void setLocation(const FloatPoint&);
void move(const FloatPoint&);
FloatPoint location() const { return m_offset; } // used in unit tests

// Sets the size of the inner viewport when unscaled in CSS pixels.
// This will be clamped to the size of the outer viewport (the main frame).
Expand Down
6 changes: 3 additions & 3 deletions Source/platform/scroll/ScrollView.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ class PLATFORM_EXPORT ScrollView : public Widget, public ScrollableArea {

virtual void notifyPageThatContentAreaWillPaint() const;

// NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
virtual void scrollTo(const IntSize& newOffset);

// The window that hosts the ScrollView. The ScrollView will communicate scrolls and repaints to the
// host window in the window's coordinate space.
virtual HostWindow* hostWindow() const = 0;
Expand Down Expand Up @@ -256,9 +259,6 @@ class PLATFORM_EXPORT ScrollView : public Widget, public ScrollableArea {
protected:
ScrollView();

// NOTE: This should only be called by the overriden setScrollOffset from ScrollableArea.
virtual void scrollTo(const IntSize& newOffset);

virtual void contentRectangleForPaintInvalidation(const IntRect&);
virtual void paintContents(GraphicsContext*, const IntRect& damageRect) = 0;

Expand Down
101 changes: 15 additions & 86 deletions Source/web/ViewportAnchor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "core/dom/Node.h"
#include "core/page/EventHandler.h"
#include "core/rendering/HitTestResult.h"
#include "platform/scroll/ScrollView.h"

namespace blink {

Expand Down Expand Up @@ -69,76 +68,31 @@ Node* findNonEmptyAnchorNode(const IntPoint& point, const IntRect& viewRect, Eve
return node;
}

void moveToEncloseRect(IntRect& outer, const FloatRect& inner)
{
IntPoint minimumPosition = ceiledIntPoint(inner.location() + inner.size() - FloatSize(outer.size()));
IntPoint maximumPosition = flooredIntPoint(inner.location());

IntPoint outerOrigin = outer.location();
outerOrigin = outerOrigin.expandedTo(minimumPosition);
outerOrigin = outerOrigin.shrunkTo(maximumPosition);

outer.setLocation(outerOrigin);
}

void moveIntoRect(FloatRect& inner, const IntRect& outer)
{
FloatPoint minimumPosition = FloatPoint(outer.location());
FloatPoint maximumPosition = minimumPosition + outer.size() - inner.size();

// Adjust maximumPosition to the nearest lower integer because
// PinchViewport::maximumScrollPosition() does the same.
// The value of minumumPosition is already adjusted since it is
// constructed from an integer point.
maximumPosition = flooredIntPoint(maximumPosition);

FloatPoint innerOrigin = inner.location();
innerOrigin = innerOrigin.expandedTo(minimumPosition);
innerOrigin = innerOrigin.shrunkTo(maximumPosition);

inner.setLocation(innerOrigin);
}

} // namespace

ViewportAnchor::ViewportAnchor(EventHandler* eventHandler)
: m_eventHandler(eventHandler) { }

void ViewportAnchor::setAnchor(const IntRect& outerViewRect, const IntRect& innerViewRect,
const FloatSize& anchorInInnerViewCoords)
void ViewportAnchor::setAnchor(const IntRect& viewRect, const FloatSize& anchorInViewCoords)
{
// Preserve the inner viewport position in document in case we won't find the anchor
m_pinchViewportInDocument = innerViewRect.location();

m_viewRect = viewRect;
m_anchorNode.clear();
m_anchorNodeBounds = LayoutRect();
m_anchorInNodeCoords = FloatSize();
m_anchorInInnerViewCoords = anchorInInnerViewCoords;
m_normalizedPinchViewportOffset = FloatSize();
m_anchorInViewCoords = anchorInViewCoords;

if (innerViewRect.isEmpty())
if (viewRect.isEmpty())
return;

// Preserve origins at the absolute screen origin
if (innerViewRect.location() == IntPoint::zero())
if (viewRect.location() == IntPoint::zero())
return;

// Inner rectangle should be within the outer one.
ASSERT(outerViewRect.contains(innerViewRect));

// Outer rectangle is used as a scale, we need positive width and height.
ASSERT(!outerViewRect.isEmpty());
FloatSize anchorOffset = viewRect.size();
anchorOffset.scale(anchorInViewCoords.width(), anchorInViewCoords.height());
const FloatPoint anchorPoint = FloatPoint(viewRect.location()) + anchorOffset;

m_normalizedPinchViewportOffset = innerViewRect.location() - outerViewRect.location();

// Normalize by the size of the outer rect
m_normalizedPinchViewportOffset.scale(1.0 / outerViewRect.width(), 1.0 / outerViewRect.height());

FloatSize anchorOffset = innerViewRect.size();
anchorOffset.scale(anchorInInnerViewCoords.width(), anchorInInnerViewCoords.height());
const FloatPoint anchorPoint = FloatPoint(innerViewRect.location()) + anchorOffset;

Node* node = findNonEmptyAnchorNode(flooredIntPoint(anchorPoint), innerViewRect, m_eventHandler);
Node* node = findNonEmptyAnchorNode(flooredIntPoint(anchorPoint), viewRect, m_eventHandler);
if (!node)
return;

Expand All @@ -148,49 +102,24 @@ void ViewportAnchor::setAnchor(const IntRect& outerViewRect, const IntRect& inne
m_anchorInNodeCoords.scale(1.f / m_anchorNodeBounds.width(), 1.f / m_anchorNodeBounds.height());
}

void ViewportAnchor::computeOrigins(const ScrollView& scrollView, const FloatSize& innerSize,
IntPoint& mainFrameOffset, FloatPoint& pinchViewportOffset) const
{
IntSize outerSize = scrollView.visibleContentRect().size();

// Compute the viewport origins in CSS pixels relative to the document.
FloatSize absPinchViewportOffset = m_normalizedPinchViewportOffset;
absPinchViewportOffset.scale(outerSize.width(), outerSize.height());

FloatPoint innerOrigin = getInnerOrigin(innerSize);
FloatPoint outerOrigin = innerOrigin - absPinchViewportOffset;

IntRect outerRect = IntRect(flooredIntPoint(outerOrigin), outerSize);
FloatRect innerRect = FloatRect(innerOrigin, innerSize);

moveToEncloseRect(outerRect, innerRect);

outerRect.setLocation(scrollView.adjustScrollPositionWithinRange(outerRect.location()));

moveIntoRect(innerRect, outerRect);

mainFrameOffset = outerRect.location();
pinchViewportOffset = FloatPoint(innerRect.location() - outerRect.location());
}

FloatPoint ViewportAnchor::getInnerOrigin(const FloatSize& innerSize) const
IntPoint ViewportAnchor::computeOrigin(const IntSize& currentViewSize) const
{
if (!m_anchorNode || !m_anchorNode->inDocument())
return m_pinchViewportInDocument;
return m_viewRect.location();

const LayoutRect currentNodeBounds = m_anchorNode->boundingBox();
if (m_anchorNodeBounds == currentNodeBounds)
return m_pinchViewportInDocument;
return m_viewRect.location();

// Compute the new anchor point relative to the node position
FloatSize anchorOffsetFromNode = currentNodeBounds.size();
anchorOffsetFromNode.scale(m_anchorInNodeCoords.width(), m_anchorInNodeCoords.height());
FloatPoint anchorPoint = currentNodeBounds.location() + anchorOffsetFromNode;

// Compute the new origin point relative to the new anchor point
FloatSize anchorOffsetFromOrigin = innerSize;
anchorOffsetFromOrigin.scale(m_anchorInInnerViewCoords.width(), m_anchorInInnerViewCoords.height());
return anchorPoint - anchorOffsetFromOrigin;
FloatSize anchorOffsetFromOrigin = currentViewSize;
anchorOffsetFromOrigin.scale(m_anchorInViewCoords.width(), m_anchorInViewCoords.height());
return flooredIntPoint(anchorPoint - anchorOffsetFromOrigin);
}

} // namespace blink
20 changes: 5 additions & 15 deletions Source/web/ViewportAnchor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ namespace blink {
class EventHandler;
class IntSize;
class Node;
class ScrollView;

// ViewportAnchor provides a way to anchor a viewport origin to a DOM node.
// In particular, the user supplies the current viewport (in CSS coordinates)
Expand All @@ -57,31 +56,22 @@ class ViewportAnchor {
public:
explicit ViewportAnchor(EventHandler*);

void setAnchor(const IntRect& outerViewRect, const IntRect& innerViewRect, const FloatSize& anchorInViewCoords);
void setAnchor(const IntRect& viewRect, const FloatSize& anchorInViewCoords);

void computeOrigins(const ScrollView&, const FloatSize& innerSize,
IntPoint& mainFrameOffset, FloatPoint& pinchViewportOffset) const;

private:
FloatPoint getInnerOrigin(const FloatSize& innerSize) const;
IntPoint computeOrigin(const IntSize& currentViewSize) const;

private:
RawPtrWillBeMember<EventHandler> m_eventHandler;

// Inner viewport origin in the reference frame of the document in CSS pixels
FloatPoint m_pinchViewportInDocument;

// Inner viewport origin in the reference frame of the outer viewport
// normalized to the outer viewport size.
FloatSize m_normalizedPinchViewportOffset;
IntRect m_viewRect;

RefPtrWillBeMember<Node> m_anchorNode;
LayoutRect m_anchorNodeBounds;

FloatSize m_anchorInInnerViewCoords;
FloatSize m_anchorInViewCoords;
FloatSize m_anchorInNodeCoords;
};

} // namespace blink

#endif
#endif
61 changes: 6 additions & 55 deletions Source/web/WebViewImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1705,10 +1705,8 @@ void WebViewImpl::resize(const WebSize& newSize)

ViewportAnchor viewportAnchor(&localFrameRootTemporary()->frame()->eventHandler());
if (shouldAnchorAndRescaleViewport) {
viewportAnchor.setAnchor(
view->visibleContentRect(),
visibleRectInDocument(),
FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
viewportAnchor.setAnchor(view->visibleContentRect(),
FloatSize(viewportAnchorXCoord, viewportAnchorYCoord));
}

// FIXME: TextAutosizer does not yet support out-of-process frames.
Expand All @@ -1728,33 +1726,15 @@ void WebViewImpl::resize(const WebSize& newSize)

if (shouldAnchorAndRescaleViewport) {
float newPageScaleFactor = oldPageScaleFactor / oldMinimumPageScaleFactor * minimumPageScaleFactor();
newPageScaleFactor = clampPageScaleFactorToLimits(newPageScaleFactor);

FloatSize pinchViewportSize = FloatSize(newSize);
pinchViewportSize.scale(1 / newPageScaleFactor);

IntPoint mainFrameOrigin;
FloatPoint pinchViewportOrigin;
viewportAnchor.computeOrigins(*view, pinchViewportSize,
mainFrameOrigin, pinchViewportOrigin);
scrollAndRescaleViewports(newPageScaleFactor, mainFrameOrigin, pinchViewportOrigin);
IntSize scaledViewportSize = newSize;
scaledViewportSize.scale(1 / newPageScaleFactor);
setPageScaleFactor(newPageScaleFactor, viewportAnchor.computeOrigin(scaledViewportSize));
}
}

sendResizeEventAndRepaint();
}

IntRect WebViewImpl::visibleRectInDocument() const
{
if (pinchVirtualViewportEnabled()) {
// Inner viewport in the document coordinates
return enclosedIntRect(page()->frameHost().pinchViewport().visibleRectInDocument());
}

// Outer viewport in the document coordinates
return localFrameRootTemporary()->frameView()->visibleContentRect();
}

void WebViewImpl::willEndLiveResize()
{
if (mainFrameImpl() && mainFrameImpl()->frameView())
Expand Down Expand Up @@ -2960,36 +2940,6 @@ WebFloatPoint WebViewImpl::pinchViewportOffset() const
return page()->frameHost().pinchViewport().visibleRect().location();
}

void WebViewImpl::scrollAndRescaleViewports(float scaleFactor,
const IntPoint& mainFrameOrigin,
const FloatPoint& pinchViewportOrigin)
{
// Old way
if (!pinchVirtualViewportEnabled()) {
setPageScaleFactor(scaleFactor, mainFrameOrigin);
return;
}

if (!page())
return;

if (!mainFrameImpl())
return;

FrameView * view = mainFrameImpl()->frameView();
if (!view)
return;

// Order is important: pinch viewport location is clamped based on
// main frame scroll position and pinch viewport scale.

view->setScrollOffset(mainFrameOrigin);

setPageScaleFactor(scaleFactor);

page()->frameHost().pinchViewport().setLocation(pinchViewportOrigin);
}

void WebViewImpl::setPageScaleFactor(float scaleFactor)
{
ASSERT(page());
Expand Down Expand Up @@ -3032,6 +2982,7 @@ void WebViewImpl::setPageScaleFactor(float scaleFactor, const WebPoint& origin)
page()->setPageScaleFactor(scaleFactor, newScrollOffset);
}


float WebViewImpl::deviceScaleFactor() const
{
if (!page())
Expand Down
4 changes: 0 additions & 4 deletions Source/web/WebViewImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,10 +505,6 @@ class WebViewImpl FINAL : public WebView
// prevent external usage
virtual void setPageScaleFactor(float scaleFactor, const WebPoint& origin) OVERRIDE;

void scrollAndRescaleViewports(float scaleFactor, const IntPoint& mainFrameOrigin, const FloatPoint& pinchViewportOrigin);

IntRect visibleRectInDocument() const;

float legibleScale() const;
void refreshPageScaleFactorAfterLayout();
void resumeTreeViewCommits();
Expand Down
Loading

0 comments on commit 2502bcc

Please sign in to comment.