Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Source/WebCore: 
REGRESSION (r268476): Sometimes zooming does not work on purpleair.com/map
https://bugs.webkit.org/show_bug.cgi?id=218680
<rdar://problem/70418612>

Reviewed by Zalan Bujtas.

Loading purpleair.com/map and quickly panning then zooming would often fail to zoom the map,
and rubberband instead. This happened because the composited layer containing the WebGL
canvas would switch to into tiled mode, and that would fail to set the event region on the
new platform layer.

The fix is to just add the 'EventRegionChanged' flag to the list of properties that need to
get updated after a tiled layer switch.

Test: fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch.html

* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::determineWheelEventProcessing):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::changeLayerTypeTo):

LayoutTests:
REGRESSION (r268476): Sometimes zooming does not worki on purpleair.com/map
https://bugs.webkit.org/show_bug.cgi?id=218680

Reviewed by Zalan Bujtas.

* fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch-expected.txt: Added.
* fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch.html: Added.


Canonical link: https://commits.webkit.org/231352@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@269561 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
@smfr
smfr committed Nov 7, 2020
1 parent 216aa7f commit 2e863ec
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 20 deletions.
10 changes: 10 additions & 0 deletions LayoutTests/ChangeLog
View file
@@ -1,3 +1,13 @@
2020-11-07 Simon Fraser <simon.fraser@apple.com>

REGRESSION (r268476): Sometimes zooming does not worki on purpleair.com/map
https://bugs.webkit.org/show_bug.cgi?id=218680

Reviewed by Zalan Bujtas.

* fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch-expected.txt: Added.
* fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch.html: Added.

2020-11-06 Simon Fraser <simon.fraser@apple.com>

Programmatic scrolls need updated scrolling geometry
Expand Down
10 changes: 10 additions & 0 deletions LayoutTests/fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch-expected.txt
View file
@@ -0,0 +1,10 @@

Test that scroll over box does not scroll page
PASS windowScrollEventCount is 0

Test that scroll over box does not scroll page after switch to tiled layer
PASS windowScrollEventCount is 0
PASS successfullyParsed is true

TEST COMPLETE

83 changes: 83 additions & 0 deletions LayoutTests/fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch.html
View file
@@ -0,0 +1,83 @@
<!DOCTYPE html>
<html>
<style>
body {
height: 2000px;
}

.box {
width: 3000px;
height: 100px;
margin: 50px;
background-color: silver;
}

.composited {
transform: translateZ(0);
}

body.changed .box {
background-image: linear-gradient(silver, gray);
}
</style>
<script src="../../../resources/js-test-pre.js"></script>
<script src="../../../resources/ui-helper.js"></script>
<script>
var jsTestIsAsync = true;

let windowScrollEventCount = 0;

async function resetScrollPositions()
{
window.scrollTo(0, 0);

// Wait for scroll events to fire.
await UIHelper.renderingUpdate();

windowScrollEventCount = 0;
}

async function testScroll()
{
await resetScrollPositions();
await UIHelper.mouseWheelScrollAt(100, 100);
shouldBe('windowScrollEventCount', '0');
}

async function scrollTest()
{
debug('');
debug('Test that scroll over box does not scroll page');
await testScroll();

document.body.classList.add('changed');

debug('');
debug('Test that scroll over box does not scroll page after switch to tiled layer');
await testScroll();

finishJSTest();
}

window.addEventListener('load', () => {

for (let active of document.querySelectorAll('.active')) {
active.addEventListener('wheel', (event) => {
event.preventDefault();
}, { passive: false });
}

window.addEventListener('scroll', () => {
++windowScrollEventCount;
}, false);

setTimeout(scrollTest, 0);
}, false);
</script>
<body>
<div class="active composited box"></div>
<div id="console"></div>
<script src="../../../resources/js-test-post.js"></script>
</body>
</html>

23 changes: 23 additions & 0 deletions Source/WebCore/ChangeLog
View file
@@ -1,3 +1,26 @@
2020-11-07 Simon Fraser <simon.fraser@apple.com>

REGRESSION (r268476): Sometimes zooming does not work on purpleair.com/map
https://bugs.webkit.org/show_bug.cgi?id=218680
<rdar://problem/70418612>

Reviewed by Zalan Bujtas.

Loading purpleair.com/map and quickly panning then zooming would often fail to zoom the map,
and rubberband instead. This happened because the composited layer containing the WebGL
canvas would switch to into tiled mode, and that would fail to set the event region on the
new platform layer.

The fix is to just add the 'EventRegionChanged' flag to the list of properties that need to
get updated after a tiled layer switch.

Test: fast/scrolling/mac/wheel-event-listener-region-tiled-layer-switch.html

