forked from flutter/packages
-
Notifications
You must be signed in to change notification settings - Fork 4
[DO NOT MERGE] Add BitmapRegistry #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
aednlaxer
wants to merge
18
commits into
main
Choose a base branch
from
add-bitmap-registry
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
1098e79
Add BitmapRegistry
aednlaxer bdd65ab
Update example code
aednlaxer 774ffae
Initialize ImageRegistry in FLTGoogleMapsPlugin.m
aednlaxer 140dbb2
Update BitmapRegistry example
aednlaxer 94fbf0c
Rename ImageRegistry to FGMImageRegistry on iOS
aednlaxer 5b8555b
Update comments
aednlaxer 06b40c2
Use toBitmapDescriptor when creating bitmaps inside ImageRegistry
aednlaxer 0f13881
Add RegisteredMapBitmap doc and example
aednlaxer e8a25eb
Add default implementation tests
aednlaxer d19b6c9
Fix existing Android tests
aednlaxer 08a408b
Add more tests
aednlaxer ebfb4eb
Clean up iOS implementation
aednlaxer 2e2bf8d
Fix tests
aednlaxer 49d9a05
Reformat code
aednlaxer c7518ee
Rename PlatformBitmapRegisteredMapBitmap to PlatformRegisteredMapBitmap
aednlaxer bd53fba
Fix tests
aednlaxer 9799b6a
Add doc for getBitmap
aednlaxer dcf3876
Add hasRegisteredMapBitmap method to maps inspector
aednlaxer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Binary file added
BIN
+324 KB
packages/google_maps_flutter/google_maps_flutter/example/assets/checkers.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
167 changes: 167 additions & 0 deletions
167
packages/google_maps_flutter/google_maps_flutter/example/lib/bitmap_registry.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,167 @@ | ||
// Copyright 2013 The Flutter Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
// ignore_for_file: public_member_api_docs | ||
|
||
import 'dart:math'; | ||
|
||
import 'package:flutter/material.dart'; | ||
import 'package:flutter/services.dart'; | ||
import 'package:google_maps_flutter/google_maps_flutter.dart'; | ||
import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart'; | ||
|
||
import 'page.dart'; | ||
|
||
class BitmapRegistryPage extends GoogleMapExampleAppPage { | ||
const BitmapRegistryPage({Key? key}) | ||
: super(const Icon(Icons.speed), 'Bitmap registry', key: key); | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
return const _BitmapRegistryBody(); | ||
} | ||
} | ||
|
||
// How many markers to place on the map. | ||
const int _numberOfMarkers = 500; | ||
|
||
class _BitmapRegistryBody extends StatefulWidget { | ||
const _BitmapRegistryBody(); | ||
|
||
@override | ||
State<_BitmapRegistryBody> createState() => _BitmapRegistryBodyState(); | ||
} | ||
|
||
class _BitmapRegistryBodyState extends State<_BitmapRegistryBody> { | ||
final Set<Marker> _markers = <Marker>{}; | ||
int? _registeredBitmapId; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
|
||
_registerBitmap(); | ||
} | ||
|
||
@override | ||
void dispose() { | ||
_unregisterBitmap(); | ||
super.dispose(); | ||
} | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
return SingleChildScrollView( | ||
child: Column( | ||
children: <Widget>[ | ||
AspectRatio( | ||
aspectRatio: 2 / 3, | ||
child: GoogleMap( | ||
markers: _markers, | ||
initialCameraPosition: const CameraPosition( | ||
target: LatLng(0, 0), | ||
zoom: 1.0, | ||
), | ||
), | ||
), | ||
MaterialButton( | ||
onPressed: () async { | ||
// Add markers to the map with a custom bitmap as the icon. | ||
// | ||
// To show potential performance issues: | ||
// * large original image is used (800x600px, ~330KB) | ||
// * bitmap is scaled down to 64x64px | ||
// * bitmap is created once and reused for all markers. This | ||
// doesn't help much because the bitmap is still sent to the | ||
// platform side for each marker. | ||
// | ||
// Adding many markers may result in a performance hit, | ||
// out of memory errors or even app crashes. | ||
final BytesMapBitmap bitmap = await _getAssetBitmapDescriptor(); | ||
_updateMarkers(bitmap); | ||
}, | ||
child: const Text('Add $_numberOfMarkers markers, no registry'), | ||
), | ||
MaterialButton( | ||
onPressed: _registeredBitmapId == null | ||
? null | ||
: () { | ||
// Add markers to the map with a custom bitmap as the icon | ||
// placed in the bitmap registry beforehand. | ||
final RegisteredMapBitmap registeredBitmap = | ||
RegisteredMapBitmap(id: _registeredBitmapId!); | ||
_updateMarkers(registeredBitmap); | ||
}, | ||
child: const Text('Add $_numberOfMarkers markers using registry'), | ||
), | ||
], | ||
), | ||
); | ||
} | ||
|
||
/// Register a bitmap in the bitmap registry. | ||
Future<void> _registerBitmap() async { | ||
if (_registeredBitmapId != null) { | ||
return; | ||
} | ||
|
||
final BytesMapBitmap bitmap = await _getAssetBitmapDescriptor(); | ||
_registeredBitmapId = | ||
await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
|
||
// If the widget was disposed before the bitmap was registered, unregister | ||
// the bitmap. | ||
if (!mounted) { | ||
_unregisterBitmap(); | ||
return; | ||
} | ||
|
||
setState(() {}); | ||
} | ||
|
||
/// Unregister the bitmap from the bitmap registry. | ||
void _unregisterBitmap() { | ||
if (_registeredBitmapId == null) { | ||
return; | ||
} | ||
|
||
GoogleMapBitmapRegistry.instance.unregister(_registeredBitmapId!); | ||
_registeredBitmapId = null; | ||
} | ||
|
||
// Create a set of markers with the given bitmap and update the state with new | ||
// markers. | ||
void _updateMarkers(BitmapDescriptor bitmap) { | ||
final List<Marker> newMarkers = List<Marker>.generate( | ||
_numberOfMarkers, | ||
(int id) { | ||
return Marker( | ||
markerId: MarkerId('$id'), | ||
icon: bitmap, | ||
position: LatLng( | ||
Random().nextDouble() * 100 - 50, | ||
Random().nextDouble() * 100 - 50, | ||
), | ||
); | ||
}, | ||
); | ||
|
||
setState(() { | ||
_markers | ||
..clear() | ||
..addAll(newMarkers); | ||
}); | ||
} | ||
|
||
/// Load a bitmap from an asset and create a scaled [BytesMapBitmap] from it. | ||
Future<BytesMapBitmap> _getAssetBitmapDescriptor() async { | ||
final ByteData byteData = await rootBundle.load('assets/checkers.png'); | ||
final Uint8List bytes = byteData.buffer.asUint8List(); | ||
return BitmapDescriptor.bytes( | ||
bytes, | ||
width: 64, | ||
height: 64, | ||
); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
packages/google_maps_flutter/google_maps_flutter/lib/src/bitmap_registry.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
part of '../google_maps_flutter.dart'; | ||
|
||
/// A bitmap registry. | ||
/// | ||
/// Bitmaps can be created before they are used in markers and then registered | ||
/// with the registry. This allows for more efficient rendering of markers | ||
/// on the map. For example, if multiple markers use the same bitmap, bitmap | ||
/// can be registered once and then reused. This eliminates the need to | ||
/// transfer the bitmap data multiple times to the platform side. | ||
/// | ||
/// Using bitmap registry is optional. | ||
/// | ||
/// Example: | ||
/// ```dart | ||
/// // Register a bitmap | ||
/// final registeredBitmap = await GoogleMapBitmapRegistry.instance.register( | ||
/// Bitmap.fromAsset('assets/image.png'), | ||
/// ); | ||
/// | ||
/// // Use the registered bitmap as marker icon | ||
/// Marker( | ||
/// markerId: MarkerId('markerId'), | ||
/// icon: registeredBitmap, | ||
/// position: LatLng(0, 0) | ||
/// ), | ||
/// ) | ||
/// ``` | ||
class GoogleMapBitmapRegistry { | ||
GoogleMapBitmapRegistry._(); | ||
|
||
/// The singleton instance of [GoogleMapBitmapRegistry]. | ||
static final GoogleMapBitmapRegistry instance = GoogleMapBitmapRegistry._(); | ||
|
||
// The number of registered images. Also used as a unique identifier for each | ||
// registered image. | ||
int _imageCount = 0; | ||
|
||
/// Registers a [bitmap] with the registry. | ||
/// | ||
/// Returns a unique identifier for the registered bitmap. | ||
Future<int> register(MapBitmap bitmap) async { | ||
_imageCount++; | ||
final int id = _imageCount; | ||
await GoogleMapsFlutterPlatform.instance.registerBitmap(id, bitmap); | ||
return id; | ||
} | ||
|
||
/// Unregisters a bitmap with the given [id]. | ||
Future<void> unregister(int id) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this take RegisteredMapBitmap as a parameter. |
||
return GoogleMapsFlutterPlatform.instance.unregisterBitmap(id); | ||
} | ||
|
||
/// Unregister all previously registered bitmaps and clear the cache. | ||
Future<void> clear() { | ||
return GoogleMapsFlutterPlatform.instance.clearBitmapCache(); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
63 changes: 63 additions & 0 deletions
63
packages/google_maps_flutter/google_maps_flutter/test/bitmap_registry_test.dart
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import 'dart:typed_data'; | ||
|
||
import 'package:flutter_test/flutter_test.dart'; | ||
import 'package:google_maps_flutter/google_maps_flutter.dart'; | ||
import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart'; | ||
|
||
import 'fake_google_maps_flutter_platform.dart'; | ||
|
||
void main() { | ||
late FakeGoogleMapsFlutterPlatform platform; | ||
|
||
setUp(() { | ||
platform = FakeGoogleMapsFlutterPlatform(); | ||
GoogleMapsFlutterPlatform.instance = platform; | ||
}); | ||
|
||
test('Adding bitmap to registry', () async { | ||
final BytesMapBitmap bitmap = BytesMapBitmap(Uint8List(20)); | ||
final int id = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
expect(id, 1); | ||
expect( | ||
platform.bitmapRegistryRecorder.bitmaps, | ||
<String>['REGISTER $id $bitmap'], | ||
); | ||
}); | ||
|
||
test('Removing bitmap from registry', () async { | ||
final BytesMapBitmap bitmap = BytesMapBitmap(Uint8List(20)); | ||
final int id = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
await GoogleMapBitmapRegistry.instance.unregister(id); | ||
expect( | ||
platform.bitmapRegistryRecorder.bitmaps, | ||
<String>['REGISTER $id $bitmap', 'UNREGISTER $id'], | ||
); | ||
}); | ||
|
||
test('Clearing bitmap registry', () async { | ||
final BytesMapBitmap bitmap = BytesMapBitmap(Uint8List(20)); | ||
final int id1 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
final int id2 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
expect( | ||
platform.bitmapRegistryRecorder.bitmaps, | ||
<String>['REGISTER $id1 $bitmap', 'REGISTER $id2 $bitmap'], | ||
); | ||
|
||
await GoogleMapBitmapRegistry.instance.clear(); | ||
expect( | ||
platform.bitmapRegistryRecorder.bitmaps, | ||
<String>['REGISTER $id1 $bitmap', 'REGISTER $id2 $bitmap', 'CLEAR CACHE'], | ||
); | ||
}); | ||
|
||
test('Bitmap ID is incremental', () async { | ||
final BytesMapBitmap bitmap = BytesMapBitmap(Uint8List(20)); | ||
final int id1 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
final int id2 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
final int id3 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
final int id4 = await GoogleMapBitmapRegistry.instance.register(bitmap); | ||
expect(id2, id1 + 1); | ||
expect(id3, id1 + 2); | ||
expect(id4, id1 + 3); | ||
}); | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could return RegisteredMapBitmap object directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I was doing initially but then there was a comment to my other PR about not mixing platform and user-facing packages. I think it will be the case here too because
RegisteredMapBitmap
is part of platform interface while this comment is about user-facing plugin.I see these options:
int
(the way it's done now)GoogleMapsRegisteredMapBitmap
or something like that to the user-facing pluginRegisteredMapBitmap
and see what maintainers sayUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, BitmapDescriptor classes are used already as icon objects etc.
I think the case was more about the internal configuration umbrella objects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIR it was about using platform interface's
MarkerType
ingoogle_maps_flutter
, exposing it for end users. All other objects are exposed but I was asked not to do that because they're changing how federated plugins are doing things.I'd rather keep it like it is now and wait for review or ask a question when main PR is created. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is quite easy to change later if they want to