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

Issue 653309 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Remoting Android] Cursor position not constrained to the viewport

Project Member Reported by yuweih@chromium.org, Oct 5 2016

Issue description

Version: ToT
OS: Android 7

What steps will reproduce the problem?
(1) Connect to a host
(2) Do swipe-down gesture to open the action bar
(3) move the cursor (in trackpad mode) toward the action bar

What is the expected output?

The cursor will be constrained by the boundary of the viewport.

What do you see instead?

The cursor goes below the action bar.

Assigned to joedow@ since it may be related to his recent changes.
 
Labels: -Pri-1 M-56 Pri-2
This is expected for the current release as it is an artifact of the current Trackpad mode impl.  I do want to improve that experience so I will use this bug to track it.

An important note is that this does not affect the remote cursor, it is still constrained to the actual desktop.  This is only a visual representation of the desired cursor position.
Since the definition of image bounds is a set of allowed cursor positions in the image space, I don't quite understand why the image bounds is shift up and get negative y-start value (e.g. (0.0, -140.70222, 2560.0, 1528.5378)). Shouldn't it always be (0, 0, image_width, image_height)?
I think we may be able to mathematically simplify the logic. I'll let you know when you are back :)

Comment 4 by joedow@chromium.org, Oct 19 2016

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 25 2016

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

commit 95ccb26c84a537f696b374097f7050d472fba289
Author: joedow <joedow@chromium.org>
Date: Tue Oct 25 20:53:58 2016

Updating the calculations used for image overpanning

This CL simplifies the calculations used for panning the remote desktop
image out from under the System UI.  It also constrains the cursor to
the image coordinates vs. the previous method which did not.

The old approach I used was to allow overpanning the remote desktop
image by adjusting the boundaries of the image.  This worked reasonably
well but had one side effect for cursor mode as it meant the cursor
arrow could be rendered outside of the actual image dimensions.
Functionally this was fine as it was still constrained on the remote
system but visually it was not optimal.

The new approach is to use accurate boundaries for the cursor and
viewport but instead accumulate an offset value (mViewportOffset) when
the user tries to 'overpan' the image past its boundaries.  The effect
is that the cursor is still constrained to the image but we can move the
image further in either dimension to ensure they can interact with it.

Once the System UI disappears, we run a per-frame animation to reduce
the offset and reposition/render the remote image.  The effect is that
the image slides back into view quickly which is much cleaner than the
snapping effect we would have if we allowed Android to reposition the
view.

BUG= 653309 

Review-Url: https://codereview.chromium.org/2441723004
Cr-Commit-Position: refs/heads/master@{#427462}

[modify] https://crrev.com/95ccb26c84a537f696b374097f7050d472fba289/remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java

Comment 6 by joedow@chromium.org, Oct 25 2016

Labels: Merge-Request-55

Comment 7 by dimu@chromium.org, Oct 25 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 25 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab6933c26cf444f61413983fb56b8700166af6a8

commit ab6933c26cf444f61413983fb56b8700166af6a8
Author: Joe Downing <joedow@google.com>
Date: Tue Oct 25 22:34:44 2016

Updating the calculations used for image overpanning

This CL simplifies the calculations used for panning the remote desktop
image out from under the System UI.  It also constrains the cursor to
the image coordinates vs. the previous method which did not.

The old approach I used was to allow overpanning the remote desktop
image by adjusting the boundaries of the image.  This worked reasonably
well but had one side effect for cursor mode as it meant the cursor
arrow could be rendered outside of the actual image dimensions.
Functionally this was fine as it was still constrained on the remote
system but visually it was not optimal.

The new approach is to use accurate boundaries for the cursor and
viewport but instead accumulate an offset value (mViewportOffset) when
the user tries to 'overpan' the image past its boundaries.  The effect
is that the cursor is still constrained to the image but we can move the
image further in either dimension to ensure they can interact with it.

Once the System UI disappears, we run a per-frame animation to reduce
the offset and reposition/render the remote image.  The effect is that
the image slides back into view quickly which is much cleaner than the
snapping effect we would have if we allowed Android to reposition the
view.

BUG= 653309 

Review-Url: https://codereview.chromium.org/2441723004
Cr-Commit-Position: refs/heads/master@{#427462}
(cherry picked from commit 95ccb26c84a537f696b374097f7050d472fba289)

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

Cr-Commit-Position: refs/branch-heads/2883@{#303}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ab6933c26cf444f61413983fb56b8700166af6a8/remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java

Comment 9 by joedow@chromium.org, Oct 25 2016

Owner: ajnolley@chromium.org
Status: Fixed (was: Started)
Hey AJ, this change updates the panning behavior when the soft input method has been invoked.  We now correctly constrain the cursor to the image rect while also allowing the user to pan the image out from under the soft keyboard.

This change has been pushed into M55 as well as M56.  Can you take a look and see if you find any problems?

Thanks!
Status: Verified (was: Fixed)
I spent some time with this and found no issues in version 55.0.2883.28
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab6933c26cf444f61413983fb56b8700166af6a8

commit ab6933c26cf444f61413983fb56b8700166af6a8
Author: Joe Downing <joedow@google.com>
Date: Tue Oct 25 22:34:44 2016

Updating the calculations used for image overpanning

This CL simplifies the calculations used for panning the remote desktop
image out from under the System UI.  It also constrains the cursor to
the image coordinates vs. the previous method which did not.

The old approach I used was to allow overpanning the remote desktop
image by adjusting the boundaries of the image.  This worked reasonably
well but had one side effect for cursor mode as it meant the cursor
arrow could be rendered outside of the actual image dimensions.
Functionally this was fine as it was still constrained on the remote
system but visually it was not optimal.

The new approach is to use accurate boundaries for the cursor and
viewport but instead accumulate an offset value (mViewportOffset) when
the user tries to 'overpan' the image past its boundaries.  The effect
is that the cursor is still constrained to the image but we can move the
image further in either dimension to ensure they can interact with it.

Once the System UI disappears, we run a per-frame animation to reduce
the offset and reposition/render the remote image.  The effect is that
the image slides back into view quickly which is much cleaner than the
snapping effect we would have if we allowed Android to reposition the
view.

BUG= 653309 

Review-Url: https://codereview.chromium.org/2441723004
Cr-Commit-Position: refs/heads/master@{#427462}
(cherry picked from commit 95ccb26c84a537f696b374097f7050d472fba289)

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

Cr-Commit-Position: refs/branch-heads/2883@{#303}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ab6933c26cf444f61413983fb56b8700166af6a8/remoting/android/java/src/org/chromium/chromoting/DesktopCanvas.java

Comment 12 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment