Add direct cutting-plane control to GLVis scripts#374
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new cutting_plane GLVis script command to set cutting-plane orientation, translation, kind, and algorithm directly in one call, avoiding the expensive step-by-step recomputation caused by replaying multiple keystrokes.
Changes:
- Introduces
Plane::SetPlane()to set cutting-plane orientation/position directly. - Adds a virtual
VisualizationSceneScalarData::SetCuttingPlane()hook and implements it forVisualizationSceneSolution3d. - Registers and parses a new
cutting_planecommand in the script controller (degrees in scripts, converted to radians internally).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/vssolution3d.hpp | Declares VisualizationSceneSolution3d::SetCuttingPlane(...) override. |
| lib/vssolution3d.cpp | Implements SetCuttingPlane(...) to update plane, kind, algorithm, and trigger recomputation. |
| lib/vsdata.hpp | Adds Plane::SetPlane(...) and a default no-op SetCuttingPlane(...) hook in the base scene class. |
| lib/vsdata.cpp | Implements Plane::SetPlane(...) using stored bounding-box center + translation along the plane normal. |
| lib/script_controller.cpp | Adds the cutting_plane script command, parses optional args, and calls vs->SetCuttingPlane(...). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CuttingPlane->SetPlane(phi, theta, translation); | ||
| FindNodePos(); | ||
|
|
||
| if (kind != -1 && kind != cplane) | ||
| { | ||
| if (cplane == 2 && cp_drawmesh == 3) | ||
| { | ||
| cp_drawmesh = 2; | ||
| } | ||
| cplane = kind; | ||
| } | ||
|
|
||
| if (algo != -1) | ||
| { | ||
| cp_algo = algo; | ||
| } | ||
|
|
||
| CPPrepare(); | ||
| if (cplane == 0 || cplane == 2) | ||
| { | ||
| Prepare(); | ||
| PrepareLines(); | ||
| PrepareOrderingCurve(); | ||
| } |
There was a problem hiding this comment.
Well, it is right, but the suggestion is unnecessarily convoluted:
cplane_oldmakes sense to savecplane_changeddoes not make sense, it is trivial to comparecplaneandcplane_old- the test condition is correct
if (cplane_old == 2 || cplane == 2) - the inner test for the ordering curve makes sense to do, but it should be only
if (cplane != cplane_old) - the second branch does not make sense, because it does not realize that it was just the leaving case in the toggling procedure as
(2+1) % 3 == 0🙃 .
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Btw, do not mind the failing CI on mac, it is happening on master as well (https://github.com/GLVis/glvis/actions/runs/29534330078/job/87741910839) 🫣 . |
|
@najlkin or @camierjs , I am not familiar enough with the GLVis flow control to resolve this copilot comment. Could you please help with this? |
| int kind = 1, algo = -1; | ||
| scr >> ws; | ||
| int ch = scr.peek(); | ||
| if (ch != std::char_traits<char>::eof() && (isdigit(ch) || ch == '-')) |
There was a problem hiding this comment.
Remove the EOF check, it does not make sense - EOF is not a digit nor minus sign 🤪 .
Motivation
Today, the only way to position the 3D cutting plane from a GLVis script is to replay raw keystrokes via keys. Even a modest plane orientation requires several keystrokes:
(5
xpresses to rotatephiby 25°, 8ypresses to rotatethetaby 40°, thenito enable the plane). Each individual keystroke triggers a full cutting-plane geometry recomputation, not just the final one. On a large mesh that's a lot of wasted computation just to reach a state that could be stated directly in one line.What this adds
A single script command:
which sets the plane's orientation (degrees), translation, kind (cut-through-elements / solid-cut-behind / off), and algorithm (accurate / fast) directly, in one shot reproducing exactly what the equivalent keystroke sequence would have produced, without the redundant intermediate recomputations.
kinddefaults to 1 (cut through elements) so the plane is visible immediately;algis optional and leaves the current value unchanged when omitted.Implementation
Plane::SetPlane: new direct setter on the existingPlaneclass (previously only relativeIncrease/Decreasemutators existed).VisualizationSceneScalarData::SetCuttingPlane: empty-default virtual hook (2D scenes no-op safely).VisualizationSceneSolution3d::SetCuttingPlane— override combining plane move + kind + algorithm changes, mirroring the existing key-handler logic.cutting_planeregistered in the script command table.