Add Bulk Edit Rotate X / Y / Z to the Layout tab by heffneil · Pull Request #6184 · xLightsSequencer/xLights

heffneil · 2026-04-20T21:34:04Z

Summary

Adds three new items to the existing Bulk Edit right-click submenu in the Layout tab so a rotation can be applied to every selected model in one operation:

Rotate X
Rotate Y
Rotate Z

Each option opens a text entry dialog pre-filled with the first selected unlocked model's current rotation on that axis, and applies the entered angle (in degrees, decimals supported) to every selected non-locked model. The existing tree selection is preserved through the reload, and a single Ctrl-Z reverts the entire batch as one undo step.

Design choices

Three menu items, one axis each rather than a combined X/Y/Z dialog, since the most common workflow is rotating a group of props around a single axis at a time.
Locked models are skipped silently (same rule the property-grid rotation handler uses via loc.IsLocked()).
Float parsing uses std::strtod instead of std::stod per CLAUDE.md ("xLights has effectively no exception handling").
Undo via CreateUndoPoint("All", ...) takes a full snapshot before any change, so one undo rolls back all affected models together. The snapshot is deferred until after the dialog is accepted and the value parses successfully, so Cancel or bad input doesn't leave a phantom entry on the undo stack.
Selection preservation mirrors BulkEditPixelSize — capture GetSelectedTreeModelPaths() before the dialog, then call ReselectTreeModels() after the reload.
The three public entry points (BulkEditRotateX/Y/Z) delegate to a single BulkEditRotateAxis(char axis) to avoid triplicate code.

Files changed

src-ui-wx/layout/LayoutPanel.h — three new ID_PREVIEW_BULKEDIT_ROTATE{X,Y,Z} IDs and method declarations
src-ui-wx/layout/LayoutPanel.cpp — ID definitions, menu entries under Bulk Edit (after Shadow Model For, before the controller port section), dispatch branches in both OnPreviewModelPopup sites, and the BulkEditRotateAxis implementation
README.txt — release note under 2026.07

No new files added, so no Xlights.vcxproj / xLights.cbp / Xcode project updates needed. All changes live in src-ui-wx/ (cross-platform UI layer), so the feature applies equally to Windows, macOS, and Linux builds.

Test plan

Build green on macOS Debug
Multi-select 3+ models, Bulk Edit → Rotate X/Y/Z → value applied to all
Locked model mixed into selection — skipped without error, others still rotate
Tree selection preserved after the operation
Cancel button on the dialog — no change, no undo entry
Invalid text (e.g. abc) — aborts silently, no change
Ctrl-Z after a bulk rotate — all models revert in one step
Windows smoke test (code is pure wx + std, no platform-specific APIs; would appreciate a Windows reviewer confirming)

Right-click with multiple models selected in the Layout tab now exposes three new items under Bulk Edit: "Rotate X", "Rotate Y", "Rotate Z". Each opens a text entry dialog pre-filled with the first unlocked selected model's current rotation for that axis, and applies the entered angle (in degrees, float OK) to every selected non-locked model. Selection is preserved across the reload by saving the tree paths before the dialog and calling ReselectTreeModels() afterward, matching the pattern used by BulkEditPixelSize and the other existing bulk-edit commands. Parsing uses std::strtod rather than std::stod per CLAUDE.md (xLights has effectively no exception handling). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Snapshot the full models XML via CreateUndoPoint("All", ...) before any rotation is applied, so a single Ctrl-Z reverts the entire batch as one undo step instead of leaving models in a partially-rotated state. The snapshot is deferred until after the dialog is accepted and the value successfully parsed, so Cancel or invalid input doesn't leave a no-op entry on the undo stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds new Bulk Edit context-menu actions in the Layout tab to apply a single-axis rotation (X/Y/Z) across the current multi-selection, while preserving tree selection and supporting one-step undo for the batch.

Changes:

