[google_maps_flutter] Add heatmap support

[Rexios80]

Transferred from https://github.com/flutter/plugins/pull/5274

Adds heatmap support for web, iOS, and Android

List which issues are fixed by this PR. You must list at least one issue.

Fixes #33586 Fixes #86811

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

IF YOU WANT TO USE THIS NOW

pubspec.yaml:

dependencies:
  google_maps_flutter: ^2.6.0

dependency_overrides:
  google_maps_flutter:
    git:
      url: https://github.com/Rexios80/packages_flutter
      ref: 01ab1db2d2ac3a13d70d25576c8aab0c5ba38a7c
      path: packages/google_maps_flutter/google_maps_flutter
  google_maps_flutter_web:
    git:
      url: https://github.com/Rexios80/packages_flutter
      ref: 01ab1db2d2ac3a13d70d25576c8aab0c5ba38a7c
      path: packages/google_maps_flutter/google_maps_flutter_web
  google_maps_flutter_android:
    git:
      url: https://github.com/Rexios80/packages_flutter
      ref: 01ab1db2d2ac3a13d70d25576c8aab0c5ba38a7c
      path: packages/google_maps_flutter/google_maps_flutter_android
  google_maps_flutter_ios:
    git:
      url: https://github.com/Rexios80/packages_flutter
      ref: 01ab1db2d2ac3a13d70d25576c8aab0c5ba38a7c
      path: packages/google_maps_flutter/google_maps_flutter_ios
  google_maps_flutter_platform_interface:
    git:
      url: https://github.com/Rexios80/packages_flutter
      ref: 01ab1db2d2ac3a13d70d25576c8aab0c5ba38a7c
      path: packages/google_maps_flutter/google_maps_flutter_platform_interface

Pre-launch Checklist

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

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

[reidbaker]

Thank you for your contribution. It looks like the tests are failing. Before we review the PR, please see what you can do to resolve the test failures. If you are unsure how to proceed, please reach out for help on the #hackers-new channel.

[Rexios80]

@reidbaker The actions seem to only be failing due to the required dependency overrides since this PR requires changes to the platform interface. Until we are ready to split out the platform interface changes into their own PR and get that merged, the actions will continue to fail on this PR.

[Rexios80]

@stuartmorgan The example ios app Podfile needs to specify platform :ios, '13.0' in order to use the latest Google Maps iOS Cocoapod. Should I make a PR just for that or include it here?

[stuartmorgan]

The example ios app Podfile needs to specify platform :ios, '13.0' in order to use the latest Google Maps iOS Cocoapod.

Why do you need the latest version? It looks like building and testing are passing as-is; doesn't that mean the APIs you are using are available back to the oldest version that supports 11.0?

[Rexios80]

I don't necessarily need it, but since we were seeing behavior changes between versions on Android I thought it might be good to make sure the example app is actually using the latest.

[stuartmorgan]

We'd lose testing that it works with the oldest version we support though. If we want testing of the latest, I would much rather we do it via having two examples with different minimum iOS versions. One can start build-only, and then we can add tests if it turns out there are specific behavioral differences we want to test.

[reidbaker]

Note from triage: @Rexios80 is creating a duplicate example something you are still planning on getting to? We understand if that is not the case and can close this pr to get it off our review queue.

[Rexios80]

@reidbaker I didn't realize we actually wanted to do that. I didn't see any difference running the latest iOS pod, so if we do want to create more examples that should probably be in its own PR.

[stuartmorgan]

For triagers: the current state here is that PR is blocked on https://github.com/flutter/packages/pull/3257#discussion_r1128552963

[stuartmorgan]

Triage update: still blocked on someone having time to investigate https://github.com/flutter/packages/pull/3257#discussion_r1128552963

