Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(399)

Issue 1005213003: Fix for the continuing issues with pointer lock operations in HiDPI Chrome. (Closed)

Created:
5 years, 9 months ago by ananta
Modified:
5 years, 9 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, jam, penghuang+watch_chromium.org, sievers+watch_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, kalyank, piman+watch_chromium.org, danakj+watch_chromium.org, James Su
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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. Committed: https://crrev.com/2de0b55dc80ac6a6741c3e3cc68e945f6876cfcf Cr-Commit-Position: refs/heads/master@{#321450}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -0 lines) Patch
M content/browser/renderer_host/render_widget_host_view_aura.cc View 1 2 chunks +23 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
ananta
5 years, 9 months ago (2015-03-14 01:10:05 UTC) #2
ananta
5 years, 9 months ago (2015-03-14 01:10:23 UTC) #4
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/1005213003/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc File content/browser/renderer_host/render_widget_host_view_aura.cc (right): https://codereview.chromium.org/1005213003/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc#newcode1935 content/browser/renderer_host/render_widget_host_view_aura.cc:1935: if (synthetic_move_sent_ && (current_device_scale_factor_ != 1)) { but !=1 ...
5 years, 9 months ago (2015-03-19 03:06:11 UTC) #5
ananta
https://codereview.chromium.org/1005213003/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc File content/browser/renderer_host/render_widget_host_view_aura.cc (right): https://codereview.chromium.org/1005213003/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc#newcode1935 content/browser/renderer_host/render_widget_host_view_aura.cc:1935: if (synthetic_move_sent_ && (current_device_scale_factor_ != 1)) { On 2015/03/19 ...
5 years, 9 months ago (2015-03-19 19:49:17 UTC) #6
cpu_(ooo_6.6-7.5)
lgtm
5 years, 9 months ago (2015-03-19 20:12:01 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1005213003/20001
5 years, 9 months ago (2015-03-19 20:18:13 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 9 months ago (2015-03-19 22:25:38 UTC) #10
commit-bot: I haz the power
5 years, 9 months ago (2015-03-19 22:26:15 UTC) #11
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/2de0b55dc80ac6a6741c3e3cc68e945f6876cfcf
Cr-Commit-Position: refs/heads/master@{#321450}

Powered by Google App Engine
This is Rietveld 408576698