Skip to content

Commit cc24a06

Browse files
andyclaude
andcommitted
Merge wip/viewer-228-226: an MO surface is built at the requested cutoff instead of flashing at 0.05 first (#226); normal-mode arrow measurements for #228
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFPK14wBcLD1s4dYUowpP4
2 parents b9441a4 + c0937cd commit cc24a06

11 files changed

Lines changed: 298 additions & 22 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
type: pitfall
3+
title: "`Builder::execute()` paints after every command, so a command sequence shows each intermediate state (#226)"
4+
area: wx-viewer
5+
paths: [src/apps/builder/Builder.C, src/wxviz/viewer/SGViewer.C, src/apps/builder/MoPanel.C, src/viz/propsgcommands/IsoSurfaceCmd.C, src/viz/propsgcommands/IsoValueCmd.C, src/inv/moiv/ChemIso.C]
6+
issues: [226]
7+
---
8+
`Builder::execute(cmd)` (non-batch) ends with `SGViewer::refreshRenderArea()`,
9+
which is `Refresh(false)` + `Update()`: a synchronous paint. A panel that
10+
reaches its final scene through several commands therefore puts every
11+
intermediate scene on screen, one frame each.
12+
13+
MoPanel's Compute ran `IsoSurfaceCmd` (built at a hard-wired 0.05, 0.002
14+
for ESP), `SurfDisplayTypeCmd`, then `IsoValueCmd` (the slider's cutoff):
15+
two frames of a 0.05 surface before the requested one. `IsoSurfaceCmd` now
16+
takes an `isovalue` parameter (0 = the field-type default) and MoPanel
17+
passes the cutoff it is about to put on the slider. Cube.C still builds at
18+
the default and never applies its slider on grid selection.
19+
20+
To see it headlessly: `tools/coin/scenes/calc-water-mopanel.scene`
21+
(`mopanel <name>`, a Builder-only scene command) presses Compute and writes
22+
every frame of the Builder canvas with the thresholds of the `ChemIso`
23+
nodes in the scene at that frame; `ECCE_DEBUG_ISO=1` also logs each surface
24+
`ChemIso::regenerate` actually generates.

‎docs/claude/wx-viewer/index.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ Read this before changing any dialog, panel, sizer, grid, ewx control or the Ope
1515
- [`ECCE_USE_COIN` (default ON) builds the viewer against Coin3D; `=OFF` keeps the vendored core for one release](ecce-use-coin-builds-the-viewer-against-coin3d.md)
1616
- [Coin SORTED_LAYERS_BLEND is the default lobe transparency (6 passes, alpha canvas); slow scenes fall back to SCREEN_DOOR](coin-sorted-layers-blend-is-the-default-lobe-transparency.md)
1717
- [`GlPlatform` is the viewer's only GLX/EGL code; `tools/nonx11/check.py` compiles the viewer as a non-X11 port (#232)](glplatform-is-the-viewers-only-glx-egl-code.md)
18+
- [`Builder::execute()` paints after every command, so a command sequence shows each intermediate state (#226)](builder-execute-paints-after-every-command.md)
19+
- [Normal-mode arrows start at the atom centre and scale uniformly; small ones end inside the sphere (#228)](normal-mode-arrows-start-at-the-atom-centre.md)
1820
- [`tools/coin/compare.sh` renders a fixed scene set with both viewer builds (#166 stage 2); harness pitfalls](tools-coin-compare-sh-renders-both-viewer-builds.md)
1921
- [Atom labels drew nothing in 9.x: the sh wrappers dropped `FL_FONT_PATH`; flclient now defaults to `$ECCE_HOME/data/client/fonts`](atom-labels-need-the-bundled-font-fl-font-path.md)
2022
- [Builder Reset View (toolbar, Render menu, Home) is camera only; the old home button restored a pre-molecule camera](builder-reset-view-is-camera-only-home-key.md)
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
type: map
3+
title: "Normal-mode arrows start at the atom centre and scale uniformly; small ones end inside the sphere (#228)"
4+
area: wx-viewer
5+
paths: [src/viz/propsgcommands/NModeVectCmd.C, src/viz/nodes/VRVector.C, src/apps/builder/NModePanel.C, src/wxviz/viewer/SceneScript.C]
6+
issues: [228]
7+
---
8+
`NModeVectCmd` puts one `VRVector` per atom at the atom centre, direction
9+
`d * 0.5/maxnorm` (Scale slider multiplies it). `VRVector` scales its whole
10+
geometry (shaft, head, thickness) by |d|; the tip is at 1.075 |d|, so the
11+
longest arrow reaches 0.5375 A from its centre.
12+
13+
Displayed sphere radii (Ball And Stick and the default style) are covalent
14+
x 0.5: O 0.365, H 0.20, C about 0.38. An atom whose displacement is below
15+
about 70 % of the largest one (any heavy atom in an X-H stretch) has its
16+
whole arrow inside its sphere. Separately, in Ball And Stick an arrow
17+
pointing along a bond lies inside the bond cylinder and is hidden too (the
18+
water symmetric stretch shows no arrows at all with Sign unticked).
19+
20+
Measured, not computed: the scene command `nmcheck <name>` reports per atom
21+
the farthest arrow vertex from the atom centre (`SoCallbackAction`
22+
triangles) and where a ray along the arrow meets that atom's sphere
23+
(`SoRayPickAction`, atom detail); `tools/coin/scenes/calc-water-nmcheck.scene`.

‎include/wxviz/SceneScript.H‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef SCENESCRIPT_H_
22
#define SCENESCRIPT_H_
33

4+
#include <functional>
45
#include <string>
56
#include <vector>
67
using std::string;
@@ -9,6 +10,7 @@ using std::vector;
910
class IPropCalculation;
1011
class SGContainer;
1112
class SGViewer;
13+
class wxWindow;
1214

1315
/**
1416
* Drives the viewer from a text script and writes offscreen snapshots, so
@@ -32,6 +34,11 @@ class SGViewer;
3234
* viewall | clear reset the camera / hide MO and modes
3335
* snap <name> the canvas, read back with glReadPixels -> <name>.ppm
3436
* thumb <name> 64x64 black background, as the calculation thumbnail
37+
* nmcheck <name> per normal-mode arrow: tip distance from its atom centre
38+
* (arrow triangles) and the atom's sphere surface along
39+
* the arrow (ray pick) -> <name>.txt
40+
* Commands this class does not know go to the caller's extension, if set
41+
* (the Builder adds "mopanel", which presses the MO panel's Compute).
3542
*/
3643
class SceneScript
3744
{
@@ -43,20 +50,32 @@ class SceneScript
4350
bool run(const string& scriptPath);
4451
const string& message() const { return p_msg; }
4552

53+
// Unknown commands go here; return false (and set fail()) on error.
54+
typedef std::function<bool(SceneScript&, const vector<string>&)> Extension;
55+
void setExtension(Extension e) { p_ext = e; }
56+
57+
// Runs act() and records every frame the render area under canvasOwner
58+
// paints meanwhile: <name>-NN.ppm plus one line per frame in
59+
// <name>-frames.txt with the thresholds of the isosurfaces drawn.
60+
bool recordFrames(wxWindow *canvasOwner, const string& name,
61+
std::function<void()> act);
62+
bool fail(const string& m) { p_msg = m; return false; }
63+
4664
private:
4765
bool exec(const vector<string>& w, const string& rest);
4866
bool pickAtoms(const string& name, const vector<string>& atoms);
4967
bool dragAtom(const string& name, int atom, int dx, int dy);
5068
bool countRedraws(const string& name, const vector<string>& steps);
5169
bool snapshot(const string& name, int size, float r, float g, float b);
5270
bool exportIso(const string& name);
53-
bool fail(const string& m) { p_msg = m; return false; }
71+
bool nmCheck(const string& name);
5472

5573
SGViewer *p_viewer;
5674
SGContainer *p_sg;
5775
IPropCalculation *p_calc;
5876
string p_outdir;
5977
string p_msg;
78+
Extension p_ext;
6079
};
6180

6281
#endif

‎src/apps/builder/Builder.C‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ using std::vector;
178178
#include "Peptide.H"
179179
#include "PropertyIndexPanel.H"
180180
#include "PropertyPanel.H"
181+
#include "MoPanel.H"
181182
#include "PropertyPanelFactory.H"
182183
#include "ShapeDropDown.H"
183184
#include "StructLib.H"
@@ -5104,6 +5105,24 @@ void Builder::updatePropertyMenus()
51045105
for (int i = 0; i < 30; i++) { wxTheApp->Yield(true); wxMilliSleep(10); }
51055106
viewer->viewAll();
51065107
SceneScript run(viewer, sg, p_calculation, outdir);
5108+
// "mopanel <name>": press the MO panel's Compute, as a user would,
5109+
// recording every frame the Builder's own canvas paints meanwhile.
5110+
run.setExtension([this](SceneScript& s, const vector<string>& w) {
5111+
if (w[0] != "mopanel" || w.size() != 2)
5112+
return s.fail("unknown command: " + w[0]);
5113+
MoPanel *mo = 0;
5114+
set<PropertyPanel*> panels =
5115+
PropertyPanel::getPanels(p_calculation->getURL().toString());
5116+
for (set<PropertyPanel*>::iterator it = panels.begin();
5117+
it != panels.end() && !mo; ++it)
5118+
mo = dynamic_cast<MoPanel*>(*it);
5119+
if (!mo) return s.fail("mopanel: no MO panel");
5120+
return s.recordFrames(p_viewer, w[1], [mo]() {
5121+
wxCommandEvent ev(wxEVT_BUTTON, MoGUI::ID_BUTTON_MO_COMPUTE);
5122+
ev.SetEventObject(mo->FindWindow(MoGUI::ID_BUTTON_MO_COMPUTE));
5123+
mo->GetEventHandler()->ProcessEvent(ev);
5124+
});
5125+
});
51075126
if (!ready || !run.run(script)) {
51085127
string msg = ready ? run.message() : "molecule never loaded";
51095128
fprintf(stderr, "ECCE_VIEWER_SCENE: %s\n", msg.c_str());

‎src/apps/builder/MoPanel.C‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -981,20 +981,6 @@ void MoPanel::OnButtonMoComputeClick( wxCommandEvent& event )
981981
double posR, posG, posB, negR, negG, negB;
982982
getSurfaceColors(posR, posG, posB, negR, negG, negB);
983983

984-
cmd = new IsoSurfaceCmd("Iso Surface", &sg, expt);
985-
applyEspRange(cmd);
986-
cmd->getParameter("transparency")->setDouble(getTransparency());
987-
cmd->getParameter("positiveRed")->setDouble(posR);
988-
cmd->getParameter("positiveGreen")->setDouble(posG);
989-
cmd->getParameter("positiveBlue")->setDouble(posB);
990-
cmd->getParameter("negativeRed")->setDouble(negR);
991-
cmd->getParameter("negativeGreen")->setDouble(negG);
992-
cmd->getParameter("negativeBlue")->setDouble(negB);
993-
fw.execute(cmd);
994-
995-
updateSurface();
996-
997-
998984
// Determine max abs value of field for the (abs value) slider
999985
// Bruce says that for spin density, its better to hardwire the max
1000986
// scale to no omre than 1.0 but probably even less.
@@ -1035,6 +1021,22 @@ void MoPanel::OnButtonMoComputeClick( wxCommandEvent& event )
10351021
if (espSurface) {
10361022
isovalue = log10(0.002);
10371023
}
1024+
1025+
cmd = new IsoSurfaceCmd("Iso Surface", &sg, expt);
1026+
applyEspRange(cmd);
1027+
cmd->getParameter("isovalue")->setDouble(pow(10.0, isovalue));
1028+
cmd->getParameter("transparency")->setDouble(getTransparency());
1029+
cmd->getParameter("positiveRed")->setDouble(posR);
1030+
cmd->getParameter("positiveGreen")->setDouble(posG);
1031+
cmd->getParameter("positiveBlue")->setDouble(posB);
1032+
cmd->getParameter("negativeRed")->setDouble(negR);
1033+
cmd->getParameter("negativeGreen")->setDouble(negG);
1034+
cmd->getParameter("negativeBlue")->setDouble(negB);
1035+
fw.execute(cmd);
1036+
1037+
updateSurface();
1038+
1039+
10381040
p_slider->SetValue(isovalue);
10391041
updateIsoValue(isovalue);
10401042

@@ -1614,6 +1616,8 @@ void MoPanel::rebuildEspSurface()
16141616

16151617
Command *cmd = new IsoSurfaceCmd("Iso Surface", &sg, expt);
16161618
applyEspRange(cmd);
1619+
cmd->getParameter("isovalue")->setDouble(
1620+
pow(10.0, static_cast<double>(p_slider->GetFloatValue())));
16171621
cmd->getParameter("transparency")->setDouble(getTransparency());
16181622
cmd->getParameter("positiveRed")->setDouble(posR);
16191623
cmd->getParameter("positiveGreen")->setDouble(posG);

‎src/inv/moiv/ChemIso.C‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555

5656
#ident "$Revision: 22147 $"
5757

58+
#include <cstdio>
59+
#include <cstdlib>
5860
#include "inv/actions/SoCallbackAction.H"
5961
#include "inv/actions/SoGetBoundingBoxAction.H"
6062
#include "inv/actions/SoGetMatrixAction.H"
@@ -485,4 +487,10 @@ RESET_TIMER();
485487

486488
isoLibIso(threshold.getValue());
487489
MARK_TIME("DONE with ISO");
490+
// ECCE_DEBUG_ISO: every surface actually generated, with its cutoff.
491+
static const bool isoDebug = getenv("ECCE_DEBUG_ISO") != 0;
492+
if (isoDebug) {
493+
fprintf(stderr, "ISO: ChemIso %p generated at threshold %g, %d vertices\n",
494+
(void *)this, threshold.getValue(), vp->vertex.getNum());
495+
}
488496
}

‎src/viz/propsgcommands/IsoSurfaceCmd.C‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ void IsoSurfaceCmd::init()
5959
addParameter(new CommandParameter("colorMin", -0.05));
6060
addParameter(new CommandParameter("colorMax", 0.05));
6161

62+
// The cutoff to build the surface at; 0 or less means the field
63+
// type's default below. A caller that knows the user's cutoff passes
64+
// it: building at the default first showed that surface for a frame
65+
// or two before IsoValueCmd replaced it (#226).
66+
addParameter(new CommandParameter("isovalue", 0.0));
67+
6268
}
6369

6470
bool IsoSurfaceCmd::execute()
@@ -287,8 +293,12 @@ bool IsoSurfaceCmd::execute()
287293
}
288294
}
289295

296+
const double requested = getParameter("isovalue")->getDouble();
297+
const float threshold = requested > 0.0 ? (float)requested : isovalue;
298+
ISO_LOG << "ISO: building at threshold " << threshold << std::endl;
299+
290300
// Since we only deal with absolute values of isovalues:
291-
if (isovalue > 0.0) {
301+
if (threshold > 0.0) {
292302
positiveLobeColor = posColor;
293303
negativeLobeColor = negColor;
294304
}
@@ -331,7 +341,7 @@ bool IsoSurfaceCmd::execute()
331341
gridStruct->colorFieldMax(), transparency);
332342
}
333343

334-
isosurf1->threshold.setValue(isovalue);
344+
isosurf1->threshold.setValue(threshold);
335345

336346
#ifndef MI10
337347
/* Well, I guess I sort of screwed you over here, but nothing that can't be
@@ -378,7 +388,7 @@ bool IsoSurfaceCmd::execute()
378388

379389
isosurf2->dataVar = 0;
380390
isosurf2->colorVar = 0;
381-
isosurf2->threshold.setValue(-isovalue);
391+
isosurf2->threshold.setValue(-threshold);
382392

383393
#ifndef MI10
384394
childList = isosurf2->getChildren();
@@ -410,13 +420,13 @@ bool IsoSurfaceCmd::execute()
410420
chemMesh->kAxis.setValue(true);
411421
chemMesh->dataVar = 0;
412422

413-
chemMesh->levels.set1Value(0, isovalue);
423+
chemMesh->levels.set1Value(0, threshold);
414424
chemMesh->orderedRGBA.set1Value(0, positiveLobeColor);
415425

416426
// For other than Density field, we want to display both positive and
417427
// negative lobes (isosurfaces)
418428
if (!densityLike) {
419-
chemMesh->levels.set1Value(1, -isovalue);
429+
chemMesh->levels.set1Value(1, -threshold);
420430
chemMesh->orderedRGBA.set1Value(1, negativeLobeColor);
421431
}
422432

@@ -438,15 +448,15 @@ bool IsoSurfaceCmd::execute()
438448
chemContour->kAxis.setValue(false);
439449
chemContour->dataVar = 0;
440450

441-
chemContour->levels.set1Value(0, isovalue);
451+
chemContour->levels.set1Value(0, threshold);
442452
chemContour->orderedRGBA.set1Value(0, positiveLobeColor);
443453
// vp = (SoVertexProperty *)chemContour->vertexProperty.getValue();
444454
// vp->orderedRGBA.setValue(positiveLobeColor);
445455

446456
// For other than Density field, we want to display both positive and
447457
// negative lobes (isosurfaces)
448458
if (!densityLike) {
449-
chemContour->levels.set1Value(1, -isovalue);
459+
chemContour->levels.set1Value(1, -threshold);
450460
chemContour->orderedRGBA.set1Value(1, negativeLobeColor);
451461
}
452462

0 commit comments

Comments
 (0)