I haven't had time yet, but it's still on the list to dig into how to resolve this (either through heuristics, or if we determine there's no good heuristic option, via a structure that captures the different input meanings).

[LuisReyes98]

I have been testing this PR in a project of mine Is there a way to know if the condition mentioned in https://github.com/flutter/packages/pull/3257#discussion_r1128552963

regarding this piece of code

radius: kIsWeb
                        ? 10
                        : defaultTargetPlatform == TargetPlatform.android
                            ? 20
                            : 40,
                  )

Is dependent of the device pixel ratio? MediaQuery.of(context).devicePixelRatio;

I have been testing in android and web in my project using those values to multiply the radius or the devicePixelRatio gives me similar results

static int calculateRadiusMagnitude({
    required double radius,
    required BuildContext context,
  }) {
    double devicePixelRatio = MediaQuery.of(context).devicePixelRatio;
    debugPrint('devicePixelRatio: $devicePixelRatio');
    int magnitude = kIsWeb
        ? 1
        : defaultTargetPlatform == TargetPlatform.android
            ? 2
            : 4;

    return (radius * devicePixelRatio).toInt();
  }

When creating custom markers for google maps I also have to use devicePixelRatio to get platform accurate size, so I thought this could be the same case. For my project it gives me results that are visually good enough.

What would be the best way to test this and find out if it is indeed the devicePixelRatio the reason that it varies in the platforms? And if using makes the heatmap be identical in all platforms?

[Rexios80]

Did you try devices of the same platform with different pixel ratios to make sure it actually behaves the same? That's the issue I'm having.

[camsim99]

@Rexios80 What is the state of this PR?

[Rexios80]

@camsim99 https://github.com/flutter/packages/pull/3257#issuecomment-1854191530

[stuartmorgan]

See https://github.com/flutter/packages/pull/3257#discussion_r1528851122 for a suggestion on how to unblock this.

[Rexios80]

@stuartmorgan Alright let's review this again

[vashworth]

cc @hellohuanlin

[Rexios80]

I'm waiting for more reviews to come in before fixing any issues but it's been a while without any. Are more reviews coming?

[trevor-nomadik]

The heatmaps are not buildable without the radius, which is defined (even if it does nothing), however every time I try to use it with the heatmap it throws an undefined specifically for the HeatmapRadius. The Heatmap and even the HeatmapId work fine. Attempting to define it myself suddenly lets the compiler know that it is actually defined already, but as soon I remove my definition it goes back to not being defined.

The method 'HeatmapRadius' isn't defined for the type 'ReportsModel'. Try correcting the name to the name of an existing method, or defining a method named 'HeatmapRadius'.

return Heatmap(heatmapId: const HeatmapId("debris"), data: data, radius: HeatmapRadius(radius: 10));

/// A wrapper around platform specific behavior for the radius of a [Heatmap].
///
/// Currently this class does nothing. See https://github.com/flutter/flutter/issues/145411
@immutable
class HeatmapRadius {
  const HeatmapRadius._(this.radius);

  /// Create a [HeatmapRadius] with a radius in pixels.
  ///
  /// This value will be used verbatim on all platforms. It is up to the
  /// developer to ensure that the value is appropriate for the platform.
  const HeatmapRadius.fromPlatformSpecificValue(int radius) : this._(radius);

  /// Create a [HeatmapRadius] from a platform value.
  ///
  /// In the future, this will do the opposite conversion that [pixels] does.
  const HeatmapRadius.fromJson(dynamic json) : this._(json as int);

  /// The platform-independent value of the radius.
  final int radius;

  /// The radius in pixels derived from [radius].
  ///
  /// In the future, this will convert [radius] to the current platform's
  /// expected value.
  int get pixels => radius;

  @override
  bool operator ==(Object other) {
    if (identical(this, other)) {
      return true;
    }
    if (other.runtimeType != runtimeType) {
      return false;
    }
    return other is HeatmapRadius && other.radius == radius;
  }

  @override
  int get hashCode => radius.hashCode;
}

I can get it working by just setting a default value for radius in the Heatmap class and removing the required keyword for what its worth.

[Rexios80]

