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

Map view should use pixel density of current screen #7819

Open
friedbunny opened this issue Jan 23, 2017 · 10 comments
Open

Map view should use pixel density of current screen #7819

friedbunny opened this issue Jan 23, 2017 · 10 comments
Labels
bug gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS navigation For the Mapbox Navigation SDK for Android or iOS or navigation use cases in general

Comments

@friedbunny
Copy link
Contributor

Platform: macOS 10.12.3 Beta (16D17a)
Mapbox SDK version: macOS 0.3.0
Hardware: MacBook Pro (Retina, 13-inch, Early 2015), Intel Iris Graphics 6100 1536 MB

When running macapp on a non-retina screen, the map graphics are aliased and chunky. This is on 10.12.3 beta — when I update to today’s official release version I will retest.

screen shot 2017-01-23 at 1 54 40 pm

/cc @1ec5 @kkaefer

@friedbunny friedbunny added bug macOS Mapbox Maps SDK for macOS labels Jan 23, 2017
@1ec5 1ec5 added this to the macos-v0.3.1 milestone Jan 23, 2017
@1ec5
Copy link
Contributor

1ec5 commented Jan 23, 2017

I wonder if this is happening because +[NSScreen mainScreen] is returning a screen other than the one this window is on. At the time you opened this window, did your MacBook Pro’s built-in Retina screen have keyboard focus?

@friedbunny
Copy link
Contributor Author

friedbunny commented Jan 23, 2017

At the time you opened this window, did your MacBook Pro’s built-in Retina screen have keyboard focus?

Yes, it turns out I was doing that — opened macapp from finder on the retina screen, but the app opened on the second, non-retina display. Focusing the non-retina screen and opening macapp on that screen works as expected (it’s anti-aliased).

@1ec5
Copy link
Contributor

1ec5 commented Jan 23, 2017

Incidentally, are you able to reproduce #6654?

@friedbunny
Copy link
Contributor Author

friedbunny commented Jan 23, 2017

Incidentally, are you able to reproduce #6654?

Moving to a screen with a different pixel density doesn’t crash (on a release build), but the GL view does stop working and goes gray/whatever-the-style’s-background-color-is. MGLMapView UI elements remain visible and map interactions appear to still work, but nothing is drawn.

@friedbunny
Copy link
Contributor Author

It looks like #3451 covered both the original aliasing issue and the disappearing GL view. That was closed in favor of #6654.

@1ec5 1ec5 changed the title Map graphics are aliased on non-retina screen Map should adjust pixel density when transitioning to non-Retina screen Jan 25, 2017
@1ec5 1ec5 changed the title Map should adjust pixel density when transitioning to non-Retina screen Map view should use pixel density of current screen Jan 25, 2017
@1ec5
Copy link
Contributor

1ec5 commented Jan 25, 2017

The root cause is that we create mbgl::Map with the backing scale factor of the main screen (the screen that has keyboard focus) at the time of the view’s creation. But if you open the window on one screen by clicking on some icon on another screen, the main screen and the window’s screen are distinct and may have different scale factors.

We’re in a catch-22. Ideally we’d use a solution similar to #7846 when creating mbgl::Map. Unfortunately, we create mbgl::Map in -commonInit, before the view is added to a window, yet #1799 removed the ability for mbgl::Map to transition to a different pixel density once the view is added to a window. We’d need to move the map’s creation and anything that depends on mbgl::Map’s presence from -commonInit to -viewDidMoveToWindow. But it’s most common to call -setStyleURL: between initializing the map view and adding it to a subview, which would trigger a crash. Therefore, the only solution would be to undo #1799, and I don’t know if that would be feasible under the current architecture.

@1ec5 1ec5 removed this from the macos-v0.3.1 milestone Jan 25, 2017
@1ec5
Copy link
Contributor

1ec5 commented Sep 24, 2018

As of #12933, mbgl requests both 1× and 2× resources unconditionally, though the decision of which to display is still made at initialization and can’t be changed dynamically.

@kkaefer
Copy link
Contributor

kkaefer commented Sep 24, 2018

@1ec5 no, that's quite what #12933 does. We only request both when creating an offline pack, not in regular operation.

@1ec5
Copy link
Contributor

1ec5 commented Jan 14, 2019

Per #13701, it’s also possible for the iOS map SDK to run in a mixed pixel density environment, specifically when connected to an external display via either AirPlay or CarPlay.

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS navigation For the Mapbox Navigation SDK for Android or iOS or navigation use cases in general labels Jan 14, 2019
@stale stale bot added the archived Archived because of inactivity label Jul 13, 2019
@stale
Copy link

stale bot commented Jul 13, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Jul 13, 2019
@julianrex julianrex reopened this Jul 15, 2019
@stale stale bot removed the archived Archived because of inactivity label Jul 15, 2019
@jmkiley jmkiley added the gl-ios label Nov 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS navigation For the Mapbox Navigation SDK for Android or iOS or navigation use cases in general
Projects
None yet
Development

No branches or pull requests

5 participants