dmah42 / dart-box2d

Box2D for Dart
http://dart-lang.github.io/dart-box2d/
34 stars 5 forks source link

[WIP] Use vector_maths Aabb2 #21

Closed laszlokorte closed 11 years ago

laszlokorte commented 11 years ago

AxisAlignBox is removed and vector_maths Aabb2 is used instead.

Performance: https://gist.github.com/laszlokorte/5974467

As I commited Aabb2 into vector_math just a few days ago it is not in the official version yet. So for now I have changed the pubspec to use vector_maths master branch. I assume we wait until the next release until before merging this.

One odd thing I noticed is that the collision behavior very slightly. For example in the blob_example demo before this change the dropping box ends up resting on the right side but after this change it ends up resting on the left side.

I spent several hours setting breakpoints and looking at the AAbb2 positions in each computation step and came to the conclusion that it seems to be a precision/rounding error of the doubles and floats.

Update: It was not about rounding errors but me introducing a change in the Aabb2#contains method.

In line 218 in polygon_shape.dart:

argAabb.min.x = lower.x - radius;

In one computation step lower.x is -50.0 and radius is 0.01. Before this change argAabb.min.x results to be exactly -50.01 and after this change its -50.00999999977648.

I could reproduce this behavior with:

Float32List l = new Float32List(2);
l[0] = -50.0;
l[1] = 0.01;

Float32List l2 = new Float64List(2);
l2[0] = -50.0;
l2[1] = 0.01;

double a = -50.0;
double b = 0.01;

double r = a-b; // -50.01
double r2 = l[0] - l[1]; //-50.00999999977648
double r3 = l[0] - l[1]; //-50.01

But I do not know why these commits have any effect like this since it does not change anything regarding the storage type. (The old AxisAlignedBox used Vector2 internally too). All this happens already after commit 3513014 where I still have kept most of the old axisalignedbox function implementations als legacy code.

dmah42 commented 11 years ago

This actually does break one of the demos: DominoTower is no longer able to stand up before it's hit. I'd rather not merge this until understanding where the accuracy differences are coming from.

laszlokorte commented 11 years ago

Yeah I know :(

laszlokorte commented 11 years ago

I found the error. I made a mistake when porting the Aabb2#contains() method over to vector_math. https://github.com/johnmccutchan/vector_math/pull/30 (is merged now)

dmah42 commented 11 years ago

I'm still seeing the same behaviour for blob and domino-tower with johnmccutchan/vector_math#30.

laszlokorte commented 11 years ago

Ok very strange. I do not know why it was working for me. The method "contains" was misnamed in the original implementation. a.contains(b) did not return if a contains b but if b contains a.

dmah42 commented 11 years ago

it's possible I merged the change in badly. I'll try to grab it again.

Dominic Hamon | Google There are no bad ideas; only good ideas that go horribly wrong.

On Thu, Jul 11, 2013 at 12:48 PM, Laszlo Korte notifications@github.comwrote:

Ok very strange. I do not know why it was working for me. The method "contains" was misnamed in the original implementation. a.contains(b) did not return if a contains b but if b contains a.

— Reply to this email directly or view it on GitHubhttps://github.com/dart-lang/dart-box2d/pull/21#issuecomment-20837277 .

dmah42 commented 11 years ago

As with all things, it was a caching problem. Will merge when johnmccutchan/vector_math next releases.

johnmccutchan commented 11 years ago

pub now has vector_math 1.3.3 which includes Laszlo's AABB fix.

dmah42 commented 11 years ago

manual merge in progress (to revert the pubspec.yaml change)

johnmccutchan commented 11 years ago

@laszlokorte

Does vector_math's a.contains(b) return true when b is inside a or when a is inside b?

laszlokorte commented 11 years ago

Vector maths a.contains(b) returns true if b is inside a. The doc comment of the old box2d's contains() said the same but the method itself did the opposite.