Skip to content

Commit

Permalink
Bug 1570124 - Enable mouse scrolling in FxR window r=masayuki
Browse files Browse the repository at this point in the history
This change addresses two issues with vrhost sending WM_MOUSEWHEEL events:
- The point from the message had an incorrect coordinate origin. Documentation specifices that it should be screen, rather than window/client, origin. Since vrhost only knows about a position in the window, it translates the point before sending the message.
- Gecko ignores the point passed in to the window message and instead uses the point from GetMessagePos. As warnings indicate, this can be incorrect, as is exposed with vrhost. This change now uses this point from the message when available.

Differential Revision: https://phabricator.services.mozilla.com/D51322
  • Loading branch information
thomasmo committed Nov 6, 2019
1 parent 064c012 commit 4eded1c
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 27 deletions.
13 changes: 12 additions & 1 deletion gfx/vr/vrhost/vrhostapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,21 @@ void SendUIMessageToVRWindow(uint32_t nVRWindowID, uint32_t msg,
HWND hwnd = VRWindowManager::GetManager()->GetHWND(nVRWindowID);
if (hwnd != nullptr) {
switch (msg) {
case WM_MOUSEWHEEL:
// For MOUSEWHEEL, the coordinates are supposed to be at Screen origin
// rather than window client origin.
// Make the conversion to screen coordinates before posting the message
// to the Fx window.
POINT pt;
POINTSTOPOINT(pt, MAKEPOINTS(lparam));
if (!::ClientToScreen(hwnd, &pt)) {
break;
}
// otherwise, fallthrough
lparam = POINTTOPOINTS(pt);
case WM_MOUSEMOVE:
case WM_LBUTTONDOWN:
case WM_LBUTTONUP:
case WM_MOUSEWHEEL:
case WM_CHAR:
case WM_KEYDOWN:
case WM_KEYUP:
Expand Down
16 changes: 15 additions & 1 deletion gfx/vr/vrhost/vrhosttest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,10 +331,24 @@ void TestCreateVRWindow() {
printf(
"5. Simulating clicking outside the URL bar, which should "
"send a keyboard blur event\n");
pt.x = 80;
pt.x = 20;
pt.y = 20;
fnSendMsg(windowId, WM_LBUTTONDOWN, 0, POINTTOPOINTS(pt));
fnSendMsg(windowId, WM_LBUTTONUP, 0, POINTTOPOINTS(pt));

::Sleep(1000);

printf("6. Simulating scrolling the web content down and then up\n");

pt.x = 100;
pt.y = 100;
for (int i = -20; i < 10; ++i) {
fnSendMsg(windowId, WM_MOUSEWHEEL,
MAKELONG(0, (i < 0) ? -WHEEL_DELTA : WHEEL_DELTA),
POINTTOPOINTS(pt));
::Sleep(100);
}

::Sleep(5000);

// Close the Firefox VR Window
Expand Down
7 changes: 7 additions & 0 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6445,6 +6445,13 @@
value: 1500
mirror: always

# Mouse wheel scroll position is determined by GetMessagePos rather than
# LPARAM msg value
- name: mousewheel.ignore_cursor_position_in_lparam
type: RelaxedAtomicBool
value: false
mirror: always

#---------------------------------------------------------------------------
# Prefs starting with "network."
#---------------------------------------------------------------------------
Expand Down
62 changes: 39 additions & 23 deletions widget/windows/WinMouseScrollHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "mozilla/MouseEvents.h"
#include "mozilla/Preferences.h"
#include "mozilla/dom/WheelEventBinding.h"
#include "mozilla/StaticPrefs_mousewheel.h"

#include <psapi.h>

Expand Down Expand Up @@ -303,21 +304,28 @@ nsresult MouseScrollHandler::SynthesizeNativeMouseScrollEvent(

/* static */
void MouseScrollHandler::InitEvent(nsWindowBase* aWidget,
WidgetGUIEvent& aEvent,
LayoutDeviceIntPoint* aPoint) {
WidgetGUIEvent& aEvent, LPARAM* aPoint) {
NS_ENSURE_TRUE_VOID(aWidget);
LayoutDeviceIntPoint point;
if (aPoint) {
point = *aPoint;

// If a point is provided, use it; otherwise, get current message point or
// synthetic point
POINTS pointOnScreen;
if (aPoint != nullptr) {
pointOnScreen = MAKEPOINTS(*aPoint);
} else {
POINTS pts = GetCurrentMessagePos();
POINT pt;
pt.x = pts.x;
pt.y = pts.y;
::ScreenToClient(aWidget->GetWindowHandle(), &pt);
point.x = pt.x;
point.y = pt.y;
pointOnScreen = GetCurrentMessagePos();
}

// InitEvent expects the point to be in window coordinates, so translate the
// point from screen coordinates.
POINT pointOnWindow;
POINTSTOPOINT(pointOnWindow, pointOnScreen);
::ScreenToClient(aWidget->GetWindowHandle(), &pointOnWindow);

LayoutDeviceIntPoint point;
point.x = pointOnWindow.x;
point.y = pointOnWindow.y;

aWidget->InitEvent(aEvent, &point);
}

Expand Down Expand Up @@ -622,7 +630,8 @@ void MouseScrollHandler::HandleMouseWheelMessage(nsWindowBase* aWidget,
RefPtr<nsWindowBase> kungFuDethGrip(aWidget);

WidgetWheelEvent wheelEvent(true, eWheel, aWidget);
if (mLastEventInfo.InitWheelEvent(aWidget, wheelEvent, modKeyState)) {
if (mLastEventInfo.InitWheelEvent(aWidget, wheelEvent, modKeyState,
aLParam)) {
MOZ_LOG(gMouseScrollLog, LogLevel::Info,
("MouseScroll::HandleMouseWheelMessage: dispatching "
"eWheel event"));
Expand Down Expand Up @@ -680,10 +689,12 @@ void MouseScrollHandler::HandleScrollMessageAsMouseWheelMessage(
return;
}
modKeyState.InitInputEvent(wheelEvent);
// XXX Current mouse position may not be same as when the original message
// is received. We need to know the actual mouse cursor position when
// the original message was received.
InitEvent(aWidget, wheelEvent);

// Current mouse position may not be same as when the original message
// is received. However, this data is not available with the original
// message, which is why nullptr is passed in. We need to know the actual
// mouse cursor position when the original message was received.
InitEvent(aWidget, wheelEvent, nullptr);

MOZ_LOG(
gMouseScrollLog, LogLevel::Info,
Expand Down Expand Up @@ -789,13 +800,14 @@ int32_t MouseScrollHandler::LastEventInfo::RoundDelta(double aDelta) {

bool MouseScrollHandler::LastEventInfo::InitWheelEvent(
nsWindowBase* aWidget, WidgetWheelEvent& aWheelEvent,
const ModifierKeyState& aModKeyState) {
const ModifierKeyState& aModKeyState, LPARAM aLParam) {
MOZ_ASSERT(aWheelEvent.mMessage == eWheel);

// XXX Why don't we use lParam value? We should use lParam value because
// our internal message is always posted by original message handler.
// So, GetMessagePos() may return different cursor position.
InitEvent(aWidget, aWheelEvent);
if (StaticPrefs::mousewheel_ignore_cursor_position_in_lparam()) {
InitEvent(aWidget, aWheelEvent, nullptr);
} else {
InitEvent(aWidget, aWheelEvent, &aLParam);
}

aModKeyState.InitInputEvent(aWheelEvent);

Expand Down Expand Up @@ -1324,7 +1336,11 @@ bool MouseScrollHandler::Device::Elantech::HandleKeyMessage(
WidgetCommandEvent appCommandEvent(
true, (aWParam == VK_NEXT) ? nsGkAtoms::Forward : nsGkAtoms::Back,
aWidget);
InitEvent(aWidget, appCommandEvent);

// In this scenario, the coordinate of the event isn't supplied, so pass
// nullptr as an argument to indicate using the coordinate from the last
// available window message.
InitEvent(aWidget, appCommandEvent, nullptr);
aWidget->DispatchWindowEvent(&appCommandEvent);
} else {
MOZ_LOG(gMouseScrollLog, LogLevel::Info,
Expand Down
4 changes: 2 additions & 2 deletions widget/windows/WinMouseScrollHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class MouseScrollHandler {
* GetCurrentMessagePos() will be used.
*/
static void InitEvent(nsWindowBase* aWidget, WidgetGUIEvent& aEvent,
LayoutDeviceIntPoint* aPoint = nullptr);
LPARAM* aPoint);

/**
* GetModifierKeyState() returns current modifier key state.
Expand Down Expand Up @@ -236,7 +236,7 @@ class MouseScrollHandler {
* Otherwise, FALSE.
*/
bool InitWheelEvent(nsWindowBase* aWidget, WidgetWheelEvent& aWheelEvent,
const ModifierKeyState& aModKeyState);
const ModifierKeyState& aModKeyState, LPARAM aLParam);

private:
static int32_t RoundDelta(double aDelta);
Expand Down

0 comments on commit 4eded1c

Please sign in to comment.