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

tvOS map SDK and tvOS demo application #9319

Closed
wants to merge 5 commits into from
Closed

tvOS map SDK and tvOS demo application #9319

wants to merge 5 commits into from

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Jun 21, 2017

This PR introduces a new tvOS-compatible flavor of the iOS map SDK. The goal is to minimize maintenance overhead, so that we can get tvOS support more or less “for free” in the course of supporting iOS. Unlike the macOS map SDK introduced in #3135, the tvOS map SDK is compiled from a strict subset of the iOS map SDK codebase and lives inside the same Xcode project generated by make iproj. Ultimately, the tvOS map SDK would be distributed as just another binary in an iOS map SDK release, alongside the dynamic and static builds.

Conditional compilation is used extensively to exclude incompatible features like user location tracking and and most gesture recognizers. Telemetry-related files are excluded outright from the target. The conditional compilation is particularly intrusive around the gesture recognition code in MGLMapView. I think MGLMapView remains more readable than, for instance, much of the MGLStyleValue code, but on the other hand my personal tolerance for conditional macros is relatively high.

tvosapp

The tvOS SDK isn’t ready for a debut, and we haven’t made any plans to officially support tvOS. This PR merely provides a convenient place to collect technical feedback. We’d need to address most of these caveats before we could even consider merging the PR:

  • The same ICU dependency that enables correctly rendered Arabic is available as an iOS static library. Using it as a tvOS or watchOS library anyways technically works, but Apple plans to remove support for this approach soon, so there are lots of URGENT warnings. Before this PR can land, we’d want Mason to produce a tvOS binary of ICU to avoid future breakage. Alternatively, maybe we could find a way to build the tvOS flavor of ICU from scratch, on demand, as part of make iproj.
  • CMake, which generates mbgl.xcodeproj, is unable to place a single file reference in multiple targetsMerge iproj and xproj? #5942 (comment) – so we have to compile mbgl-core as a single fat binary supporting both iOS and tvOS. This shouldn’t add to the iOS map SDK’s size, since unused symbols would get stripped away as part of the SDK build process.
  • Due to the Apple Remote’s limited gesture support, it’s expected that most applications would rely on programmatic camera changes more than user gestures. However, it would be desirable to figure out how to implement the same pan and zoom menu that MapKit has implemented.
  • The attribution button does nothing when pressed. MapKit puts a clickable Legal button inside the pan/zoom menu. We should do likewise, but there’s also enough horizontal real estate to display an unobtrusive copyright notice on-screen, similar to the macOS map SDK’s attribution view.

To do:

  • Sort out ICU build situation
  • Implement menu with pan, zoom, and attribution buttons
  • Implement zoom slider
  • Add macOS-style attribution view
  • Implement keyboard shortcuts (Add key commands for keyboard navigation #7625)
  • Make tvosapp more compelling
  • Set up CircleCI for tvOS targets
  • Add CocoaPods podspec
  • Add tvOS-specific jazzy docset generator
  • Update DEVELOPING.md

Prior art in #2340 and #2778.

/cc @friedbunny @kkaefer

@1ec5 1ec5 added build iOS Mapbox Maps SDK for iOS MapKit parity For feature parity with MapKit on iOS or macOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Jun 21, 2017
@1ec5 1ec5 self-assigned this Jun 21, 2017
@1ec5 1ec5 added the tvOS Porting the Mapbox Maps SDK for iOS to tvOS label Jun 21, 2017
@boundsj
Copy link
Contributor

boundsj commented Jun 21, 2017

This is 😎

I've been thinking lately about refactoring MGLMapView to make it more testable and less... long 😄. It'd be nice to address (or at least plan to address) some of that debt before this eventually gets merged. One improvement I can think of is that some of the map view behaviors could be captured in classes and the map view could be composed with subclasses of those behaviors depending on the platform it is compiled for which could reduce some of the conditionals and keep things more readable.

@1ec5

This comment has been minimized.

@1ec5

This comment has been minimized.

@1ec5 1ec5 changed the base branch from master to release-agua October 6, 2017 05:33
@1ec5 1ec5 changed the base branch from release-agua to master December 2, 2017 02:03
@1ec5

This comment has been minimized.

@wmanth
Copy link

wmanth commented Jan 22, 2018

With the help of this Makefile I was able to build universal versions of the ICU 58.1 library that work for tvOS (device and simulator) without the link error mentioned above (copy the file into the ICU root directory and call make).

The question is how to distribute such tvOS specific libraries via mason if there is no separate platform available for tvOS but instead iOS being reused.

Given I've got runtime issues of the tvOS test app (I assume several things under the hood of tvOS work slightly different than iOS) I would actually consider adding an additional platform. This way also the amount of ifdef statements could be reduced and features could be adopted to the different environment of a TV app. Admittedly some code might be duplicated if no better solution could be found.

@stale stale bot added the archived Archived because of inactivity label Feb 20, 2019
@stale
Copy link

stale bot commented Feb 20, 2019

This pull request 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 Feb 20, 2019
@1ec5
Copy link
Contributor Author

1ec5 commented Feb 26, 2019

Oh no, not after all this tvOS quest has been through.

@1ec5 1ec5 reopened this Feb 26, 2019
@stale stale bot removed the archived Archived because of inactivity label Feb 26, 2019
@1ec5 1ec5 force-pushed the 1ec5-tvos branch 2 times, most recently from 4d18c83 to bfbb585 Compare February 27, 2019 01:45
@1ec5 1ec5 force-pushed the 1ec5-tvos branch 3 times, most recently from 530d990 to 817d7ff Compare March 23, 2019 06:26
@stale stale bot added the archived Archived because of inactivity label Jun 28, 2019
@stale
Copy link

stale bot commented Jun 28, 2019

This pull request 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 Jun 28, 2019
@julianrex julianrex reopened this Jun 28, 2019
@stale stale bot removed the archived Archived because of inactivity label Jun 28, 2019
1ec5 added 5 commits July 28, 2019 15:15
Allow mbgl-platform-core to be used as a tvOS static library. Added a tvOS dynamic framework target and corresponding unit test target. Conditionally omitted lots of code specific to iOS, particularly regarding Core Location and certain gesture recognizers that are incompatible with the Apple Remote. Also omitted the entirety of Mapbox Telemetry and Fabric integration.
Also read access token from environment or user default.
@tmpsantos
Copy link
Contributor

No longer relevant.

@tmpsantos tmpsantos closed this Mar 3, 2020
@tmpsantos tmpsantos deleted the 1ec5-tvos branch March 3, 2020 09:01
@nvolpe
Copy link

nvolpe commented Jan 18, 2021

Hello there. I was hopeful to bring these changes to a fork of mine to create a tvOS build. After looking a little closer at the build steps, I am guessing that will not be possible? The mapbox native core framework is built internally and likely not built for tvOS. Therefore I am guessing it will not allow me to add it to a tvOS project. Is that correct? If it is possible I would be interested in contributing support for tvOS. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold iOS Mapbox Maps SDK for iOS MapKit parity For feature parity with MapKit on iOS or macOS tvOS Porting the Mapbox Maps SDK for iOS to tvOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants