feat(common): archive tracked nodes absent from an update round (+ propagate archived attrs to the backend) - #156
Conversation
Object nodes arrived is_active=true and nothing ever cleared the flag, so Pairwise merge candidates (archived-only) were always empty and backend object merges could never fire (box_6/7: 144/144 active). Opt-in per layer via LayerTracker archive_missing (default false).
Second hop of the same gate: graph_builder's frontend->backend_graph mergeGraph also skipped archived-node attribute updates, so the extractor's image_folder still never left the frontend (box_10: hook saw empty attrs, 0 renames).
Same gate as the frontend hop: mergeGraph skips attribute updates for archived target copies, so attributes written after a node archives never reached the backend's odometric mirror.
nathanhhughes
left a comment
nathanhhughes
left a comment
There was a problem hiding this comment.
I'm skeptical that this is useful; this is what I use to control whether or not objects should be archived (it defaults false because it messes with how the original Hydra mesh-based objects work but should be true in your case). Two reasons why this should be somewhere else:
- Archiving nodes that are missing from an update constrains how the active window behaves and/or makes it impossible to have two sources updating the same layer (which is not the end of the world, but seems not worth it if we don't need to do it this way)
- The archival / active window logic of whatever is sending the graph update may not be the same as the archival / active window logic of the frontend, and the frontend should have final say about what should still be active
| // archived nodes still receive attribute updates (e.g. the object extractor's | ||
| // image_folder lands after the track archives) that the backend must see | ||
| GraphMergeConfig merge_config; | ||
| merge_config.update_archived_attributes = true; | ||
| state_->backend_graph->graph->mergeGraph(*dsg_->graph, merge_config); |
There was a problem hiding this comment.
This is way more expensive for no benefit. mergeGraph copies in attributes while the destination is active, so the last update to attributes (when is_active is flipped to false) is always caught. This is actually how is_active flips from true to false in the backend (so a lot more things would be broken if this didn't work)
|
I'm going to close this for now, we can talk about what this should look like if the current way of handling active/archived doesn't actually work for you |
2 participants
Problem (two halves of one lifecycle gap)
1. Track-driven nodes never archive.
GraphUpdaterstampsis_active=trueon every add/update of a tracked node and nothing ever clears it (places/frontiers have explicit archival; track-driven layers don't). Consequence:Pairwise::candidatesfilters to!is_active, so backend merge proposals for such layers are structurally empty — object dedup can never fire. Measured on a 38-min indoor bag: 144/144 object nodes still active at shutdown, zero merges ever, in every run.2. Archived copies are frozen forever. Once a downstream copy of a node is inactive,
mergeGraphskips its attribute updates (scene_graph_layer.cpparchived gate) — and sinceis_activeitself only updates via that same clone, the copy can never be reactivated. Any attribute written after archival (e.g. an extractor finalizing a node after its track leaves the active window) silently never reaches the backend.Fix
LayerTracker::Config::archive_missing(default false — no behavior change unless opted in): tracked nodes absent from a layer's update round getis_active=false; a returning track re-activates via the normal update path. Unit tests included.mergeGraphcalls setGraphMergeConfig::update_archived_attributes = true, so late attribute updates reach the backend mirror regardless of archival state.Measured (fork, with
archive_missing: truefor objects)Under a real 235-edge loop-closure stream: 11 duplicate objects merged by the existing backend merge machinery (previously structurally impossible), map quality unchanged (objects-on-mesh median 0.11 m).
Notes: exposed a latent
MergeTrackercrash once merges started firing — fixed separately in #155. Orthogonal to #153/#154.