refactor(json): Dispatch surface material (de)serialization via TypeDispatcher - #6023
Merged
paulgessinger merged 5 commits intoSep 17, 2026
Conversation
paulgessinger
marked this pull request as ready for review
September 3, 2026 13:28
github-actions
Bot
added
Component - Core
Contributor
Public API surface diff+12 added, 0 breaking. ➕ Added public APINew types / aliases / enums / variables / concepts (2)
New call signatures (incl. defaulted-arg overloads) (8)
New public data members (2)
|
Contributor
paulgessinger
marked this pull request as draft
September 4, 2026 12:31
paulgessinger
marked this pull request as ready for review
September 8, 2026 09:51
paulgessinger
force-pushed
the
feat/material-json-typedispatcher
branch
from
September 8, 2026 16:27
b422291 to
34135ab
Compare
paulgessinger
marked this pull request as draft
September 8, 2026 16:27
github-actions
Bot
added
Component - Detray
…ispatcher The `ISurfaceMaterial` <-> JSON conversion in `Plugins/Json` was a hand written `dynamic_cast` chain on the write side and a shape-guessing reader on the read side. This replaces both with the dispatcher pattern already used by `SurfaceJsonConverter`: a new `Acts::SurfaceMaterialJsonConverter` holds a `TypeDispatcher` encoder and a `JsonKindDispatcher` decoder in a `Config`, with `Config::defaultConfig()` registering everything. The payload of every type that already round-tripped is unchanged, so existing material maps (e.g. `Tests/Data/material-map.json`) keep loading. What this buys: - The reader dispatches strictly on the type tag instead of inferring the type from the payload shape. A missing or unknown tag is an error. As a consequence a "binned" payload with a single bin now decodes as `BinnedSurfaceMaterial` rather than `HomogeneousSurfaceMaterial`. - `ProtoGridSurfaceMaterial` (the `MultiAxisSpec2D` proto material) is covered under the new "proto-grid" tag; it used to serialize to nothing. - Globally indexed grids read back with their slab store populated. The free-function reader refuses them outright, because the shared vector has no source at single-surface scope; the encoder here inlines it under `storage_vector`, and the next commit adds the document-wide store table that lets the sharing itself survive. - `mappingType` round-trips for every type that can carry one, and the `mapMaterial == false` short circuit is applied once in `fromJson` before dispatch. Grid payloads gain the `mapMaterial` flag, which the free-function writer never emitted. The whole grid material family needs exactly one encoder and one decoder: `GridSurfaceMaterial` is a single concrete class whose storage backend is a runtime `std::variant`, so the encoder branches with `std::visit` and never names a concrete axis type. The reader hands out a `std::unique_ptr<const ISurfaceMaterial>` so ownership is explicit. `MaterialMapJsonConverter` correspondingly stores `std::shared_ptr<const ISurfaceMaterial>` in its geometry hierarchy map instead of an owning raw pointer. The free `to_json`/`from_json` overloads for `surfaceMaterialPointer` stay as deprecated forwarders; all in-tree callers use the new class directly. Volume material conversion is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSH9Jg53ECS7DmzTYJdsZs
…dispatch Globally indexed grid material indexes into a slab store that several grids of a material map can share. The JSON payload had no way to express that sharing: the previous commit makes each entry carry its own copy of the store, which is correct but turns one shared vector into one vector per surface on the way back. Introduce an encode/decode context that a whole document is written and read with. On encoding, the grid encoder registers the store it encounters with the context, which keys stores on `shared_ptr` identity (never on content, so two stores holding equal slabs stay distinct) and hands back an id assigned in encounter order. The entry then only writes `"store": <id>`, and the map-level converter can emit the stores once as a top-level table. On decoding, the table is read first into the context and `"store": <id>` resolves to the shared allocation, so the grids that shared a store before the round trip share it again after. A default constructed context has no store table. The grid encoder then inlines the store as `storage_vector`, exactly as the locally indexed backend does, which keeps a surface serialized on its own -- through `SurfaceJsonConverter`, `JsonSurfacesWriter` or the tracking geometry converter -- self-contained. A `"store"` reference read without a table, or one past the end of the table, is an error rather than a silently empty store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSH9Jg53ECS7DmzTYJdsZs
The store context work exposed more than callers need. Pull it back in. The two context classes move to `detail/MaterialJsonContext.hpp` in `Acts::detail`, following the plugin's existing detail convention. The public header forward declares them and keeps the `EncodeContext`/`DecodeContext` aliases, which is all the dispatcher signatures need; only the converter sources and the unit tests see the definitions. `toJson` and `fromJson` collapse from two overloads each to one, with the context as an optional trailing pointer. A null pointer means what the context-less overload used to mean: the encoder inlines the slab store, and the decoder rejects a payload that references one. The duplicate cached `SurfaceMaterialJsonConverter::defaultConfig()` is gone; `Config::defaultConfig()` does the caching, as in `SurfaceJsonConverter`. The free `to_json`/`from_json` on `std::shared_ptr<const ISurfaceMaterial>` are removed. Their two callers -- the surface-and-material tuple encoder and the material map converter's surface hierarchy -- now go through the class API. The map converter encodes each entry itself and hands the hierarchy container ready json, which is also what will let it thread a document-wide context through the entries later. With this the branch touches no Core header at all; it is confined to `Plugins/Json`, its tests and the docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSH9Jg53ECS7DmzTYJdsZs
Addresses the two SonarCloud code smells on the PR: push_back of a brace-initialised pair becomes emplace_back, and map insert of a pair becomes try_emplace. The volume-side insert is changed too so the two loops read the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YVsEtWnNPh2DejfRkESkTJ
paulgessinger
force-pushed
the
feat/material-json-typedispatcher
branch
from
September 11, 2026 13:02
34135ab to
d86fbb4
Compare
github-actions
Bot
removed
Component - Core
The JSON plugin's most user-facing artifact is the material map file, and nothing described it. This adds a page that walks the document layout, every surface material payload the dispatcher can write, and the volume material payloads of the older converter. No JSON on the page is typed out by hand, so none of it can drift. `MaterialJsonDocumentation` builds the example, encodes it through `MaterialMapJsonConverter`, checks that the result reads back, and compares it against `docs/examples/material_map_example.json`; a format change that is not reflected in the docs fails the test, and running it with `ACTS_UPDATE_DOC_EXAMPLES=1` refreshes the file. The example is kept minimal, because it is included verbatim and every line of it is a line someone reads. It carries the three surface payloads that differ in shape -- a plain slab, a binned matrix, a grid indexing into a slab store -- plus one volume entry, with the smallest bin counts the format allows and a single material throughout. The types it leaves out differ only in keys, which the tables on the page list; that the encoder covers all of them is already asserted by SurfaceMaterialJsonConverterTests. The dump uses four spaces so the repository's json formatting hook leaves it alone, and .gitignore gets an exception for it next to the codegen manifest, since `*.json` is ignored wholesale. The test gets the example's path as a command line argument rather than a compile definition, so it does not have to be rebuilt when it moves and the path stays visible in the ctest invocation. `add_unittest` now forwards any argument after the source file to the executable, behind the `--` separator that Boost.Test wants for tokens that are not its own options; they arrive as `master_test_suite().argv`, so this stays an ordinary Boost unit test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSH9Jg53ECS7DmzTYJdsZs
paulgessinger
force-pushed
the
feat/material-json-typedispatcher
branch
from
September 11, 2026 13:17
d86fbb4 to
5c63c90
Compare
paulgessinger
marked this pull request as ready for review
September 11, 2026 15:00
|
noemina
reviewed
Sep 15, 2026
noemina
left a comment
noemina
left a comment
Contributor
There was a problem hiding this comment.
All looks good to me. I have one comment on the description.
noemina
approved these changes
Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
The grid dispatch is written against the
GridSurfaceMaterialfrom #5871(now merged): one encoder on the concrete class branching over the storage
variant, no bound/global-to-grid-local delegates, 2D only,
accessor.type: "direct". The store context below is also what letsglobally indexed grids be read back at all -- #5871's reader throws on
them, because the shared material vector has no source at single-surface
scope.
Surface material JSON conversion was a pair of free
to_json/from_jsonfunctions on
surfaceMaterialPointer: a longdynamic_castchain on write,and a decoder that guessed the type from whichever keys happened to be
present. It could not be extended from outside the plugin, and the two sides
had drifted apart -- a
GloballyIndexedSurfaceMaterialwas read back as alocally indexed one with an empty store.
This replaces them with
SurfaceMaterialJsonConverter, built likeSurfaceJsonConverter:TypeDispatcherwith one encoder per material type. The wholegrid family is registered against the two abstract
IGridSurfaceMaterialbases, so the concrete grid shapes stay out of the translation unit.
JsonKindDispatcherkeyed on thetypetag. The tag is nowauthoritative -- a missing or unknown one throws instead of falling through
to a guess.
Both directions take an optional context. Grids that index into a shared slab
store register it once per document and reference it by id, so the sharing
survives a round trip instead of becoming one copy per surface. Without a
context the store is inlined and the payload stays self-contained, which is
what standalone surface entries need.
The old free functions stay, marked
[[deprecated]]. The on-disk payloads areunchanged;
Tests/Data/material-map.jsonstill round-trips byte for byte.Also adds a page documenting the material map file format. Its example is
generated by a unit test that fails if the file stops matching what the
converters write.