diff --git a/docs/ezycad_code_style.md b/docs/ezycad_code_style.md index f11698d..d0b3cdb 100644 --- a/docs/ezycad_code_style.md +++ b/docs/ezycad_code_style.md @@ -67,7 +67,7 @@ User-facing Markdown (now in `docs/`: `usage.md`, `usage-*.md`, `scripting.md`, - **Reader-first order** (`.cpp`): Put **public API and high-level workflow** at the top of the file (constructors, main entry points, orchestration). Put **lower-level details** below so the first screen shows what the module does before how it does it. - **Helper functions** (`.cpp`): Prefer **file-local static helpers** in an anonymous namespace at the **bottom** of the implementing `.cpp` file. Forward-declare them near the top (or above their first use) when needed. Avoid large helper blocks at the top of the file; the goal is high-level code first, helpers last for readability. -- **PIMPL** (`class Foo; class Foo::Impl`): Use when hiding implementation details or when you want to swap implementations (e.g. `Sketch_nodes`, `Sketch_delta`). Keep the public surface in the header; put data members, private record types, and apply/clone logic in `Impl` inside the `.cpp`. +- **PIMPL** (`class Foo; class Foo::Impl`): Use when hiding implementation details or when you want to swap implementations (e.g. `Sketch_nodes`, `Sketch_op_recorder`). Keep the public surface in the header; put data members, private record types, and apply/clone logic in `Impl` inside the `.cpp`. - **Templates**: Prefer putting template implementations in `.inl` files included from the header (e.g. `utl_types.inl`, `utl.inl`). - **OCCT handles**: Use `opencascade::handle` and project aliases (e.g. `AIS_Shape_ptr`, `Shp_ptr`). - **Result/error handling**: See **Fail fast** below. Use `Result` and `Status` from `utl.h`, `CHK_RET(...)` for early return on failure. diff --git a/src/doc/shape.md b/src/doc/shape.md index 1cdf1f0..f29348a 100644 --- a/src/doc/shape.md +++ b/src/doc/shape.md @@ -171,7 +171,7 @@ Full GLFW -> `GUI` routing before these delegates: [`src/doc/gui.md`](gui.md). | --- | --- | | `Occt_view::push_undo_snapshot()` | Before booleans, fillet/chamfer, primitive add, polar dup, transform finalize, extrude commit, revolve | | Transform preview only | No undo until **finalize** (not on each mouse move) | -| `Sketch_delta` (sketch subsystem) | Separate from shape ops; shape list captured in view snapshots | +| `Sketch_op_delta` (sketch subsystem) | Separate from shape ops; shape list captured in view snapshots | ## Persistence diff --git a/src/doc/sketch.md b/src/doc/sketch.md index 0a411bf..0ea5771 100644 --- a/src/doc/sketch.md +++ b/src/doc/sketch.md @@ -75,10 +75,10 @@ Supporting (not owned sub-objects): sketch_display.cpp visibility, edge styling, list hover, set_current sketch_operations.cpp operation axis, mirror, revolve sketch_json.* JSON serialization - sketch_delta.* undo/redo (Sketch_op_recorder, Sketch_delta) + sketch_op_recorder.* undo/redo (Sketch_op_recorder; Sketch_op_delta in .cpp) ``` -`Sketch` grants `friend` access to `Sketch_json`, `Sketch_delta`, `Sketch_topo`, `Sketch_edges`, `Sketch_dims`, `Sketch_node_marks`, and `Sketch_tools` for coordinated mutations without exposing internals publicly. +`Sketch` grants `friend` access to `Sketch_json`, `Sketch_op_delta`, `Sketch_topo`, `Sketch_edges`, `Sketch_dims`, `Sketch_node_marks`, and `Sketch_tools` for coordinated mutations without exposing internals publicly. ## Public API overview @@ -209,7 +209,7 @@ Prefer these visitors in JSON/delta/topo code over iterating `std::list` (`Sketch_delta` today) | +| **Element delta** | Sketch geometry edits | `std::unique_ptr` (`Sketch_op_delta` in `sketch_op_recorder.cpp`) | | **Full JSON snapshot** | Everything else | `to_json()` string of the whole document | Sketch deltas are cheaper and precise; JSON snapshots are simple and cover arbitrary document changes (shape booleans, sketch add/remove, file load, etc.). @@ -58,7 +58,7 @@ Snapshot undo/redo calls `load(state.json, false)` so **camera pan/zoom/rotate a ### Stable sketch identity -`Sketch_delta` keys sketches by stable `sketch_id` (not display name or list index). Node indices are never compacted; undo tombstones nodes created during an operation. See [sketch.md](sketch.md#requirements-and-invariants). +`Sketch_op_delta` keys sketches by stable `sketch_id` (not display name or list index). Node indices are never compacted; undo tombstones nodes created during an operation. See [sketch.md](sketch.md#requirements-and-invariants). ## Architecture @@ -69,13 +69,13 @@ GUI (menus, hotkeys, some UI-driven snapshots) | m_undo_stack / m_redo_stack (Undo_entry) | +-- push_undo_snapshot() --> entry.json = to_json() - +-- push_undo_delta() --> entry.delta = Sketch_delta + +-- push_undo_delta() --> entry.delta = Sketch_op_delta +-- pop_undo_snapshot() --> drop last entry (failed/aborted edit) Sketch edits Sketch_op_recorder (RAII) note_prev_* / note_curr_* during mutation - commit() --> push_undo_delta(Sketch_delta) + commit() --> push_undo_delta(Sketch_op_delta) cancel() --> discard (destructor auto-cancels if not committed) Delta (abstract) @@ -84,7 +84,7 @@ Delta (abstract) clone() copy for opposite stack ``` -`Sketch_delta` is currently the only `Delta` subclass. +`Sketch_op_delta` is currently the only `Delta` subclass (defined in [`sketch_op_recorder.cpp`](../sketch_op_recorder.cpp), not the public header). ## Core types @@ -100,11 +100,11 @@ class Delta { Subclasses record the **forward** change. Undo applies `apply_reverse`; redo reapplies `apply_forward`. -### `Sketch_op_recorder` ([`sketch_delta.h`](../sketch_delta.h)) +### `Sketch_op_recorder` ([`sketch_op_recorder.h`](../sketch_op_recorder.h)) RAII scope around one sketch edit: -1. **Construction** — captures live node positions and linear edges at operation start; allocates a `Sketch_delta` for the sketch's `get_id()`. +1. **Construction** — captures live node positions and linear edges at operation start; initializes `Sketch_op_data` for the sketch's `get_id()`. 2. **Mutation** — sketch code calls `note_*` helpers as it changes geometry. 3. **`commit()`** — if any notes were recorded, pushes the delta via `Occt_view::push_undo_delta()`. 4. **`cancel()`** or scope exit without commit — nothing is pushed (used for no-op mirror, failed tools, etc.). @@ -116,13 +116,13 @@ Recorder notes (deduplicated by geometry): | `note_prev_linear_edge` | Edge removed or split away (must have existed at op start) | | `note_curr_linear_edge` | New linear segment added | | `note_prev_arc_edge` / `note_curr_arc_edge` | Arc circle segment removed / added | -| `note_curr_node` | New node not live at op start | +| `note_curr_node` | New node not live at op start (stores `permanent` for redo) | | `note_prev_length_dim` / `note_curr_length_dim` | Dimension removed / added | | `note_prev_operation_axis` / `note_curr_operation_axis` | Operation axis before / after | `note_prev_linear_edge` only records edges that were present when the recorder opened (`linear_edge_at_op_start_`), so incidental splits of pre-existing geometry are not mistaken for undoable removals of the user's new work. -### `Sketch_delta` apply logic ([`sketch_delta.cpp`](../sketch_delta.cpp)) +### `Sketch_op_delta` apply logic ([`sketch_op_recorder.cpp`](../sketch_op_recorder.cpp)) **Forward (redo):** add `curr_*` linear edges and arcs, restore `curr_*` dimensions and operation axis, refresh faces. @@ -199,7 +199,7 @@ Hotkeys are handled in `GUI::on_key` (`gui_mode.cpp`) when dist/angle edit popup 1. Open a `Sketch_op_recorder` around the mutation. 2. Call `note_prev_*` before removing or splitting existing geometry; `note_curr_*` after adding. 3. `commit()` at success; `cancel()` on no-op or error. -4. If redo/undo needs logic not covered by existing note types, extend `Sketch_delta::Impl` and both `apply_forward_` / `apply_reverse_`. +4. If redo/undo needs logic not covered by existing note types, extend `Sketch_op_data` and both `apply_forward_` / `apply_reverse_` in `sketch_op_recorder.cpp`. ### Non-sketch document change diff --git a/src/sketch.cpp b/src/sketch.cpp index 074f443..7c8c1fe 100644 --- a/src/sketch.cpp +++ b/src/sketch.cpp @@ -14,7 +14,7 @@ #include "utl_geom.h" #include "mode.h" #include "gui_occt_view.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "utl.h" using namespace glm; diff --git a/src/sketch.h b/src/sketch.h index 704e60f..6582874 100644 --- a/src/sketch.h +++ b/src/sketch.h @@ -161,7 +161,8 @@ class Sketch private: friend class Sketch_json; friend class Sketch_access; - friend class Sketch_delta; + friend class Sketch_op_delta; + friend struct Sketch_op_data; friend class Sketch_op_recorder; friend class Sketch_topo; friend class Sketch_dims; diff --git a/src/sketch_dims.cpp b/src/sketch_dims.cpp index c033870..157d637 100644 --- a/src/sketch_dims.cpp +++ b/src/sketch_dims.cpp @@ -12,7 +12,7 @@ #include "mode.h" #include "gui_occt_view.h" #include "sketch.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_edge.h" #include "utl_geom.h" diff --git a/src/sketch_edges.cpp b/src/sketch_edges.cpp index 9b8da3e..c41cdd8 100644 --- a/src/sketch_edges.cpp +++ b/src/sketch_edges.cpp @@ -11,7 +11,7 @@ #include "gui_occt_view.h" #include "sketch.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_edge.h" #include "sketch_topo.h" #include "utl_geom.h" diff --git a/src/sketch_edges.h b/src/sketch_edges.h index 3f25c41..e89b580 100644 --- a/src/sketch_edges.h +++ b/src/sketch_edges.h @@ -48,7 +48,6 @@ class Sketch_edges private: friend class Sketch; friend class Sketch_topo; - friend class Sketch_delta; void add_edge_raw_(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b); void add_edge_impl_(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b, Sketch_op_recorder* rec); diff --git a/src/sketch_node_marks.h b/src/sketch_node_marks.h index 8eff4b0..ea48c7a 100644 --- a/src/sketch_node_marks.h +++ b/src/sketch_node_marks.h @@ -20,8 +20,6 @@ class Sketch_node_marks void trim_trailing(); private: - friend class Sketch_delta; - void apply_style_(AIS_Shape_ptr& shp, bool origin) const; Sketch& m_sketch; diff --git a/src/sketch_delta.cpp b/src/sketch_op_recorder.cpp similarity index 66% rename from src/sketch_delta.cpp rename to src/sketch_op_recorder.cpp index 15e8776..692ecdf 100644 --- a/src/sketch_delta.cpp +++ b/src/sketch_op_recorder.cpp @@ -1,4 +1,4 @@ -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include #include @@ -6,6 +6,7 @@ #include #include +#include "delta.h" #include "utl_geom.h" #include "gui_occt_view.h" #include "sketch.h" @@ -21,11 +22,8 @@ bool pts_equal_(const gp_Pnt2d& a, const gp_Pnt2d& b); } // namespace -class Sketch_delta::Impl +struct Sketch_op_data { -public: - friend class Sketch_op_recorder::Impl; - struct Prev_edge_rec { gp_Pnt2d pt_a; @@ -56,25 +54,34 @@ class Sketch_delta::Impl std::string name; }; + struct Curr_node_record + { + gp_Pnt2d pt; + bool permanent{false}; + }; + Sketch* m_sketch{nullptr}; size_t m_sketch_id{0}; std::vector prev_linear_edges; std::vector curr_linear_edges; std::vector prev_arc_edges; std::vector curr_arc_edges; - std::vector curr_node_pts; + std::vector curr_nodes; std::vector prev_length_dims; std::vector curr_length_dims; std::optional prev_operation_axis; std::optional curr_operation_axis; - explicit Impl(size_t sketch_id); - Impl(Sketch& sketch, size_t sketch_id); + bool empty() const + { + return prev_linear_edges.empty() && curr_linear_edges.empty() && prev_arc_edges.empty() && curr_arc_edges.empty() && + curr_nodes.empty() && prev_length_dims.empty() && curr_length_dims.empty() && + !prev_operation_axis.has_value() && !curr_operation_axis.has_value(); + } - Sketch* resolve_sketch_(Occt_view& view) const; - void apply_forward_(Occt_view& view) const; - void apply_reverse_(Occt_view& view) const; - std::unique_ptr clone() const; + Sketch* resolve_sketch_(Occt_view& view) const; + void apply_forward_(Occt_view& view) const; + void apply_reverse_(Occt_view& view) const; static bool prev_linear_equal_(const Prev_edge_rec& x, const Prev_edge_rec& y); static bool curr_linear_equal_(const Curr_linear_edge_record& x, const Curr_linear_edge_record& y); @@ -86,7 +93,8 @@ class Sketch_delta::Impl static void remove_arc_edge_(Sketch& sketch, const Arc_edge_record& rec); static void remove_length_dim_(Sketch& sketch, const Length_dim_record& rec); static void tombstone_node_at_pt_(Sketch& sketch, const gp_Pnt2d& pt); - static void restore_curr_node_at_pt_(Sketch& sketch, const gp_Pnt2d& pt, const std::vector& curr_arc_edges); + static void restore_curr_node_at_pt_(Sketch& sketch, const Curr_node_record& rec, + const std::vector& curr_arc_edges); static void restore_prev_linear_edge_(Sketch& sketch, const Prev_edge_rec& rec); static void restore_length_dim_(Sketch& sketch, const Length_dim_record& rec); static void restore_prev_operation_axis_(Sketch& sketch, const Prev_edge_rec& rec); @@ -94,17 +102,36 @@ class Sketch_delta::Impl static bool linear_edge_at_op_start_(const std::vector& at_start, const Prev_edge_rec& rec); }; +/// One sketch operation step: `prev_*` is restored on undo; `curr_*` is re-applied on redo. +class Sketch_op_delta : public Delta +{ +public: + explicit Sketch_op_delta(Sketch_op_data data) + : m_data(std::move(data)) + { + } + + void apply_forward(Occt_view& view) override { m_data.apply_forward_(view); } + + void apply_reverse(Occt_view& view) override { m_data.apply_reverse_(view); } + + std::unique_ptr clone() const override { return std::make_unique(m_data); } + +private: + Sketch_op_data m_data; +}; + class Sketch_op_recorder::Impl { public: - Occt_view& m_view; - Sketch& m_sketch; - Sketch_op_recorder* m_owner{nullptr}; - bool m_active{true}; - bool m_committed{false}; - std::vector m_live_node_pts_at_start; - std::vector m_linear_edges_at_start; - std::unique_ptr m_delta; + Occt_view& m_view; + Sketch& m_sketch; + Sketch_op_recorder* m_owner{nullptr}; + bool m_active{true}; + bool m_committed{false}; + std::vector m_live_node_pts_at_start; + std::vector m_linear_edges_at_start; + Sketch_op_data m_data; Impl(Occt_view& view, Sketch& sketch); @@ -125,28 +152,9 @@ class Sketch_op_recorder::Impl void cancel(); private: - bool empty_() const; void unregister_owner_(); }; -Sketch_delta::Sketch_delta(Sketch& sketch, const size_t sketch_id) - : m_impl(std::make_unique(sketch, sketch_id)) -{ -} - -Sketch_delta::Sketch_delta(const size_t sketch_id) - : m_impl(std::make_unique(sketch_id)) -{ -} - -Sketch_delta::~Sketch_delta() = default; - -void Sketch_delta::apply_forward(Occt_view& view) { m_impl->apply_forward_(view); } - -void Sketch_delta::apply_reverse(Occt_view& view) { m_impl->apply_reverse_(view); } - -std::unique_ptr Sketch_delta::clone() const { return m_impl->clone(); } - Sketch_op_recorder::Sketch_op_recorder(Occt_view& view, Sketch& sketch) : m_impl(std::make_unique(view, sketch)) { @@ -216,8 +224,9 @@ Sketch_op_recorder::Impl::Impl(Occt_view& view, Sketch& sketch) if (!sketch.m_nodes[i].deleted) m_live_node_pts_at_start.push_back(sketch.m_nodes[i]); - Sketch_delta::Impl::capture_linear_edges_at_start_(sketch, m_linear_edges_at_start); - m_delta = std::make_unique(sketch, sketch.get_id()); + Sketch_op_data::capture_linear_edges_at_start_(sketch, m_linear_edges_at_start); + m_data.m_sketch = &sketch; + m_data.m_sketch_id = sketch.get_id(); } void Sketch_op_recorder::Impl::register_owner_(Sketch_op_recorder& owner) { m_owner = &owner; } @@ -234,7 +243,7 @@ void Sketch_op_recorder::Impl::on_destroy_() void Sketch_op_recorder::Impl::note_prev_linear_edge(size_t node_idx_a, size_t node_idx_b, std::optional node_idx_mid, const std::string& name) { - if (!m_active || !m_delta) + if (!m_active) return; const gp_Pnt2d pt_a = m_sketch.m_nodes[node_idx_a]; @@ -243,60 +252,59 @@ void Sketch_op_recorder::Impl::note_prev_linear_edge(size_t node_idx_a, size_t n if (node_idx_mid.has_value()) pt_mid = m_sketch.m_nodes[*node_idx_mid]; - Sketch_delta::Impl::Prev_edge_rec rec{pt_a, pt_b, pt_mid, name}; - if (!Sketch_delta::Impl::linear_edge_at_op_start_(m_linear_edges_at_start, rec)) + Sketch_op_data::Prev_edge_rec rec{pt_a, pt_b, pt_mid, name}; + if (!Sketch_op_data::linear_edge_at_op_start_(m_linear_edges_at_start, rec)) return; - Sketch_delta::Impl& d = *m_delta->m_impl; - for (const Sketch_delta::Impl::Prev_edge_rec& x : d.prev_linear_edges) - if (Sketch_delta::Impl::prev_linear_equal_(x, rec)) + for (const Sketch_op_data::Prev_edge_rec& x : m_data.prev_linear_edges) + if (Sketch_op_data::prev_linear_equal_(x, rec)) return; - d.prev_linear_edges.push_back(std::move(rec)); + m_data.prev_linear_edges.push_back(std::move(rec)); } void Sketch_op_recorder::Impl::note_curr_linear_edge(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b) { - if (!m_active || !m_delta) + if (!m_active) return; - Sketch_delta::Impl::Curr_linear_edge_record rec{pt_a, pt_b}; - for (const Sketch_delta::Impl::Curr_linear_edge_record& x : m_delta->m_impl->curr_linear_edges) - if (Sketch_delta::Impl::curr_linear_equal_(x, rec)) + Sketch_op_data::Curr_linear_edge_record rec{pt_a, pt_b}; + for (const Sketch_op_data::Curr_linear_edge_record& x : m_data.curr_linear_edges) + if (Sketch_op_data::curr_linear_equal_(x, rec)) return; - m_delta->m_impl->curr_linear_edges.push_back(rec); + m_data.curr_linear_edges.push_back(rec); } void Sketch_op_recorder::Impl::note_prev_arc_edge(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b, const gp_Pnt2d& pt_c) { - if (!m_active || !m_delta) + if (!m_active) return; - Sketch_delta::Impl::Arc_edge_record rec{pt_a, pt_b, pt_c}; - for (const Sketch_delta::Impl::Arc_edge_record& x : m_delta->m_impl->prev_arc_edges) - if (Sketch_delta::Impl::arc_equal_(x, rec)) + Sketch_op_data::Arc_edge_record rec{pt_a, pt_b, pt_c}; + for (const Sketch_op_data::Arc_edge_record& x : m_data.prev_arc_edges) + if (Sketch_op_data::arc_equal_(x, rec)) return; - m_delta->m_impl->prev_arc_edges.push_back(rec); + m_data.prev_arc_edges.push_back(rec); } void Sketch_op_recorder::Impl::note_curr_arc_edge(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b, const gp_Pnt2d& pt_c) { - if (!m_active || !m_delta) + if (!m_active) return; - Sketch_delta::Impl::Arc_edge_record rec{pt_a, pt_b, pt_c}; - for (const Sketch_delta::Impl::Arc_edge_record& x : m_delta->m_impl->curr_arc_edges) - if (Sketch_delta::Impl::arc_equal_(x, rec)) + Sketch_op_data::Arc_edge_record rec{pt_a, pt_b, pt_c}; + for (const Sketch_op_data::Arc_edge_record& x : m_data.curr_arc_edges) + if (Sketch_op_data::arc_equal_(x, rec)) return; - m_delta->m_impl->curr_arc_edges.push_back(rec); + m_data.curr_arc_edges.push_back(rec); } void Sketch_op_recorder::Impl::note_curr_node(size_t node_idx) { - if (!m_active || !m_delta) + if (!m_active) return; const gp_Pnt2d pt = m_sketch.m_nodes[node_idx]; @@ -305,68 +313,56 @@ void Sketch_op_recorder::Impl::note_curr_node(size_t node_idx) if (pts_equal_(live, pt)) return; - std::vector& nodes = m_delta->m_impl->curr_node_pts; - for (const gp_Pnt2d& x : nodes) - if (pts_equal_(x, pt)) + for (const Sketch_op_data::Curr_node_record& x : m_data.curr_nodes) + if (pts_equal_(x.pt, pt)) return; - nodes.push_back(pt); + m_data.curr_nodes.push_back({pt, m_sketch.m_nodes[node_idx].permanent}); } void Sketch_op_recorder::Impl::note_prev_length_dim(size_t lo, size_t hi, bool visible, std::optional flyout, const std::string& name) { - if (!m_active || !m_delta) + if (!m_active) return; - Sketch_delta::Impl::Length_dim_record rec{m_sketch.m_nodes[lo], m_sketch.m_nodes[hi], visible, flyout, name}; - for (const Sketch_delta::Impl::Length_dim_record& x : m_delta->m_impl->prev_length_dims) - if (Sketch_delta::Impl::length_dim_equal_(x, rec)) + Sketch_op_data::Length_dim_record rec{m_sketch.m_nodes[lo], m_sketch.m_nodes[hi], visible, flyout, name}; + for (const Sketch_op_data::Length_dim_record& x : m_data.prev_length_dims) + if (Sketch_op_data::length_dim_equal_(x, rec)) return; - m_delta->m_impl->prev_length_dims.push_back(std::move(rec)); + m_data.prev_length_dims.push_back(std::move(rec)); } void Sketch_op_recorder::Impl::note_curr_length_dim(size_t lo, size_t hi, bool visible, std::optional flyout, const std::string& name) { - if (!m_active || !m_delta) + if (!m_active) return; - Sketch_delta::Impl::Length_dim_record rec{m_sketch.m_nodes[lo], m_sketch.m_nodes[hi], visible, flyout, name}; - for (const Sketch_delta::Impl::Length_dim_record& x : m_delta->m_impl->curr_length_dims) - if (Sketch_delta::Impl::length_dim_equal_(x, rec)) + Sketch_op_data::Length_dim_record rec{m_sketch.m_nodes[lo], m_sketch.m_nodes[hi], visible, flyout, name}; + for (const Sketch_op_data::Length_dim_record& x : m_data.curr_length_dims) + if (Sketch_op_data::length_dim_equal_(x, rec)) return; - m_delta->m_impl->curr_length_dims.push_back(std::move(rec)); + m_data.curr_length_dims.push_back(std::move(rec)); } void Sketch_op_recorder::Impl::note_prev_operation_axis(size_t node_idx_a, size_t node_idx_b) { - if (!m_active || !m_delta) + if (!m_active) return; - m_delta->m_impl->prev_operation_axis = - Sketch_delta::Impl::Prev_edge_rec{m_sketch.m_nodes[node_idx_a], m_sketch.m_nodes[node_idx_b], std::nullopt, {}}; + m_data.prev_operation_axis = + Sketch_op_data::Prev_edge_rec{m_sketch.m_nodes[node_idx_a], m_sketch.m_nodes[node_idx_b], std::nullopt, {}}; } void Sketch_op_recorder::Impl::note_curr_operation_axis(const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b) { - if (!m_active || !m_delta) + if (!m_active) return; - m_delta->m_impl->curr_operation_axis = Sketch_delta::Impl::Curr_linear_edge_record{pt_a, pt_b}; -} - -bool Sketch_op_recorder::Impl::empty_() const -{ - if (!m_delta) - return true; - - const Sketch_delta::Impl& d = *m_delta->m_impl; - return d.prev_linear_edges.empty() && d.curr_linear_edges.empty() && d.prev_arc_edges.empty() && d.curr_arc_edges.empty() && - d.curr_node_pts.empty() && d.prev_length_dims.empty() && d.curr_length_dims.empty() && - !d.prev_operation_axis.has_value() && !d.curr_operation_axis.has_value(); + m_data.curr_operation_axis = Sketch_op_data::Curr_linear_edge_record{pt_a, pt_b}; } void Sketch_op_recorder::Impl::commit() @@ -379,8 +375,8 @@ void Sketch_op_recorder::Impl::commit() unregister_owner_(); - if (!empty_()) - m_view.push_undo_delta(std::move(m_delta)); + if (!m_data.empty()) + m_view.push_undo_delta(std::make_unique(std::move(m_data))); } void Sketch_op_recorder::Impl::cancel() @@ -391,18 +387,7 @@ void Sketch_op_recorder::Impl::cancel() unregister_owner_(); } -Sketch_delta::Impl::Impl(const size_t sketch_id) - : m_sketch_id(sketch_id) -{ -} - -Sketch_delta::Impl::Impl(Sketch& sketch, const size_t sketch_id) - : m_sketch(&sketch) - , m_sketch_id(sketch_id) -{ -} - -Sketch* Sketch_delta::Impl::resolve_sketch_(Occt_view& view) const +Sketch* Sketch_op_data::resolve_sketch_(Occt_view& view) const { for (const Sketch::sptr& s : view.get_sketches()) if (s->get_id() == m_sketch_id) @@ -419,7 +404,7 @@ Sketch* Sketch_delta::Impl::resolve_sketch_(Occt_view& view) const return m_sketch; } -void Sketch_delta::Impl::apply_forward_(Occt_view& view) const +void Sketch_op_data::apply_forward_(Occt_view& view) const { Sketch* sketch = resolve_sketch_(view); if (!sketch) @@ -437,15 +422,15 @@ void Sketch_delta::Impl::apply_forward_(Occt_view& view) const if (curr_operation_axis.has_value()) sketch->sketch_json_set_operation_axis_(curr_operation_axis->pt_a, curr_operation_axis->pt_b); - for (const gp_Pnt2d& pt : curr_node_pts) - restore_curr_node_at_pt_(*sketch, pt, curr_arc_edges); + for (const Curr_node_record& node : curr_nodes) + restore_curr_node_at_pt_(*sketch, node, curr_arc_edges); sketch->m_node_marks.sync(); sketch->m_nodes.hide_snap_annos(); sketch->update_faces_(); } -void Sketch_delta::Impl::apply_reverse_(Occt_view& view) const +void Sketch_op_data::apply_reverse_(Occt_view& view) const { Sketch* sketch = resolve_sketch_(view); if (!sketch) @@ -475,31 +460,14 @@ void Sketch_delta::Impl::apply_reverse_(Occt_view& view) const for (const Length_dim_record& d : prev_length_dims) restore_length_dim_(*sketch, d); - for (const gp_Pnt2d& pt : curr_node_pts) - tombstone_node_at_pt_(*sketch, pt); + for (const Curr_node_record& node : curr_nodes) + tombstone_node_at_pt_(*sketch, node.pt); sketch->m_nodes.hide_snap_annos(); sketch->update_faces_(); } -std::unique_ptr Sketch_delta::Impl::clone() const -{ - auto copy = std::make_unique(m_sketch_id); - Impl& copy_impl = *copy->m_impl; - copy_impl.m_sketch = m_sketch; - copy_impl.prev_linear_edges = prev_linear_edges; - copy_impl.curr_linear_edges = curr_linear_edges; - copy_impl.prev_arc_edges = prev_arc_edges; - copy_impl.curr_arc_edges = curr_arc_edges; - copy_impl.curr_node_pts = curr_node_pts; - copy_impl.prev_length_dims = prev_length_dims; - copy_impl.curr_length_dims = curr_length_dims; - copy_impl.prev_operation_axis = prev_operation_axis; - copy_impl.curr_operation_axis = curr_operation_axis; - return copy; -} - -bool Sketch_delta::Impl::prev_linear_equal_(const Prev_edge_rec& x, const Prev_edge_rec& y) +bool Sketch_op_data::prev_linear_equal_(const Prev_edge_rec& x, const Prev_edge_rec& y) { if (!pts_equal_(x.pt_a, y.pt_a) || !pts_equal_(x.pt_b, y.pt_b)) return false; @@ -513,20 +481,20 @@ bool Sketch_delta::Impl::prev_linear_equal_(const Prev_edge_rec& x, const Prev_e return true; } -bool Sketch_delta::Impl::curr_linear_equal_(const Curr_linear_edge_record& x, const Curr_linear_edge_record& y) +bool Sketch_op_data::curr_linear_equal_(const Curr_linear_edge_record& x, const Curr_linear_edge_record& y) { return x.pt_a.SquareDistance(y.pt_a) <= Precision::SquareConfusion() && x.pt_b.SquareDistance(y.pt_b) <= Precision::SquareConfusion(); } -bool Sketch_delta::Impl::arc_equal_(const Arc_edge_record& x, const Arc_edge_record& y) +bool Sketch_op_data::arc_equal_(const Arc_edge_record& x, const Arc_edge_record& y) { return x.pt_a.SquareDistance(y.pt_a) <= Precision::SquareConfusion() && x.pt_b.SquareDistance(y.pt_b) <= Precision::SquareConfusion() && x.pt_c.SquareDistance(y.pt_c) <= Precision::SquareConfusion(); } -bool Sketch_delta::Impl::arc_edge_matches_record_(const Sketch& sketch, const Sketch::Edge& e, const Arc_edge_record& rec) +bool Sketch_op_data::arc_edge_matches_record_(const Sketch& sketch, const Sketch::Edge& e, const Arc_edge_record& rec) { if (!sketch_edge_is_arc(e) || !e.node_idx_b.has_value() || e.shp.IsNull()) return false; @@ -546,12 +514,12 @@ bool Sketch_delta::Impl::arc_edge_matches_record_(const Sketch& sketch, const Sk return pts_equal_(expected_mid, actual_mid); } -bool Sketch_delta::Impl::length_dim_equal_(const Length_dim_record& x, const Length_dim_record& y) +bool Sketch_op_data::length_dim_equal_(const Length_dim_record& x, const Length_dim_record& y) { return pts_equal_(x.pt_lo, y.pt_lo) && pts_equal_(x.pt_hi, y.pt_hi); } -void Sketch_delta::Impl::remove_linear_edges_on_segment_(Sketch& sketch, const gp_Pnt2d& seg_a, const gp_Pnt2d& seg_b) +void Sketch_op_data::remove_linear_edges_on_segment_(Sketch& sketch, const gp_Pnt2d& seg_a, const gp_Pnt2d& seg_b) { for (auto itr = sketch.m_edges.edges().begin(); itr != sketch.m_edges.edges().end();) { @@ -573,12 +541,12 @@ void Sketch_delta::Impl::remove_linear_edges_on_segment_(Sketch& sketch, const g } } -void Sketch_delta::Impl::remove_linear_edges_on_node_segment_(Sketch& sketch, const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b) +void Sketch_op_data::remove_linear_edges_on_node_segment_(Sketch& sketch, const gp_Pnt2d& pt_a, const gp_Pnt2d& pt_b) { remove_linear_edges_on_segment_(sketch, pt_a, pt_b); } -void Sketch_delta::Impl::remove_arc_edge_(Sketch& sketch, const Arc_edge_record& rec) +void Sketch_op_data::remove_arc_edge_(Sketch& sketch, const Arc_edge_record& rec) { for (auto itr = sketch.m_edges.edges().begin(); itr != sketch.m_edges.edges().end();) { @@ -599,7 +567,7 @@ void Sketch_delta::Impl::remove_arc_edge_(Sketch& sketch, const Arc_edge_record& } } -void Sketch_delta::Impl::remove_length_dim_(Sketch& sketch, const Length_dim_record& rec) +void Sketch_op_data::remove_length_dim_(Sketch& sketch, const Length_dim_record& rec) { for (auto it = sketch.m_dims.dimensions().begin(); it != sketch.m_dims.dimensions().end(); ++it) { @@ -616,7 +584,7 @@ void Sketch_delta::Impl::remove_length_dim_(Sketch& sketch, const Length_dim_rec } } -void Sketch_delta::Impl::tombstone_node_at_pt_(Sketch& sketch, const gp_Pnt2d& pt) +void Sketch_op_data::tombstone_node_at_pt_(Sketch& sketch, const gp_Pnt2d& pt) { for (size_t i = 0, n = sketch.m_nodes.size(); i < n; ++i) { @@ -633,27 +601,20 @@ void Sketch_delta::Impl::tombstone_node_at_pt_(Sketch& sketch, const gp_Pnt2d& p } } -void Sketch_delta::Impl::restore_curr_node_at_pt_(Sketch& sketch, const gp_Pnt2d& pt, - const std::vector& curr_arc_edges) +void Sketch_op_data::restore_curr_node_at_pt_(Sketch& sketch, const Curr_node_record& rec, + const std::vector& curr_arc_edges) { - bool is_arc_defining_pt = false; - bool is_arc_bulge = false; + bool is_arc_bulge = false; for (const Arc_edge_record& e : curr_arc_edges) { - if (pts_equal_(pt, e.pt_b)) - { - is_arc_defining_pt = true; - is_arc_bulge = true; - break; - } - if (pts_equal_(pt, e.pt_a) || pts_equal_(pt, e.pt_c)) + if (pts_equal_(rec.pt, e.pt_b)) { - is_arc_defining_pt = true; + is_arc_bulge = true; break; } } - const size_t node_idx = sketch.m_nodes.get_node_exact(pt, !is_arc_defining_pt); + const size_t node_idx = sketch.m_nodes.get_node_exact(rec.pt, rec.permanent); if (is_arc_bulge) return; @@ -661,7 +622,7 @@ void Sketch_delta::Impl::restore_curr_node_at_pt_(Sketch& sketch, const gp_Pnt2d sketch.m_topo.split_arcs_at_node_if_interior(node_idx); } -void Sketch_delta::Impl::restore_prev_linear_edge_(Sketch& sketch, const Prev_edge_rec& rec) +void Sketch_op_data::restore_prev_linear_edge_(Sketch& sketch, const Prev_edge_rec& rec) { remove_linear_edges_on_segment_(sketch, rec.pt_a, rec.pt_b); @@ -684,19 +645,19 @@ void Sketch_delta::Impl::restore_prev_linear_edge_(Sketch& sketch, const Prev_ed } } -void Sketch_delta::Impl::restore_length_dim_(Sketch& sketch, const Length_dim_record& rec) +void Sketch_op_data::restore_length_dim_(Sketch& sketch, const Length_dim_record& rec) { const size_t lo = sketch.m_nodes.get_node_exact(rec.pt_lo); const size_t hi = sketch.m_nodes.get_node_exact(rec.pt_hi); sketch.json_add_length_dimension_(lo, hi, rec.visible, rec.flyout_offset, rec.name); } -void Sketch_delta::Impl::restore_prev_operation_axis_(Sketch& sketch, const Prev_edge_rec& rec) +void Sketch_op_data::restore_prev_operation_axis_(Sketch& sketch, const Prev_edge_rec& rec) { sketch.sketch_json_set_operation_axis_(rec.pt_a, rec.pt_b); } -void Sketch_delta::Impl::capture_linear_edges_at_start_(Sketch& sketch, std::vector& out) +void Sketch_op_data::capture_linear_edges_at_start_(Sketch& sketch, std::vector& out) { for (const Sketch::Edge& e : sketch.m_edges.edges()) { @@ -711,7 +672,7 @@ void Sketch_delta::Impl::capture_linear_edges_at_start_(Sketch& sketch, std::vec } } -bool Sketch_delta::Impl::linear_edge_at_op_start_(const std::vector& at_start, const Prev_edge_rec& rec) +bool Sketch_op_data::linear_edge_at_op_start_(const std::vector& at_start, const Prev_edge_rec& rec) { return std::find_if(at_start.begin(), at_start.end(), [&](const Prev_edge_rec& x) { return prev_linear_equal_(x, rec); }) != at_start.end(); diff --git a/src/sketch_delta.h b/src/sketch_op_recorder.h similarity index 59% rename from src/sketch_delta.h rename to src/sketch_op_recorder.h index 7741722..17f3a8a 100644 --- a/src/sketch_delta.h +++ b/src/sketch_op_recorder.h @@ -1,7 +1,5 @@ #pragma once -#include "delta.h" - #include #include @@ -11,7 +9,7 @@ class Occt_view; class Sketch; -/// Records `prev`/`curr` lists during one sketch edit; pushes a `Sketch_delta` on commit. +/// Records `prev`/`curr` lists during one sketch edit; pushes a `Sketch_op_delta` on commit. class Sketch_op_recorder { public: @@ -39,29 +37,3 @@ class Sketch_op_recorder private: std::unique_ptr m_impl; }; - -/// One sketch undo step: `prev_*` is restored on undo; `curr_*` is re-applied on redo. -/// Node indices are never compacted; undo tombstones `curr_node_idxs` and redo revives them -/// through the normal node lookup when edges are added again. -class Sketch_delta : public Delta -{ -public: - Sketch_delta(Sketch& sketch, size_t sketch_id); - ~Sketch_delta() override; - - /// Clone path: identity by stable sketch id (no live `Sketch` pointer). - explicit Sketch_delta(size_t sketch_id); - - Sketch_delta(const Sketch_delta&) = delete; - Sketch_delta& operator=(const Sketch_delta&) = delete; - - void apply_forward(Occt_view& view) override; - void apply_reverse(Occt_view& view) override; - std::unique_ptr clone() const override; - -private: - friend class Sketch_op_recorder::Impl; - - class Impl; - std::unique_ptr m_impl; -}; diff --git a/src/sketch_operations.cpp b/src/sketch_operations.cpp index 46b334e..014f90b 100644 --- a/src/sketch_operations.cpp +++ b/src/sketch_operations.cpp @@ -7,7 +7,7 @@ #include "gui.h" #include "mode.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_edge.h" #include "utl_geom.h" #include "utl_occt.h" diff --git a/src/sketch_tools.cpp b/src/sketch_tools.cpp index df2386f..4068ea1 100644 --- a/src/sketch_tools.cpp +++ b/src/sketch_tools.cpp @@ -14,7 +14,7 @@ #include "mode.h" #include "gui_occt_view.h" #include "sketch.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_edge.h" #include "utl_geom.h" #include "utl_occt.h" diff --git a/src/sketch_topo.cpp b/src/sketch_topo.cpp index a7fc66d..878c62c 100644 --- a/src/sketch_topo.cpp +++ b/src/sketch_topo.cpp @@ -21,7 +21,7 @@ #include "gui_occt_view.h" #include "sketch.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_edge.h" #include "utl.h" #include "utl_geom.h" diff --git a/src/utl_geom.cpp b/src/utl_geom.cpp index 96a2803..0b15665 100644 --- a/src/utl_geom.cpp +++ b/src/utl_geom.cpp @@ -1527,6 +1527,7 @@ std::vector arc_arc_intersections_2d(const TopoDS_Edge& arc_a, const T if (!point_on_open_arc_interior_2d(p2d, arc_a, pln)) continue; + if (!point_on_open_arc_interior_2d(p2d, arc_b, pln)) continue; diff --git a/tests/sketch_tests.cpp b/tests/sketch_tests.cpp index 810c23b..aa1d2c8 100644 --- a/tests/sketch_tests.cpp +++ b/tests/sketch_tests.cpp @@ -16,7 +16,7 @@ #include "gui_occt_view.h" #include "sketch.h" #include "sketch_ais.h" -#include "sketch_delta.h" +#include "sketch_op_recorder.h" #include "sketch_json.h" #include "sketch_edge.h" #include "sketch_nodes.h" @@ -688,6 +688,66 @@ TEST_F(Sketch_test, Undo_two_crossing_edges_via_finalize) << "Undo of the first edge should remove it"; } +TEST_F(Sketch_test, Redo_two_crossing_edges_keeps_intersection_node_non_permanent) +{ + gp_Pln default_plane(gp::Origin(), gp::DZ()); + Sketch sketch("TestSketch", view(), default_plane); + + auto count_permanent_nodes = [](Sketch& s) { + size_t n = 0; + for (const auto& node : s.get_nodes()) + if (node.permanent && !node.deleted) + ++n; + return n; + }; + + auto find_live_node_at = [](Sketch& s, const gp_Pnt2d& pt) -> std::optional { + for (size_t i = 0, n = s.get_nodes().size(); i < n; ++i) + { + const auto& node = s.get_nodes()[i]; + if (node.deleted) + continue; + + if (node.SquareDistance(pt) <= Precision::SquareConfusion()) + return i; + } + return std::nullopt; + }; + + { + Sketch_op_recorder rec(view(), sketch); + Sketch_access::add_edge_(sketch, gp_Pnt2d(0.0, 0.0), gp_Pnt2d(10.0, 0.0), rec); + rec.commit(); + } + + { + Sketch_op_recorder rec(view(), sketch); + Sketch_access::add_edge_(sketch, gp_Pnt2d(3.0, -2.0), gp_Pnt2d(3.0, 4.0), rec); + rec.commit(); + } + + EXPECT_EQ(Sketch_access::get_linear_edge_count(sketch), 4); + const size_t perm_after_create = count_permanent_nodes(sketch); + + const std::optional intersection_before = find_live_node_at(sketch, gp_Pnt2d(3.0, 0.0)); + ASSERT_TRUE(intersection_before.has_value()) << "Crossing should create an interior node"; + EXPECT_FALSE(sketch.get_nodes()[*intersection_before].permanent) + << "Auto-split intersection nodes should not be permanent"; + + EXPECT_TRUE(view().undo()); + EXPECT_TRUE(view().undo()); + EXPECT_TRUE(view().redo()); + EXPECT_TRUE(view().redo()); + + EXPECT_EQ(Sketch_access::get_linear_edge_count(sketch), 4); + EXPECT_EQ(count_permanent_nodes(sketch), perm_after_create) << "Redo should not add permanent nodes"; + + const std::optional intersection_after = find_live_node_at(sketch, gp_Pnt2d(3.0, 0.0)); + ASSERT_TRUE(intersection_after.has_value()) << "Crossing intersection should be restored"; + EXPECT_FALSE(sketch.get_nodes()[*intersection_after].permanent) + << "Redo must not promote the intersection node to permanent"; +} + TEST_F(Sketch_test, Undo_extrude_snapshot_keeps_single_sketch) { Sketch::Sketch_ptr sk = view().curr_sketch_shared(); diff --git a/tools/pimpl_gen.py b/tools/pimpl_gen.py index fe07323..f1ee0be 100644 --- a/tools/pimpl_gen.py +++ b/tools/pimpl_gen.py @@ -5,8 +5,8 @@ with out-of-line definitions that delegate to m_impl->method(...). Example: - python tools/pimpl_gen.py src/sketch_delta.h --class Sketch_op_recorder - -> sketch_delta_impl_000.inl + python tools/pimpl_gen.py src/sketch_op_recorder.h --class Sketch_op_recorder + -> sketch_op_recorder_impl_000.inl The output is a starting point: review, rename, and merge into your .cpp. """