1167379 - Timeline selections are offset when DevTools are zoomed in

Status: VERIFIED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Dan Callahan [:callahad]]

Attachment: Zoomed in, the scrubber appears to the right of my cursor

Steps to Reproduce:

1. Open the Performance panel, create a profile.
2. Press Cmd-+ to zoom the DevTools
3. Mouse over the timeline. Try clicking-and-dragging to select a range of time.

What Happens:

1. The selection scrubber appears tens to hundreds of pixels to the right of my actual cursor.

What I Expected:

1. I expected to be able to target things directly below my cursor while zoomed in, rather than having my selection offset to the right of my cursor.

My cursor is at the same location (~250ms) in both of the attached screenshots. Notice how far over the scrubber is from my cursor when zoomed in.

Comment 1

[Dan Callahan [:callahad]]

Attachment: Unzoomed, the scrubber appears directly under my cursor

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

ni?ing victor, guessing this is due to screen DPI, or was this fixed in another patch in last week's flurry?

Comment 3

[Brian Grinstead [:bgrins]]

I can confirm this problem in a current build on tip.  Maybe this is also what you were seeing in Bug 1165228?

Comment 4

[Brian Grinstead [:bgrins]]

I think maybe getBoxQuads is not taking the zoomed value into account.  We may need to use LayoutHelpers.getAdjustedQuads or something else.  I'll see if I can get something working

Comment 5

[Victor Porof [:vporof][:vp]]

Canvases don't change size when zooming in. They just stretch. We need to take the stretch amount into consideration when calculating mouse coords.

Comment 6

[Victor Porof [:vporof][:vp]]

midair!

Comment 7

[Brian Grinstead [:bgrins]]

OK, I think we just need to divide the coords by LayoutHelpers.getCurrentZoom(this._canvas)

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: zoom-coords.patch

Fix with test.  Try push for testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10b7112ca029

Comment 9

[Victor Porof [:vporof][:vp]]

Comment on attachment 8612557 [details] [diff] [review]
zoom-coords.patch

Review of attachment 8612557 [details] [diff] [review]:
-----------------------------------------------------------------

Very nice. Sorry for making you rebase this on top of bug 1169135, and make the LayoutHelpers a lazy one.

Comment 10

[Brian Grinstead [:bgrins]]

Some rounding errors on the Windows 7 try box: TEST-UNEXPECTED-FAIL | browser/devtools/shared/test/browser_graphs-07e.js | The current selection end value is correct (3). - Got 448.5, expected 500
Return code: 1 .  Looking into that

Comment 12

[Brian Grinstead [:bgrins]]

There is still some weirdness happening in Windows 7 on try (but not in a local build).  I have some more logging on this push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=91c5a54eb235 and this one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1eb1215c6fb, but will not be able to look further at it for a few days.  Feel free to steal this if it's high priority.

Comment 13

[Brian Grinstead [:bgrins]]

Attachment: zoom-coords.patch

The latest work - which calls the mouse functions directly with screenX/screenY which fixed things in a local win7 build but for some reason not on try.

Comment 14

[Brian Grinstead [:bgrins]]

Attachment: zoom-coords.patch

Figured out the win7 failure - the resolution on the test machines weren't large enough so the upper bound of the selection was being capped by the maxX available

Comment 15

[Brian Grinstead [:bgrins]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d68bbc0b25ad

Comment 16

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/44b73d061b0e

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/44b73d061b0e

Comment 18

[Brian Grinstead [:bgrins]]

Comment on attachment 8616129 [details] [diff] [review]
zoom-coords.patch

Approval Request Comment
[Feature/regressing bug #]: Devtools Perf Tool
[User impact if declined]: The graph selections will not match up with the mouse coordinates when the toolbox is zoomed in
[Describe test coverage new/current, TreeHerder]: There's a new test for this change
[Risks and why]: Risk focused inside of DevTools toolbox, specifically within the the Performance panel.  This is a small change with a test so pretty low
[String/UUID change made/needed]: None

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8616129 [details] [diff] [review]
zoom-coords.patch

New feature, we want it to be polished for release day. Taking it.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/eaa49c6a1e8f

Comment 21

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 41.0a1 (2015-05-21) using Windows 7 64-bit.
Verified fixed on Firefox 42.0a1 (2015-07-13), Firefox 41.0a2 (2015-07-13) and Firefox 40 Beta 4 (20150713153304) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5. 

I have noticed that the Performance Panel line appears a bit to the right compared to the cursor under Windows 7 64-bit. Filed a follow-up bug: Bug 1183589

I am marking this bug as Verified since the other issue is tracked separately.