Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ source_group("web" FILES ${WEB_FILES_REL})

# Root documentation (README.md, CHANGELOG.md) and docs/ folder contents are made
# visible in the IDE via the modern glob + source_group block near the end of this
# file (consistent with agents/, github-workflows, res/, scripts/). See below.
# file (consistent with agents/, github-workflows, res/, scripts/, tools/). See below.

# Add Google Test only for non-Emscripten builds
if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Emscripten")
Expand Down Expand Up @@ -735,6 +735,22 @@ if(EZYCAD_SCRIPT_FILES)
set_source_files_properties(${EZYCAD_SCRIPT_FILES_REL} PROPERTIES HEADER_FILE_ONLY TRUE)
endif()

# Dev tools: show tools/ in the IDE (codegen, asset helpers; not compiled)
file(GLOB EZYCAD_TOOL_FILES CONFIGURE_DEPENDS
"${CMAKE_SOURCE_DIR}/tools/*.py"
"${CMAKE_SOURCE_DIR}/tools/*.ps1")
if(EZYCAD_TOOL_FILES)
source_group(TREE "${CMAKE_SOURCE_DIR}/tools" PREFIX "tools" FILES ${EZYCAD_TOOL_FILES})
set(EZYCAD_TOOL_FILES_REL)
foreach(f ${EZYCAD_TOOL_FILES})
file(RELATIVE_PATH f_rel "${CMAKE_SOURCE_DIR}" "${f}")
list(APPEND EZYCAD_TOOL_FILES_REL "${f_rel}")
endforeach()
target_sources(${PROJECT_NAME}_lib PRIVATE ${EZYCAD_TOOL_FILES_REL})
target_sources(${PROJECT_NAME} PRIVATE ${EZYCAD_TOOL_FILES_REL})
set_source_files_properties(${EZYCAD_TOOL_FILES_REL} PROPERTIES HEADER_FILE_ONLY TRUE)
endif()

