Skip to content

Commit b14d32e

Browse files
CendioHalimCendioHalim
committed
Move viewport dragging handling up one layer
This commit moves the viewport dragging handling up one layer, to the mouse handling and gesture handling layer. Moving the handling here means we can separate viewport dragging between mouse events and gesture events. This has the added benefit of removing the "down" argument from _handleMouseButton().
1 parent 0cd6427 commit b14d32e

File tree

2 files changed

+165
-77
lines changed

2 files changed

+165
-77
lines changed

‎core/rfb.js

Lines changed: 124 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,46 +1091,61 @@ export default class RFB extends EventTargetMixin {
10911091

10921092
let bmask = RFB._convertButtonMask(ev.buttons);
10931093

1094+
let down = ev.type == 'mousedown';
10941095
switch (ev.type) {
10951096
case 'mousedown':
1096-
setCapture(this._canvas);
1097-
this._handleMouseButton(pos.x, pos.y, true, bmask);
1098-
break;
10991097
case 'mouseup':
1100-
this._handleMouseButton(pos.x, pos.y, false, bmask);
1098+
if (this.dragViewport) {
1099+
if (down && !this._viewportDragging) {
1100+
this._viewportDragging = true;
1101+
this._viewportDragPos = {'x': pos.x, 'y': pos.y};
1102+
this._viewportHasMoved = false;
1103+
1104+
// Skip sending mouse events, instead save the current
1105+
// mouse mask so we can send it later.
1106+
this._mouseButtonMask = bmask;
1107+
break;
1108+
} else {
1109+
this._viewportDragging = false;
1110+
1111+
// If we actually performed a drag then we are done
1112+
// here and should not send any mouse events
1113+
if (this._viewportHasMoved) {
1114+
break;
1115+
}
1116+
1117+
// Otherwise we treat this as a mouse click event.
1118+
// Send the previously saved button mask, followed
1119+
// by the current button mask at the end of this
1120+
// function.
1121+
this._sendMouse(pos.x, pos.y, this._mouseButtonMask);
1122+
}
1123+
}
1124+
setCapture(this._canvas);
1125+
this._handleMouseButton(pos.x, pos.y, bmask);
11011126
break;
11021127
case 'mousemove':
1103-
this._handleMouseMove(pos.x, pos.y);
1104-
break;
1105-
}
1106-
}
1128+
if (this._viewportDragging) {
1129+
const deltaX = this._viewportDragPos.x - pos.x;
1130+
const deltaY = this._viewportDragPos.y - pos.y;
11071131

1108-
_handleMouseButton(x, y, down, bmask) {
1109-
if (this.dragViewport) {
1110-
if (down && !this._viewportDragging) {
1111-
this._viewportDragging = true;
1112-
this._viewportDragPos = {'x': x, 'y': y};
1113-
this._viewportHasMoved = false;
1132+
if (this._viewportHasMoved || (Math.abs(deltaX) > dragThreshold ||
1133+
Math.abs(deltaY) > dragThreshold)) {
1134+
this._viewportHasMoved = true;
11141135

1115-
// Skip sending mouse events
1116-
this._mouseButtonMask = bmask;
1117-
return;
1118-
} else {
1119-
this._viewportDragging = false;
1136+
this._viewportDragPos = {'x': pos.x, 'y': pos.y};
1137+
this._display.viewportChangePos(deltaX, deltaY);
1138+
}
11201139

1121-
// If we actually performed a drag then we are done
1122-
// here and should not send any mouse events
1123-
if (this._viewportHasMoved) {
1124-
return;
1140+
// Skip sending mouse events
1141+
break;
11251142
}
1126-
1127-
// Otherwise we treat this as a mouse click event.
1128-
// Send the button down event here, as the button up
1129-
// event is sent at the end of this function.
1130-
this._sendMouse(x, y, this._mouseButtonMask);
1131-
}
1143+
this._handleMouseMove(pos.x, pos.y);
1144+
break;
11321145
}
1146+
}
11331147

1148+
_handleMouseButton(x, y, bmask) {
11341149
// Flush waiting move event first
11351150
if (this._mouseMoveTimer !== null) {
11361151
clearTimeout(this._mouseMoveTimer);
@@ -1143,22 +1158,6 @@ export default class RFB extends EventTargetMixin {
11431158
}
11441159

11451160
_handleMouseMove(x, y) {
1146-
if (this._viewportDragging) {
1147-
const deltaX = this._viewportDragPos.x - x;
1148-
const deltaY = this._viewportDragPos.y - y;
1149-
1150-
if (this._viewportHasMoved || (Math.abs(deltaX) > dragThreshold ||
1151-
Math.abs(deltaY) > dragThreshold)) {
1152-
this._viewportHasMoved = true;
1153-
1154-
this._viewportDragPos = {'x': x, 'y': y};
1155-
this._display.viewportChangePos(deltaX, deltaY);
1156-
}
1157-
1158-
// Skip sending mouse events
1159-
return;
1160-
}
1161-
11621161
this._mousePos = { 'x': x, 'y': y };
11631162

11641163
// Limit many mouse move events to one every MOUSE_MOVE_DELAY ms
@@ -1227,22 +1226,22 @@ export default class RFB extends EventTargetMixin {
12271226
// for one of the axes is large enough.
12281227
if (Math.abs(this._accumulatedWheelDeltaX) >= WHEEL_STEP) {
12291228
if (this._accumulatedWheelDeltaX < 0) {
1230-
this._handleMouseButton(pos.x, pos.y, true, bmask | 1 << 5);
1231-
this._handleMouseButton(pos.x, pos.y, false, bmask);
1229+
this._handleMouseButton(pos.x, pos.y, bmask | 1 << 5);
1230+
this._handleMouseButton(pos.x, pos.y, bmask);
12321231
} else if (this._accumulatedWheelDeltaX > 0) {
1233-
this._handleMouseButton(pos.x, pos.y, true, bmask | 1 << 6);
1234-
this._handleMouseButton(pos.x, pos.y, false, bmask);
1232+
this._handleMouseButton(pos.x, pos.y, bmask | 1 << 6);
1233+
this._handleMouseButton(pos.x, pos.y, bmask);
12351234
}
12361235

12371236
this._accumulatedWheelDeltaX = 0;
12381237
}
12391238
if (Math.abs(this._accumulatedWheelDeltaY) >= WHEEL_STEP) {
12401239
if (this._accumulatedWheelDeltaY < 0) {
1241-
this._handleMouseButton(pos.x, pos.y, true, bmask | 1 << 3);
1242-
this._handleMouseButton(pos.x, pos.y, false, bmask);
1240+
this._handleMouseButton(pos.x, pos.y, bmask | 1 << 3);
1241+
this._handleMouseButton(pos.x, pos.y, bmask);
12431242
} else if (this._accumulatedWheelDeltaY > 0) {
1244-
this._handleMouseButton(pos.x, pos.y, true, bmask | 1 << 4);
1245-
this._handleMouseButton(pos.x, pos.y, false, bmask);
1243+
this._handleMouseButton(pos.x, pos.y, bmask | 1 << 4);
1244+
this._handleMouseButton(pos.x, pos.y, bmask);
12461245
}
12471246

12481247
this._accumulatedWheelDeltaY = 0;
@@ -1281,8 +1280,8 @@ export default class RFB extends EventTargetMixin {
12811280
this._gestureLastTapTime = Date.now();
12821281

12831282
this._fakeMouseMove(this._gestureFirstDoubleTapEv, pos.x, pos.y);
1284-
this._handleMouseButton(pos.x, pos.y, true, bmask);
1285-
this._handleMouseButton(pos.x, pos.y, false, 0x0);
1283+
this._handleMouseButton(pos.x, pos.y, bmask);
1284+
this._handleMouseButton(pos.x, pos.y, 0x0);
12861285
}
12871286

12881287
_handleGesture(ev) {
@@ -1303,12 +1302,26 @@ export default class RFB extends EventTargetMixin {
13031302
this._handleTapEvent(ev, 0x2);
13041303
break;
13051304
case 'drag':
1306-
this._fakeMouseMove(ev, pos.x, pos.y);
1307-
this._handleMouseButton(pos.x, pos.y, true, 0x1);
1305+
if (this.dragViewport) {
1306+
this._viewportHasMoved = false;
1307+
this._viewportDragging = true;
1308+
this._viewportDragPos = {'x': pos.x, 'y': pos.y};
1309+
} else {
1310+
this._fakeMouseMove(ev, pos.x, pos.y);
1311+
this._handleMouseButton(pos.x, pos.y, 0x1);
1312+
}
13081313
break;
13091314
case 'longpress':
1310-
this._fakeMouseMove(ev, pos.x, pos.y);
1311-
this._handleMouseButton(pos.x, pos.y, true, 0x4);
1315+
if (this.dragViewport) {
1316+
// If dragViewport is true, we need to wait to see
1317+
// if we have dragged outside the threshold before
1318+
// sending any events to the server.
1319+
this._viewportHasMoved = false;
1320+
this._viewportDragPos = {'x': pos.x, 'y': pos.y};
1321+
} else {
1322+
this._fakeMouseMove(ev, pos.x, pos.y);
1323+
this._handleMouseButton(pos.x, pos.y, 0x4);
1324+
}
13121325
break;
13131326
case 'twodrag':
13141327
this._gestureLastMagnitudeX = ev.detail.magnitudeX;
@@ -1331,6 +1344,22 @@ export default class RFB extends EventTargetMixin {
13311344
break;
13321345
case 'drag':
13331346
case 'longpress':
1347+
if (this.dragViewport) {
1348+
this._viewportDragging = true;
1349+
const deltaX = this._viewportDragPos.x - pos.x;
1350+
const deltaY = this._viewportDragPos.y - pos.y;
1351+
1352+
if (this._viewportHasMoved || (Math.abs(deltaX) > dragThreshold ||
1353+
Math.abs(deltaY) > dragThreshold)) {
1354+
this._viewportHasMoved = true;
1355+
1356+
this._viewportDragPos = {'x': pos.x, 'y': pos.y};
1357+
this._display.viewportChangePos(deltaX, deltaY);
1358+
}
1359+
1360+
// Skip sending mouse events
1361+
break;
1362+
}
13341363
this._fakeMouseMove(ev, pos.x, pos.y);
13351364
break;
13361365
case 'twodrag':
@@ -1339,23 +1368,23 @@ export default class RFB extends EventTargetMixin {
13391368
// every update.
13401369
this._fakeMouseMove(ev, pos.x, pos.y);
13411370
while ((ev.detail.magnitudeY - this._gestureLastMagnitudeY) > GESTURE_SCRLSENS) {
1342-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x8);
1343-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x8);
1371+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x8);
1372+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x8);
13441373
this._gestureLastMagnitudeY += GESTURE_SCRLSENS;
13451374
}
13461375
while ((ev.detail.magnitudeY - this._gestureLastMagnitudeY) < -GESTURE_SCRLSENS) {
1347-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x10);
1348-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x10);
1376+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x10);
1377+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x10);
13491378
this._gestureLastMagnitudeY -= GESTURE_SCRLSENS;
13501379
}
13511380
while ((ev.detail.magnitudeX - this._gestureLastMagnitudeX) > GESTURE_SCRLSENS) {
1352-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x20);
1353-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x20);
1381+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x20);
1382+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x20);
13541383
this._gestureLastMagnitudeX += GESTURE_SCRLSENS;
13551384
}
13561385
while ((ev.detail.magnitudeX - this._gestureLastMagnitudeX) < -GESTURE_SCRLSENS) {
1357-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x40);
1358-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x40);
1386+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x40);
1387+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x40);
13591388
this._gestureLastMagnitudeX -= GESTURE_SCRLSENS;
13601389
}
13611390
break;
@@ -1368,13 +1397,13 @@ export default class RFB extends EventTargetMixin {
13681397
if (Math.abs(magnitude - this._gestureLastMagnitudeX) > GESTURE_ZOOMSENS) {
13691398
this._handleKeyEvent(KeyTable.XK_Control_L, "ControlLeft", true);
13701399
while ((magnitude - this._gestureLastMagnitudeX) > GESTURE_ZOOMSENS) {
1371-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x8);
1372-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x8);
1400+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x8);
1401+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x8);
13731402
this._gestureLastMagnitudeX += GESTURE_ZOOMSENS;
13741403
}
13751404
while ((magnitude - this._gestureLastMagnitudeX) < -GESTURE_ZOOMSENS) {
1376-
this._handleMouseButton(pos.x, pos.y, true, this._mouseButtonMask | 0x10);
1377-
this._handleMouseButton(pos.x, pos.y, false, this._mouseButtonMask & ~0x10);
1405+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask | 0x10);
1406+
this._handleMouseButton(pos.x, pos.y, this._mouseButtonMask & ~0x10);
13781407
this._gestureLastMagnitudeX -= GESTURE_ZOOMSENS;
13791408
}
13801409
}
@@ -1392,12 +1421,32 @@ export default class RFB extends EventTargetMixin {
13921421
case 'twodrag':
13931422
break;
13941423
case 'drag':
1395-
this._fakeMouseMove(ev, pos.x, pos.y);
1396-
this._handleMouseButton(pos.x, pos.y, false, 0x0);
1424+
if (this.dragViewport) {
1425+
this._viewportDragging = false;
1426+
} else {
1427+
this._fakeMouseMove(ev, pos.x, pos.y);
1428+
this._handleMouseButton(pos.x, pos.y, 0x0);
1429+
}
13971430
break;
13981431
case 'longpress':
1399-
this._fakeMouseMove(ev, pos.x, pos.y);
1400-
this._handleMouseButton(pos.x, pos.y, false, 0x0);
1432+
if (this._viewportHasMoved) {
1433+
// We don't want to send any events if we have moved
1434+
// our viewport
1435+
break;
1436+
}
1437+
1438+
if (this.dragViewport && !this._viewportHasMoved) {
1439+
this._fakeMouseMove(ev, pos.x, pos.y);
1440+
// If dragViewport is true, we need to wait to see
1441+
// if we have dragged outside the threshold before
1442+
// sending any events to the server.
1443+
this._handleMouseButton(pos.x, pos.y, 0x4);
1444+
this._handleMouseButton(pos.x, pos.y, 0x0);
1445+
this._viewportDragging = false;
1446+
} else {
1447+
this._fakeMouseMove(ev, pos.x, pos.y);
1448+
this._handleMouseButton(pos.x, pos.y, 0x0);
1449+
}
14011450
break;
14021451
}
14031452
break;

