Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

zooming is janky #4712

Merged
merged 2 commits into from
Apr 26, 2016
Merged

zooming is janky #4712

merged 2 commits into from
Apr 26, 2016

Conversation

kkaefer
Copy link
Contributor

@kkaefer kkaefer commented Apr 19, 2016

Zooming (easiest using Shift + drag) in the osxapp is noticeably janky, even in a Release build.

(Doesn't seem to have been caused by #2909; it was janky before that PR too.)

@jfirebaugh jfirebaugh added the performance Speed, stability, CPU usage, memory usage, or power usage label Apr 15, 2016
@1ec5 1ec5 added the macOS Mapbox Maps SDK for macOS label Apr 15, 2016
@kkaefer
Copy link
Contributor

kkaefer commented Apr 15, 2016

@jfirebaugh I noticed that too and debugging lead me to believe that canceling work tasks was the culprit; in particular it sometimes took over 1 second to cancel a work task.

@jfirebaugh
Copy link
Contributor Author

I'm not sure that's it. I rebased #3724 and it didn't seem to help. Logs say:

2016-04-14 18:34:59.027 Mapbox GL[76244:16334421] [WARNING] {Main}[General]: Map thread blocked for 0 ms.
2016-04-14 18:34:59.178 Mapbox GL[76244:16334421] [WARNING] {Main}[General]: Map thread blocked for 0 ms.
2016-04-14 18:34:59.206 Mapbox GL[76244:16334421] [WARNING] {Main}[General]: Map thread blocked for 0 ms.
2016-04-14 18:35:00.994 Mapbox GL[76244:16334421] [WARNING] {Main}[General]: Map thread blocked for 0 ms.
2016-04-14 18:35:01.058 Mapbox GL[76244:16334421] [WARNING] {Main}[General]: Map thread blocked for 0 ms.

@jfirebaugh
Copy link
Contributor Author

Zooming in glfw-app (with #4666 applied) is silky smooth.

@kkaefer
Copy link
Contributor

kkaefer commented Apr 19, 2016

Issue was that the view object was eating many low-delta scroll events, interpreting them as touch events instead.

@kkaefer
Copy link
Contributor

kkaefer commented Apr 19, 2016

@jfirebaugh will merge as soon as you give the go-ahead that this fixes your issues.

@jfirebaugh
Copy link
Contributor Author

@kkaefer It's a definite improvement, but still lacking the smoothness and fluidity of glfw-app. If you compare them side-by-side, the difference is striking: with glfw-app, you feel like you're directly manipulating the map surface. With osxapp, input is noticeably delayed and janky.

@@ -375,7 +375,6 @@ - (void)installAttributionView {

/// Adds gesture recognizers for manipulating the viewport and selecting annotations.
- (void)installGestureRecognizers {
self.acceptsTouchEvents = YES;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks touch events, right? Even if we conditionalize it on MGLScrollWheelZoomsMapViewDefaultKey, gestures like rotation and two-finger tap will be broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested both of these before submitting, and both rotation and two-finger tap still work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried out this one change (without the rest of the changes in this PR), and all the gestures continue to work fine. In fact, performance improves dramatically with this one change. I think acceptsTouchEvents is about raw touch events only, but MGLMapView uses gesture recognizers, which are unaffected by this property.

@1ec5
Copy link
Contributor

1ec5 commented Apr 19, 2016

@jfirebaugh, is this remaining jankiness limited to zooming, or is it consistent with what you see when rotating or scrolling?

@jfirebaugh
Copy link
Contributor Author

With these changes, it's about equally noticeable across all gestures.

@kkaefer
Copy link
Contributor

kkaefer commented Apr 19, 2016

With these changes, it's about equally noticeable across all gestures.

Does that mean that scrolling becomes worse for you with this patch?

they are canceling out trackpad scrolling events
- uses the same algorithm as the GLFW app to determine scrollwheel zoom speed
- disables transitions which makes the trackpad feel faster due to immediate response
@kkaefer kkaefer force-pushed the 4712-janky-scrollwheel-zooming branch from fb0ac3d to 4770a9e Compare April 26, 2016 08:42
@kkaefer kkaefer merged commit 4770a9e into master Apr 26, 2016
@kkaefer kkaefer deleted the 4712-janky-scrollwheel-zooming branch April 26, 2016 09:34
@1ec5 1ec5 modified the milestone: osx-v0.1.0 May 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
macOS Mapbox Maps SDK for macOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants