diff --git a/AGENTS.md b/AGENTS.md index f0e40d119..73ecde45f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,9 +5,11 @@ The role of this file is to describe common mistakes and confusion points that a - `Milky2018/wgpu_mbt@0.14.2` and `Milky2018/window@0.5.1` provide the current macOS `NSView*` renderer integration path. Use `Instance::create_surface_macos_ns_view_u64` with `Window::content_view_handle()` data instead of reintroducing project-local `CAMetalLayer` attach stubs or unsafe casts. Use `RenderPassDescBuilder::set_color_attachment_resolve_target` for MSAA color resolve attachments instead of project-local render-pass descriptor native stubs. - When `moon test` fails with a tcc framework lookup error, rerun the same test with `--release`. - Long `moon run` commands, especially native release builds and examples that compile large generated C files, may spend several minutes in clang/link/executable generation. Do not treat the long wait as a failure or switch debugging strategy just to avoid waiting; keep polling until the command naturally completes or produces a real error. +- On current MoonBit toolchains, `moon test -f` is the short form of `--filter`, not a test filename selector. Pass one test file as a positional path (for example, `moon test ecs/foo_test.mbt -f 'test name'`) and use only one filter flag. - On newer MoonBit toolchains, a repository-wide `moon fmt` can migrate legacy `options("is-main": true)` entries to `pkgtype(kind: "executable")` and regenerate hundreds of package/interface files. For surgical changes, inspect `git status` immediately afterward and exclude unrelated migration churn from the task. - Use this fixed native screenshot command shape for visual parity captures, for example: `MGSTUDIO_PARITY_CAPTURE_DELAY_FRAMES=90 MGSTUDIO_PARITY_RUN_TIMEOUT_SECONDS=240 mgstudio-engine/scripts/p0_visual_capture_native.sh examples/2d/bloom_2d /tmp/mgstudio_iss025_camera_render_graph_bloom_2d.png`. +- A fixed-frame capture represents simulated time, not the wall-clock age of a checked-in reference. For physics-heavy examples such as `examples/3d/despawn3`, frame 90 at the default 16,667 microseconds is only about 1.5 seconds and should not be compared with a later settled-pile reference. If an exact delayed frame is required, set `MGSTUDIO_PARITY_CAPTURE_DISABLE_RETRY=1` and inspect `capture_delay_frames_used` in the generated metadata; otherwise a timeout retry can replace the requested output with a fresh-process frame-1 capture. A larger `MGSTUDIO_PARITY_FIXED_FRAME_TIME_US` can validate later fixed-update state with fewer expensive render frames. - When an example runs correctly for the first time, save a reference screenshot in that example's source directory. Future agents should compare against that checked-in local reference before deciding that the example still renders correctly. - Do not infer Bevy parity from a visually plausible example alone. The `examples/3d/motion_blur` audit showed that a scene can render and respond to input while still diverging from Bevy in ECS shape and data flow: Bevy used a `Text` root with `TextSpan` children and `TextUiWriter`, while mgstudio had been replacing one whole `Text` value every frame. For render/PBR/mesh/text/UI work, verify source-owner structure, component/resource ownership, schedule placement, dirty/change tracking, example behavior, and visual/performance gates before raising completion scores. - If a bug appears in one feature area, check adjacent Bevy boundaries before assigning ownership. Input symptoms in `motion_blur` crossed window/AppKit event-loop timing, ECS schedule entry, UI text updates, and PBR postprocess state; treating the first visible subsystem as the root cause is usually wrong. diff --git a/CONTEXT.md b/CONTEXT.md index 30507c55d..59a313c6b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -134,7 +134,7 @@ _Avoid_: committed Bevy patch, permanent baseline fork > **Dev:** "Should bridge packages own behavior just because code lives there?" > **Domain expert:** "No — **Runtime Owner Semantics** stays with the **Bevy Source Owner**; bridges only keep the port buildable and traceable." > -> **Dev:** "If `moon_taffy` lacks a Bevy-required layout capability, is that a mgstudio parity gap?" +> **Dev:** "If `chicle` lacks a Bevy-required layout capability, is that a mgstudio parity gap?" > **Domain expert:** "No — that is an **External Blocker** and should be tracked so the community can build the missing dependency capability." > > **Dev:** "If MoonBit cannot express the typed system parameter shape Bevy uses, can we encode the state as JSON?" diff --git a/docs/ECS_PERFORMANCE_API_MIGRATION.md b/docs/ECS_PERFORMANCE_API_MIGRATION.md new file mode 100644 index 000000000..cf7e6ec9f --- /dev/null +++ b/docs/ECS_PERFORMANCE_API_MIGRATION.md @@ -0,0 +1,220 @@ +# ECS Performance Branch Public API Migration + +This document records the source-breaking public API changes retained by the +ECS and render performance work tracked by `ISS-442`, `ISS-444`, `ISS-456`, +`ISS-461`, and `ISS-463`. + +These changes are intentional, but they are not source compatible. The +generated interfaces expose struct fields, struct mutability, return types, +error variants, and generic bounds as public API. Downstream code that used +those details must be migrated before updating to this branch. + +The audit compares this branch against its `main` merge base, +`df70870662fe5fb4ad151098fe6edd329431838b`. + +## Value-Type Semantics + +`Duration`, `Stopwatch`, `TimerMode`, `Timer`, `TextureAtlas`, `Aabb`, `Sphere`, +`InheritedVisibility`, and `ViewVisibility` are now MoonBit value types. Copies +no longer share object identity. Value updates return replacement values that +the caller must store. + +MoonBit value types cannot contain the reference-typed fields used by the old +representations. The flattened fields are an implementation detail; downstream +code should use constructors and methods instead of depending on the new scalar +layout. + +## Time + +`Stopwatch` previously exported `elapsed : Duration`; it now stores +`elapsed_nanos : Int64`. `Timer` previously exported `stopwatch`, `duration`, +and `mode`; those fields are now flattened scalar state. + +| Previous source | Replacement | +| --- | --- | +| `stopwatch.elapsed` | `stopwatch.elapsed()` | +| `Stopwatch::{ elapsed, paused: false }` | `Stopwatch::new().set_elapsed(elapsed)` | +| `Stopwatch::{ elapsed, paused: true }` | `Stopwatch::new().set_elapsed(elapsed).pause()` | +| `timer.stopwatch.elapsed()` | `timer.elapsed()` | +| `timer.stopwatch.is_paused()` | `timer.is_paused()` | +| `timer.duration` | `timer.duration()` | +| `timer.mode` | `timer.mode()` | +| `Timer::{ stopwatch, duration, mode, .. }` | Start with `Timer::new(duration, mode)`, then apply `set_elapsed`, `pause`, `finish`, or `tick` as required | + +There is no supported replacement for constructing arbitrary inconsistent +`Timer` internals through a struct literal. Use the public state-transition +methods. Existing `Timer` and `Stopwatch` mutators already return a replacement +value, so retain the result, for example `timer = timer.tick(delta)`. + +## Camera Primitives + +`Aabb` no longer exports `inner : Aabb3d`; it stores six scalar center and +half-extent values. `Sphere` no longer exports `center : Vec3`; it stores three +scalar center values. + +| Previous source | Replacement | +| --- | --- | +| `Aabb::{ inner }` | `Aabb::from_math(inner)` | +| `aabb.inner` | `aabb.to_math()` | +| `aabb.inner.center` | `aabb.center()` | +| `aabb.inner.half_size` | `aabb.half_size()` | +| `Sphere::{ center, radius }` | `Sphere::new(center, radius)` | +| `sphere.center` | `sphere.center()` | +| Scalar sphere construction | `Sphere::from_xyz(x, y, z, radius)` | + +The scalar getters such as `center_x()` and `half_extents_x()` exist for hot +paths, but callers should not construct the flattened representation directly. + +## Visibility + +`InheritedVisibility` is now a value type with otherwise unchanged constructor +and getter APIs. + +`ViewVisibility.bits` is no longer mutable. The former mutating +`ViewVisibility::update` method is deliberately not retained with a +value-returning signature: an old `view.update()` call would still compile, +discard the returned value, and silently preserve the old state. The +replacement is named `advance_frame` so unmigrated calls fail at compile time. + +| Previous source | Replacement | +| --- | --- | +| `view.update()` | `view = view.advance_frame()` | +| `view.set_visible()` | `view = view.with_visible()` | +| `view.bits = bits` | Use `ViewVisibility::new`, `visible`, `hidden`, `with_visible`, or `advance_frame` | +| Component mutation through direct field access | Use `Write::set`, `Write::update`, or the `SetViewVisibility` API | + +Code using ECS change detection must choose ordinary mutation or the explicit +bypass-change-detection API according to the same semantics as before; do not +replace every visibility write with a bypass update. + +The former public `check_visibility_system` blanket producer was removed. It +marked every inherited-visible renderable as view-visible without applying the +camera-, layer-, range-, light-, or frustum-specific visibility owners. Normal +applications should install the root visibility plugin together with the +camera visibility plugin and relevant light plugins. A custom visibility +producer should set `ViewVisibility` through `SetViewVisibility` and run in +`visibility_set_check_visibility`; there is no source-faithful one-function +replacement for the old blanket scan. + +## Texture Atlas + +`TextureAtlas.layout` is now a private scalar asset id because the value type +cannot contain a reference-typed `Handle`. + +| Previous source | Replacement | +| --- | --- | +| `TextureAtlas::{ layout, index }` | `TextureAtlas::new(layout, index)` | +| `atlas.layout` | `atlas.layout()` | +| Direct layout asset lookup through `atlas.layout` | `atlas.layout_asset(layout_assets)` | + +`TextureAtlas.index` remains public. Prefer `index()` and `with_index()` when +the caller does not need field-pattern matching. + +## ECS Change-Location Storage + +`RawTableColumnKernel.changed_caller_ids` changed from `Array[Int]` to +`Array[Int]?`. Change-location allocation is now opt-in through +`World::new(track_change_locations=true)`. + +Direct consumers of `RawTableColumnKernel` must handle `None`. `None` means the +world does not track caller locations; it is not an empty tracked column. +`RawTableColumnKernel` is a low-level storage type, so normal downstream ECS +code should use `Read::changed_by` and the corresponding query APIs instead of +accessing the raw field. + +`EcsError` also gained `TableBatchUnavailable(String)`. Exhaustive downstream +matches must add that case or an explicit fallback arm. + +`World::new` now accepts the optional `track_change_locations` argument. +Existing `World::new()` calls remain valid. Code that stored `World::new` as a +zero-argument function value should replace it with `() => World::new()`; +callers that need change locations should use +`World::new(track_change_locations=true)`. + +## Bindless Material Allocation + +`MaterialBindGroupBindlessAllocator::allocate` changed from +`MaterialBindingId?` to `MaterialBindingId`. A bindless allocator now grows a +new slab and cannot fail because the current slab is full. + +Remove `Some`/`None` matching or `.unwrap()` from direct bindless allocator +calls: + +```mbt nocheck +let binding = allocator.allocate(material_id) +``` + +The higher-level material allocator enum may still return `None` when a +bindless allocation is requested from a non-bindless allocator. + +## Frustum System Generic Bound + +`render3d_update_frusta_system_by_projection_key` now requires +`T : @ecs.Component`. Generic wrappers calling this function must add the same +bound. Concrete projection components that already implement `Component` need +no source change. + +## Table-Batch Mutable Column Scope + +`TableBatchWrite::update_inplace` and +`update_inplace_bypass_change_detection` now pass +`TableBatchMutValues[T]` instead of a raw `MutArrayView[T]`. The old view could +escape its callback and continue replacing ECS column values without a valid +lease, change ticks, or finalizer handling. A retained `TableBatchMutValues` +becomes inactive when the callback returns, and every subsequent indexed read +or write aborts. + +Callbacks with inferred parameter types keep the same source shape: + +```mbt nocheck +values.update_inplace(fn(items) { + for index in 0..