[DO NOT MERGE] Add BitmapRegistry

[aednlaxer]

Bitmap registry PR for internal review. Idea is simple: register bitmap with a registry then reuse it by using RegisteredBitmap as marker icon, this improves performance since bitmaps are cached on the platform side.

TODO:

• Provide numbers on performance improvement
• Example pages for Android and iOS
• Check if solution works with cluster
• Test with bigger images
• Check how to update marker image
• Check how it works before map is initlialized
• Check how it works with multiple registries (Dart singleton?)
• Tests
• Check if iOS performance can be improved (I see no changes here)

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jokerttu]

Looks good, few review comments added

[jokerttu]

this could return RegisteredMapBitmap object directly

[aednlaxer]

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:

• Use int (the way it's done now)
• Add GoogleMapsRegisteredMapBitmap or something like that to the user-facing plugin
• Use RegisteredMapBitmap and see what maintainers say

[jokerttu]

Oh, BitmapDescriptor classes are used already as icon objects etc.
I think the case was more about the internal configuration umbrella objects?

[aednlaxer]

AFAIR it was about using platform interface's MarkerType in google_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?

[jokerttu]

Yes, it is quite easy to change later if they want to

[jokerttu]

should this take RegisteredMapBitmap as a parameter.

[aednlaxer]

@illuminati1911 can you please review the iOS part? Feel free to leave comments about everything else too, I'm just very unsure about the Objective-C code

[aednlaxer]

I've added some tests. What else should be added/changed before PR can be created in the main repository @jokerttu?

[aednlaxer]

This is some results of my performance tests (will be copied to the main PR). I've done plenty of tests in different configurations but I think it's important to show that using registry in the example page makes a difference.

Performance improvements

There's a new example page called Bitmap registry to evaluate performance when adding many markers with and without the registry. It's a simple page that adds 500 markers with a bitmap. Bitmap is relatively large (800x600px, 332KB). This example shows performance drop on iOS and Android. Other examples I've tried (more markers and smaller bitmaps) don't show much improvements on iOS. Web is quite performant with or without the registry.

The data was measured using Stopwatch in updateMarkers call. First button press adds first set of markers with random positions. Consecutive calls clear existing markers and add next set of markers.

There are two buttons in the example. First button adds markers and sets their icon by sending each marker's icon to the platform layer. Second button adds bitmap to the registry and adds markers with the registered bitmap.

1. Not using registry

• First call ~ 11 seconds
• Consecutive call ~ 12 seconds

2. Using registry

• First call ~ 0.17 second
• Consecutive call ~ 0.61 second

I've tried this on iOS and Android multiple times with different bitmaps. The results are different depending on current platform, number of markers and bitmap configuration (size, scaling)

[jokerttu]

I'll continue with this later... here is first foundings.

App facing package is missing intergration tests as well!

[jokerttu]

We should extend the inspector api to have methods
bool supportsBitmapRegistry => false (overrided in platform implementations)
bool hasRegisteredMapBitmap => api call to check if there is registered bitmap...

[aednlaxer]

Why do we need the first method?

I'm hitting an obstacle with these - implementation needs to have mapId to call inspectorProvider in google_map_inspector_android.dart but for bitmap registry mapId is not used. BitmapRegistry is not attached to a map, it's a singleton which part of the plugin that different map instances could use

[jokerttu]

even when this is now supported by all platforms, it seems its recommend to have supports queries for certain calls (that are called for example in tests).
We have added similar to other features lately,
not sure if supportsBitmapRegistry is too generic, it could actually be supportsHasRegisteredMapBitmap

[jokerttu]

I'm hitting an obstacle with these - implementation needs to have mapId to call inspectorProvider in google_map_inspector_android.dart but for bitmap registry mapId is not used. BitmapRegistry is not attached to a map, it's a singleton which part of the plugin that different map instances could use

I can check this next week, but it is always possible to create separate inspector api for bitmapregistry that does not require mapid

[aednlaxer]

I've ended up pumping a map widget and getting a map ID from it for these tests. I've added just the hasRegisteredMapBitmap because I still don't see why we should have the supportsBitmapRegistry method if it's never used

[jokerttu]

You should be able to for example skip tests if supportsBitmapRegistry return false.
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#api-support-queries