Skip to content

Commit c0937cd

Browse files
andyclaude
andcommitted
MO Compute: build the isosurface at the requested cutoff (#226)
IsoSurfaceCmd built every surface at a hard-wired 0.05 (0.002 for ESP) and MoPanel applied the slider's cutoff afterwards with IsoValueCmd. Each Builder::execute() ends in a synchronous paint (refreshRenderArea is Refresh + Update), so the 0.05 surface was drawn twice, after IsoSurfaceCmd and after SurfDisplayTypeCmd, before the requested one. IsoSurfaceCmd takes an "isovalue" parameter (0 keeps the field-type default, so other callers are unchanged); MoPanel computes the initial cutoff first and passes it, and the ESP colour-range rebuild passes the slider's. The ESP colour shell still uses the default cutoff, as before. Verified with tools/coin/scenes/calc-water-mopanel.scene on calc-water-vib (HOMO): before, frames 4-5 carry thresholds +/-0.05 and ChemIso generates at 0.05 then 0.150818; after, every frame carries +/-0.150818 and only that surface is generated. Final frame pixel-identical before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFPK14wBcLD1s4dYUowpP4
1 parent e5f15df commit c0937cd

4 files changed

Lines changed: 61 additions & 21 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)

‎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/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)