Skip to content
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

[google_maps_flutter_platform_interface] Convert BitmapDescriptor to typesafe subclasses #7699

Merged
merged 15 commits into from
Oct 4, 2024
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.9.5

* Converts `BitmapDescriptor` to typesafe structures.

## 2.9.4

* Converts `PatternItem` to typesafe structure.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import 'dart:async' show Future;
import 'dart:typed_data' show Uint8List;

import 'package:flutter/foundation.dart' show kIsWeb;
import 'package:flutter/foundation.dart' show kIsWeb, visibleForTesting;
import 'package:flutter/material.dart'
show
AssetBundleImageKey,
Expand All @@ -29,6 +29,14 @@ enum MapBitmapScaling {
none,
}

/// Convert a string from provided JSON to a MapBitmapScaling enum.
@visibleForTesting
MapBitmapScaling mapBitmapScalingFromString(String mode) => switch (mode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be private so we're not increasing the API surface of the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be visible for testing and keep the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be my thought as well, @stuartmorgan do you agree with this decision?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's fine; from a semver standpoint we consider those equivalent.

'auto' => MapBitmapScaling.auto,
'none' => MapBitmapScaling.none,
_ => throw ArgumentError('Unrecognized MapBitmapScaling $mode', 'mode'),
};

// The default pixel ratio for custom bitmaps.
const double _naturalPixelRatio = 1.0;

