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

Load map resources from local assets #579

Closed
ljbade opened this issue Nov 9, 2014 · 31 comments
Closed

Load map resources from local assets #579

ljbade opened this issue Nov 9, 2014 · 31 comments
Labels
performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@ljbade
Copy link
Contributor

ljbade commented Nov 9, 2014

Currently takes the app a second or two to load.

This is due to extracting the asset folder from APK.

Need to make this faster, get rid of it (by reading directly from APK), or only do it once at install time (can hook this), or only if the files are not there.

TODO: more detail on this so you guys understand what I am talking about

@ljbade ljbade added the Android Mapbox Maps SDK for Android label Nov 9, 2014
@ljbade
Copy link
Contributor Author

ljbade commented Nov 11, 2014

Found Context.getPackageCodePath that may allow the C++ code to read styles directly from the APK zip file if we include C++ zip library.

Also I am going to restrict Assetbridge to extract only the files we need. This should make things faster. See #585

@kkaefer
Copy link
Member

kkaefer commented Nov 11, 2014

Cache shaders: #588

@ljbade
Copy link
Contributor Author

ljbade commented Nov 11, 2014

Also getCodeCacheDir sounds useful:

Returns the absolute path to the application specific cache directory on the filesystem designed for storing cached code. The system will delete any files stored in this location both when your specific application is upgraded, and when the entire platform is upgraded.

But it is only on Android 5+

@ljbade
Copy link
Contributor Author

ljbade commented Nov 11, 2014

OK de1f37f includes the stuff to limit which files are extracted.

@ljbade ljbade added the performance Speed, stability, CPU usage, memory usage, or power usage label Nov 16, 2014
@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

@kkaefer what would be required to get the URL styles to load directly from a zip file? This would save extacting all the styles even if you are not using all of them

Also need to get curl to read the ca-bundle file from the zip too.

@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

Okay it looks like libzip is the way to go.

@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

I am proposing a URL scheme like: zip://path/to/zipfile#path/in/zipfile

@kkaefer is that OK?

@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

OK that will not work with relative files, instead I will just use asset://path/to/file which means we will load from the APK's assets dir.

@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

Implemented asset:// URLs in f0e4e27

Now I need to get curl to load ca-bundle from memory. See http://curl.haxx.se/libcurl/c/cacertinmem.html and http://blog.steffenl.com/2013/02/load-certificate-from-memory-using-curl.html

@ljbade
Copy link
Contributor Author

ljbade commented Nov 17, 2014

Implemented loading CA bundle from memory: 80be6c4

I removed Assetbridge now that we no longer need it.

Done.

@ljbade ljbade closed this as completed Nov 17, 2014
@kkaefer
Copy link
Member

kkaefer commented Nov 24, 2014

+1 on asset:// URLs. Why make it Android-specific though? This will make stylesheets that use that URL scheme platform specific. Instead, we could use this scheme to provide a couple of default styles/sprites/fonts that always ship with all applications.

I refrained from implementing the certificate loading from memory since it requires OpenSSL headers, which we don't necessarily have available on all platform, and I didn't want to add a direct dependency on it, as compiling OpenSSL can get quite hairy.

@ljbade
Copy link
Contributor Author

ljbade commented Nov 24, 2014

Just discovered that Android actually has a native version of the AssetManager in <android/asset_manager.h> so I might be able to get rid of libzip. Reopening.

@kkaefer no specific reason for it to be Android specific. I can refactor it to allow other platforms to implement it.

Interesting about OpenSSL headers. Perhaps we can just make it an #ifdef that is set on platforms that have OpenSSL headers and want to load certs from asset://?

@springmeyer was looking at switching from OpenSSL to Google's BoringSSL fork at least for Android. This is needed as OpenSSL has a compile bug on ARM Android that Google patched in their fork.

@ljbade ljbade reopened this Nov 24, 2014
@ljbade
Copy link
Contributor Author

ljbade commented Nov 30, 2014

Looks like NDK AssetManager has some flaws.

Will keep using libzip.

@ljbade
Copy link
Contributor Author

ljbade commented Nov 30, 2014

