New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 411634 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Pointer Lock Broken on high DPI displays

Reported by bren...@annable.id.au, Sep 6 2014

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.103 Safari/537.36

Steps to reproduce the problem:
1. Open http://mdn.github.io/pointer-lock-demo/
2. Click inside canvas
3. Move mouse

What is the expected behavior?
The red circle to move smoothly in relation to your mouse movements.

What went wrong?
The red circle gets 'stuck', and will not move. Sometimes jitters around the place.

Did this work before? Yes Before I updated to chrome 37.

Chrome version: 37.0.2062.103  Channel: stable
OS Version: 8.1
Flash Version: Shockwave Flash 14.0 r0

The technical details is that event.movementX and event.movementY are not reporting valid values.

This appears to only occur when I have the 150% text size in Windows, if I set it back to 100% it works correctly, but everything is very small due to my high DPI display (3200x1800).

Works in FireFox, but not chrome.
 
Labels: Needs-Feedback M-37
Unable to repro this issue on Windows7 using: 37.0.2062.103 & 39.0.2149.0 canary. 

Please find the attached screenshot as reference. 

Is this specific to windows 8.1 issue? Could you please re-check on clean profile and conform this issue. 
411634.mp4
429 KB Download
Status: Untriaged
Able to reproduce the issue on Windows and Chrome Version: 37.0.2062.103

However this is fixed on Latest Canary i.e. 39.0.2149.0 canary (64-bit), can you please verify the same ?

You can download and install Canary from http://www.google.co.in/intl/en/chrome/browser/canary.html
I downloaded and tried the latest Chrome 39.0.2149.0 canary (64-bit) and I still experienced this issue.

I just updated from Windows 7 to Windows 8.1 to try and resolve the problem, so it appears to occur on both operating systems for me.

Comment 4 by amin...@google.com, Sep 8 2014

Labels: -M-37 MovedFrom-37 M-38
Moving all non essential bugs to the next Milestone.
Labels: -M-38 MovedFrom-38
This issue has already been moved once and is lower than Priority 1,therefore removing mstone.

Comment 6 by jimno...@gmail.com, Dec 3 2014

Discovered this today while trying to author an FPS game. It basically makes the API unusable and so the project is not achievable until this is fixed.

This is on 200% zoom in windows 8.1, with chrome 39.0.2171.62 beta-m.

I went ahead and verified that this is still occurring with the latest canary build 41.0.2237.0 canary (64-bit)

The movementX and movementY properties seem to report in conflicting pairs, with the first event having negative coordinates, and the second event having the same coordinates, but positive: http://screencast.com/t/uHe0CBDl

The net effect is the movements cancel each other out and no overall movement is acheived. I don't believe there is any information to construct a workaround either.

Comment 7 by jimno...@gmail.com, Dec 3 2014

Also noteworthy is that the mousemove events when the pointer is locked seem to arrive sporadically, as if some samples are being dropped or omitted.

Comment 8 by bbudge@chromium.org, Feb 20 2015

Mergedinto: 459551
Status: Duplicate

Comment 9 by bbudge@chromium.org, Feb 20 2015

Mergedinto:
Status: Available
Reopening, I think HiDpi is a better way to look at this. Merging the newer bug into this one.
 Issue 459551  has been merged into this issue.
Cc: scheib@chromium.org
Talked with Ananta - perhaps we're not properly scaling the mouse in HiDpi situations:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/renderer_host/render_widget_host_view_aura.cc&l=1863
Adding Vince who might know.
Labels: Cr-IO-PointerLock
Plugins using this API become unusable with a mouse after changing font to 150%.

Since it is quite common for users to change the font size on Windows, can you please increase the priority of this bug?
Cc: ananta@chromium.org
ananta, would you mind looking at this?
Ping. Can anyone please take a look?
Owner: ananta@chromium.org
Status: Assigned
Will look at this tomorrow
I'll also add, this issue is even worse when you combine multiple monitors at different DPI settings.

At work I used 3 screens, and if I request a pointer lock on one monitor (different settings to native laptop screen), the mouse gets visibly locked to a quad on ANOTHER monitor which I can move the mouse around in.
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 5 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05f7888960f6e3393b2ad5f5835028404737c584

commit 05f7888960f6e3393b2ad5f5835028404737c584
Author: ananta <ananta@chromium.org>
Date: Thu Mar 05 01:01:46 2015

Ensure that pointer lock works correctly in Windows 7+ with HiDPI scale factors above 1

We were not scaling the mouse coordinate by the scale factor before invoking the ClientToScreen API. With this
change the blink::WebMouseEvent.globalX and globalY values have to be converted back to DIP after convering them to screen.

This should fix the mouse locking issue.

BUG= 411634 
TEST= The main test case in  bug 411634 

Review URL: https://codereview.chromium.org/973123003

Cr-Commit-Position: refs/heads/master@{#319181}

[modify] http://crrev.com/05f7888960f6e3393b2ad5f5835028404737c584/content/browser/renderer_host/input/web_input_event_builders_win.cc

Thank you! :)
I checked with a nightly downloaded from https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?prefix=Win/319225/ and I could still reproduce the issue.

I chose that based on "Cr-Commit-Position: refs/heads/master@{#319181}" from the fix.

Is this the right nightly to pick up?
Tried with today's canary 43.0.2326.2
It is much better than previous, but is not completely fixed.
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 10 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef5361d976c30ad91cd9df5fccf4c046295bac0a

commit ef5361d976c30ad91cd9df5fccf4c046295bac0a
Author: ananta <ananta@chromium.org>
Date: Tue Mar 10 00:23:55 2015

Second attempt at fixing pointer lock issues with Windows HiDPI.

The earlier attempt https://codereview.chromium.org/973123003/ was incorrect. Reverted parts that patch
which changed the WebMouseEventBuilder::Build function to convert from DIP to pixels and back. The initial
conversion from DIP to pixels was not needed as the values are picked up from the Windows message which gives
us these coordinates in pixels.

Changes in this patch are as below:-
1. The WebMouseEventBuilder::Build function has been changed to convert the WebMouseEvent::globalX and globalY values
   to DIP. The other changes in the earlier patch as mentioned above have been reverted.

2. The RenderWidgetHostViewAura::UpdateMouseLockRegion function which executes on Windows was clipping the windows cursor
   to a rectangle in DIPs. We need to convert this rectangle to pixels before calling the ClipCursor API. This was the
   main reason for the lock operation not working correctly.

3. I was seeing a DCHECK in the renderer process in the WebViewImpl::pointerLockMouseEvent function as we were sending
   the mouse leave message. Added code in the CanRendererHandleEvent function in the render_widget_host_view_aura.cc file
   to not send this message if we are in the mouse locked state.

BUG= 411634 
TEST= The main test case in  bug 411634 

Review URL: https://codereview.chromium.org/981393002

