fix: Use 32bit Vector2 in Flame to be compatible with forge2d, flame_forge2d [draft]

[zeyus]

Description

This fixes a problem where, if you use imports from both flame, and flame_forge2d that use Vector2 objects, the app will fail to compile (in addition to showing errors in the IDE) due to flame using vector_math_64.dart and forge2d / flame_forge2d using vector_math.dart). This occurs because of the merged PR: flame-engine/forge2d#96

import 'package:flame/components.dart';
import 'package:flame_forge2d/flame_forge2d.dart';

class Ball extends BodyComponent<RiseTogetherGame> with ContactCallbacks {
  bool isMoving = false;
  bool isRising = false;
  final double radius;

  Ball({required this.radius, super.paint, Vector2? pos})
      : startPosition = pos != null ? Vector2.copy(pos) : Vector2.zero(), // <---- Error here
        super();
}

I would have kept this in a local branch but I saw at least one issue referencing this (including the Flame 2 issue): #3406 and #1938

Fixes #3406

I'm not sure if this change breaks anything, I'm currently setting up / running the test suite, which so far is going fine, but there may be an issue with the PNG transform, perhaps someone else knows far better than I do.

I'll gladly add some tests for the pool, but before I want to do any work I would appreciate some feedback on:

a) if there is any chance this will be merged
b) if this is even the right approach

Thanks 🪄

Replace this text.

Checklist

• I have followed the Contributor Guide when preparing my PR.
• I have updated/added tests for ALL new/updated/fixed functionality.
• I have updated/added relevant documentation in docs and added dartdoc comments with ///.
• I have updated/added relevant examples in examples or docs.

Breaking Change?

• Yes, this PR is a breaking change. Probably?
• No, this PR is not a breaking change.

Related Issues

[spydon]

These should not be removed, the warnings you are seeing will be removed (and fixed) in the next Flutter release.

[spydon]

This and it's sibling PRs is what we're waiting for:
flutter/flutter#160443

[zeyus]

Right, that makes more sense, I'll revert that commit.

The other test errors seem to be floating point math issues due to the change in precision. I didn't want to just change the tests to make them pass, but if is acceptable, maybe the tolerance can be increased a little

[spydon]

Can you check how much the tolerance would have to change? It should just be the default of the closeToVector matcher (or whatever it is called).

[zeyus]

Of course. I'll have a look when I get to work. I remember seeing something like 1e-10 (maybe it was more) in the test output, and the one fail that stood out to me was the PNG related one...I think I will try and manually inspect the output of that test to see what is going on.

[zeyus]

Can you check how much the tolerance would have to change? It should just be the default of the closeToVector matcher (or whatever it is called).

I'm going through it right now, and the current tolerance is set to 1e-14, and now it is within 1e-6 (actually ~9.5e-7) which looks to me to fit with reduction in precision. I'll adjust the relevant tolerances and see if anything still fails.

For completeness' sake, today I'm working on OSX and yesterday was Windows, and the results are pretty much the same.

[zeyus]

It should just be the default of the closeToVector matcher

So, the default closeTo... delta parameter is 1e-15 which is OK for 64 bit, but now it's less with 32 bit.

I've updated the relevant tests but there are some edge cases here where my intuition is reaching its limits and I don't know enough about how exactly dart handles things. That said, only 9 tests are failing now (huge improvement, still not great)

I'm fairly certain some of the problems are coming from toDouble in the Vector2 extension, because the doubles have 53 bits of precision, and conversion from a 32 bit float to a double will approximate a value...but that's kind of as far as I got with troubleshooting it...

Then there are 2 other "classes" of failures besides the toDouble failures:

• The image related issues (which are no doubt consequences of the change in precision) <-- I checked the output, and you can see that it's clearly 1px off, which is a rounding issue. Here's the isolatedDiff.png
• The "contains" fails, again, it's precision related, because if it checks for <= 32.0 and its 32.00001, then it fails

And a final weird one, that I don't even understand where it's coming from, because in flame/test/components/position_component_test.dart:996:1 the failure shows the two values the same:

00:17 +933 -13: /Users/au662726/github_projects/flame/packages/flame/test/components/position_component_test.dart: PositionComponent Bounding rectangle rotated component [E]                          
  Expected: Rect:<Rect.fromLTRB(-0.3, 0.0, 4.9, 2.8)>
    Actual: Rect:<Rect.fromLTRB(-0.3, 0.0, 4.9, 2.8)>
  
  package:matcher                                      expect
  test/components/position_component_test.dart 996:11  main.<fn>.<fn>.<fn>

Anyway that's where I'm at, and I am happy to dig further, but perhaps you have a suggestion as to how to deal with the float64 / double => float32 handling and rounding.

[spydon]

The image related issues (which are no doubt consequences of the change in precision) <-- I checked the output, and you can see that it's clearly 1px off, which is a rounding issue. Here's the isolatedDiff.png

That is fine, we'll just regenerate the goldens.

The "contains" fails, again, it's precision related, because if it checks for <= 32.0 and its 32.00001, then it fails

I guess we could add the epsilon here too.

And a final weird one, that I don't even understand where it's coming from, because in flame/test/components/position_component_test.dart:996:1 the failure shows the two values the same:

That's a very odd one... No idea what is happening there.

Sorry for the late reply, I thought I already replied.

[zeyus]

Great, I would love to help get this done (I'm currently using this branch in my experiment I'm developing). Please let me know what I can / should do to finish it up.

That's a very odd one... No idea what is happening there.

I figured it out, it's the same issue as the others, but the toString() method rounds the output: pkg/sky_engine/lib/ui/geometry.dart

class Rect {
  // ...
  /// The offset of the left edge of this rectangle from the x axis.
  final double left;

  /// The offset of the top edge of this rectangle from the y axis.
  final double top;

  /// The offset of the right edge of this rectangle from the x axis.
  final double right;

  /// The offset of the bottom edge of this rectangle from the y axis.
  final double bottom;

  // ...

  @override
  bool operator ==(Object other) {
    if (identical(this, other)) {
      return true;
    }
    if (runtimeType != other.runtimeType) {
      return false;
    }
    return other is Rect &&
        other.left == left &&
        other.top == top &&
        other.right == right &&
        other.bottom == bottom;
  }

  @override
  int get hashCode => Object.hash(left, top, right, bottom);

  @override
  String toString() =>
      'Rect.fromLTRB(${left.toStringAsFixed(1)}, ${top.toStringAsFixed(1)}, ${right.toStringAsFixed(1)}, ${bottom.toStringAsFixed(1)})';

So despite it showing the same value in the output, the double values are not identical.

[spydon]

I updated the goldens here: 413282b
To update them you just need to run melos update-goldens, the problem is that it needs to be done on a linux machine, since there are small discrepancies between the platforms in Flutter, and our pipeline is running Linux.

I figured it out, it's the same issue as the others, but the toString() method rounds the output: pkg/sky_engine/lib/ui/geometry.dart

Aah, that makes sense!