* page/scrolling/ScrollingTree.cpp:
(WebCore::ScrollingTree::determineWheelEventProcessing):
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::changeLayerTypeTo):

2020-11-06 Simon Fraser <simon.fraser@apple.com>

A programmatic scroll to a new location should stop rubberbanding
Expand Down
45 changes: 25 additions & 20 deletions Source/WebCore/page/scrolling/ScrollingTree.cpp
View file
Expand Up @@ -76,35 +76,40 @@ OptionSet<WheelEventProcessingSteps> ScrollingTree::determineWheelEventProcessin

m_latchingController.receivedWheelEvent(wheelEvent, m_allowLatching);

if (!m_rootNode)
return { WheelEventProcessingSteps::ScrollingThread };
auto processingSteps = [&]() -> OptionSet<WheelEventProcessingSteps> {
if (!m_rootNode)
return { WheelEventProcessingSteps::ScrollingThread };

FloatPoint position = wheelEvent.position();
position.move(m_rootNode->viewToContentsOffset(m_treeState.mainFrameScrollPosition));
FloatPoint position = wheelEvent.position();
position.move(m_rootNode->viewToContentsOffset(m_treeState.mainFrameScrollPosition));

if (!m_treeState.eventTrackingRegions.isEmpty()) {
const EventNames& names = eventNames();
IntPoint roundedPosition = roundedIntPoint(position);
if (!m_treeState.eventTrackingRegions.isEmpty()) {
const EventNames& names = eventNames();
IntPoint roundedPosition = roundedIntPoint(position);

// Event regions are affected by page scale, so no need to map through scale.
bool isSynchronousDispatchRegion = m_treeState.eventTrackingRegions.trackingTypeForPoint(names.wheelEvent, roundedPosition) == TrackingType::Synchronous
|| m_treeState.eventTrackingRegions.trackingTypeForPoint(names.mousewheelEvent, roundedPosition) == TrackingType::Synchronous;
LOG_WITH_STREAM(Scrolling, stream << "\n\nScrollingTree::shouldHandleWheelEventSynchronously: wheelEvent " << wheelEvent << " mapped to content point " << position << ", in non-fast region " << isSynchronousDispatchRegion);
// Event regions are affected by page scale, so no need to map through scale.
bool isSynchronousDispatchRegion = m_treeState.eventTrackingRegions.trackingTypeForPoint(names.wheelEvent, roundedPosition) == TrackingType::Synchronous
|| m_treeState.eventTrackingRegions.trackingTypeForPoint(names.mousewheelEvent, roundedPosition) == TrackingType::Synchronous;
LOG_WITH_STREAM(Scrolling, stream << "\nScrollingTree::determineWheelEventProcessing: wheelEvent " << wheelEvent << " mapped to content point " << position << ", in non-fast region " << isSynchronousDispatchRegion);

if (isSynchronousDispatchRegion)
return { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };
}
if (isSynchronousDispatchRegion)
return { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };
}

#if ENABLE(WHEEL_EVENT_REGIONS)
auto eventListenerTypes = eventListenerRegionTypesForPoint(position);
if (eventListenerTypes.contains(EventListenerRegionType::NonPassiveWheel))
return { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };
auto eventListenerTypes = eventListenerRegionTypesForPoint(position);
if (eventListenerTypes.contains(EventListenerRegionType::NonPassiveWheel))
return { WheelEventProcessingSteps::MainThreadForScrolling, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };

if (eventListenerTypes.contains(EventListenerRegionType::Wheel))
return { WheelEventProcessingSteps::ScrollingThread, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };
if (eventListenerTypes.contains(EventListenerRegionType::Wheel))
return { WheelEventProcessingSteps::ScrollingThread, WheelEventProcessingSteps::MainThreadForDOMEventDispatch };
#endif
return { WheelEventProcessingSteps::ScrollingThread };
}();

LOG_WITH_STREAM(Scrolling, stream << "ScrollingTree::determineWheelEventProcessing: processingSteps " << processingSteps);

return { WheelEventProcessingSteps::ScrollingThread };
return processingSteps;
}

WheelEventHandlingResult ScrollingTree::handleWheelEvent(const PlatformWheelEvent& wheelEvent, OptionSet<WheelEventProcessingSteps> processingSteps)
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
View file
Expand Up @@ -3965,6 +3965,7 @@ void GraphicsLayerCA::changeLayerTypeTo(PlatformCALayer::LayerType newLayerType)
| BackdropFiltersChanged
| MaskLayerChanged
| OpacityChanged
| EventRegionChanged
| NameChanged
| DebugIndicatorsChanged);

Expand Down

0 comments on commit 2e863ec

Please sign in to comment.