Add three new Bulk Edit menu items: Rotate X / Rotate Y / Rotate Z.
Implement BulkEditRotateAxis(char axis) to prompt for an angle and apply it to all selected, unlocked models.
Add a 2026.07 release note entry documenting the new Bulk Edit rotation options.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src-ui-wx/layout/LayoutPanel.h	Adds new menu IDs and Bulk Edit rotation method declarations.
src-ui-wx/layout/LayoutPanel.cpp	Wires new menu items, dispatches events, and implements bulk rotation + undo + selection restore.
README.txt	Documents the enhancement in the 2026.07 release notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Two fixes rolled up: 1. Use WORK_SCREEN_LOCATION_CHANGE (not WORK_RELOAD_ALLMODELS) to persist the rotation change. This is the same work item Nudge() and the property-grid rotation handlers use. It re-computes transforms, redraws the preview, and reloads the property grid without tearing down and rebuilding every model - so selection survives naturally and we don't need the ClearSelectedModel / ReselectTreeModels dance. 2. After SetRotateX/Y/Z, call Reload() + Init() on the screen location. SetRotate*() only writes the raw rotatex/y/z fields; the cached rotate_quat (used by TranslatePoint, which drives the selection bounding box math) is rebuilt only inside Init() when rotation_init==true. Reload() sets that flag but nothing in the draw loop calls Init() to honor it, so the selection wireframe kept using the pre-rotation quaternion until the layout was closed and reopened. Calling both immediately makes the cache consistent so the white selection box rotates with the model on the first apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

heffneil · 2026-04-20T21:55:24Z

Pushed a follow-up commit fixing two real bugs found in live testing:

1. Wrong work item type: was using WORK_RELOAD_ALLMODELS (which tears down and rebuilds every model from XML). Switched to WORK_SCREEN_LOCATION_CHANGE to match how Nudge() and the property-grid rotation handlers persist geometry changes. This also removes the need for the ClearSelectedModel / ReselectTreeModels dance since the models do not get torn down.

2. Selection bounding box not rotating with the model: BoxedScreenLocation caches a rotate_quat quaternion that is rebuilt only inside Init() when rotation_init==true. SetRotateX/Y/Z only writes the raw rotatex/y/z fields; Reload() sets the flag but nothing in the draw loop calls Init() to honor it. So the model drew at the new angle (ApplyModelViewMatrices uses the raw fields directly) but TranslatePoint (which drives the selection bounding box bounds transform) kept using the stale quaternion. The user found they could work around it by closing and reopening the Layout tab, which naturally triggers a load-time Init. Calling Reload() + Init() immediately after each SetRotate*() makes the cache consistent on the first apply.

Live-tested on macOS Debug: white selection wireframe now rotates with the model on the first click; undo still rolls the whole batch back in one step.

Copilot noted that CreateUndoPoint("All", ...) was executed before the loop that could determine no models would actually change. When every selected model is locked (or the selection is empty) we would leave a no-op entry on the undo stack and have paid the cost of serializing the full models XML for nothing. Restructured to: 1. Build editableModels (non-null, non-locked) from modelsToEdit 2. Early-return if editableModels is empty 3. Pre-fill the dialog from editableModels.front() directly 4. Parse the entered angle (return on cancel / unparseable input) 5. Only then take the undo snapshot 6. Apply SetRotate* to each editable model in a simple loop Also reverts the README.txt release-note entry per maintainer guidance not to modify the release notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

heffneil · 2026-04-20T22:01:13Z

Addressed the Copilot finding plus reverted the README change (per maintainer guidance not to touch release notes):

Copilot: undo snapshot before editability check
The CreateUndoPoint("All", ...) call was running before the loop could determine whether any model would actually change. When every selected model is locked, the function would return early with changed == 0 but the undo stack already had a no-op entry and the full XML snapshot work was wasted.

Restructured to filter editableModels (non-null, non-locked) up front, early-return if empty, then take the undo snapshot only after we know at least one model will change AND the entered angle parsed successfully.

Release-note revert
Dropped the README.txt lines this PR previously added.

Build green on macOS Debug.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Two Copilot findings from the last review pass: 1. Input validation: std::strtod happily parses "nan" / "inf" and any numeric string including out-of-range values like 720. Non-finite would propagate garbage into the transform matrices; out-of-range values get silently reset to 0 by BoxedScreenLocation::Init() - so a user typing 200 would lose their input without warning. Reject !std::isfinite() and show a clear wxMessageBox for values outside [-180, 180] (the same range enforced by the rotation property grid via Min/Max attributes in ScreenLocationPropertyHelper). 2. Undo selection hint: CreateUndoPoint("All", ...) stores the second argument into undoBuffer[idx].model, which DoUndo() later passes as the "selectedModel" hint to AddASAPWork (LayoutPanel.cpp:8209). A label like "BulkRotateX" is not a real model name and would cause post-undo selection logic to resolve a nonexistent model. Pass editableModels.front()->name instead, and move the operation label to the key field. Added <cmath> include explicitly for std::isfinite so the build holds up under NO_PCH per CLAUDE.md guidance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

heffneil · 2026-04-20T22:55:32Z

Addressed the two new Copilot findings from the latest review round:

