feat: KMZ support #625
feat: KMZ support #625
Conversation
BitmapDescriptorFactory.fromBitmap() copies the Bitmap on every call. Avoid duplicating Bitmaps for each Marker by caching the scaled BitmapDescriptors.
googlebot
added
the
cla: yes
| @@ -553,7 +553,7 @@ private void putMarkerImagesCache(String url, String scale, BitmapDescriptor bit | |||
| * @param url image URL | |||
| * @param bitmap image bitmap | |||
| */ | |||
| protected void cacheBitmap(String url, Bitmap bitmap) { | |||
| public void cacheBitmap(String url, Bitmap bitmap) { | |||
There was a problem hiding this comment.
Please revert—changing the scope of cacheBitmap and checkClearBitmapCache are implementation leaks. I'll comment on KmlLayer on another suggested approach.
| * Use this constructor with shared object managers in order to handle multiple layers with | ||
| * their own event handlers on the map. | ||
| * | ||
| * @param map GoogleMap object | ||
| * @param stream InputStream containing KML file | ||
| * @param stream InputStream containing KML or KMZ file |
There was a problem hiding this comment.
I think it would be simpler if another constructor was created, with the following signature, to handle the KMZ files explicitly:
public KmlLayer(GoogleMap map, ZipInputStream stream, FragmentActivity activity, MarkerManager markerManager, PolygonManager polygonManager, PolylineManager polylineManager, GroundOverlayManager groundOverlayManager)This way, it's left to the caller to use the correct constructor for their use-case.
There was a problem hiding this comment.
I thought about that. Since we can differentiate between the two types with the logic I implemented, I thought a single universal constructor was simpler for the caller to use. Also, since there are two constructors currently, the other supports opening a KML by resource ID, how could we implement opening a KMZ by resource ID? We'd have to imply the resource ID is either a KML or KMZ or we'd need the same logic to differentiate between them and call the appropriate stream constructor anyway.
There was a problem hiding this comment.
Good point about invoking by resource ID. Let's stick to this implementation.
| if (parser == null && entry.getName().toLowerCase().endsWith(".kml")) { | ||
| parser = parseKml(zip); | ||
| } else { | ||
| Bitmap bitmap = BitmapFactory.decodeStream(zip); |
There was a problem hiding this comment.
Since other file types might also be used (e.g. .mp3), it would be good to check that an image is used and if not, log via Log.w that the file type is not supported.
There was a problem hiding this comment.
Will this handle subdirectories?
sample.kmz
- doc.kml
- image.png
- dir/image2.png
- dir/image3.png
- dir/dir2/image4.png
...
There was a problem hiding this comment.
Yes, subdirectories are handled. The name includes the full file path within the zip archive.
There was a problem hiding this comment.
Added warning log: 3d1d0e7
| } else { | ||
| Bitmap bitmap = BitmapFactory.decodeStream(zip); | ||
| if (bitmap != null) { | ||
| renderer.cacheBitmap(entry.getName(), bitmap); |
There was a problem hiding this comment.
Instead of caching the bitmap in KmlLayer, a helper method in KmlRenderer would be cleaner:
e.g.
KmlRenderer.storeKmzData(HashMap<String, KmlStyle> styles,
HashMap<String, String> styleMaps,
HashMap<KmlPlacemark, Object> features, ArrayList<KmlContainer> folders,
HashMap<KmlGroundOverlay, GroundOverlay> groundOverlays,
Set<String> bitmapInputStreams)This way, all bitmap caching is done within KmlRenderer and the scoping of the functions within Renderer can be reverted.
There was a problem hiding this comment.
I added a similar method. It's not possible to pass a set of bitmap InputStreams, as each bitmap uses the same ZipInputStream as the ZipEntrys are iterated over. So I'm just passing a map of decoded bitmaps. 482c748
| * Use this constructor with shared object managers in order to handle multiple layers with | ||
| * their own event handlers on the map. | ||
| * | ||
| * @param map GoogleMap object | ||
| * @param stream InputStream containing KML file | ||
| * @param stream InputStream containing KML or KMZ file |
There was a problem hiding this comment.
Good point about invoking by resource ID. Let's stick to this implementation.
Support opening a KMZ file through the same mechanism as opening a KML file, either from a resource ID or
InputStream, inKmlLayer. Images within the KMZ archive are decoded and cached for points and ground overlays the same as downloaded images from URLs.Fixes #619, #472