Closed Bug 1167379 Opened 9 years ago Closed 9 years ago

Timeline selections are offset when DevTools are zoomed in

Categories

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

defect
Not set
normal

Tracking

(firefox40 verified, firefox41 verified, firefox42 verified)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- verified
firefox42 --- verified

People

(Reporter: callahad, Assigned: bgrins)

References

Details

(Keywords: DevAdvocacy)

Attachments

(3 files, 2 obsolete files)

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.
ni?ing victor, guessing this is due to screen DPI, or was this fixed in another patch in last week's flurry?
Flags: needinfo?(vporof)
I can confirm this problem in a current build on tip.  Maybe this is also what you were seeing in Bug 1165228?
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
Canvases don't change size when zooming in. They just stretch. We need to take the stretch amount into consideration when calculating mouse coords.
Flags: needinfo?(vporof)
midair!
OK, I think we just need to divide the coords by LayoutHelpers.getCurrentZoom(this._canvas)
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attached patch zoom-coords.patch (obsolete) — Splinter Review
Fix with test.  Try push for testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=10b7112ca029
Attachment #8612557 - Flags: review?(vporof)
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.
Attachment #8612557 - Flags: review?(vporof) → review+
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
Blocks: 1169135
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.
Attached patch zoom-coords.patch (obsolete) — Splinter Review
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.
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
Attachment #8612557 - Attachment is obsolete: true
Attachment #8613204 - Attachment is obsolete: true
Attachment #8616129 - Flags: review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/44b73d061b0e
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/44b73d061b0e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
Blocks: 1169035
Flags: qe-verify+
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
Attachment #8616129 - Flags: approval-mozilla-aurora?
Comment on attachment 8616129 [details] [diff] [review]
zoom-coords.patch

New feature, we want it to be polished for release day. Taking it.
Attachment #8616129 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.