Expand All @@ -44,14 +52,14 @@ const double _naturalPixelRatio = 1.0;
/// a default marker icon.
/// Use the [BitmapDescriptor.defaultMarkerWithHue] to create a
/// [BitmapDescriptor] for a default marker icon with a hue value.
class BitmapDescriptor {
const BitmapDescriptor._(this._json);
abstract class BitmapDescriptor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was a breaking change but a spot check of the public usages in github show I am incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only ways to construct a BitmapDescriptor without constructing one of its subclasses was either through this private _ factory, which is removed here, or the below fromJson public factory, which this PR converts to a static method, so unless we are expecting public usages of this package to add new subclasses of BitmapDescriptor, I believe that neither making it abstract nor removing the _ factory should impact any public uses.

const BitmapDescriptor._();

/// The inverse of .toJson.
// TODO(stuartmorgan): Remove this in the next breaking change.
@Deprecated('No longer supported')
BitmapDescriptor.fromJson(Object json) : _json = json {
assert(_json is List<dynamic>);
static BitmapDescriptor fromJson(Object json) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the : _json syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR removes the _json field from BitmapDescriptor, and since it also makes BitmapDescriptor an abstract class with a concrete subtype for each possible set of member data, fromJson needs to return an instance of one of those subtypes.

assert(json is List<dynamic>);
final List<dynamic> jsonList = json as List<dynamic>;
assert(_validTypes.contains(jsonList[0]));
switch (jsonList[0]) {
Expand All @@ -61,19 +69,25 @@ class BitmapDescriptor {
assert(jsonList[1] is num);
final num secondElement = jsonList[1] as num;
assert(0 <= secondElement && secondElement < 360);
return DefaultMarker(hue: secondElement);
}
return const DefaultMarker();
case _fromBytes:
assert(jsonList.length == 2);
assert(jsonList[1] != null && jsonList[1] is List<int>);
assert((jsonList[1] as List<int>).isNotEmpty);
return BytesBitmap(byteData: jsonList[1] as Uint8List);
case _fromAsset:
assert(jsonList.length <= 3);
assert(jsonList[1] != null && jsonList[1] is String);
assert((jsonList[1] as String).isNotEmpty);
if (jsonList.length == 3) {
assert(jsonList[2] != null && jsonList[2] is String);
assert((jsonList[2] as String).isNotEmpty);
return AssetBitmap(
name: jsonList[1] as String, package: jsonList[2] as String);
}
return AssetBitmap(name: jsonList[1] as String);
case _fromAssetImage:
assert(jsonList.length <= 4);
assert(jsonList[1] != null && jsonList[1] is String);
Expand All @@ -82,7 +96,15 @@ class BitmapDescriptor {
if (jsonList.length == 4) {
assert(jsonList[3] != null && jsonList[3] is List<dynamic>);
assert((jsonList[3] as List<dynamic>).length == 2);
final List<dynamic> sizeList = jsonList[3] as List<dynamic>;
return AssetImageBitmap(
name: jsonList[1] as String,
scale: jsonList[2] as double,
size: Size((sizeList[0] as num).toDouble(),
(sizeList[1] as num).toDouble()));
}
return AssetImageBitmap(
name: jsonList[1] as String, scale: jsonList[2] as double);
case AssetMapBitmap.type:
assert(jsonList.length == 2);
assert(jsonList[1] != null && jsonList[1] is Map<String, dynamic>);
Expand All @@ -96,6 +118,16 @@ class BitmapDescriptor {
assert(jsonMap['imagePixelRatio'] is double);
assert(!jsonMap.containsKey('width') || jsonMap['width'] is double);
assert(!jsonMap.containsKey('height') || jsonMap['height'] is double);
final double? width =
jsonMap.containsKey('width') ? jsonMap['width'] as double : null;
final double? height =
jsonMap.containsKey('height') ? jsonMap['height'] as double : null;
return AssetMapBitmap(jsonMap['assetName'] as String,
bitmapScaling:
mapBitmapScalingFromString(jsonMap['bitmapScaling'] as String),
imagePixelRatio: jsonMap['imagePixelRatio'] as double,
width: width,
height: height);
case BytesMapBitmap.type:
assert(jsonList.length == 2);
assert(jsonList[1] != null && jsonList[1] is Map<String, dynamic>);
Expand All @@ -109,9 +141,20 @@ class BitmapDescriptor {
assert(jsonMap['imagePixelRatio'] is double);
assert(!jsonMap.containsKey('width') || jsonMap['width'] is double);
assert(!jsonMap.containsKey('height') || jsonMap['height'] is double);
final double? width =
jsonMap.containsKey('width') ? jsonMap['width'] as double : null;
final double? height =
jsonMap.containsKey('height') ? jsonMap['height'] as double : null;
return BytesMapBitmap(jsonMap['byteData'] as Uint8List,
bitmapScaling:
mapBitmapScalingFromString(jsonMap['bitmapScaling'] as String),
width: width,
height: height,
imagePixelRatio: jsonMap['imagePixelRatio'] as double);
default:
break;
}
throw ArgumentError('Unrecognized BitmapDescriptor type ${jsonList[0]}');
}

static const String _defaultMarker = 'defaultMarker';
Expand Down Expand Up @@ -160,15 +203,14 @@ class BitmapDescriptor {
static const double hueRose = 330.0;

/// Creates a BitmapDescriptor that refers to the default marker image.
static const BitmapDescriptor defaultMarker =
BitmapDescriptor._(<Object>[_defaultMarker]);
static const BitmapDescriptor defaultMarker = DefaultMarker();

/// Creates a BitmapDescriptor that refers to a colorization of the default
/// marker image. For convenience, there is a predefined set of hue values.
/// See e.g. [hueYellow].
static BitmapDescriptor defaultMarkerWithHue(double hue) {
assert(0.0 <= hue && hue < 360.0);
return BitmapDescriptor._(<Object>[_defaultMarker, hue]);
return DefaultMarker(hue: hue);
}

/// Creates a [BitmapDescriptor] from an asset image.
Expand All @@ -189,27 +231,17 @@ class BitmapDescriptor {
}) async {
final double? devicePixelRatio = configuration.devicePixelRatio;
if (!mipmaps && devicePixelRatio != null) {
return BitmapDescriptor._(<Object>[
_fromAssetImage,
assetName,
devicePixelRatio,
]);
return AssetImageBitmap(name: assetName, scale: devicePixelRatio);
}
final AssetImage assetImage =
AssetImage(assetName, package: package, bundle: bundle);
final AssetBundleImageKey assetBundleImageKey =
await assetImage.obtainKey(configuration);
final Size? size = configuration.size;
return BitmapDescriptor._(<Object>[
_fromAssetImage,
assetBundleImageKey.name,
assetBundleImageKey.scale,
if (kIsWeb && size != null)
<Object>[
size.width,
size.height,
],
]);
final Size? size = kIsWeb ? configuration.size : null;
return AssetImageBitmap(
name: assetBundleImageKey.name,
scale: assetBundleImageKey.scale,
size: size);
}

/// Creates a BitmapDescriptor using an array of bytes that must be encoded
Expand All @@ -222,15 +254,7 @@ class BitmapDescriptor {
static BitmapDescriptor fromBytes(Uint8List byteData, {Size? size}) {
assert(byteData.isNotEmpty,
'Cannot create BitmapDescriptor with empty byteData');
return BitmapDescriptor._(<Object>[
_fromBytes,
byteData,
if (kIsWeb && size != null)
<Object>[
size.width,
size.height,
]
]);
return BytesBitmap(byteData: byteData, size: size);
}

/// Creates a [BitmapDescriptor] from an asset using [AssetMapBitmap].
Expand Down Expand Up @@ -306,10 +330,100 @@ class BitmapDescriptor {
);
}

final Object _json;

/// Convert the object to a Json format.
Object toJson() => _json;
Object toJson();
}

/// A BitmapDescriptor using the default marker.
class DefaultMarker extends BitmapDescriptor {
/// Provide an optional [hue] for the default marker.
const DefaultMarker({this.hue}) : super._();

/// Optional hue of the colorization of the default marker.
final num? hue;

@override
Object toJson() => (hue == null)
? const <Object>[BitmapDescriptor._defaultMarker]
: <Object>[BitmapDescriptor._defaultMarker, hue!];
}

/// A BitmapDescriptor using an array of bytes that must be encoded
/// as PNG.
@Deprecated('Use BytesMapBitmap instead')
class BytesBitmap extends BitmapDescriptor {
/// On the web, the [size] parameter represents the *physical size* of the
/// bitmap, regardless of the actual resolution of the encoded PNG.
/// This helps the browser to render High-DPI images at the correct size.
/// `size` is not required (and ignored, if passed) in other platforms.
@Deprecated('Use BytesMapBitmap instead')
const BytesBitmap({required Uint8List byteData, Size? size})
: this._(byteData, kIsWeb ? size : null);

@Deprecated('Use BytesMapBitmap instead')
const BytesBitmap._(this.byteData, this.size) : super._();

/// Array of bytes encoding a PNG.
final Uint8List byteData;

/// On web, the physical size of the bitmap. Null on all other platforms.
final Size? size;

@override
Object toJson() => <Object>[
BitmapDescriptor._fromBytes,
byteData,
if (size != null) <Object>[size!.width, size!.height]
];
}

/// A bitmap specified by a name and optional package.
class AssetBitmap extends BitmapDescriptor {
/// Provides an asset name with [name] and optionally a [package].
const AssetBitmap({required this.name, this.package}) : super._();

/// Name of the asset backing the bitmap.
final String name;

/// Optional package of the asset.
final String? package;

@override
Object toJson() => <Object>[
BitmapDescriptor._fromAsset,
name,
if (package != null) package!
];
}

/// A [BitmapDescriptor] from an asset image.
@Deprecated('Use AssetMapBitmap instead')
class AssetImageBitmap extends BitmapDescriptor {
/// Creates a [BitmapDescriptor] from an asset image with specified [name] and [scale], and an optional [size].
/// Asset images in flutter are stored per:
/// https://flutter.dev/to/resolution-aware-images
/// This method takes into consideration various asset resolutions
/// and scales the images to the right resolution depending on the dpi.
@Deprecated('Use AssetMapBitmap instead')
const AssetImageBitmap({required this.name, required this.scale, this.size})
: super._();

/// Name of the image asset.
final String name;

/// Scaling factor for the asset image.
final double scale;

/// Size of the image if using mipmaps.
final Size? size;

@override
Object toJson() => <Object>[
BitmapDescriptor._fromAssetImage,
name,
scale,
if (size != null) <Object>[size!.width, size!.height]
];
}

/// Represents a [BitmapDescriptor] base class for map bitmaps.
Expand Down Expand Up @@ -344,7 +458,7 @@ abstract class MapBitmap extends BitmapDescriptor {
required this.imagePixelRatio,
this.width,
this.height,
}) : super._(const <Object>[]);
}) : super._();

/// The scaling method of the bitmap.
final MapBitmapScaling bitmapScaling;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ repository: https://github.com/flutter/packages/tree/main/packages/google_maps_f
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 2.9.4
version: 2.9.5

environment:
sdk: ^3.3.0
Expand Down
Loading