- Revision
- 125434
- Author
- [email protected]
- Date
- 2012-08-13 12:24:17 -0700 (Mon, 13 Aug 2012)
Log Message
[BlackBerry] Rounding error somewhere when translating CompositingLayer 's for in-region scrolling
https://bugs.webkit.org/show_bug.cgi?id=93848
PR #190986
Reviewed by Yong Li.
Patch by Antonio Gomes <[email protected]>
Patch adds a WebKit thread setScrollPosition-like API to InRegionScroller.
It fixes a mismatch we had of setting a layer's scroll positing directly via
::setScrollPosition on the UI thread, but dispatching a coalesceable
scrollBy(delta) message to WebKit thread.
* Api/InRegionScroller.cpp:
(BlackBerry::WebKit::InRegionScroller::setScrollPositionCompositingThread):
Changed the API name to match other thread specific methods naming.
(WebKit):
(BlackBerry::WebKit::InRegionScroller::setScrollPositionWebKitThread):
Added a setScrollPosition-like method to InRegionScroller public API. Used
together with its UI-thread counterpart, it can fix some rounding
errors we have due to mixing ::setScrollPosition and ::scrollBy methods.
(BlackBerry::WebKit::InRegionScrollerPrivate::setScrollPositionCompositingThread):
Method renamed. See reasons above.
(BlackBerry::WebKit::InRegionScrollerPrivate::setScrollPositionWebKitThread):
Calls fowards the call to ::setLayerScrollPosition (below).
(BlackBerry::WebKit::InRegionScrollerPrivate::setLayerScrollPosition):
Sets the scroll position of a given RenderLayer.
* Api/InRegionScroller.h:
(InRegionScroller):
* Api/InRegionScroller_p.h:
(WebCore):
(InRegionScrollerPrivate):
* WebKitSupport/InRegionScrollableArea.cpp:
(BlackBerry::WebKit::InRegionScrollableArea::InRegionScrollableArea):
Cache the RenderLayer object associated to a given scrollable area
instead of its LayerCompositingThread. This way we can use it for
scrolling from both Compositing/UI and WebKit threads.
Modified Paths
Diff
Modified: trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.h (125433 => 125434)
--- trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.h 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.h 2012-08-13 19:24:17 UTC (rev 125434)
@@ -133,6 +133,8 @@
Image* contents() const { return m_contents.get(); }
void setOwner(GraphicsLayerBlackBerry* owner) { m_owner = owner; }
+ // NOTE: Can be 0.
+ GraphicsLayerBlackBerry* owner() const { return m_owner; }
bool drawsContent() const { return m_owner && m_owner->drawsContent(); }
void setDrawable(bool);
Modified: trunk/Source/WebKit/blackberry/Api/InRegionScroller.cpp (125433 => 125434)
--- trunk/Source/WebKit/blackberry/Api/InRegionScroller.cpp 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebKit/blackberry/Api/InRegionScroller.cpp 2012-08-13 19:24:17 UTC (rev 125434)
@@ -26,9 +26,11 @@
#include "InRegionScrollableArea.h"
#include "InRegionScroller_p.h"
#include "LayerCompositingThread.h"
+#include "LayerWebKitThread.h"
#include "Page.h"
#include "RenderBox.h"
#include "RenderLayer.h"
+#include "RenderLayerBacking.h"
#include "RenderObject.h"
#include "RenderView.h"
#include "WebPage_p.h"
@@ -56,12 +58,22 @@
delete d;
}
-bool InRegionScroller::compositedSetScrollPosition(unsigned camouflagedLayer, const Platform::IntPoint& scrollPosition)
+bool InRegionScroller::setScrollPositionCompositingThread(unsigned camouflagedLayer, const Platform::IntPoint& scrollPosition)
{
ASSERT(Platform::userInterfaceThreadMessageClient()->isCurrentThread());
- return d->compositedSetScrollPosition(camouflagedLayer, d->m_webPage->mapFromTransformed(scrollPosition));
+
+ // FIXME: Negative values won't work with map{To,From}Transform methods.
+ return d->setScrollPositionCompositingThread(camouflagedLayer, d->m_webPage->mapFromTransformed(scrollPosition));
}
+bool InRegionScroller::setScrollPositionWebKitThread(unsigned camouflagedLayer, const Platform::IntPoint& scrollPosition)
+{
+ ASSERT(Platform::webKitThreadMessageClient()->isCurrentThread());
+
+ // FIXME: Negative values won't work with map{To,From}Transform methods.
+ return d->setScrollPositionWebKitThread(camouflagedLayer, d->m_webPage->mapFromTransformed(scrollPosition));
+}
+
InRegionScrollerPrivate::InRegionScrollerPrivate(WebPagePrivate* webPagePrivate)
: m_webPage(webPagePrivate)
{
@@ -92,15 +104,35 @@
return hasNode();
}
-bool InRegionScrollerPrivate::compositedSetScrollPosition(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition)
+bool InRegionScrollerPrivate::setScrollPositionCompositingThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition)
{
- LayerCompositingThread* scrollLayer = reinterpret_cast<LayerCompositingThread*>(camouflagedLayer);
- scrollLayer->override()->setBoundsOrigin(WebCore::FloatPoint(scrollPosition.x(), scrollPosition.y()));
+ LayerCompositingThread* scrollLayer = reinterpret_cast<LayerWebKitThread*>(camouflagedLayer)->layerCompositingThread();
+ // FIXME: Clamp maximum and minimum scroll positions as a last attempt to fix round errors.
+ scrollLayer->override()->setBoundsOrigin(scrollPosition);
m_webPage->scheduleCompositingRun();
return true;
}
+bool InRegionScrollerPrivate::setScrollPositionWebKitThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition)
+{
+ RenderLayer* layer = 0;
+
+ LayerWebKitThread* layerWebKitThread = reinterpret_cast<LayerWebKitThread*>(camouflagedLayer);
+ ASSERT(layerWebKitThread);
+ if (layerWebKitThread->owner()) {
+ GraphicsLayer* graphicsLayer = layerWebKitThread->owner();
+ RenderLayerBacking* backing = static_cast<RenderLayerBacking*>(graphicsLayer->client());
+ layer = backing->owningLayer();
+ }
+
+ if (!layer)
+ return false;
+
+ // FIXME: Clamp maximum and minimum scroll positions as a last attempt to fix round errors.
+ return setLayerScrollPosition(layer, scrollPosition);
+}
+
bool InRegionScrollerPrivate::scrollBy(const Platform::IntSize& delta)
{
ASSERT(Platform::webkitThreadMessageClient()->isCurrentThread());
@@ -192,6 +224,29 @@
return validReturn;
}
+bool InRegionScrollerPrivate::setLayerScrollPosition(RenderLayer* layer, const IntPoint& scrollPosition)
+{
+ RenderObject* layerRenderer = layer->renderer();
+ ASSERT(layerRenderer);
+
+ if (layerRenderer->isRenderView()) { // #document case.
+ FrameView* view = toRenderView(layerRenderer)->frameView();
+ ASSERT(view);
+
+ Frame* frame = view->frame();
+ ASSERT_UNUSED(frame, frame);
+
+ view->setScrollPosition(scrollPosition);
+ return true;
+ }
+
+ // RenderBox-based elements case (scrollable boxes (div's, p's, textarea's, etc)).
+ layer->scrollToOffset(scrollPosition.x(), scrollPosition.y());
+ // FIXME_agomes: Please recheck if it is needed still!
+ layer->renderer()->repaint(true);
+ return true;
+}
+
bool InRegionScrollerPrivate::scrollNodeRecursively(WebCore::Node* node, const WebCore::IntSize& delta)
{
if (delta.isZero())
Modified: trunk/Source/WebKit/blackberry/Api/InRegionScroller.h (125433 => 125434)
--- trunk/Source/WebKit/blackberry/Api/InRegionScroller.h 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebKit/blackberry/Api/InRegionScroller.h 2012-08-13 19:24:17 UTC (rev 125434)
@@ -35,7 +35,8 @@
InRegionScroller(WebPagePrivate*);
~InRegionScroller();
- bool compositedSetScrollPosition(unsigned /*camouflagedLayer*/, const Platform::IntPoint& /*scrollPosition*/);
+ bool setScrollPositionCompositingThread(unsigned camouflagedLayer, const Platform::IntPoint& /*scrollPosition*/);
+ bool setScrollPositionWebKitThread(unsigned camouflagedLayer, const Platform::IntPoint& /*scrollPosition*/);
private:
friend class WebPagePrivate;
Modified: trunk/Source/WebKit/blackberry/Api/InRegionScroller_p.h (125433 => 125434)
--- trunk/Source/WebKit/blackberry/Api/InRegionScroller_p.h 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebKit/blackberry/Api/InRegionScroller_p.h 2012-08-13 19:24:17 UTC (rev 125434)
@@ -29,6 +29,7 @@
class Frame;
class Node;
class RenderObject;
+class RenderLayer;
}
namespace BlackBerry {
@@ -48,13 +49,18 @@
bool hasNode() const;
bool scrollBy(const Platform::IntSize& delta);
- bool compositedSetScrollPosition(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition);
+ bool setScrollPositionCompositingThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition);
+ bool setScrollPositionWebKitThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition);
+
std::vector<Platform::ScrollViewBase*> inRegionScrollableAreasForPoint(const WebCore::IntPoint&);
WebPagePrivate* m_webPage;
private:
+ bool setLayerScrollPosition(WebCore::RenderLayer*, const WebCore::IntPoint& scrollPosition);
+
+ // Obsolete codepath.
bool scrollNodeRecursively(WebCore::Node*, const WebCore::IntSize& delta);
bool scrollRenderer(WebCore::RenderObject*, const WebCore::IntSize& delta);
void adjustScrollDelta(const WebCore::IntPoint& maxOffset, const WebCore::IntPoint& currentOffset, WebCore::IntSize& delta) const;
Modified: trunk/Source/WebKit/blackberry/ChangeLog (125433 => 125434)
--- trunk/Source/WebKit/blackberry/ChangeLog 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebKit/blackberry/ChangeLog 2012-08-13 19:24:17 UTC (rev 125434)
@@ -1,3 +1,41 @@
+2012-08-13 Antonio Gomes <[email protected]>
+
+ [BlackBerry] Rounding error somewhere when translating CompositingLayer 's for in-region scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=93848
+ PR #190986
+
+ Reviewed by Yong Li.
+
+ Patch adds a WebKit thread setScrollPosition-like API to InRegionScroller.
+ It fixes a mismatch we had of setting a layer's scroll positing directly via
+ ::setScrollPosition on the UI thread, but dispatching a coalesceable
+ scrollBy(delta) message to WebKit thread.
+
+ * Api/InRegionScroller.cpp:
+ (BlackBerry::WebKit::InRegionScroller::setScrollPositionCompositingThread):
+ Changed the API name to match other thread specific methods naming.
+ (WebKit):
+ (BlackBerry::WebKit::InRegionScroller::setScrollPositionWebKitThread):
+ Added a setScrollPosition-like method to InRegionScroller public API. Used
+ together with its UI-thread counterpart, it can fix some rounding
+ errors we have due to mixing ::setScrollPosition and ::scrollBy methods.
+ (BlackBerry::WebKit::InRegionScrollerPrivate::setScrollPositionCompositingThread):
+ Method renamed. See reasons above.
+ (BlackBerry::WebKit::InRegionScrollerPrivate::setScrollPositionWebKitThread):
+ Calls fowards the call to ::setLayerScrollPosition (below).
+ (BlackBerry::WebKit::InRegionScrollerPrivate::setLayerScrollPosition):
+ Sets the scroll position of a given RenderLayer.
+ * Api/InRegionScroller.h:
+ (InRegionScroller):
+ * Api/InRegionScroller_p.h:
+ (WebCore):
+ (InRegionScrollerPrivate):
+ * WebKitSupport/InRegionScrollableArea.cpp:
+ (BlackBerry::WebKit::InRegionScrollableArea::InRegionScrollableArea):
+ Cache the RenderLayer object associated to a given scrollable area
+ instead of its LayerCompositingThread. This way we can use it for
+ scrolling from both Compositing/UI and WebKit threads.
+
2012-08-12 Arvid Nilsson <[email protected]>
[BlackBerry] Tap highlight flashes checkerboard after pinch zoom
Modified: trunk/Source/WebKit/blackberry/WebKitSupport/InRegionScrollableArea.cpp (125433 => 125434)
--- trunk/Source/WebKit/blackberry/WebKitSupport/InRegionScrollableArea.cpp 2012-08-13 19:24:13 UTC (rev 125433)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/InRegionScrollableArea.cpp 2012-08-13 19:24:17 UTC (rev 125434)
@@ -20,7 +20,6 @@
#include "InRegionScrollableArea.h"
#include "Frame.h"
-#include "LayerCompositingThread.h"
#include "LayerWebKitThread.h"
#include "RenderBox.h"
#include "RenderLayer.h"
@@ -90,7 +89,7 @@
if (m_layer->usesCompositedScrolling()) {
m_supportsCompositedScrolling = true;
ASSERT(m_layer->backing()->hasScrollingLayer());
- m_cachedCompositedScrollableLayer = reinterpret_cast<unsigned>(m_layer->backing()->scrollingLayer()->platformLayer()->layerCompositingThread());
+ m_cachedCompositedScrollableLayer = reinterpret_cast<unsigned>(m_layer->backing()->scrollingLayer()->platformLayer());
}
m_overscrollLimitFactor = 0.0;
