[BUG] CustomPoint has potential cast exception at runtime

[josxha]

What is the bug?

While working on greensopinion/flutter-vector-map-tiles#149 i discovered that the current CustomPoint implementation lead to a cast exception at runtime.

This behaviour is documented in the parent class math.Point but isn't mentioned in CustomPoint:

/// Important Note: This function accepts a num as its argument only so
/// that you can scale Point<double> objects by an int factor. Because the
/// * operator always returns the same type of Point as it is called on,
/// passing in a double [factor] on a Point<int> causes a
/// runtime error.

How can we reproduce it?

final pDouble = CustomPoint<double>(1, 2);
final pInt = CustomPoint<int>(3,4);
var point;

point = pInt + pDouble; // runtime exception
point = pInt + pDouble; // runtime exception
point = pInt * pDouble; // runtime exception
point = pInt / pDouble; // runtime exception

Potentially this would throw an exception too:

point = CustomPoint<int, int>(5, 5) / CustomPoint<int, int>(2, 2); // runtime exception

Do you have a potential solution?

The issue is that other may be a num or double and turns the result into a double while the function casts it to an int.

For example here:

  /// Create new [CustomPoint] where [x] and [y] values are added to [other]
  /// point [x] and [y] values
  @override
  CustomPoint<T> operator +(math.Point other) {
    return CustomPoint<T>((x + other.x) as T, (y + other.y) as T);
  }

Possible solutions:

• Add documentation to the dangerous functions like it's done in math.Point. This could lead to potential errors for users but might not be that problematic since CustomPoint is mostly used internally or by plugin developers.
• force other to be the same type as the CustomPoint instance. This requires transforming every working double/int compination to be converted to a double/double.
• Use named functions like:

CustomPoint<T> multiplyWithInt(CustomPoint<int> other) {} // int/int, double/int
CustomPoint<double> multiplyWithDouble(CustomPoint<double> other) {} // double/double, int/double

Platforms

tested on android but should

Severity

Erroneous: Prevents normal functioning and causes errors in the console

[josxha]

The flutter_map implementation had this issue at some point too and used a workaround:

final tileZoomDouble = tileZoom.toDouble();
final scale = mapState.getZoomScale(viewingZoom, tileZoomDouble);
final pixelCenter =
    mapState.project(center, tileZoomDouble).floor().toDoublePoint();

Jump to code

I don't think that this is the right solution. I can't see why .floor() is necessary in the first hand.

[ibrierley]

Looks like that was introduced as part of a larger rewrite on #572 and it's not clear (is it ever :D)

Just be aware, sometimes the floors and things like that are semi fudges where oddities get introduced like hairline white gaps between tiles and things like that, so double check all that kind of stuff if playing with it.

[josxha]

Thanks for the explaination of the floor() usage. (: