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

OS X SDK and revamped OS X demo application #3135

Merged
merged 25 commits into from
Dec 14, 2015
Merged

OS X SDK and revamped OS X demo application #3135

merged 25 commits into from
Dec 14, 2015

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Nov 25, 2015

This PR replaces the minimal, glfw-based OS X support in this project with a proper SDK that mirrors (and in many places shares code with) the Mapbox iOS SDK. make xproj now creates an Xcode project with a framework target and a rewritten osxapp application target. The vestigial static library target is immediately repackaged as a bone fide dynamic framework (#828) – not the kind of faux framework that framework.sh creates for iOS. (In the future, we can cut out the intermediate static library step.) make xpackage simply builds the project that make xproj creates.

Inside the SDK is almost all the same functionality as you’d find in the iOS SDK: MGLMapView has support for gestures, animations, annotations, callouts, etc. It lacks support for the user dot, user location tracking, or telemetry. However, the optimized framework weighs in at only 5 MB compared to 290 MB for the iOS SDK. It’s also easier to install: you just plop Mapbox.framework into your application’s Xcode project, set the access token in Info.plist, and add the MGLMapView to MainMenu.xib or Main.storyboard.

osxapp is now a typical Cocoa application that links against Mapbox.framework and allows you to view any GL style in an MGLMapView. osxapp no longer depends on glfw. Go to Help ‣ Mapbox GL Help for an explanation of the various gestures that are supported.

Both SDKs share almost all of the model object classes and most geometry conversion code – basically anything that doesn’t touch AppKit or UIKit. They have separate implementations of view classes, annotation images (because I didn’t want to confuse appledoc with conditional compilation; see #3203), gesture recognition (because the Mac has different standard gestures), and callouts (because NSPopover is such an elegant solution to the problem).

The biggest architectural difference between the two SDKs, other than the use of a framework, is that the OS X version of MGLMapView holds a mapping from numeric “annotation tags” to annotation context objects that contain the annotations themselves (#3159). This optimizes nearly every operation involving annotations. It allowed me to fix longstanding bugs and performance bottlenecks such as #1504 right off the bat and implement some nifty Mac-only features like annotation tooltips, custom cursors, and proper hit testing for annotations. (A doughnut-shaped annotation can only be selected by clicking the dough, not the transparent hole in the middle.)

The goal of this PR is to ensure that when we say we target OS X (as we do in the readme), we do so in a way that others can reuse outside of the Mapbox GL project. Hopefully the way I’ve architected this port will keep it going with minimal maintenance cost. Even if it requires slightly more attention than the glfw-based demo application, I think it’ll be much easier for Mac developers to contribute to the new framework and osxapp. We don’t have to think about releasing official builds of the OS X SDK right now; I don’t think it’s too onerous to expect developers to clone the repository and run make xpackage. Plus, osxapp is now an enjoyable little application to debug with.

This PR covers a massive list of features:

  • Rendering
  • Finger and mouse gestures (scroll, drag, magnify, rotate, tilt)
  • Keyboard shortcuts
  • MGLAccountManager
  • Delegate methods
  • Completion handlers
  • MGLMapCamera
    • Flying to cameras
  • Geometric and geographic functions
  • User dot
  • User location tracking
  • Annotation display
  • Bindings
  • Printing
  • Zoom controls
  • Compass
  • Logo and attribution
  • IB inspectables and designable
  • State restoration
  • Right-to-left layout
  • Fix crash closing a window containing an MGLMapView
  • Fix incompatibilities with full size content view windows, Cocoa controls overlapping MGLMapView
    • Need to find a way to switch from NSOpenGLView to NSOpenGLLayer
  • Reenable VAOs
  • Access token preference in osxapp
  • Custom styles in osxapp
  • URL event handling in osxapp
  • Share button in osxapp for comparing with GL JS
  • Fix build issues in other targets arising from OS X build changes
  • Switch from static library to framework
  • Switch to more popular Objective-C coding style
  • Rewrite inline comments
  • Rewrite documentation comments
  • Automated tests (blocked by upgrade gyp #3193)
  • 🍆

blueprint

Fixes #8, #3043.

/cc @incanus @kkaefer @friedbunny

@1ec5 1ec5 added feature refactor macOS Mapbox Maps SDK for macOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold in progress labels Nov 25, 2015
@friedbunny
Copy link
Contributor

Great start — this app is already pretty damn nice.

Noticed that make xproj downloads glfw from mason, is this still required elsewhere?

@1ec5 1ec5 self-assigned this Nov 26, 2015
@tkrajacic
Copy link

It's an excellent starting point! Thanks for your work @1ec5 !

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 27, 2015

MGLMapView is now layer-backed via NSOpenGLLayer, addressing the crash-on-close, eliminating CPU usage when idle, and enabling Cocoa controls like the toolbar to appear above the map view:

layer

The map view stays visible while resizing, too. However, animation performance is suffering, probably due in part to:

2015-11-26 17:42:27.300 Mapbox GL[59178:4428209] [WARNING] {Map}[OpenGL]: Not using Vertex Array Objects

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 28, 2015

Noticed that make xproj downloads glfw from mason, is this still required elsewhere?

glfw is still used by linuxapp, which is ironically built as part of the xproj workspace.

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

As part of this PR, I’ve taken a fresh look at a lot of the iOS SDK’s MGLMapView code, with an eye towards modularity. The annotation classes are now responsible for styling themselves, and the annotation management and selection code is a lot simpler now that IDs are mapped to annotations rather than the other way around. (That approach is also required for #1496.) Where necessary for code sharing, I’ve updated the iOS SDK, but I’ll need to backport more sweeping changes in a separate PR.

I’m particularly pleased with how callouts turned out. On iOS, MGLMapView acts as a middleman between SMCalloutView and MGLMapViewDelegate. On OS X, MGLMapView gets out of the way, allowing MGLMapViewDelegate to supply any NSViewController (such as one instantiated from a nib); MGLMapView wraps it in an NSPopover for display. The default view controller uses bindings to keep everything tidy:

callout

The callout respects the annotation image’s alignment rect, solving #1504 and #2151. (Although it would be more natural for MGLAnnotationImage to know its anchorPoint, by analogy with -[NSCursor hotspot].)

@mb12
Copy link

mb12 commented Nov 30, 2015

Thanks @1ec5 for rewriting OSX app using Cocoa.
I am getting the following compilation errors when trying to build target osxapp using Xcode Version 6.3.1 (6D1002).
This is what I did.

  1. git reset --hard 73f83c4
  2. make xproj
  3. Build target osxapp for 'My Mac'.
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/AppDelegate.m:104:92: error: property 'doubleValue' not found on object of type 'id'
            self.mapView.centerCoordinate = CLLocationCoordinate2DMake(coordinateValues[0].doubleValue,
                                                                                           ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/AppDelegate.m:105:92: error: property 'doubleValue' not found on object of type 'id'
                                                                       coordinateValues[1].doubleValue);

/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:32: error: expected ')'
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                               ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:24: note: to match this '('
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                       ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:141: error: expected ')'
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                                                                                                                                            ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:110: note: to match this '('
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                                                                                                             ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:60: warning: conflicting parameter types in implementation of 'getObjectValue:forString:errorDescription:': '__autoreleasing id *' vs '__strong id' [-Wmismatched-parameter-types]
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                            ~~                             ^
In file included from /Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:1:
In file included from /Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.h:1:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:19:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSDateFormatter.h:5:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSFormatter.h:53:34: note: previous definition is here
- (BOOL)getObjectValue:(out id *)obj forString:(NSString *)string errorDescription:(out NSString **)error;
                            ~~~~ ^
/Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:34:153: warning: conflicting parameter types in implementation of 'getObjectValue:forString:errorDescription:': 'NSString *__autoreleasing *' vs 'NSString *__autoreleasing' [-Wmismatched-parameter-types]
- (BOOL)getObjectValue:(out id _Nullable __autoreleasing *)obj forString:(NSString *)string errorDescription:(out NSString *__autoreleasing _Nullable *)error {
                                                                                                                                                        ^
In file included from /Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.m:1:
In file included from /Users/mb12/Documents/MAPBOX_GL_MAC_1EC5/mapbox-gl-native/macosx/LocationCoordinate2DFormatter.h:1:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:19:
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSDateFormatter.h:5:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSFormatter.h:53:101: note: previous definition is here
- (BOOL)getObjectValue:(out id *)obj forString:(NSString *)string errorDescription:(out NSString **)error;
                                                                                        ~~~~~~~~~~~ ^
2 warnings and 2 errors generated.


@1ec5 1ec5 changed the title [WIP] Minimal OS X SDK and revamped OS X demo application [WIP] OS X SDK and revamped OS X demo application Nov 30, 2015
@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

@mb12, thanks for trying out this branch! Those errors should be fixed now. They’re specific to Xcode 6.x (which doesn’t support lightweight generics) and crept in because I’ve been developing in Xcode 7.0 or 7.1 lately.

@kkaefer
Copy link
Contributor

kkaefer commented Nov 30, 2015

Not sure how I triggered this, but I got stuck in a mode where I can't pan the map anymore with click + drag. Trackpad panning still works though.

How can I rotate the map? In the GLFW version, I could right-click and drag to rotate the map.

Here's a small list of things that we should also add to the app:

  • Quit app when you close the window, otherwise you'll end up with a menu only app which isn't useful.
  • Implement VBO support. For this to work, we need to load GL extensions in the setup code, like GLFW does
  • Add a way to tilt the map

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

How can I rotate the map? In the GLFW version, I could right-click and drag to rotate the map.

  1. On a trackpad, rotate with two fingers, like on iOS.
  2. Hold down Option while dragging the cursor left and right.
  3. Hold down Option while pressing the left and right arrow keys.
  4. Fiddle with the compass in the lower right.

Implement VBO support.

This was working originally when I used an NSOpenGLView (similar to what GLFW used from glfw/glfw@81bcefe to glfw/glfw@8f0fd7e), but now that I’m using an NSOpenGLLayer, the extensions aren’t being initialized correctly. Given that we have to explicitly activate the context on the Map thread, I suspect that mbgl::gl::InitializeExtensions() needs to be called on the Map thread, rather than on the main thread where it’s being called now.

Add a way to tilt the map

Hold down Option while dragging the cursor up and down. I haven’t bothered to implement the compass-inclinometer control that Maps.app has.

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

Quit app when you close the window, otherwise you'll end up with a menu only app which isn't useful.

Fixed in 66066bd.

@kkaefer, regarding VAOs, InitializeExtensions() is bailing here because glGetString(GL_EXTENSIONS) comes up null. Any ideas?

@tkrajacic
Copy link

Might be because it seems deprecated in the OpenGL spec.

Quick search turned up that you should use glGetStringi:
https://www.opengl.org/sdk/docs/man/html/glGetString.xhtml

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

InitializeExtensions() was being called before the context was activated. Fixed in 94dc1dc; now VBOs are being used and performance is 👌.

@mb12
Copy link

mb12 commented Nov 30, 2015

On OS X Yosemite (10.10.1), its crashing with the following error.

2015-11-30 10:58:26.053 Mapbox GL[23588:454865] -[NSSegmentedControl setTrackingMode:]: unrecognized selector sent to instance 0x608000181380

This is the specific assignment in commonInit that's causing the problem. If I comment it out it launches and runs successfully. Without this mode, the zoom button stays blue even after the mouse click.

_zoomControls.trackingMode = NSSegmentSwitchTrackingMomentary;

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

Ah, thanks. I was afraid I'd use a 10.11-only API; I don't have a Yosemite machine handy to test with.

Edit: In fact, it was introduced in 10.10.3, so kudos for having a great OS version for testing with. 😄

@1ec5
Copy link
Contributor Author

1ec5 commented Nov 30, 2015

@mb12, the crash in 10.10.1 should be fixed as of fb1fbc4. Let me know if you run into any other issues.

@mb12
Copy link

mb12 commented Dec 1, 2015

Thanks @1ec5 for fixing this. I've just verified it on Yosemite 10.10.. The look and feel and callouts are very close to native Apple Maps on Mac.

Stripping the framework means you can’t debug the framework.
Shared MGLMapCamera between iOS and OS X. Unfortunately -camera and -setCamera: implementations need to be copy-pasted for now.
Also fixed an issue where removing a selected annotation failed to deselect it.
appledoc can’t understand conditional compilation. This is the best we can do until we move to Jazzy.
Copied some pixel-reading code from HeadlessView.
Corrected the path to the build output. Also added a missing inline comment.
friedbunny added a commit that referenced this pull request Dec 14, 2015
- Change all smart quotes to dumb quotes to workaround SourceKitten bug.
- Port changes from OS X (#3135)
@1ec5 1ec5 merged commit a01dd8a into master Dec 14, 2015
@1ec5 1ec5 removed the in progress label Dec 14, 2015
@1ec5 1ec5 deleted the 1ec5-osx branch December 14, 2015 04:34
friedbunny added a commit that referenced this pull request Dec 21, 2015
- Change all smart quotes to dumb quotes to workaround SourceKitten bug.
- Port changes from OS X (#3135)
@1ec5 1ec5 mentioned this pull request Dec 22, 2015
friedbunny added a commit that referenced this pull request Dec 23, 2015
- Change all smart quotes to dumb quotes to workaround SourceKitten bug.
- Port changes from OS X (#3135)
friedbunny added a commit that referenced this pull request Jan 5, 2016
- Change all smart quotes to dumb quotes to workaround SourceKitten bug.
- Port changes from OS X (#3135)
@1ec5 1ec5 modified the milestone: osx-v0.1.0 May 10, 2016
@1ec5 1ec5 mentioned this pull request Jun 21, 2017
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature macOS Mapbox Maps SDK for macOS refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pinch zoom & finger rotate on OS X
6 participants