I will get android-mason branch merged, then I can come back and expand asset:// for other platforms.

@incanus do you think asset:// will work for iOS too?

@incanus
Copy link
Contributor

incanus commented Dec 1, 2014

To make sure I understand, the idea is to use asset:// for ease of reference cross-platform?

This is easy enough to do as a layer on top of NSBundle, which searches by filename + extension through the bundled resources. For example we do this already here:

const std::string path([[[NSBundle mainBundle] pathForResource:@"bright-v6" ofType:@"json" inDirectory:@"styles/"] UTF8String]);

@ljbade
Copy link
Contributor Author

ljbade commented Dec 1, 2014

Yeah that is what I understand from @kkaefer

Ensures same style JSON can be used on both Android and iOS

@incanus
Copy link
Contributor

incanus commented Dec 2, 2014

Working on asset:// on iOS but it's held up by cutting-room-floor/mapbox-gl-cocoa#72 since this needs to work for both the library (which has a MapboxGL.bundle for resources, including styles) and the iOS test app, which includes the styles directly in its own bundle.

@ljbade
Copy link
Contributor Author

ljbade commented Dec 2, 2014

@incanus I have refactored asset_request to use the cross platform pattern.

Currently Linux, OSX and iOS use a noop implementation that just returns a 500 error.

So you will need to copy this and add as necessary to load from iOS.

@kkaefer
Copy link
Member

kkaefer commented Dec 2, 2014

Note that we'll also have to figure out a way to handle asset:// URLs on JS.

@ljbade
Copy link
Contributor Author

ljbade commented Dec 2, 2014

Hmm, yes.

Perhaps just remap asset:// to a HTTP request with prefix to some static URL, e.g. person might have static HTTP server/S3 for serving assets like images, scripts etc for their website

@incanus
Copy link
Contributor

incanus commented Dec 2, 2014

Ok @ljbade what else are we working on here to "make the app faster"? We should rename the ticket if nothing else or else split off into separate, actionable, assignable tickets. Right now for example we're discussing asset:// across a couple tickets.

@ljbade ljbade changed the title Make app load faster Load map resources from local assets Dec 2, 2014
@ljbade
Copy link
Contributor Author

ljbade commented Dec 2, 2014

@incanus I renamed the issue, is that better?

Anyway this issue was more about the Android implementation of asset://, but that has now grown to cross-platform concept.

Now Android is done with asset URL we should close this and have seperate tickets for asset support on other platforms?

@incanus
Copy link
Contributor

incanus commented Dec 2, 2014

Nah let's do this since it's a single issue thing now:

@ljbade ljbade removed Android Mapbox Maps SDK for Android labels Dec 2, 2014
@kkaefer
Copy link
Member

kkaefer commented Dec 5, 2014

I think we should merge FileRequest and AssetRequest, and move the actual implementation to a platform-specific file, like HTTPRequest already does (it uses Cocoa stuff on OS X/iOS, and CURL on all other platforms).

FileRequest and AssetRequest are already very similar: relative paths in a file:// URLS are relative to the executable or bundle (iOS/OS X), which means they are already used to access "bundled" files like the stylesheet or image sprites.

@jfirebaugh
Copy link
Contributor

👍

@mikemorris
Copy link
Contributor

👍 to merging into FileRequest with platform-specific implementations

@ljbade
Copy link
Contributor Author

ljbade commented Dec 6, 2014

Okay will work on this then.

kkaefer added a commit that referenced this issue Dec 17, 2014
refs #579: Android can now rename AssetRequest to FileRequest and implement it differently
@kkaefer
Copy link
Member

kkaefer commented Dec 17, 2014

@ljbade merged this. You can now move the Android implementation to the new scheme.

@ljbade
Copy link
Contributor Author

ljbade commented Dec 17, 2014

@kkaefer awesome

@ljbade
Copy link
Contributor Author

ljbade commented Jan 27, 2015

@kkaefer So where did this get to? I think Android was done right?

@kkaefer
Copy link
Member

kkaefer commented Jan 27, 2015

This is implemented in other systems as well. OS X and iPhone bundles load data from the resources folder, Linux builds well load assets relative to the binary's location

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

5 participants