Bug 1662800 - Fix matrix multiplication order. r=botond

When combining transform matrices for deferred transforms, we were multiplying
them in the wrong order. This caused incorrect behaviour when one of the matrices
had a scale factor.

Differential Revision: https://phabricator.services.mozilla.com/D96199
This commit is contained in:
Kartikaya Gupta 2020-11-07 11:19:56 +00:00
parent 7e58b12632
commit c9fa323a9d
4 changed files with 72 additions and 2 deletions

View file

@ -226,6 +226,8 @@ function getTargetOrigin(aTarget) {
// Convert (aX, aY), in CSS pixels relative to aTarget's bounding rect
// to device pixels relative to the screen.
// TODO: this function currently does not incorporate some CSS transforms on
// elements enclosing aTarget, e.g. scale transforms.
function coordinatesRelativeToScreen(aX, aY, aTarget) {
// Note that |window| might not be the root content window, for two
// possible reasons:
@ -759,6 +761,10 @@ function promiseMoveMouseAndScrollWheelOver(
// processed by the widget code can be detected by listening for the mousemove
// events in the caller, or for some other event that is triggered by the
// mousemove, such as the scroll event resulting from the scrollbar drag.
// The scaleFactor argument should be provided if the scrollframe has been
// scaled by an enclosing CSS transform. (TODO: this is a workaround for the
// fact that coordinatesRelativeToScreen is supposed to do this automatically
// but it currently does not).
// Note: helper_scrollbar_snap_bug1501062.html contains a copy of this code
// with modifications. Fixes here should be copied there if appropriate.
// |target| can be an element (for subframes) or a window (for root frames).
@ -766,7 +772,8 @@ function* dragVerticalScrollbar(
target,
testDriver,
distance = 20,
increment = 5
increment = 5,
scaleFactor = 1
) {
var targetElement = elementForTarget(target);
var w = {},
@ -780,6 +787,8 @@ function* dragVerticalScrollbar(
var upArrowHeight = verticalScrollbarWidth; // assume square scrollbar buttons
var mouseX = targetElement.clientWidth + verticalScrollbarWidth / 2;
var mouseY = upArrowHeight + 5; // start dragging somewhere in the thumb
mouseX *= scaleFactor;
mouseY *= scaleFactor;
dump(
"Starting drag at " +

View file

@ -0,0 +1,59 @@
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width; initial-scale=1.0">
<title>Dragging the mouse on a scrollbar for a scrollframe inside nested transforms with a scale component</title>
<script type="application/javascript" src="apz_test_native_event_utils.js"></script>
<script type="application/javascript" src="apz_test_utils.js"></script>
<script src="/tests/SimpleTest/paint_listener.js"></script>
<script type="text/javascript">
function* test(testDriver) {
var scrollableDiv = document.getElementById("scrollable");
scrollableDiv.addEventListener("scroll", () => setTimeout(testDriver, 0), {once: true});
// Scroll down a small amount (7px). The bug in this case is that the
// scrollthumb "jumps" most of the way down the scroll track because with
// the bug, the code was incorrectly combining the transforms.
// Given the scrollable height of 0.7*2000px and scrollframe height of 0.7*400px,
// the scrollthumb should be approximately 0.7*80px = 56px tall. Dragging it 7px
// should scroll approximately 50 (unscaled) pixels. If the bug manifests, it will get
// dragged by a lot more and scroll to approximately 1300px.
var dragFinisher = yield* dragVerticalScrollbar(scrollableDiv, testDriver, 7, 7, 0.7);
if (!dragFinisher) {
ok(true, "No scrollbar, can't do this test");
return;
}
// the events above might be stuck in APZ input queue for a bit until the
// layer is activated, so we wait here until the scroll event listener is
// triggered.
yield;
yield* dragFinisher();
// Flush everything just to be safe
yield flushApzRepaints(testDriver);
// Ensure the scroll position ended up roughly where we wanted it (around
// 50px, but definitely less than 1300px).
ok(scrollableDiv.scrollTop < 100, "Scrollbar drag resulted in a scroll position of " + scrollableDiv.scrollTop);
}
waitUntilApzStable()
.then(runContinuation(test))
.then(subtestDone, subtestFailed);
</script>
</head>
<body>
<div style="width: 500px; height: 300px; transform: translate(500px, 500px) scale(0.7)">
<div id="scrollable" style="transform: translate(-600px, -600px); overflow: scroll">
<div style="width: 600px; height: 400px">
<div style="width: 600px; height: 2000px; background-image: linear-gradient(red,blue)"></div>
</div>
</div>
</div>
</body>
</html>

View file

@ -42,6 +42,8 @@ var subtests = [
// Drag-select some text after reconstructing the RSF of a non-RCD to ensure
// the pending visual offset update doesn't get stuck
{"file": "helper_visualscroll_nonrcd_rsf.html"},
// Scrollbar-dragging on scrollframes inside nested transforms with scale
{"file": "helper_bug1662800.html"},
];
if (isApzEnabled()) {

View file

@ -124,7 +124,7 @@ Maybe<gfx::Matrix4x4> StackingContextHelper::GetDeferredTransformMatrix()
gfx::Matrix4x4 result =
(*mDeferredTransformItem)->GetTransform().GetMatrix();
if (mDeferredAncestorTransform) {
result = *mDeferredAncestorTransform * result;
result = result * *mDeferredAncestorTransform;
}
return Some(result);
} else {