Cr-Commit-Position: refs/heads/master@{#319791}

[modify] http://crrev.com/ef5361d976c30ad91cd9df5fccf4c046295bac0a/content/browser/renderer_host/input/web_input_event_builders_win.cc
[modify] http://crrev.com/ef5361d976c30ad91cd9df5fccf4c046295bac0a/content/browser/renderer_host/input/web_input_event_builders_win.h
[add] http://crrev.com/ef5361d976c30ad91cd9df5fccf4c046295bac0a/content/browser/renderer_host/input/web_input_event_unittest.cc
[modify] http://crrev.com/ef5361d976c30ad91cd9df5fccf4c046295bac0a/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] http://crrev.com/ef5361d976c30ad91cd9df5fccf4c046295bac0a/content/content_tests.gypi

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 11 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/449c2485458f40a9627a9f2517c6297bd180aa80

commit 449c2485458f40a9627a9f2517c6297bd180aa80
Author: ananta <ananta@chromium.org>
Date: Wed Mar 11 05:02:15 2015

Fix for the WebInputEventBuilderTest.TestMouseEventScale content_unittests failures on the DrMemory Windows builder.

The test attempts to force a device scale factor via the force-device-scale-factor command line switch. While this works
well in isolation, the test would fail if another test before it forces the device scale factor to something else.

Reason being the device scale factor code uses static variables to determine if the scale factor has been forced and
caches the value. This is done as an optimization as there are lots of calls to the GetDPIScale and related functions.

Fix is to get rid of the static variables used to track if we checked for a forced device scale factor and the forced
scale factor value. These are now global variables which can be reset via a static function
Display::ResetForceDeviceScaleFactor.

The WebInputEventBuilderTest.TestMouseEventScale test calls the above function to reset the forced device scale factor
state.

BUG= 411634 
TBR=cpu

Review URL: https://codereview.chromium.org/992383002

Cr-Commit-Position: refs/heads/master@{#320049}

[modify] http://crrev.com/449c2485458f40a9627a9f2517c6297bd180aa80/content/browser/renderer_host/input/web_input_event_unittest.cc
[modify] http://crrev.com/449c2485458f40a9627a9f2517c6297bd180aa80/ui/gfx/display.cc
[modify] http://crrev.com/449c2485458f40a9627a9f2517c6297bd180aa80/ui/gfx/display.h

Tested with 43.0.2329.0 refs/heads/master@{#320066}
Issue still repros. I am trying to do one full circular motion on http://mdn.github.io/pointer-lock-demo/ but I am unable to. Pointer jumps out in between, mostly at the bottom right corner of the circle.
Cc: pbomm...@chromium.org
It works fine on Windows 8.1 and Win7 with latest Chrome Canary i.e., 43.0.2330.0 (32 and 64bit) with display scaling set to 100% and 150%. 

Status: Fixed
I tested on Windows 7 with latest Chrome canary 64 bit (43.0.2330.0) with 100 and 150% display scales. Works fine.

Marking as fixed.

Comment 27 Deleted

Perhaps you did a circular motion in the anti clockwise direction. With the fixes this works fine. Here's a video recording of where I see the issue, in clockwise direction:
https://www.youtube.com/watch?v=tjGe2YkI6AQ

This is on Win 8, canary chrome version 43.0.2330.3 with 150% display scale.
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 19 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2de0b55dc80ac6a6741c3e3cc68e945f6876cfcf

commit 2de0b55dc80ac6a6741c3e3cc68e945f6876cfcf
Author: ananta <ananta@chromium.org>
Date: Thu Mar 19 22:25:27 2015

Fix for the continuing issues with pointer lock operations in HiDPI Chrome.

While the recent fixes to the pointer lock code have helped, we still have problems when we move the cursor near
the bounds of the content window. The pointer lock code in RWHVA detects that and forces a move to the center of
the window via a SetCursorPos operation. The expectation here is that we would see a mouse move message with the
location specified by SetCursorPos which would ignore.

With fractional scale factors like 1.25/1.5, etc it turns out that the conversions of the mouse messages from pixel
to DPI and vice versa causes us to end up with messages which are off by 1 or 2 px which causes us to miss the move
to center event in the locked mode. The renderer sees the message and bounces the cursor back to the center leading
to erratic behavior.

Proposed fix is to add an allowance of 2 for the mouse event x and y coordinates if we are waiting for the move to
center event.

BUG= 411634 
TEST=As described in the bug. Please try a clockwise and anticlockwise mouse rotation with device scales set to 1.25/
1.5, etc.

Review URL: https://codereview.chromium.org/1005213003

Cr-Commit-Position: refs/heads/master@{#321450}

[modify] http://crrev.com/2de0b55dc80ac6a6741c3e3cc68e945f6876cfcf/content/browser/renderer_host/render_widget_host_view_aura.cc

The issue is fully fixed now. Thanks @ananta, @bbudge for taking this up.

Since this impacts the end user behaviour quite a lot, is it possible to pull these fixes into an earlier release of Chrome? I would be glad to see this working at end user level as early as possible. :-)
Labels: M-42 Merge-Requested
We would need to merge the 4 fixes here to ensure that it goes through without conflicts. Overall it is low risk. Given that it has regressed since M37 I think we should merge it to 42.

Comment 32 by amin...@google.com, Mar 20 2015

Labels: -Merge-Requested Merge-Review Hotlist-Merge-Review
[Automated comment] Reverts referenced in bugdroid comments, needs manual review.
Labels: -Merge-Review Merge-Rejected
We only have a few weeks left on branch, and given the number of CLs we'd need to merge (plus how long things have been broken on trunk and a relatively low issue star count), I would prefer not to merge to M42 at this time.

Comment 34 Deleted

Components: -IO>PointerLock Blink>Input>PointerLock
Deprecate IO>PointerLock

Sign in to add a comment