‎tests/test.rfb.js

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,20 @@ describe('Remote Frame Buffer protocol client', function () {
768768
13, 9, 0x0);;
769769
});
770770

771+
it('should send button messages when longpress with small movement', function () {
772+
// Small movement
773+
gestureStart('longpress', 13, 9, client);
774+
gestureEnd('longpress', 15, 14, client);
775+
776+
expect(RFB.messages.pointerEvent).to.have.been.calledThrice;
777+
expect(RFB.messages.pointerEvent.firstCall).to.have.been.calledWith(client._sock,
778+
15, 14, 0x0);
779+
expect(RFB.messages.pointerEvent.secondCall).to.have.been.calledWith(client._sock,
780+
15, 14, 0x4);
781+
expect(RFB.messages.pointerEvent.thirdCall).to.have.been.calledWith(client._sock,
782+
15, 14, 0x0);
783+
});
784+
771785
it('should not send button messages when in view only', function () {
772786
client._viewOnly = true;
773787

@@ -822,7 +836,7 @@ describe('Remote Frame Buffer protocol client', function () {
822836
gestureStart('drag', 13, 9, client);
823837
gestureMove('drag', 43, 9, client);
824838

825-
expect(RFB.messages.pointerEvent).to.have.been.calledOnce;
839+
expect(RFB.messages.pointerEvent).to.not.have.been.called;
826840
expect(client._display.viewportChangePos).to.have.been.calledOnce;
827841
expect(client._display.viewportChangePos).to.have.been.calledWith(-30, 0);
828842

@@ -845,7 +859,7 @@ describe('Remote Frame Buffer protocol client', function () {
845859
gestureStart('longpress', 13, 9, client);
846860
gestureMove('longpress', 14, 9, client);
847861

848-
expect(RFB.messages.pointerEvent).to.have.been.calledOnce;
862+
expect(RFB.messages.pointerEvent).to.not.have.been.called;
849863
expect(client._display.viewportChangePos).to.not.have.been.called;
850864

851865
client._display.viewportChangePos.resetHistory();
@@ -856,7 +870,32 @@ describe('Remote Frame Buffer protocol client', function () {
856870
expect(RFB.messages.pointerEvent).to.not.have.been.called;
857871
expect(client._display.viewportChangePos).to.have.been.calledOnce;
858872
expect(client._display.viewportChangePos).to.have.been.calledWith(-30, 0);
873+
});
874+
875+
it('should send button messages on small longpress gesture movement', function () {
876+
sinon.spy(client._display, "viewportChangePos");
859877

878+
// A small movement below the threshold should not move.
879+
gestureStart('longpress', 13, 9, client);
880+
gestureMove('longpress', 14, 10, client);
881+
882+
expect(RFB.messages.pointerEvent).to.not.have.been.called;
883+
expect(client._display.viewportChangePos).to.not.have.been.called;
884+
885+
client._display.viewportChangePos.resetHistory();
886+
RFB.messages.pointerEvent.resetHistory();
887+
888+
gestureEnd('longpress', 14, 9, client);
889+
890+
expect(RFB.messages.pointerEvent).to.have.been.calledThrice;
891+
expect(RFB.messages.pointerEvent.firstCall).to.have.been.calledWith(client._sock,
892+
14, 9, 0x0);
893+
expect(RFB.messages.pointerEvent.secondCall).to.have.been.calledWith(client._sock,
894+
14, 9, 0x4);
895+
expect(RFB.messages.pointerEvent.thirdCall).to.have.been.calledWith(client._sock,
896+
14, 9, 0x0);
897+
898+
expect(client._display.viewportChangePos).to.not.have.been.called;
860899
});
861900

862901
it('should not send button messages when dragging ends', function () {

0 commit comments

Comments
 (0)