# GitHub workflow files: show .github/workflows/*.yml in the IDE under 'github-workflows' folder
# (so they are visible in Visual Studio Solution Explorer etc. without being compiled)
file(GLOB_RECURSE GITHUB_WORKFLOW_FILES CONFIGURE_DEPENDS
Expand Down Expand Up @@ -772,7 +788,7 @@ endif()

# Documentation folder: show docs/ (and the moved usage guides, style guides, building-occt.md etc.)
# in the IDE under a 'docs' folder for both the library and executable targets.
# (non-compiled; uses the same pattern as agents/, github-workflows, res/, scripts/)
# (non-compiled; uses the same pattern as agents/, github-workflows, res/, scripts/, tools/)
file(GLOB_RECURSE EZYCAD_DOCS_FILES CONFIGURE_DEPENDS
LIST_DIRECTORIES false
"${CMAKE_SOURCE_DIR}/docs/*")
Expand Down
88 changes: 88 additions & 0 deletions agents/issues/017-sketch-undo-stable-ids.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Sketch undo: stable ids, operation-axis undo, stack budget

**Suggested labels:** `bug`, `sketch`, `enhancement`

**Branch:** `Trailcode/sketch-undo-stable-ids` (base: `main`)

---

## Title (GitHub)

Sketch undo: stable sketch ids for delta resolve, operation-axis undo, and undo stack byte budget

## Body (GitHub)

### Summary

Harden sketch delta undo/redo on branch **`Trailcode/sketch-undo-stable-ids`**: assign each sketch a stable persisted **id** for `Sketch_delta` lookup (display names may duplicate), fix operation-axis undo/clear and OCCT handle teardown, add undo-stack hygiene (entry kind + byte budget), and regression tests including a GUI-driven undo/redo path.

### Problem

- **Release crash on redo:** `Sketch_delta` cached a raw `Sketch*` and returned it without validation; after JSON reload or sketch recreation the pointer was dangling (`add_edge_impl_` read access violation).
- **Name-based resolve was fragile:** duplicate sketch names (rename UI, `"Sketch from face"`, load) could target the wrong sketch silently.
- **Operation axis undo incomplete:** Clear axis was not undoable; redo-after-clear could leave a stale axis; OCCT `Standard_ProgramError` when clearing axis if AIS handle was not nulled after `Remove`.
- **Undo stack memory:** 50 full JSON snapshots could dominate memory; no byte budget or debug footprint summary.

### Implemented scope

**Stable sketch ids**

- `Sketch::get_id()` / `uint64_t m_id`; allocated via `Occt_view::allocate_sketch_id()`.
- Persisted in sketch JSON as `"id"`; backward compatible when field missing.
- `Sketch_delta` resolves target sketch by **id**, not name.

**Operation axis undo**

- `clear_operation_axis_undoable()` for Options **Clear axis**; records prev axis in delta.
- `finalize_operation_axis_` records axis endpoint nodes for undo tombstone.
- `apply_forward_` clears axis when redoing a clear step.
- `clear_operation_axis()`: `Nullify()` AIS handle after `Remove`.

**Undo stack hygiene**

- `Undo_entry_kind`: `Sketch_delta` vs `Document_snapshot`.
- `k_max_undo_bytes` (32 MB): trim oldest document snapshots first when over budget.
- Debug panel shows undo/redo step count and total MB (Debug builds only).

**Tests**

- `Gui_UndoRedo_edge_then_operation_axis_four_undo_two_redo` (GUI path).
- `Undo_delta_resolves_sketch_by_id_after_reload`.
- Existing undo tests use `curr_sketch()` (document-owned sketches).

### Acceptance criteria

- [ ] Add edge + operation axis, undo/redo repeatedly — no crash (Release).
- [ ] Clear axis is undoable; redo clear works.
- [ ] JSON save/load preserves sketch `id`; redo after reload applies to correct sketch.
- [ ] Duplicate sketch display names do not break delta undo.
- [ ] `EzyCad_tests`: `Sketch_test.Undo*`, `Sketch_test.Gui_UndoRedo*`.

### Files touched

- `src/delta.h`, `src/sketch.h`, `src/sketch.cpp`, `src/sketch_delta.h`, `src/sketch_delta.cpp`, `src/sketch_json.cpp`
- `src/occt_view.h`, `src/occt_view.cpp`, `src/gui.cpp`, `src/gui_mode.cpp`
- `tests/sketch_tests.cpp`, `docs/bugs.md`
- `agents/issues/017-sketch-undo-stable-ids.md`, `agents/prs/012-sketch-undo-stable-ids.md`

### Related

- Builds on sketch delta undo (#144 / #145).
- Compare: https://github.com/trailcode/EzyCad/compare/main...Trailcode/sketch-undo-stable-ids
- GitHub issue: https://github.com/trailcode/EzyCad/issues/150
- GitHub PR: https://github.com/trailcode/EzyCad/pull/151

### Test plan

- [ ] Build Release: `EzyCad`, `EzyCad_tests`.
- [ ] Run: `EzyCad_tests.exe --gtest_filter=Sketch_test.Undo*:Sketch_test.Gui_UndoRedo*`
- [ ] Manual: edge + operation axis; undo 4x, redo 2x (former Release crash).
- [ ] Manual: Clear axis → undo → redo.
- [ ] Manual: save `.ezy`, reload, redo sketch delta step.

---

**Post-creation notes:**

- GitHub issue: https://github.com/trailcode/EzyCad/issues/150
- GitHub PR: https://github.com/trailcode/EzyCad/pull/151
38 changes: 38 additions & 0 deletions agents/prs/012-sketch-undo-stable-ids.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# PR - `Trailcode/sketch-undo-stable-ids`

## Title

Sketch undo: stable sketch ids for delta resolve, operation-axis undo, and undo stack byte budget

## Summary

- **Stable sketch ids:** Each sketch gets a monotonic `uint64_t` id (persisted in JSON). `Sketch_delta` resolves the target sketch by id, not display name — fixes Release redo crash from dangling `Sketch*` after reload/recreation.
- **Operation axis undo:** `clear_operation_axis_undoable()` for Options **Clear axis**; axis node recording on finalize; redo-clear via `apply_forward_`. OCCT handle `Nullify()` after `Remove` in `clear_operation_axis()`.
- **Undo stack hygiene:** `Undo_entry_kind`, per-entry `footprint_bytes`, 32 MB cap trimming oldest document snapshots first; debug panel shows stack MB (Debug builds).
- **Tests:** GUI undo/redo regression; reload-by-id redo test; undo tests on `curr_sketch()`.

Closes sketch undo stability work tracked in `agents/issues/017-sketch-undo-stable-ids.md`.

## Files Changed

12 files (+325 / -64 vs `main`): `src/sketch*.cpp/h`, `src/sketch_delta.*`, `src/sketch_json.cpp`, `src/occt_view.*`, `src/delta.h`, `src/gui.cpp`, `src/gui_mode.cpp`, `tests/sketch_tests.cpp`, `docs/bugs.md`.

## Related

- Issue: https://github.com/trailcode/EzyCad/issues/150
- PR: https://github.com/trailcode/EzyCad/pull/151
- Compare: https://github.com/trailcode/EzyCad/compare/main...Trailcode/sketch-undo-stable-ids
- Prior: #145 sketch delta undo

## Test Plan

- [x] Build Release: `EzyCad_tests`.
- [x] Run: `EzyCad_tests.exe --gtest_filter=Sketch_test.Undo*:Sketch_test.Gui_UndoRedo*`
- [ ] Manual: edge + operation axis; undo 4x, redo 2x (Release — former crash).
- [ ] Manual: Clear axis → undo → redo.
- [ ] Manual: save document, reload, redo sketch delta.

## Notes

- Old `.ezy` files without `"id"` allocate new ids on load (backward compatible).
- `approximate_undo_bytes()` is internal; user sees summed MB only in Debug **dbg** window.
7 changes: 3 additions & 4 deletions docs/usage-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Between those, the pane has **six** collapsible sections. Expand a section to se

3. **View presentation** — **Background color 1** and **Background color 2** (float RGB fields and swatches). **Gradient blend** — combo: **Horizontal**, **Vertical**, **Diagonal 1**, **Diagonal 2**, **Corner 1** … **Corner 4**. **Element hover color** — highlight for rows hovered in the **Shape List** or **Sketch List** (**Dimensions** table); stored as **`gui.elm_list_hover_color`**.

4. **3D view grid** — **Show grid** (checkbox; grid is drawn on the **active sketch** plane). **Fine grid lines** and **Major grid lines** (passed to Open CASCADE `Aspect_Grid::SetColors`: dense lines vs every-tenth emphasis lines). **Grid step**, **Grid extent X / Y** (full span edge-to-edge), and **Grid display Z offset** in the Settings pane use the **same length scale as sketch length dimensions** (display value = model value / internal `dimension_scale`, default **100**). Saved JSON (`occt_view`) stores **half-extent** in model units for OCCT (`grid_graphic_*`); Settings shows **full** extent (twice the stored half-extent).
4. **3D view grid** — **Show grid** (checkbox; grid is drawn on the **active sketch** plane, behind coplanar geometry). **Fine grid lines** and **Major grid lines** (dense lines vs every-tenth emphasis). **Grid step**, **Grid padding** (margin around sketch content when sizing the grid), and **Grid display Z offset** in the Settings pane use the **same length scale as sketch length dimensions** (display value = model value / internal `dimension_scale`, default **100**). Saved JSON (`occt_view`) stores padding in model units (`grid_padding`).

5. **Sketch** — Expand **Dimensions** (nested, open by default) for length-dimension appearance and behavior (most rows have **?** help). Other sketch rows stay in the parent **Sketch** section:
- **Dimension line width** — slider **0.5** to **8.0**
Expand Down Expand Up @@ -148,9 +148,8 @@ String: ImGui `.ini` text for window positions and docking saved with **SaveIniS
| `grid_color1` | array of 3 numbers | Fine (dense) grid lines (`Aspect_Grid` main color). |
| `grid_color2` | array of 3 numbers | Major (sparse / every-tenth) grid lines (`Aspect_Grid` tenth-line color). |
| `grid_visible` | boolean | When **true**, the OCCT reference grid is drawn in the 3D view (default **true**). |
| `grid_step` | number | Grid line spacing in **model** units (`Aspect_RectangularGrid`; default **10**). Legacy `grid_x_step` / `grid_y_step` load as this value (X preferred). |
| `grid_graphic_x_size` | number | Grid half-extent on X in **model** units (OCCT `SetGraphicValues`; default **1000**). Settings **Grid extent X** shows full span (**20** with those defaults). |
| `grid_graphic_y_size` | number | Grid half-extent on Y in **model** units. Settings **Grid extent Y** shows full span. |
| `grid_step` | number | Grid line spacing in **model** units (default **10**). Legacy `grid_x_step` / `grid_y_step` load as this value (X preferred). |
| `grid_padding` | number | Margin around active sketch content when sizing the grid, in **model** units (default **1000**; Settings shows display units). Legacy `grid_graphic_x_size` loads as padding if `grid_padding` is absent. |
| `grid_graphic_z_offset` | number | Grid plane offset along Z in **model** units. |

### `gui`
Expand Down
3 changes: 1 addition & 2 deletions res/ezycad_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@
0.11791712790727615,
0.13513511419296265
],
"grid_graphic_x_size": 1000,
"grid_graphic_y_size": 1000,
"grid_padding": 1000,
"grid_graphic_z_offset": 0,
"grid_step": 10,
"grid_visible": true
Expand Down
52 changes: 24 additions & 28 deletions src/gui_settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ nlohmann::json build_occt_view_settings_object(const Occt_view& view)
{"grid_color1", {g1[0], g1[1], g1[2]}},
{"grid_color2", {g2[0], g2[1], g2[2]}},
{"grid_step", grid_rect.step},
{"grid_graphic_x_size", grid_rect.graphic_x_size},
{"grid_graphic_y_size", grid_rect.graphic_y_size},
{"grid_padding", grid_rect.grid_padding},
{"grid_graphic_z_offset", grid_rect.graphic_z_offset},
{"grid_visible", view.get_grid_visible()},
};
Expand Down Expand Up @@ -228,16 +227,20 @@ void GUI::parse_occt_view_settings_(const std::string& content)
else if (ov.contains("grid_y_step"))
apply_num("grid_y_step", grid_rect.step);
}
apply_num("grid_graphic_x_size", grid_rect.graphic_x_size);
apply_num("grid_graphic_y_size", grid_rect.graphic_y_size);
apply_num("grid_padding", grid_rect.grid_padding);
if (!ov.contains("grid_padding") && ov.contains("grid_graphic_x_size"))
{
// Legacy: treat old half-extent as padding margin around sketch content.
apply_num("grid_graphic_x_size", grid_rect.grid_padding);
}
apply_num("grid_graphic_z_offset", grid_rect.graphic_z_offset);
m_view->set_occt_grid_rect_params(grid_rect);

