From 3037def78b9e185189e4ed92b087a3f8b0ece8f3 Mon Sep 17 00:00:00 2001 From: Lukas Klingsbo Date: Mon, 19 Oct 2026 10:08:27 +0200 Subject: [PATCH] fix: Freeze the length unit across isolates The flag that freezes the length unit once a world exists was a Dart static, so each isolate had its own copy while the length unit itself is process-wide. The native backend now keeps the flag next to the world lock and checks it under that lock, and the WebAssembly backend keeps it in the backend instance. --- packages/forge2d/lib/src/api/world.dart | 2 - .../forge2d/lib/src/backend/raw_box2d.dart | 12 +++-- .../lib/src/backend/raw_box2d_ffi.dart | 7 ++- .../lib/src/backend/raw_box2d_wasm.dart | 44 +++++++++++++------ .../forge2d/lib/src/ffi/f2d_world_lock.dart | 14 ++++++ packages/forge2d/lib/src/initialize.dart | 15 ++----- packages/forge2d/native/ffi/f2d_world_lock.c | 27 ++++++++++-- packages/forge2d/test/api/isolates_test.dart | 20 +++++++++ .../forge2d/test/api/length_unit_test.dart | 6 ++- 9 files changed, 110 insertions(+), 37 deletions(-) diff --git a/packages/forge2d/lib/src/api/world.dart b/packages/forge2d/lib/src/api/world.dart index fa9be2f0..7f454d77 100644 --- a/packages/forge2d/lib/src/api/world.dart +++ b/packages/forge2d/lib/src/api/world.dart @@ -53,8 +53,6 @@ class World { 'Box2D supports at most 128 worlds at a time; destroy one first.', ); } - // Freezes the length unit: it is baked into this world's tolerances. - markWorldCreated(); } /// The packed native world id. diff --git a/packages/forge2d/lib/src/backend/raw_box2d.dart b/packages/forge2d/lib/src/backend/raw_box2d.dart index 0f94f659..2d80461a 100644 --- a/packages/forge2d/lib/src/backend/raw_box2d.dart +++ b/packages/forge2d/lib/src/backend/raw_box2d.dart @@ -10,11 +10,15 @@ abstract interface class RawBox2D { // Global tuning. - /// Sets the process-wide length unit, mirroring `b2SetLengthUnitsPerMeter`. + /// Sets the process-wide length unit, mirroring `b2SetLengthUnitsPerMeter`, + /// unless a world has been created in this process. /// - /// Box2D must not have been called before this, which the API layer - /// enforces in `initializeForge2D`. - void setLengthUnitsPerMeter(double lengthUnits); + /// Returns whether the length unit was set. + bool trySetLengthUnitsPerMeter(double lengthUnits); + + /// Forgets that a world has been created, so that + /// [trySetLengthUnitsPerMeter] can succeed again. + void resetLengthUnitLock(); /// The process-wide length unit, mirroring `b2GetLengthUnitsPerMeter`. double getLengthUnitsPerMeter(); diff --git a/packages/forge2d/lib/src/backend/raw_box2d_ffi.dart b/packages/forge2d/lib/src/backend/raw_box2d_ffi.dart index 4a3f5c8b..1f8d1197 100644 --- a/packages/forge2d/lib/src/backend/raw_box2d_ffi.dart +++ b/packages/forge2d/lib/src/backend/raw_box2d_ffi.dart @@ -65,8 +65,11 @@ final class RawBox2DFfi implements RawBox2D { // Global tuning. @override - void setLengthUnitsPerMeter(double lengthUnits) => - b2.b2SetLengthUnitsPerMeter(lengthUnits); + bool trySetLengthUnitsPerMeter(double lengthUnits) => + f2d.f2dSetLengthUnitsPerMeterLocked(lengthUnits); + + @override + void resetLengthUnitLock() => f2d.f2dResetLengthUnitLock(); @override double getLengthUnitsPerMeter() => b2.b2GetLengthUnitsPerMeter(); diff --git a/packages/forge2d/lib/src/backend/raw_box2d_wasm.dart b/packages/forge2d/lib/src/backend/raw_box2d_wasm.dart index 79ff839b..f36ebfb2 100644 --- a/packages/forge2d/lib/src/backend/raw_box2d_wasm.dart +++ b/packages/forge2d/lib/src/backend/raw_box2d_wasm.dart @@ -47,6 +47,8 @@ final class RawBox2DWasm implements RawBox2D { final WasmRuntime _runtime; + bool _worldHasBeenCreated = false; + // Scratch arena layout (see WasmRuntime.scratch, 4096 bytes): // - byte 0: small out-parameters (up to 128 bytes) // - byte 128: the f2dShapeDef struct (19 * 4 bytes) @@ -114,8 +116,16 @@ final class RawBox2DWasm implements RawBox2D { // Global tuning. @override - void setLengthUnitsPerMeter(double lengthUnits) => - _call('f2d_set_length_units_per_meter', [lengthUnits]); + bool trySetLengthUnitsPerMeter(double lengthUnits) { + if (_worldHasBeenCreated) { + return false; + } + _call('f2d_set_length_units_per_meter', [lengthUnits]); + return true; + } + + @override + void resetLengthUnitLock() => _worldHasBeenCreated = false; @override double getLengthUnitsPerMeter() => @@ -135,18 +145,24 @@ final class RawBox2DWasm implements RawBox2D { required double maximumLinearSpeed, required bool enableSleep, required bool enableContinuous, - }) => _callI('f2d_create_world', [ - gravityX, - gravityY, - restitutionThreshold, - hitEventThreshold, - contactHertz, - contactDampingRatio, - maxContactPushSpeed, - maximumLinearSpeed, - _b(enableSleep), - _b(enableContinuous), - ]); + }) { + final id = _callI('f2d_create_world', [ + gravityX, + gravityY, + restitutionThreshold, + hitEventThreshold, + contactHertz, + contactDampingRatio, + maxContactPushSpeed, + maximumLinearSpeed, + _b(enableSleep), + _b(enableContinuous), + ]); + if (id != 0) { + _worldHasBeenCreated = true; + } + return id; + } @override void destroyWorld(int worldId) => _call('f2d_destroy_world', [worldId]); diff --git a/packages/forge2d/lib/src/ffi/f2d_world_lock.dart b/packages/forge2d/lib/src/ffi/f2d_world_lock.dart index a69398c2..e94261f0 100644 --- a/packages/forge2d/lib/src/ffi/f2d_world_lock.dart +++ b/packages/forge2d/lib/src/ffi/f2d_world_lock.dart @@ -18,3 +18,17 @@ external b2WorldId f2dCreateWorldLocked( /// `b2DestroyWorld` under the same lock as [f2dCreateWorldLocked]. @ffi.Native(symbol: 'f2d_destroy_world_locked') external void f2dDestroyWorldLocked(b2WorldId id); + +/// `b2SetLengthUnitsPerMeter` under the same lock as [f2dCreateWorldLocked], +/// refused once any isolate has created a world. +/// +/// Returns whether the length unit was set. +@ffi.Native( + symbol: 'f2d_set_length_units_per_meter_locked', +) +external bool f2dSetLengthUnitsPerMeterLocked(double lengthUnits); + +/// Forgets that a world has been created, so that the length unit can be set +/// again. +@ffi.Native(symbol: 'f2d_reset_length_unit_lock') +external void f2dResetLengthUnitLock(); diff --git a/packages/forge2d/lib/src/initialize.dart b/packages/forge2d/lib/src/initialize.dart index c3898757..cc840864 100644 --- a/packages/forge2d/lib/src/initialize.dart +++ b/packages/forge2d/lib/src/initialize.dart @@ -10,13 +10,6 @@ RawBox2D? _rawBox2D; /// Internal to forge2d; not exported by the package. RawBox2D get rawBox2D => _rawBox2D ??= createRawBox2D(); -bool _worldHasBeenCreated = false; - -/// Records that a world exists, which freezes the length unit. -/// -/// Internal to forge2d; not exported by the package. -void markWorldCreated() => _worldHasBeenCreated = true; - /// Rounds [value] the way Box2D stores it, so that a length unit that was /// handed to the native side compares equal when it is read back. double _toFloat32(double value) => (Float32List(1)..[0] = value)[0]; @@ -69,10 +62,11 @@ Future initializeForge2D({ if (current == _toFloat32(lengthUnitsPerMeter)) { return; } - if (_worldHasBeenCreated) { + if (!backend.trySetLengthUnitsPerMeter(lengthUnitsPerMeter)) { throw StateError( 'The length unit cannot be changed from $current to ' - '$lengthUnitsPerMeter because a World has already been created. Box2D ' + '$lengthUnitsPerMeter because a World has already been created in this ' + 'process. Box2D ' 'bakes the length unit into the defaults of the definitions it hands ' 'out and into the tolerances of live simulations, so it has to be set ' 'before the first world exists. Await ' @@ -80,7 +74,6 @@ Future initializeForge2D({ 'world is created.', ); } - backend.setLengthUnitsPerMeter(lengthUnitsPerMeter); } /// Forgets that a world has been created, so that a test can set a different @@ -90,4 +83,4 @@ Future initializeForge2D({ /// world that is still alive are left alone, so destroy the worlds of the /// previous test first. @visibleForTesting -void debugResetLengthUnitLock() => _worldHasBeenCreated = false; +void debugResetLengthUnitLock() => rawBox2D.resetLengthUnitLock(); diff --git a/packages/forge2d/native/ffi/f2d_world_lock.c b/packages/forge2d/native/ffi/f2d_world_lock.c index 5be3f83c..67d3fd06 100644 --- a/packages/forge2d/native/ffi/f2d_world_lock.c +++ b/packages/forge2d/native/ffi/f2d_world_lock.c @@ -1,11 +1,13 @@ -// Box2D's world table is process-wide and b2CreateWorld and b2DestroyWorld -// are not thread safe (https://github.com/erincatto/box2d/issues/904), but -// Dart isolates are threads over one copy of this library. +// Box2D's world table and length unit are process-wide and b2CreateWorld and +// b2DestroyWorld are not thread safe +// (https://github.com/erincatto/box2d/issues/904), but Dart isolates are +// threads over one copy of this library. #include "atomic.h" #include "box2d/box2d.h" static b2AtomicInt f2d_world_lock; +static bool f2d_world_created; static void f2d_lock(void) { while (b2AtomicLoadInt(&f2d_world_lock) != 0 || @@ -18,6 +20,9 @@ static void f2d_unlock(void) { b2AtomicStoreInt(&f2d_world_lock, 0); } BOX2D_EXPORT b2WorldId f2d_create_world_locked(const b2WorldDef* definition) { f2d_lock(); b2WorldId id = b2CreateWorld(definition); + if (id.index1 != 0) { + f2d_world_created = true; + } f2d_unlock(); return id; } @@ -27,3 +32,19 @@ BOX2D_EXPORT void f2d_destroy_world_locked(b2WorldId id) { b2DestroyWorld(id); f2d_unlock(); } + +BOX2D_EXPORT bool f2d_set_length_units_per_meter_locked(float length_units) { + f2d_lock(); + bool can_set = !f2d_world_created; + if (can_set) { + b2SetLengthUnitsPerMeter(length_units); + } + f2d_unlock(); + return can_set; +} + +BOX2D_EXPORT void f2d_reset_length_unit_lock(void) { + f2d_lock(); + f2d_world_created = false; + f2d_unlock(); +} diff --git a/packages/forge2d/test/api/isolates_test.dart b/packages/forge2d/test/api/isolates_test.dart index 32a1742b..0eccb855 100644 --- a/packages/forge2d/test/api/isolates_test.dart +++ b/packages/forge2d/test/api/isolates_test.dart @@ -27,6 +27,26 @@ void main() { expect(results, everyElement(everyElement(expected))); }, ); + + test( + 'a world in one isolate freezes the length unit in the others', + () async { + final world = World(); + addTearDown(world.destroy); + + final error = await Isolate.run(() async { + try { + await initializeForge2D(lengthUnitsPerMeter: 2); + return null; + } on StateError catch (error) { + return error.message; + } + }); + + expect(error, contains('a World has already been created')); + expect(Tolerances.lengthUnitsPerMeter, 1); + }, + ); } /// Drops a short stack of boxes onto the ground and returns where they end diff --git a/packages/forge2d/test/api/length_unit_test.dart b/packages/forge2d/test/api/length_unit_test.dart index 03e6bbbf..265cc98a 100644 --- a/packages/forge2d/test/api/length_unit_test.dart +++ b/packages/forge2d/test/api/length_unit_test.dart @@ -5,7 +5,11 @@ import 'package:test/test.dart'; // shares, so suites run one at a time; see dart_test.yaml. Within this file // the tests run in declaration order and build on each other. void main() { - setUpAll(initializeForge2D); + // Earlier suites in the same process have created worlds. + setUpAll(() async { + await initializeForge2D(); + debugResetLengthUnitLock(); + }); // Puts the length unit back, so that the suites that run after this one see // the default whatever order they end up in.