New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scrolling by pressing space bar has regressed in trunk, is choppy (not 120Hz) #5541
Scrolling by pressing space bar has regressed in trunk, is choppy (not 120Hz) #5541
Conversation
EWS run on previous version of this PR (hash 73b11e7) |
73b11e7
to
ce0f011
Compare
EWS run on previous version of this PR (hash ce0f011) |
ce0f011
to
74d00df
Compare
EWS run on previous version of this PR (hash 74d00df) |
74d00df
to
e863e9a
Compare
EWS run on previous version of this PR (hash e863e9a) |
e863e9a
to
f525300
Compare
EWS run on previous version of this PR (hash f525300) |
EWS run on previous version of this PR (hash 3f4b850) |
m_velocity = { }; | ||
} | ||
|
||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This return value is unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is because animateScroll
overrides a virtual function, so it is forced to return a value, and in the case of ScrollAnimationKeyboard
, the value doesn't need to be used
e503278
to
4800b95
Compare
EWS run on previous version of this PR (hash dc4576d) |
EWS run on previous version of this PR (hash e503278) |
EWS run on previous version of this PR (hash 4800b95) |
4800b95
to
ab3a31a
Compare
EWS run on previous version of this PR (hash ab3a31a) |
ab3a31a
to
7f90527
Compare
EWS run on previous version of this PR (hash 7f90527) |
@@ -199,6 +200,13 @@ void ScrollingStateScrollingNode::setSynchronousScrollingReasons(OptionSet<Synch | |||
} | |||
#endif | |||
|
|||
|
|||
void ScrollingStateScrollingNode::setKeyboardScrollData(const RequestedKeyboardScrollData& scrollData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be use RequestedKeyboardScrollData&&
with WTFMove
at the call site and WTFMove
into the member variable.
7f90527
to
fc8ff7d
Compare
EWS run on current version of this PR (hash fc8ff7d) |
β¦t 120Hz) https://bugs.webkit.org/show_bug.cgi?id=246878 rdar://101052130 Reviewed by Simon Fraser. Moves keyboard smooth scrolling onto the scrolling thread instead of the main thread. A new `ScrollAnimation` is created to facilitate this, `ScrollAnimationKeyboard`. Most of the changes are to allow the state to propogate through the various classes and the scrolling tree. Since keyboard scrolling is now on a different thread, some tests had to be modified to enable the `AsyncOverflowScrollingEnabled` and `AsyncFrameScrollingEnabled` settings. * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/page/FrameView.cpp: (WebCore::FrameView::requestStartKeyboardAnimation): (WebCore::FrameView::requestFinishKeyboardAnimation): * Source/WebCore/page/FrameView.h: * Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp: (WebCore::AsyncScrollingCoordinator::requestStartKeyboardAnimation): (WebCore::AsyncScrollingCoordinator::requestFinishKeyboardAnimation): * Source/WebCore/page/scrolling/AsyncScrollingCoordinator.h: * Source/WebCore/page/scrolling/ScrollingCoordinator.h: (WebCore::ScrollingCoordinator::requestStartKeyboardAnimation): (WebCore::ScrollingCoordinator::requestFinishKeyboardAnimation): * Source/WebCore/page/scrolling/ScrollingStateNode.h: * Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp: (WebCore::ScrollingStateScrollingNode::ScrollingStateScrollingNode): (WebCore::ScrollingStateScrollingNode::setKeyboardScrollData): (WebCore::ScrollingStateScrollingNode::setKeyboardScrollEndData): * Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h: (WebCore::ScrollingStateScrollingNode::keyboardScrollData const): (WebCore::ScrollingStateScrollingNode::keyboardScrollEndData const): * Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp: (WebCore::ScrollingTreeScrollingNode::commitStateAfterChildren): (WebCore::ScrollingTreeScrollingNode::handleKeyboardScrollRequest): (WebCore::ScrollingTreeScrollingNode::handleKeyboardScrollEndRequest): * Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h: * Source/WebCore/page/scrolling/ScrollingTreeScrollingNodeDelegate.h: (WebCore::ScrollingTreeScrollingNodeDelegate::handleKeyboardScrollRequest): (WebCore::ScrollingTreeScrollingNodeDelegate::handleKeyboardScrollEndRequest): * Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.h: * Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeDelegateMac.mm: (WebCore::ScrollingTreeScrollingNodeDelegateMac::handleKeyboardScrollRequest): (WebCore::ScrollingTreeScrollingNodeDelegateMac::handleKeyboardScrollEndRequest): * Source/WebCore/platform/KeyboardScrollingAnimator.cpp: (WebCore::KeyboardScrollingAnimator::KeyboardScrollingAnimator): (WebCore::KeyboardScrollingAnimator::beginKeyboardScrollGesture): (WebCore::KeyboardScrollingAnimator::handleKeyUpEvent): (WebCore::KeyboardScrollingAnimator::stopScrollingImmediately): (WebCore::perpendicularAbsoluteUnitVector): Deleted. (WebCore::KeyboardScrollingAnimator::updateKeyboardScrollPosition): Deleted. (WebCore::farthestPointInDirection): Deleted. (WebCore::KeyboardScrollingAnimator::stopKeyboardScrollAnimation): Deleted. * Source/WebCore/platform/KeyboardScrollingAnimator.h: * Source/WebCore/platform/ScrollAnimation.cpp: (WebCore::operator<<): * Source/WebCore/platform/ScrollAnimation.h: * Source/WebCore/platform/ScrollAnimationKeyboard.cpp: Added. (WebCore::perpendicularAbsoluteUnitVector): (WebCore::boxSideForDirection): (WebCore::farthestPointInDirection): (WebCore::ScrollAnimationKeyboard::scrollableDirectionsFromPosition): (WebCore::ScrollAnimationKeyboard::ScrollAnimationKeyboard): (WebCore::ScrollAnimationKeyboard::retargetActiveAnimation): (WebCore::ScrollAnimationKeyboard::serviceAnimation): (WebCore::ScrollAnimationKeyboard::startKeyboardScroll): (WebCore::ScrollAnimationKeyboard::stopKeyboardScrollAnimation): (WebCore::ScrollAnimationKeyboard::finishKeyboardScroll): (WebCore::ScrollAnimationKeyboard::animateScroll): (WebCore::ScrollAnimationKeyboard::debugDescription const): * Source/WebCore/platform/ScrollAnimationKeyboard.h: Copied from Source/WebCore/platform/ScrollAnimation.cpp. * Source/WebCore/platform/ScrollAnimator.cpp: (WebCore::ScrollAnimator::ScrollAnimator): * Source/WebCore/platform/ScrollableArea.cpp: (WebCore::ScrollableArea::beginKeyboardScroll): (WebCore::ScrollableArea::endKeyboardScroll): * Source/WebCore/platform/ScrollableArea.h: (WebCore::ScrollableArea::requestStartKeyboardAnimation): (WebCore::ScrollableArea::requestFinishKeyboardAnimation): * Source/WebCore/platform/ScrollingEffectsController.cpp: (WebCore::ScrollingEffectsController::animationCallback): (WebCore::ScrollingEffectsController::startKeyboardScroll): (WebCore::ScrollingEffectsController::finishKeyboardScroll): (WebCore::ScrollingEffectsController::updateKeyboardScrollingAnimatingState): (WebCore::ScrollingEffectsController::scrollAnimationWillStart): * Source/WebCore/platform/ScrollingEffectsController.h: (WebCore::ScrollingEffectsControllerClient::updateKeyboardScrollPosition): Deleted. * Source/WebCore/rendering/RenderLayerScrollableArea.cpp: (WebCore::RenderLayerScrollableArea::requestStartKeyboardAnimation): (WebCore::RenderLayerScrollableArea::requestFinishKeyboardAnimation): * Source/WebCore/rendering/RenderLayerScrollableArea.h: Canonical link: https://commits.webkit.org/256266@main
fc8ff7d
to
cb5925e
Compare
Committed 256266@main (cb5925e): https://commits.webkit.org/256266@main Reviewed commits have been landed. Closing PR #5541 and removing active labels. |
cb5925e
fc8ff7d
π§ͺ mac-wk1π§ͺ mac-AS-debug-wk2