124 lines
7.9 KiB
Markdown
124 lines
7.9 KiB
Markdown
## Context
|
|
|
|
The codebase is a C++17 Windows desktop application using OpenUSD + ImGui + OpenGL. All scene mutations are applied immediately to the live `UsdStageRefPtr` with no reversal mechanism. Edits come from five surfaces: (1) viewport gizmo drag (`TransformManipulator`), (2) Property panel TRS fields (`PropertyPanel`), (3) scene hierarchy context menus (`SceneHierarchyPanel` — create/delete/ref), (4) generic attribute edits (`PropertyManager`), and (5) layer CRUD (`LayerPanel`/`LayerManager`). There is currently no command pattern, history stack, or deferred-dispatch infrastructure.
|
|
|
|
USD provides `SdfLayer`-level undo via `SdfLayer::BeginChangeBlock` / `EndChangeBlock` + `SdfLayer::DumpLayerInfo`, but it does not expose a first-class undo stack accessible to application code. The simpler and more reliable approach for this scope is application-level command objects that store enough pre/post state to replay or reverse themselves.
|
|
|
|
## Goals / Non-Goals
|
|
|
|
**Goals:**
|
|
- Single-level command history with unlimited depth (bounded only by memory).
|
|
- `Ctrl+Z` / `Ctrl+Y` / `Ctrl+Shift+Z` hotkeys processed in the main ImGui loop.
|
|
- **Edit → Undo / Redo** menu items that grey out when the respective stack end is reached.
|
|
- All five edit surfaces wrapped in reversible commands.
|
|
- Gizmo drag treated as one atomic command (not a command per frame).
|
|
- History cleared on stage open/close/new.
|
|
|
|
**Non-Goals:**
|
|
- Branching undo trees.
|
|
- Persisting history across sessions.
|
|
- Undoing camera movement.
|
|
- Undoing layer mute/unmute (mute state is cosmetic / non-destructive; deferred to a follow-up).
|
|
- Collaborative / multi-user conflict resolution.
|
|
|
|
## Decisions
|
|
|
|
### D1 — Classic Command Pattern over USD change-block replay
|
|
|
|
**Decision**: Store pre/post value snapshots in application-level `ICommand` objects; do not attempt to record and replay USD change blocks.
|
|
|
|
**Rationale**: USD's change notification system is designed for hydra update propagation, not user-facing undo. Reconstructing "what changed" from `SdfNotice` events is complex and fragile. Application-level commands with explicit `Undo()` / `Execute()` are simpler, debuggable, and already the standard approach in DCC tools.
|
|
|
|
**Alternatives considered**: `SdfLayer::StateDelegate` (undocumented, internal), `UsdStage::GetMutedLayers` diffing (covers only mute state).
|
|
|
|
---
|
|
|
|
### D2 — `CommandHistory` owned by `Application`, passed as pointer to subsystems
|
|
|
|
**Decision**: `Application` owns a single `CommandHistory` instance. Panels and managers receive a raw `CommandHistory*` alongside existing manager pointers. No global singleton.
|
|
|
|
**Rationale**: Mirrors the existing pattern for managers (raw pointers, no DI framework). Avoids threading concerns (single-threaded ImGui loop). A singleton would make testing harder.
|
|
|
|
---
|
|
|
|
### D3 — Gizmo drag commits one command on mouse-button-release
|
|
|
|
**Decision**: `TransformManipulator` stores drag-start TRS when drag begins (`m_isDragging` transitions false→true). On drag-end (mouse released), it pushes a single `TransformCommand(prim, startTRS, endTRS)` to `CommandHistory`.
|
|
|
|
**Rationale**: The current code already accumulates delta from drag-start each frame (`m_dragStartTranslate/Rotate/Scale`) instead of applying per-frame deltas, so the start state is already cached. A single command per gesture is the correct granularity for a user (one Undo reverses one drag, not 60 frames of micro-moves).
|
|
|
|
**Change required**: `TransformManipulator` needs a `CommandHistory*` member set during construction/init; on drag-end it pushes the command instead of — or after — the final `Apply*Delta`.
|
|
|
|
---
|
|
|
|
### D4 — Snapshot-based commands for attribute edits
|
|
|
|
**Decision**: `AttributeSetCommand<T>` stores `(UsdAttribute, T oldValue, T newValue, UsdTimeCode)`. `Undo()` calls `attr.Set(oldValue, timeCode)`, `Execute()` / `Redo()` calls `attr.Set(newValue, timeCode)`.
|
|
|
|
**Rationale**: USD attributes hold typed values; snapshot is cheap for scalar/vector types. The full type-erase is handled via `std::function<void()>` closures rather than a deep template hierarchy, keeping the concrete command count manageable.
|
|
|
|
**Command structure**:
|
|
```cpp
|
|
struct AttributeSetCommand : ICommand {
|
|
std::string description;
|
|
std::function<void()> executeFunc; // captures new value
|
|
std::function<void()> undoFunc; // captures old value
|
|
};
|
|
```
|
|
|
|
---
|
|
|
|
### D5 — Prim create/delete commands use path + type + layer targeting
|
|
|
|
**Decision**: `CreatePrimCommand` stores `(SdfPath, TfToken typeName, SdfLayerHandle targetLayer)`. `Undo()` removes the prim. `DeletePrimCommand` stores the full serialized prim spec via `SdfLayer::ExportToString` scoped to that prim, and `Undo()` re-imports it.
|
|
|
|
**Rationale**: Prim deletion must round-trip the full spec (attributes, relationships, metadata). `SdfLayer::ExportToString` + `SdfLayer::ImportFromString` provides safe serialization without custom recursion.
|
|
|
|
---
|
|
|
|
### D6 — Layer operations use index-based snapshots
|
|
|
|
**Decision**: `LayerReorderCommand` stores the full ordered layer-path list before and after; `Undo()` restores the prior list. `LayerCreateCommand` and `LayerRemoveCommand` store the layer file path + insertion index.
|
|
|
|
**Rationale**: Layer order is a simple list; full-list snapshots are small and unambiguous. Per-swap tracking would be more complex for multi-step reorders.
|
|
|
|
---
|
|
|
|
### D7 — Stack invalidation on stage lifecycle events
|
|
|
|
**Decision**: `CommandHistory::Clear()` is called from `Application::RefreshManagers()` (already invoked on open/close/new). No special hook needed.
|
|
|
|
**Rationale**: Commands hold `UsdAttribute`, `UsdPrim`, and `SdfLayerHandle` references that become dangling or point to a different stage after a stage reload. Clearing is the only safe option.
|
|
|
|
## Risks / Trade-offs
|
|
|
|
- **[Risk] USD prim/attribute handles become stale** if the user opens a different stage between Undo calls.
|
|
→ Mitigation: `Clear()` on every stage lifecycle event (D7).
|
|
|
|
- **[Risk] `SdfLayer::ExportToString` for large prims may be slow**.
|
|
→ Mitigation: Export is done only at delete time (not every frame). Acceptable for scene-level operations.
|
|
|
|
- **[Risk] `TransformManipulator` drag-end detection** requires distinguishing "drag just ended this frame" from "not dragging". Currently `m_isDragging` is set to false before `HandleInput` returns; the transition must be captured.
|
|
→ Mitigation: Detect `wasDrawing && !m_isDragging` transition within `HandleInput` to push the command at the right moment.
|
|
|
|
- **[Risk] Property panel writes TRS continuously as user types** (no commit-on-enter today). Pushing a command per keystroke floods the history.
|
|
→ Mitigation: Add a deferred-commit pattern (push command on `ImGui::IsItemDeactivatedAfterEdit()`), matching standard DCC behaviour.
|
|
|
|
- **[Risk] Multi-attribute edits (e.g., all three translate components at once)** would push three commands.
|
|
→ Mitigation: Acceptable for now; a `MacroCommand` (composite) can be added later if needed.
|
|
|
|
## Migration Plan
|
|
|
|
1. Introduce `ICommand` / `CommandHistory` as new files with no side effects on existing code.
|
|
2. Thread `CommandHistory*` through constructors/init methods of each subsystem (additive changes, no existing interface removed).
|
|
3. Wrap each edit surface one at a time (Transform → Property → SceneHierarchy → Attribute → Layer). Build after each group.
|
|
4. Add hotkeys and menu items last (safest to add once all commands are wired).
|
|
5. No data migration needed — history is in-memory only.
|
|
|
|
**Rollback**: Removing the `CommandHistory*` parameter and the `Push()` calls restores pre-change behaviour; no persisted format to migrate.
|
|
|
|
## Open Questions
|
|
|
|
- Should `Ctrl+Z` also undo layer **mute/unmute**? Currently scoped out (Non-Goal), but the architecture supports it trivially. Revisit if stakeholders request it.
|
|
- Should there be a **maximum history depth** (e.g., 200 steps) to cap memory? Not required at this scope; can be added to `CommandHistory` later as a constructor parameter.
|