1. Input validation (non-finite + out-of-range)
std::strtod accepts "nan" / "inf" and any numeric string. Non-finite would propagate garbage into the transform matrices; out-of-range values get silently reset to 0 by BoxedScreenLocation::Init() - so a user typing 200 would lose their input without warning.

Now rejects !std::isfinite() silently, and shows a wxMessageBox warning for values outside [-180, 180] (the same range the rotation property grid enforces via Min/Max attributes in ScreenLocationPropertyHelper).

2. Undo selection hint
CreateUndoPoint("All", model, ...) stores the second argument into undoBuffer[idx].model, which DoUndo() later passes as the "selectedModel" hint to AddASAPWork (LayoutPanel.cpp:8209). A label like "BulkRotateX" is not a real model name - post-undo selection logic would try to resolve a nonexistent model.

Now passes editableModels.front()->name as the model hint, with the operation label moved to the key field.

Also added include explicitly so std::isfinite resolves under NO_PCH per CLAUDE.md guidance.

Build green on macOS Debug.

hssusc · 2026-04-22T04:42:10Z

This is another step toward making "organic" Peace Stakes (or similar props) easier to set up.

When Peace Stakes were introduced, 3D was just coming into xLights. The cube model quickly became the preferred method for these props just for simplicity and ease of setup. That is great, but makes it difficult for those that want to lay them out in a more "organic" or random pattern.

heffneil · 2026-04-29T12:45:53Z

Hi @dkulp / maintainers — friendly bump on this one. PR has been open since 2026-04-20, CI is green, mergeable, all Copilot threads addressed. Other PRs of mine that landed after it (#6186, #6241) have already merged.

Happy to rebase if it's drifted, just let me know if anything is still blocking. Thanks!

Removed comments to streamline the BulkEditRotateAxis function.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+    std::string entered = dlg.GetValue().ToStdString();
+    char* endp = nullptr;
+    double angle = std::strtod(entered.c_str(), &endp);
+    if (endp == entered.c_str() || !std::isfinite(angle)) {
+        // Unparseable or NaN/Inf - abort silently rather than letting garbage
+        // propagate into transform matrices.
+        return;
+    }


+void LayoutPanel::BulkEditRotateAxis(char axis) {
+    std::vector<Model*> modelsToEdit = GetSelectedModelsForEdit();
+
+    std::vector<Model*> editableModels;
+    editableModels.reserve(modelsToEdit.size());
+    for (Model* model : modelsToEdit) {
+        if (model != nullptr && !model->GetBaseObjectScreenLocation().IsLocked()) {
+            editableModels.push_back(model);
+        }
+    }
+    if (editableModels.empty()) {
+        return;
+    }


 #include <wx/tglbtn.h>
 #include <wx/srchctrl.h>
 #include <pugixml.hpp>
+#include <cmath>


neil heuer and others added 2 commits April 20, 2026 17:09

Copilot AI review requested due to automatic review settings April 20, 2026 21:34

Copilot started reviewing on behalf of heffneil April 20, 2026 21:34 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread src-ui-wx/layout/LayoutPanel.cpp Outdated

Copilot AI review requested due to automatic review settings April 20, 2026 22:01

Copilot started reviewing on behalf of heffneil April 20, 2026 22:01 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread src-ui-wx/layout/LayoutPanel.cpp

Comment thread src-ui-wx/layout/LayoutPanel.cpp Outdated

heffneil mentioned this pull request Apr 21, 2026

docs: add PR self-review checklist to CLAUDE.md #6200

Closed

Clean up comments in BulkEditRotateAxis function

9543049

Removed comments to streamline the BulkEditRotateAxis function.

Copilot AI review requested due to automatic review settings May 20, 2026 20:31

Copilot started reviewing on behalf of derwin12 May 20, 2026 20:31 View session

derwin12 merged commit 9947151 into xLightsSequencer:master May 20, 2026
5 checks passed

Copilot AI reviewed May 20, 2026

View reviewed changes

heffneil mentioned this pull request Jun 9, 2026

Bulk Edit Rotate: skip models whose type doesn't support that axis #6521

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Bulk Edit Rotate X / Y / Z to the Layout tab#6184

Add Bulk Edit Rotate X / Y / Z to the Layout tab#6184
derwin12 merged 6 commits into
xLightsSequencer:masterfrom
heffneil:feature/bulk-edit-xyz-axis

heffneil commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

hssusc commented Apr 22, 2026

Uh oh!

heffneil commented Apr 29, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

heffneil commented Apr 20, 2026

Summary

Design choices

Files changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

heffneil commented Apr 20, 2026

Uh oh!

hssusc commented Apr 22, 2026

Uh oh!

heffneil commented Apr 29, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants