Faster island split

[erincatto]

Using union find. This required changing how bodies are stored in islands.

Added implot to samples app.

[Copilot]

Pull request overview

This PR refactors island splitting to be faster by switching to a union-find approach and by changing island body storage from a linked list to an indexable array. It also extends the samples app UI with an ImPlot-based “Frame Time” plot and adds new container unit tests.

Changes:

• Replace island body linked-list bookkeeping with an array + per-body islandIndex, enabling union-find based island splitting.
• Introduce a lightweight dynamic/stack array utility (container.h) and add a dedicated container test suite.
• Add ImPlot to the samples build and render a new real-time frame/profile plot UI.

Reviewed changes

Copilot reviewed 23 out of 26 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/island.c	Implements union-find based island split and updates island lifecycle for new body storage.
src/island.h	Updates b2Island to store bodies as a dynamic array instead of a linked list.
src/body.h	Replaces island linked-list pointers with islandIndex for O(1) union-find mapping.
src/body.c	Updates island membership add/remove logic to maintain islandIndex and island body arrays.
src/solver_set.c	Updates sleep logic and migration loops to iterate island body arrays.
src/solver.c	Switches island lookup to the new container access helper.
src/physics_world.h	Changes world island storage to a persistent dynamic array and declares its container type.
src/physics_world.c	Updates world island initialization/destruction and island debug drawing iteration.
src/container.h	Adds new dynamic array + stack array utilities and helpers.
src/core.h	Adds B2_TYPE_OF/B2_SWAP helpers and tweaks B2_ARRAY_COUNT.
src/core.c	Renames and changes behavior of internal assert function (breakpoint handling).
include/box2d/base.h	Updates B2_ASSERT macro and exported internal assert symbol name.
src/CMakeLists.txt	Adds container.h to the source list.
test/test_container.c	Adds a new container-focused test suite for dynamic/stack arrays.
test/main.c	Registers and runs the new ContainerTest.
test/CMakeLists.txt	Adds test_container.c to the test target sources.
samples/sample.h	Adds persistent profile ring-buffer fields and frameTime flag plumbing.
samples/sample.cpp	Records per-step profiles into a ring buffer and adds ImPlot-based plotting UI.
samples/sample_benchmark.cpp	Ensures benchmark sample UI calls base Sample::UpdateGui().
samples/main.cpp	Initializes/shuts down ImPlot context and adds “Frame Time” checkbox.
samples/CMakeLists.txt	Builds and links ImPlot into the samples app.
extern/implot/implot.h	Adds vendored ImPlot public header.
extern/implot/implot_internal.h	Adds vendored ImPlot internal header.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 27 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Indentation uses a tab for test_container.c while the surrounding entries use spaces. This can create noisy diffs and inconsistent formatting in generated project files; align the indentation with the rest of the list.

[erincatto]

@claude

[claude]

Claude finished @erincatto's task in 3m 18s —— View job

---

Code Review — Faster Island Split

• Gathering context
• Reading key changed files
• Analyzing island splitting logic
• Analyzing container utilities
• Reviewing core.h changes
• Writing review feedback

---

Overall Assessment

The union-find approach for island splitting is algorithmically sound. The data structure changes (linked list → body array + islandIndex) are well-executed and enable the O(α·N) split pass. The implementation is careful about memory safety in the tricky area where b2CreateIsland can reallocate world->islands. Below are the findings, ordered by severity.

---

Issues

1. B2_TYPE_OF breaks MSVC C builds (Critical)

src/core.h:128:

#else
#define B2_TYPE_OF( A ) __typeof__( A )  // MSVC C does not support __typeof__
#endif

__typeof__ is a GCC/Clang extension. MSVC's C compiler does not support it, so every use of B2_TYPE_OF — the casts in container.h (b2Array_Reserve, b2Array_Push, b2Array_CreateN, b2Array_Emplace, b2StackArray_Reserve) and B2_SWAP — will fail to compile on Windows under C mode.

The void* implicit cast works in C (not C++), so the simplest fix for C/MSVC is to drop the cast entirely. For B2_SWAP on MSVC, a memcpy-based swap is the standard workaround. The Copilot suggestion covers this well. Fix this →

---

2. b2Array_CreateN(a, 0) triggers an assertion (Minor)

src/container.h:37:

( a ).data = (B2_TYPE_OF( ( a ).data ))b2GrowAlloc( NULL, 0, ( n ) * sizeof( *( a ).data ) );

b2GrowAlloc asserts newSize > oldSize (see src/core.c:203). Calling b2Array_CreateN(a, 0) passes newSize = 0, tripping that assert. The current call sites use n = 8 and n = 16, so this is not a live bug today, but the API is latently unsafe. A guard if (n > 0) before the allocation (as Copilot suggests) would make it robust.

---

3. maxValue clips collide/solve lines in ImPlot (Minor)

samples/sample.cpp:761:

maxValue = b2MaxFloat( stepTimes[i], maxValue );

The y-axis limit is computed only from step, but collide and solve are also plotted. If either exceeds step, those lines will render clipped. Taking the max across all three series before calling SetupAxesLimits is a one-liner fix. Fix this →

---

Correctness Observations (No Action Needed)

componentCount == 1 early-out IS implemented — The Copilot review claimed the early-out was missing, but it is present at src/island.c:507-515. The check correctly clears constraintRemoveCount, frees arena items, and returns without splitting.

Memory safety in b2SplitIsland — The code carefully caches raw pointers (baseBodyIds, baseContacts, baseJoints) before calling b2CreateIsland, which may reallocate world->islands. The arrays are detached from the base island (zeroed out at lines 518–528) before b2DestroyIsland is called, then freed manually. This is the right approach and it is correct.

Union-find contact/joint counting — The accumulation of contactCounts/jointCounts per root during b2IslandUnion is correct. Merging propagates accumulated counts to the new root, and each contact/joint is then counted once at the post-union root. The pre-computed capacities used in b2Array_Reserve will be exact.

uint64_t ring-buffer indices — m_profileReadIndex and m_profileWriteIndex are declared as uint64_t in sample.h:112-113, so Copilot's concern about overflow with int indices does not apply.

TestArrayPop — The test uses b2Array_Resize(a, 100) followed by 100 b2Array_Push calls. The Resize sets count=100, so the Pushes grow the backing array and append at indices 100–199. The 100 subsequent Pops correctly retrieve only the pushed values. The loop bound uses n (not a.count), so Copilot's concern about the termination condition is also not applicable to the current code.

---

Nit

implot vendored as raw source files — Vendoring ~13k lines directly into extern/implot/ is simple but inflates repo history permanently. A git submodule or FetchContent in CMake would keep the repo leaner and make it easier to track upstream versions. This is a style preference, not a bug.

---