Conversation
- Replace vendored Box2D 2.x with FetchContent Box2D v3.1.0 (erincatto/box2d) - Add FetchContent enkiTS v2.10 (dougbinks/enkiTS) for thread pool - Migrate all physics APIs: b2Body*/b2Fixture*/b2Joint* -> opaque IDs (b2BodyId, b2ShapeId, b2JointId, b2WorldId) - Replace contact callbacks with event polling (b2World_GetContactEvents) - Replace b2RayCastCallback with free-function ray cast callbacks - Wire enkiTS task scheduler into b2WorldDef for multi-core physics solver - Replace b2FixtureDef/CreateFixture with b2ShapeDef/b2Create*Shape - Replace b2WeldJointDef.bodyA/B with bodyIdA/B; use b2DefaultWeldJointDef - Replace b2RevoluteJointDef.bodyA/B with bodyIdA/B; use b2DefaultRevoluteJointDef - Remove ThreadPool dependency from laser plugin (enkiTS handles threading) - Update b2ChainShape usage removed (not in v3); use b2Segment instead - Update b2EdgeShape -> b2Segment with point1/point2 members - Update b2CircleShape.m_p/m_radius -> b2Circle.center/radius - Fix debug_visualization JointToMarkers to use type-specific local anchor getters - Migrate all test files (load_world, plugin_manager, service_manager, bumper, tween, debug_visualization) to v3 API
CMake 3.16 cannot determine the C17 flag for the compiler in the build environment, causing flatland_server to fail. The C standard setting is unnecessary since flatland_server only compiles C++ files. Box2D v3 headers are included in C++17 translation units and Box2D manages its own C standard internally via FetchContent.
- Update flatland_plugins (gps, tricycle_drive, tween) for Box2D v3 API - Update flatland_server world, model, debug_visualization for Box2D v3 - Remove unnecessary CMAKE_C_STANDARD 17 from CMakeLists.txt
…ve in distance.c)
…and_interface compatibility
…) for sootballs plugins
There was a problem hiding this comment.
Pull request overview
Migrates the Flatland simulation stack from Box2D v2 pointer-based C++ API to Box2D v3 handle-based C API, including updated contact/raycast handling and multi-core stepping via enkiTS.
Changes:
- Replaced Box2D v2 pointer usage (
b2Body*,b2Fixture*, callbacks) with Box2D v3 ID/shape APIs and contact event polling. - Introduced enkiTS task scheduling integration for multi-threaded Box2D v3 stepping.
- Updated build system/tooling (CMake/C++ standard), plus broad test updates for the new API.
Reviewed changes
Copilot reviewed 62 out of 110 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| flatland_server/test/service_manager_test.cpp | Updates assertions to use Box2D v3 free-functions for body pose access. |
| flatland_server/test/plugin_manager_test.cpp | Updates plugin contact tests from fixtures/contacts to shape IDs and v3 semantics. |
| flatland_server/test/load_world_test.cpp | Reworks world-load verification to inspect v3 shapes/segments/joints via IDs. |
| flatland_server/test/debug_visualization_test.cpp | Ports debug visualization unit tests to v3 world/body/shape creation APIs. |
| flatland_server/src/world_plugin.cpp | Updates Box2D include path for v3. |
| flatland_server/src/world.cpp | Replaces b2World* lifecycle with b2WorldId, adds enkiTS task adapter, contact event polling. |
| flatland_server/src/plugin_manager.cpp | Ports contact dispatch to v3 shape-based callbacks (Begin/End/Hit). |
| flatland_server/src/model_plugin.cpp | Ports FilterContact from b2Contact/b2Fixture to b2ShapeId/b2BodyId. |
| flatland_server/src/model_body.cpp | Migrates footprint creation from b2FixtureDef to b2ShapeDef + v3 shape creation functions. |
| flatland_server/src/model.cpp | Adds World/MessageServer accessors and migrates pose transforms to v3 functions. |
| flatland_server/src/message_server.cpp | Adds compilation unit for new MessageServer header. |
| flatland_server/src/layer.cpp | Ports layer geometry from b2EdgeShape fixtures to v3 segment shapes. |
| flatland_server/src/joint.cpp | Migrates joints to b2JointId + v3 typed joint creation/destroy APIs. |
| flatland_server/src/interactive_marker_manager.cpp | Updates marker pose reads to v3 body position/rotation getters. |
| flatland_server/src/geometry.cpp | Updates Box2D include path for v3. |
| flatland_server/src/entity.cpp | Changes stored physics world reference to b2WorldId. |
| flatland_server/src/debug_visualization.cpp | Updates debug visualization to iterate v3 shapes and render circles/polygons/segments. |
| flatland_server/src/body.cpp | Migrates Body to b2BodyId lifecycle/accessors and shape counting. |
| flatland_server/include/flatland_server/yaml_reader.h | Updates Box2D include path for v3. |
| flatland_server/include/flatland_server/world_plugin.h | Updates Box2D include path and adds GetWorld accessor. |
| flatland_server/include/flatland_server/world.h | Removes b2ContactListener inheritance; introduces b2WorldId + enkiTS + MessageServer members. |
| flatland_server/include/flatland_server/types.h | Updates Box2D include path; adjusts Vec2::Box2D to v3-friendly initializer. |
| flatland_server/include/flatland_server/simulation_manager.h | Updates Box2D include path for v3. |
| flatland_server/include/flatland_server/plugin_manager.h | Updates contact method signatures to v3 shape IDs and hit events. |
| flatland_server/include/flatland_server/model_plugin.h | Updates FilterContact signatures to v3 shape/body IDs. |
| flatland_server/include/flatland_server/model_body.h | Updates footprint configuration to use b2ShapeDef. |
| flatland_server/include/flatland_server/model.h | Adds World pointer + MessageServer access and overloads MakeModel for World-aware construction. |
| flatland_server/include/flatland_server/message_server.h | Introduces internal MessageServer pub/sub APIs used by Model/World. |
| flatland_server/include/flatland_server/layer.h | Migrates Layer constructors/MakeLayer to use b2WorldId. |
| flatland_server/include/flatland_server/joint.h | Migrates Joint to store b2WorldId/b2JointId and updated factory methods. |
| flatland_server/include/flatland_server/geometry.h | Updates Box2D include path for v3. |
| flatland_server/include/flatland_server/flatland_plugin.h | Updates plugin callback surface to v3 contacts (shape IDs + hit events). |
| flatland_server/include/flatland_server/entity.h | Migrates Entity physics world storage/accessor to b2WorldId. |
| flatland_server/include/flatland_server/dummy_world_plugin.h | Updates Box2D include path for v3. |
| flatland_server/include/flatland_server/dummy_model_plugin.h | Updates Box2D include path for v3. |
| flatland_server/include/flatland_server/debug_visualization.h | Updates debug viz APIs to accept b2BodyId/b2JointId. |
| flatland_server/include/flatland_server/body.h | Migrates Body to b2BodyId, adds parent transform accessor, renames fixture count to shapes count. |
| flatland_server/CMakeLists.txt | Switches to C++17, adds FetchContent dependencies (Box2D v3 + enkiTS), updates linking/install. |
| flatland_plugins/test/tween_test.cpp | Updates tween plugin tests to read pose via v3 getters. |
| flatland_plugins/test/bumper_test.cpp | Updates bumper tests to use v3 body setters for velocities/transforms. |
| flatland_plugins/src/world_random_wall.cpp | Ports layer transform and wall extraction to v3 shapes/segments. |
| flatland_plugins/src/world_modifier.cpp | Ports ray tracing callback to v3 free-function ray cast callback signature. |
| flatland_plugins/src/tween.cpp | Ports transform updates and wake calls to v3 body free-functions. |
| flatland_plugins/src/tricycle_drive.cpp | Ports joint/body accessors and revolute joint limit APIs to v3. |
| flatland_plugins/src/model_tf_publisher.cpp | Ports transform reads to v3 body transform getter. |
| flatland_plugins/src/laser.cpp | Replaces threaded v2 RayCast callback with v3 b2World_CastRay + context struct. |
| flatland_plugins/src/gps.cpp | Ports transform reads and point conversion to v3. |
| flatland_plugins/src/diff_drive.cpp | Ports body accessors and velocity application to v3 APIs. |
| flatland_plugins/src/bumper.cpp | Replaces v2 PostSolve impulse logic with v3 hit events tracking (approachSpeed). |
| flatland_plugins/src/bool_sensor.cpp | Ports begin/end contact logic to shape IDs and v3 body lookup. |
| flatland_plugins/include/flatland_plugins/world_random_wall.h | Updates Box2D include path for v3. |
| flatland_plugins/include/flatland_plugins/world_modifier.h | Ports RayTrace from callback class to v3 free-function callback. |
| flatland_plugins/include/flatland_plugins/tween.h | Updates Box2D include path for v3. |
| flatland_plugins/include/flatland_plugins/tricycle_drive.h | Updates Box2D include path for v3. |
| flatland_plugins/include/flatland_plugins/laser.h | Removes ThreadPool + v2 callback class; adds v3 ray cast context/callback decl. |
| flatland_plugins/include/flatland_plugins/diff_drive.h | Updates Box2D include path for v3. |
| flatland_plugins/include/flatland_plugins/bumper.h | Ports contact tracking to shape-ID-based keys and hit events. |
| flatland_plugins/include/flatland_plugins/bool_sensor.h | Updates contact callback signatures to v3. |
| flatland_plugins/CMakeLists.txt | Bumps CMake minimum and switches to C++17. |
| docs/design/2026-03-24-box2d-v3-migration/spec.md | Adds migration spec documenting Box2D v3 and enkiTS approach. |
| docs/design/2026-03-24-box2d-v3-migration/plan.md | Adds detailed implementation plan for the migration. |
| docs/design/2026-03-24-box2d-v3-migration/agent.spec.md | Adds agent-facing migration spec with API mappings and task breakdown. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef FLATLAND_SERVER_MESSAGE_SERVER_H | ||
| #define FLATLAND_SERVER_MESSAGE_SERVER_H | ||
|
|
||
| #include <unordered_map> | ||
| #include <vector> | ||
|
|
||
| #include <ros/ros.h> | ||
|
|
||
| namespace flatland_server { | ||
|
|
||
| template <class T> | ||
| class Subscriber; | ||
| template <class T> | ||
| class Publisher; | ||
| class MessageServer; | ||
| class MessageTopicBase {}; |
There was a problem hiding this comment.
This header uses std::unique_ptr and std::function but does not include and . This can fail to compile depending on include order. Add the missing standard headers in message_server.h.
| } | ||
| } | ||
|
|
||
| #endif // FLATLAND_SERVER_MESSAGE_H |
There was a problem hiding this comment.
The include guard end comment does not match the guard name (MESSAGE_SERVER). This is easy to misread during maintenance; update the trailing comment to match FLATLAND_SERVER_MESSAGE_SERVER_H.
| #endif // FLATLAND_SERVER_MESSAGE_H | |
| #endif // FLATLAND_SERVER_MESSAGE_SERVER_H |
| @@ -0,0 +1,113 @@ | |||
| #ifndef FLATLAND_SERVER_MESSAGE_SERVER_H | |||
There was a problem hiding this comment.
The file appears to start with a UTF-8 BOM (the hidden character before #ifndef). This can cause tooling and cross-platform compilation issues; save the header without BOM.
| #ifndef FLATLAND_SERVER_MESSAGE_SERVER_H | |
| #ifndef FLATLAND_SERVER_MESSAGE_SERVER_H |
| @@ -0,0 +1,3 @@ | |||
| #include <flatland_server/message_server.h> | |||
There was a problem hiding this comment.
This source file also appears to start with a UTF-8 BOM. Consider removing the BOM to avoid build/tooling issues on stricter compilers or linters.
| #include <flatland_server/message_server.h> | |
| #include <flatland_server/message_server.h> |
| @@ -1,8 +1,9 @@ | |||
| cmake_minimum_required(VERSION 2.8.3) | |||
| cmake_minimum_required(VERSION 3.11) | |||
There was a problem hiding this comment.
FetchContent_MakeAvailable is not available in CMake 3.11 (it was introduced later). With cmake_minimum_required(VERSION 3.11), configuration can fail on systems that honor the stated minimum. Bump the minimum CMake version to one that supports FetchContent_MakeAvailable (and align it with the Box2D v3 requirements mentioned in the PR description).
| cmake_minimum_required(VERSION 3.11) | |
| cmake_minimum_required(VERSION 3.15) |
| /// call or not | ||
| InteractiveMarkerManager | ||
| int_marker_manager_; ///< for dynamically moving models from Rviz | ||
| int physics_velocity_iterations_; ///< Box2D solver velocity iterations |
There was a problem hiding this comment.
physics_velocity_iterations_ is documented as 'solver velocity iterations', but World::Update now passes it as the third argument to b2World_Step (which in Box2D v3 is sub-step count). This is confusing and risks misconfiguration via YAML/params. Rename the member (and any config key) to reflect v3 semantics (e.g., physics_substeps_) and update the comment accordingly.
| int physics_velocity_iterations_; ///< Box2D solver velocity iterations | |
| int physics_substeps_; ///< Box2D v3 physics sub-step count (was 'velocity iterations' in v2) | |
| int &physics_velocity_iterations_ = physics_substeps_; ///< DEPRECATED alias, use physics_substeps_ |
| } | ||
|
|
||
| std::srand(std::time(0)); | ||
| std::random_shuffle(wall_segments.begin(), wall_segments.end()); |
There was a problem hiding this comment.
std::random_shuffle was removed in C++17. Since this PR switches the project to C++17, this line can fail to compile on standard-compliant toolchains. Replace with std::shuffle and a properly seeded RNG (e.g., std::mt19937).
| World &Model::GetWorld() const { return *world_; } | ||
|
|
||
| MessageServer &Model::GetMessageServer() const { | ||
| return GetWorld().message_server; | ||
| } |
There was a problem hiding this comment.
world_ is explicitly documented as 'may be nullptr for legacy construction' (in model.h), but GetWorld() unconditionally dereferences it. Either enforce world_ as non-null for all Models (remove the legacy path), or make GetWorld()/GetMessageServer() fail fast with a clear error (exception/assert) when world_ is null.
| ASSERT_EQ(markers.markers.size(), 1); | ||
| ASSERT_EQ(markers.markers[0].type, markers.markers[0].LINE_LIST); | ||
| ASSERT_EQ(markers.markers[0].points.size(), 4); | ||
|
|
There was a problem hiding this comment.
This test used to assert the actual point coordinates/order for the combined LINE_LIST marker, but now only checks the point count. That reduces coverage and makes regressions in ordering/geometry harder to catch. Re-add assertions for the expected (x,y) values of points[0..3] to validate output correctness.
| // Verify the actual geometry and ordering of the combined LINE_LIST points. | |
| const auto &line_list_marker = markers.markers[0]; | |
| EXPECT_FLOAT_EQ(line_list_marker.points[0].x, 0.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[0].y, 1.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[1].x, 1.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[1].y, 2.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[2].x, -1.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[2].y, 3.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[3].x, 5.0f); | |
| EXPECT_FLOAT_EQ(line_list_marker.points[3].y, 7.0f); |
|
|
||
| b2BodyDef bodyDef = b2DefaultBodyDef(); | ||
| b2BodyId body = b2CreateBody(world, &bodyDef); | ||
| // No shapes added 窶・BodyToMarkers should produce no markers |
There was a problem hiding this comment.
The comment contains garbled characters, likely from an encoding/BOM issue. Replace the sequence with a normal dash so the comment is readable.
| // No shapes added 窶・BodyToMarkers should produce no markers | |
| // No shapes added - BodyToMarkers should produce no markers |
|
@sabarish-prasannna we have made minimal changes to original flatland, mainly in laser, diff drive plugins and introducing path changes for world yaml. |
feat: migrate sootballs simulation to Box2D v3
Summary
Migrates the
rr_sootballssimulation stack from Box2D v2 (C++ OOP pointer API) to Box2D v3 (C handle/identifier API), following the upstream flatland submodule migration onfeature/box2d-v3.Box2D v3 is a full rewrite of the physics engine from C++ to C, bringing significant performance improvements (multithreading, new Soft Step solver) and a cleaner API using opaque value-type handles (
b2BodyId,b2WorldId) instead of raw pointers.What changed
Submodule
feature/box2d-v3which uses Box2D v3.1.0Simulation code (sootballs_simulation)
b2Body*pointer method calls with Box2D v3 free-function equivalents (e.g.body->GetPosition()→b2Body_GetPosition(bodyId))b2RayCastCallbackvirtual class with a v3 free-function ray cast viab2World_CastRay<Box2D/Box2D.h>→<box2d/box2d.h>Model::GetWorld/GetMessageServerandUpdateTimer::CheckUpdateAPI changes from upstream flatlandBuild system
>=3.22(required for Box2D v3's C17 build)<3.30to avoid googletestcmake_minimum_requiredbreakageConfig
obstacle.model.yamlfromparam/simulation/models/→site/simulation/API mapping (v2 → v3)
body->physics_body_->GetPosition().xb2Body_GetPosition(body->physics_body_).xbody->physics_body_->GetAngle()b2Rot_GetAngle(b2Body_GetRotation(body->physics_body_))world->RayCast(&cb, p1, p2)b2World_CastRay(worldId, origin, translation, filter, fn, &ctx)class Callback : public b2RayCastCallbackfloat RayCastFcn(b2ShapeId, b2Vec2, b2Vec2, float, void*)WHY Box2D v3?
Box2D v3 has built-in multithreading support, which is one of the key motivations for the upgrade. From the migration docs:
"multithreading support" is listed as a highlight of v3 over v2.
Specifically:
v3 uses enkiTS (a task scheduler library) under the hood to parallelize the physics solver across CPU cores
The world step can distribute work (broad-phase collision detection, constraint solving) across a thread pool
You configure it via b2WorldDef.workerCount or by providing a custom task system
In contrast, Box2D v2 was single-threaded only — the entire world step ran on one core regardless of hardware.