@trevor-nomadik There is no un-named constructor for the HeatmapRadius class. Thus HeatmapRadius(radius: 10) is invalid code.

[vishaldhaduk9986]

Any update on this?

[Rexios80]

https://github.com/googlemaps/google-maps-ios-utils/issues/472

Getting no response in the google-maps-ios-utils repo on its status which isn't great...

@stuartmorgan I see that this PR has been split into it's separate platform pieces and is just blocked on merging the iOS changes. Should we go ahead and do that here?

I'm also thinking about what to do about that pod... Is there any way we can escalate this internally? Or as a last resort is forking the repo and updating it ourselves feasible?

[stuartmorgan]

@stuartmorgan I see that this PR has been split into it's separate platform pieces and is just blocked on merging the iOS changes. Should we go ahead and do that here?

Once this is fully reviewed and approved, we can start landing components, yes. (And eventually in both cases we could potentially reach a point where we land the app-facing changes with the feature marked as Android-only for now, but I would strongly prefer to avoid that.)

I'm also thinking about what to do about that pod... Is there any way we can escalate this internally?

If by escalate you mean informing the relevant people that there are google_maps_flutter PRs blocked on https://github.com/googlemaps/google-maps-ios-utils/issues/456, that has already happened internally. If you mean anything beyond that, the Flutter team does not control the priorities of other teams.

Or as a last resort is forking the repo and updating it ourselves feasible?

Maintaining Google Maps SDK features for iOS is not in scope for the Flutter team. The google_maps_flutter plugin is intended as a wrapper around the native first-party Google Maps SDKs, and will only support features that those SDKs support.

[Rexios80]

Good news! We got a response on the status of the iOS utils package:

• https://github.com/googlemaps/google-maps-ios-utils/issues/473

For anyone tracking this, WASM compatibility is taking priority:

• https://github.com/flutter/packages/pull/7077

I also have a lot of other work coming my way soon, and this PR needs a large migration. Pigeon is the big one. The google_maps: 8.0.0 one should be easy 🤞. I'll try my best to get the reviews dealt with within a reasonable timeframe! If anyone would like to attempt those migrations, please let me know here.

[stuartmorgan]

this PR needs a large migration. Pigeon is the big one

What Pigeon migration does it still need? It looks like you've updated the calls to use Pigeon, and per my comment earlier it's fine to land this without updating the JSON in the object class.

[stuartmorgan]

Once this is fully reviewed and approved, we can start landing components, yes.

It looks like once the last round of comments from @reidbaker and I are addressed, this will probably be ready to break up into sub-PRs 🎉

[reidbaker]

Once this is fully reviewed and approved, we can start landing components, yes.

It looks like once the last round of comments from @reidbaker and I are addressed, this will probably be ready to break up into sub-PRs 🎉

I agree and I would like to reiterate my appreciation for your dedication to getting this feature landed.

[Rexios80]

What Pigeon migration does it still need? It looks like you've updated the calls to use Pigeon, and per my comment earlier it's fine to land this without updating the JSON in the object class.

Oh I misread what you said earlier. If you want to land this without those changes then that's fine with me. I thought there would be a bunch of work to merge with main since there are conflicts in native code files, but it actually doesn't look that bad.

@stuartmorgan Should we create an issue to update the JSON conversion code? I'm not really sure how to express what needs done so can you create the issue if we need it?

[stuartmorgan]

If you want to land this without those changes then that's fine with me.

Yes, I'm very much in favor of landing as-is; I don't want to saddle this existing PR with a large amount of extra work just because it was blocked on the radius issue for so long.

@stuartmorgan Should we create an issue to update the JSON conversion code? I'm not really sure how to express what needs done so can you create the issue if we need it?

It'll just be covered by the existing https://github.com/flutter/flutter/issues/117907, which has tracking for updating the individual map objects from JSON to typed structures.

[Rexios80]

@reidbaker @stuartmorgan finally all checks passed! This is ready for another look.