bool grid_visible = m_view->get_grid_visible();
if (ov.contains("grid_visible") && ov["grid_visible"].is_boolean())
grid_visible = ov["grid_visible"].get<bool>();
// Always apply: default m_grid_visible may already match JSON, but the OCCT grid
// is not activated until apply_grid_visibility_() runs.
// Always apply: default m_grid_visible may already match JSON, but the shader grid
// is not displayed until apply_grid_visibility_() runs.
m_view->set_grid_visible(grid_visible);
}
catch (...)
Expand Down Expand Up @@ -796,10 +799,8 @@ void GUI::settings_()
m_view->get_occt_grid_rect_params(gr);
const double dim_scale = m_view->get_dimension_scale();
// Settings show the same length units as sketch dimensions (model / dimension_scale).
// Grid extent UI is full span; OCCT SetGraphicValues uses half-extent (x0.5 on apply).
double step_ui = gr.step / dim_scale;
double graphic_x_ui = (gr.graphic_x_size * 2.0) / dim_scale;
double graphic_y_ui = (gr.graphic_y_size * 2.0) / dim_scale;
double padding_ui = gr.grid_padding / dim_scale;
double graphic_z_off_ui = gr.graphic_z_offset / dim_scale;
bool grid_changed = false;
bool geom_changed = false;
Expand Down Expand Up @@ -848,31 +849,23 @@ void GUI::settings_()
ImGui::AlignTextToFramePadding();
ImGui::TextUnformatted("Grid step");
ImGui::TableSetColumnIndex(1);
if (ImGui::DragScalar("##gstep", ImGuiDataType_Double, &step_ui, spd_s, nullptr, nullptr, "%.8g"))
geom_changed = true;

