Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions packages/flame/lib/src/collisions/broadphase/sweep/sweep.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,21 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {

@override
void update() {
items.sort((a, b) => a.aabb.min.x.compareTo(b.aabb.min.x));
// Between two ticks the hitboxes only move a little, so [items] is
// always nearly sorted: an insertion sort runs in close to linear time
// here, where a general-purpose sort would pay its full O(n log n) on
// every tick. This also avoids allocating a comparator closure per tick.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd much rather we extracted it to a commons sort util, I know we'd still get the tear-off, but the main benefit is the sort algorithm change anyway? maybe we can use the prefer-inline pragma? or how about creating our own simple data structure class instead of List that wraps the sort and the remove, the comparator can be set at list creator level, no per-tick tear-off

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

honestly compared to the actual sorting operation (even in the linear case) the comparator tear-off (once per tick) does not feel significant

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

actually I think the tearoff trick from your other PRs will pair really nicely hear, you don't need the closure at all!

like int compareAabbs(aabb1, aabb2) => ...
and whateverSort(list, compareAabbs)

final items = this.items;
for (var i = 1; i < items.length; i++) {
final item = items[i];
final minX = item.aabb.min.x;
var previousIndex = i - 1;
while (previousIndex >= 0 && items[previousIndex].aabb.min.x > minX) {
items[previousIndex + 1] = items[previousIndex];
previousIndex--;
}
items[previousIndex + 1] = item;
}
}

@override
Expand All @@ -44,7 +58,10 @@ class Sweep<T extends Hitbox<T>> extends Broadphase<T> {
_prospectPool.acquire(item, activeItem);
}
} else {
_active.remove(activeItem);
// The order of the active list does not matter, so the removal can
// swap in the last element instead of searching and shifting.
_active[i] = _active.last;
_active.removeLast();
}
}
_active.add(item);
Expand Down
Loading