ImGui::TableNextRow();
ImGui::TableSetColumnIndex(0);
ImGui::AlignTextToFramePadding();
ImGui::TextUnformatted("Grid extent X");
ImGui::TableSetColumnIndex(1);
if (ImGui::DragScalar("##ggx", ImGuiDataType_Double, &graphic_x_ui, spd_extent, nullptr, nullptr, "%.8g"))
// Step must stay positive; grid extent is derived from sketch bounds + padding.
const double step_min_ui = 1e-6;
if (ImGui::DragScalar("##gstep", ImGuiDataType_Double, &step_ui, spd_s, &step_min_ui, nullptr, "%.8g"))
geom_changed = true;
ImGui::SameLine(0.f, ImGui::GetStyle().ItemInnerSpacing.x);
GUI_DOC_HELP_("Full width of the drawn grid on X (edge to edge through the center). Same length scale as sketch "
"dimensions. OCCT uses half this value internally. Click ? to open the user guide.",
doc_urls::k_occt_view);

ImGui::TableNextRow();
ImGui::TableSetColumnIndex(0);
ImGui::AlignTextToFramePadding();
ImGui::TextUnformatted("Grid extent Y");
ImGui::TextUnformatted("Grid padding");
ImGui::TableSetColumnIndex(1);
if (ImGui::DragScalar("##ggy", ImGuiDataType_Double, &graphic_y_ui, spd_extent, nullptr, nullptr, "%.8g"))
const double padding_min_ui = 0.0;
if (ImGui::DragScalar("##gpad", ImGuiDataType_Double, &padding_ui, spd_extent, &padding_min_ui, nullptr, "%.8g"))
geom_changed = true;
ImGui::SameLine(0.f, ImGui::GetStyle().ItemInnerSpacing.x);
GUI_DOC_HELP_("Full height of the drawn grid on Y (edge to edge through the center). OCCT uses half this value "
"internally. Click ? to open the user guide.",
GUI_DOC_HELP_("Margin added around the active sketch when sizing the grid (same length scale as sketch "
"dimensions). The grid extent follows sketch geometry plus this padding. Click ? to open the user "
"guide.",
doc_urls::k_occt_view);

ImGui::TableNextRow();
Expand All @@ -890,10 +883,13 @@ void GUI::settings_()
if (geom_changed)
{
gr.step = step_ui * dim_scale;
gr.graphic_x_size = graphic_x_ui * dim_scale * 0.5;
gr.graphic_y_size = graphic_y_ui * dim_scale * 0.5;
gr.grid_padding = padding_ui * dim_scale;
gr.graphic_z_offset = graphic_z_off_ui * dim_scale;
m_view->set_occt_grid_rect_params(gr);
m_view->get_occt_grid_rect_params(gr);
step_ui = gr.step / dim_scale;
padding_ui = gr.grid_padding / dim_scale;
graphic_z_off_ui = gr.graphic_z_offset / dim_scale;
}
if (grid_changed || geom_changed)
save_occt_view_settings();
Expand Down
Loading
Loading