Skip to content

Commit ce4f851

Browse files
andyclaude
andcommitted
Merge wip/166-coin-port: Coin3D port stages 2-3 (comparison tools, isosurfaces, thumbnails via EGL, picking, transparency state, redraw sensor fix #99), still off by default
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9WkbXZ96rqCJxYuumcrgF
2 parents 51dbe45 + 9a84e31 commit ce4f851

35 files changed

Lines changed: 2759 additions & 14 deletions

‎.github/workflows/build.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,32 @@ jobs:
9494
name: ecce-debian-trixie-deb
9595
path: build-cmake/*.deb
9696

97+
# Build only, no packaging: keeps -DECCE_USE_COIN=ON (#166) compiling
98+
# while the vendored Inventor core is still the default.
99+
coin:
100+
name: Debian, Coin3D viewer (build only)
101+
runs-on: ubuntu-latest
102+
timeout-minutes: 90
103+
container: debian:trixie
104+
steps:
105+
- name: Install build dependencies
106+
run: |
107+
apt-get update
108+
apt-get install -y \
109+
build-essential gfortran cmake ninja-build python3 \
110+
libwxgtk3.2-dev libxerces-c-dev libgl-dev libglu1-mesa-dev \
111+
libgtk-3-dev libx11-dev libice-dev libxt-dev libjpeg-dev \
112+
libmosquitto-dev mosquitto-dev libaprutil1-dev mosquitto git dpkg-dev file libssh-dev \
113+
libcoin-dev
114+
115+
- uses: actions/checkout@v4
116+
117+
- name: Configure
118+
run: cmake -G Ninja -B build-coin -DECCE_USE_COIN=ON -DECCE_REQUIRE_LIBSSH=ON
119+
120+
- name: Build
121+
run: cmake --build build-coin
122+
97123
ubuntu:
98124
name: Ubuntu (apt)
99125
runs-on: ubuntu-latest

‎.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
/lib/
55
/obj/
66
/build-cmake/
7+
/build-coin/
8+
/build-coin-compare/
9+
/inst-cmake/
10+
/inst-coin/
711
src/apps/symmetry/*.mod
812
__pycache__/
913

‎CMakeLists.txt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,10 @@ ecce_library(eccewxinv src/inv/wxinv)
293293
target_compile_definitions(eccewxinv PRIVATE NDEBUG ${wxWidgets_DEFINITIONS})
294294
target_include_directories(eccewxinv PRIVATE ${wxWidgets_INCLUDE_DIRS})
295295
target_link_libraries(eccewxinv PRIVATE ${wxWidgets_LIBRARIES} OpenGL::GL OpenGL::GLU)
296+
if(ECCE_USE_COIN AND UNIX AND NOT APPLE)
297+
find_package(OpenGL REQUIRED COMPONENTS EGL)
298+
target_link_libraries(eccewxinv PUBLIC OpenGL::EGL)
299+
endif()
296300
297301
# --- src/wxviz -> libeccewxviz.a ---
298302
ecce_library(eccewxviz src/wxviz/viewer src/wxviz/viztools)
@@ -465,6 +469,19 @@ ecce_app(builder src/apps/builder WX GL
465469
ecce_app(vizthumbnail src/apps/vizthumbnail WX GL
466470
LINK eccewxviz eccewxinv eccevizsg eccemoiv ecceinv eccewxgui eccewxplotctrl eccewxthings eccedsi eccedav eccecipc eccefaces eccexml eccetdat ecceutil X11::X11 jpeg freetype)
467471
472+
# Headless scene renderer for comparing the vendored and Coin viewer builds
473+
# (tools/coin/compare.sh); built only on request.
474+
ecce_app(viewer-scenes tools/coin WX GL
475+
LINK eccewxviz eccewxinv eccevizsg eccemoiv ecceinv eccewxgui eccewxplotctrl eccewxthings eccedsi eccedav eccecipc eccefaces eccexml eccetdat ecceutil X11::X11 jpeg freetype)
476+
set_target_properties(viewer-scenes PROPERTIES EXCLUDE_FROM_ALL TRUE)
477+
478+
# Coin only: the EGL offscreen path with no wx and no display.
479+
if(ECCE_USE_COIN)
480+
add_executable(offscreen-check EXCLUDE_FROM_ALL tools/coin/offscreen/offscreen-check.C)
481+
target_link_libraries(offscreen-check PRIVATE eccewxinv ${COIN_LIBRARY} ${wxWidgets_LIBRARIES} OpenGL::GL)
482+
target_include_directories(offscreen-check PRIVATE ${wxWidgets_INCLUDE_DIRS})
483+
endif()
484+
468485
# --- src/apps/symmetry -> 5 standalone Fortran executables, no wx involvement.
469486
# Mirrors the old Makefile's "default:" target exactly (testnames is a
470487
# dev/test-only program the old default build skipped - left out here too).
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
type: pitfall
3+
title: "Coin SORTED_LAYERS_BLEND orders lobes per pixel but needs an alpha visual and costs a full scene pass per layer"
4+
area: wx-viewer
5+
paths: [tools/coin/isoref.py, src/inv/wxinv/SoWxViewer.C, tools/coin/lobes.py, tools/coin/angles.py, src/wxviz/viewer/SceneScript.C]
6+
issues: [166]
7+
---
8+
Two MO lobes (the `ChemIso` positive and negative surfaces) have identical
9+
bounding boxes, so any per-object sort ties; the vendored core and Coin break
10+
the tie in opposite order. Depth peeling is order independent, but:
11+
- Needs an 8-bit alpha in the framebuffer; the Builder's canvas has none
12+
(`SoWxViewer` passes its own attribute list), and Coin then warns and uses
13+
SORTED_OBJECTS_BLEND. `ECCE_COIN_ALPHA=1` requests alpha (dev only). The EGL
14+
thumbnail FBO is RGBA8/D24 and peels fine; llvmpipe supports it.
15+
- Correct where measured (`tools/coin/lobes.py`, depth-tested opaque renders
16+
as oracle): real MOs 100% of overlap pixels, synthetic pi fields 97.6% at 6
17+
passes (Coin's default 4: 94.9%; 12+ is worse, not better). The per-object
18+
sort is 50-55% on both builds.
19+
- Cost: every pass redraws the whole scene. Software GL, 800x800: 10x on small
20+
molecules (2 -> 26 ms), 6x on the 5184-atom water box (170 -> ~1000 ms).
21+
- ECCE never blends MO lobes: they are SCREEN_DOOR; SORTED_OBJECT_BLEND is
22+
used only for ESP maps, which are opaque. So the tie only shows in
23+
`tools/coin/angles.py`, and peeling would be pure cost in the Builder.
24+
- Independent reference (`tools/coin/isoref.py`, `lobes.py --ref`): ray-casts
25+
exported lobe triangles, composites all hits by depth, no Inventor
26+
transparency code. Its lighting is assumed (headlight, two-sided) and does
27+
not match either library (~78% of lobe pixels differ by >24/255), so judge
28+
by dominant lobe colour in overlap pixels: layers 85%, Coin per-object sort
29+
57%, vendored 50%. The reference's nearest-lobe agreement is lower than the
30+
depth-test oracle's 97.6% where lobes interleave in depth.
31+
32+
SCREEN_DOOR (what the Builder uses), measured the same way (`LOBE_MODES=sd
33+
tools/coin/lobes.py`, then `--sd` and `--ref`; real MOs plus synthetic lobes,
34+
0/45/90 deg, alpha 0.5, both builds). Where both lobes' stipple passes in an
35+
overlap pixel (232k px vendored, 205k Coin; the stipple masks of the two lobes
36+
are identical, zero pixels differ), the visible lobe is the nearer one in 100%
37+
of pixels on both builds, and the back lobe shows in 0%: the pixel is front
38+
lobe or background, never a mix. Against `isoref`'s dominant colour on the same
39+
pixels: 96.0% vendored, 95.6% Coin (nearest lobe 99.3% / 99.1%); the reference
40+
is an alpha composite that would show the back lobe at weight 0.25
41+
((1-a)*a), which a screen door cannot do. Counted over all overlap pixels
42+
(stipple holes included) agreement is only 75% / 68%. Vendored and Coin agree on
43+
which lobe is dominant in 100% of pixels both draw, but the stipple density
44+
differs: lobe covers 58.5% of overlap pixels in the vendored core, 51.6% in
45+
Coin at alpha 0.5. So ordering is a non-issue in SCREEN_DOOR.

‎docs/claude/wx-viewer/ecce-use-coin-builds-the-viewer-against-coin3d.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,21 @@ core is removed only at stage 4. Use separate build directories
2626
(`build-cmake`, `build-coin`): switching the option in one tree rebuilds
2727
everything. After a change under `src/inv/moiv`, `src/inv/wxinv` or
2828
`include/inv/{ChemKit,SoWx}`, build both.
29+
30+
Thumbnails (`SoOffscreenRenderer`): Coin's own GLX offscreen context fails
31+
under Xvfb and crashes with no `DISPLAY`. `src/inv/wxinv/CoinEglOffscreen.C`
32+
(installed from `SoWx::init`, Coin builds on Linux only) supplies a Mesa EGL
33+
context + FBO through `cc_glglue_context_set_offscreen_cb_functions`. Traps:
34+
- Coin asserts a GLX context is current when it first sees a context; the
35+
check cannot see EGL, so `COIN_GL_NO_CURRENT_CONTEXT_CHECK` is set in code.
36+
- glvnd refuses `eglMakeCurrent` (`EGL_BAD_ACCESS`) while a GLX context, such
37+
as the wx canvas's, is current: release it first and give it back.
38+
- Coin casts our callback handle to its own GLX context struct in
39+
`cc_glglue_context_max_dimensions` (reads +0x40/+0x48/+0x50); the handle
40+
starts with zeroed padding so that path answers "no pbuffer".
41+
- Coin prints one harmless `glxglue_isdirect()` warning per run.
42+
`make offscreen-check` (target, Coin build) renders with no wx and no display.
43+
44+
Renderer differences between the libraries that need code in our layer are
45+
listed in the `tools/coin/compare.sh` entry (transparency default, SoCylinder
46+
material, handle-event viewport, stipple leak).

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Read this before changing any dialog, panel, sizer, grid, ewx control or the Ope
1313

1414
- [Open Inventor's redraw sensor is a ONE-SHOT that re-arms on render, and with a render callback installed nothing re-arms it](open-inventors-redraw-sensor-is-a-one.md)
1515
- [`ECCE_USE_COIN` builds the viewer stack against Coin3D; both builds coexist until stage 4](ecce-use-coin-builds-the-viewer-against-coin3d.md)
16+
- [`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)
1617
- [wx3.2/GTK3 layout reentrancy](wx3-2-gtk3-layout-reentrancy.md)
1718
- [`wxGrid::CreateGrid()`/`SetTable()` synchronously fires `wxEVT_GRID_SELECT_CELL`](wxgrid-creategrid-settable-synchronously-fires-wxevt-grid.md)
1819
- [The `.pjd` (DialogBlocks) files are reference only; never regenerate code from them](the-pjd-dialogblocks-files-are-reference-only.md)

‎docs/claude/wx-viewer/open-inventors-redraw-sensor-is-a-one.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,12 @@ service). If a viewer stops updating while the data demonstrably
2424
changes, instrument `renderCB` first: `ECCE_DEBUG_GEOMTRACE=1` prints
2525
`[RENDERCB]` lines alongside the step trace, and their *absence* is
2626
the finding.
27+
28+
**UPDATE 2026-10-04 (#166): the actual cause.**
29+
`SoWxEventHandler::setUpCallbacks()` is the sensor manager's changed-callback.
30+
It runs inside `SoDelayQueueSensor::schedule()` before that sets `scheduled`,
31+
and drained the queue synchronously (`ProcessEvent(wxIdleEvent)`), so the
32+
redraw sensor fired and was then marked scheduled for good. It now calls
33+
`wxWakeUpIdle()`. Measured with the scene command `redraws` (six trace steps,
34+
no forced paint): renders per change 1/6 vendored, 0/6 Coin before; 6/6 on
35+
both after. The `refreshRenderArea()` calls stay as a belt-and-braces.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
type: map
3+
title: "tools/coin/compare.sh renders a fixed scene set with both viewer builds (#166 stage 2); pitfalls of the harness"
4+
area: wx-viewer
5+
paths: [tools/coin/compare.sh, tools/coin/compare.py, tools/coin/viewer-scenes.C, tools/coin/scenes, src/wxviz/viewer/SceneScript.C, include/wxviz/SceneScript.H, src/apps/builder/Builder.C, src/inv/wxinv/SoWxRenderArea.C]
6+
issues: [166]
7+
---
8+
`SceneScript` (wxviz) runs a text script (`style`, `labels`, `select`, `mo`,
9+
`nmvect`, `nmstep`, `gtstep`, `snap`, `thumb`) against an SGViewer. Two
10+
callers: `viewer-scenes` (built-in XYZ systems, EXCLUDE_FROM_ALL target) and
11+
the Builder, when `ECCE_VIEWER_SCENE=<script>` and `ECCE_VIEWER_SCENE_OUT=<dir>`
12+
are set (inert otherwise; it renders in a second 480x480 viewer on the
13+
Builder's scene graph, then closes). `compare.py` drives both on a private
14+
Xvfb and writes `vendored.png/coin.png/diff.png` per scene plus a contact
15+
sheet of pairs that differ after removing the vendored build's constant
16+
colour offset (#83), estimated per pair as a per-channel median.
17+
18+
Traps found building it:
19+
- `snap` reads the canvas with `glReadPixels` through
20+
`SoWxRenderArea::setFrameCallback` (called before the buffer swap). The
21+
Builder's own canvas is only as big as the pane layout leaves it (181x671
22+
headless), hence the second viewer.
23+
- `SoOffscreenRenderer` (the thumbnail path) cannot make a GLX context on
24+
Coin under Xvfb ("Couldn't create GLX context"), (stage 3a fixed this with an EGL context, see the ECCE_USE_COIN entry). The vendored class
25+
crashes inside Mesa when destroyed, so the driver keeps one and `_exit`s.
26+
- Atom labels are drawn in the foreground colour (black on the default black
27+
background until `ForegroundCmd` runs) and need `FL_FONT_PATH` pointing at
28+
`data/client/fonts/` (with a trailing slash) when the Builder wrapper's
29+
`ecce_env` is not used.
30+
- A selected atom shows only if it goes through the viewer's `SGSelection`
31+
node (`getSelectionPath` + `ChemDisplayPath` + `merge`); filling
32+
`m_atomHighLight` alone changes nothing on screen.
33+
- The vendored build's offset is about +41 on every channel for lit
34+
surfaces, not only green; compare after a per-channel correction.
35+
- `IsoValueCmd`'s "Value" is a log10 slider value (10^Value is the isovalue);
36+
passing 0.05 puts the surface above the field maximum and draws nothing.
37+
- Normal-mode indices are 0-based and the fixture has 3 modes; an index past
38+
the table gives zero displacement, not an error.
39+
- `SO_NODE_INIT_CLASS`'s third argument is the PARENT's name, looked up
40+
with `SoType::fromName`. `PropSGFragment` passed its own name, which gave
41+
a type outside the SoNode tree: Coin aborted on it, the vendored core
42+
left null action methods (crash in `SoSearchAction`). Fixed; it is now
43+
registered from `SGContainer::initClass()`, so every app that builds a
44+
container scene gets it.
45+
- `IsoLib` wrote a leading end-of-strip `-1` in `coordIndex`; Coin takes it
46+
for an erroneous polygon and draws nothing. Skipped under `OIV_COIN`.
47+
(`#ifdef __coin` blocks elsewhere in `moiv` are dead: nothing defines it.)
48+
- Stage 3b findings (each one a visible difference, found by diffing GL state
49+
per frame with `tools/coin/gltrace.c`):
50+
- Coin's render action starts in BLEND, Open Inventor's in SCREEN_DOOR; MO
51+
lobes (alpha 0.5) were blended onto black = half brightness. Set in
52+
`SoWxRenderArea`'s constructor.
53+
- Coin's `SoCylinder::GLRender` sends the state's grey diffuse on every
54+
call, overwriting `ChemUnitCylinder`'s per-bond `glColor3fv`, and uses 20
55+
slices where Open Inventor uses 16. The SOCYLINDER branch (the Builder's
56+
default style) now draws the Open Inventor geometry itself.
57+
- Coin's `SoSceneManager::setWindowSize` sizes only the GL action; the
58+
handle-event action stayed 400x400, so `SGSelection`'s ray pick missed
59+
every atom at 480x480. Real picking in a Coin Builder was broken.
60+
- Coin leaves `GL_POLYGON_STIPPLE` on after a stippled lobe; `ChemDisplay`
61+
(glColor, no lazy-element alpha send) then drew atoms half missing.
62+
- Scene commands `pick`, `drag` (SoEvents built in code and sent to the
63+
scene manager, so SoHandleEventAction -> SGSelection -> processMotion run;
64+
a listener copies `Builder::motionChanged`), `redraws` (frames per scene
65+
change, no forced paint), `vizthumb`, `transparency`. Text results are
66+
compared by `compare.py`. `snap` forces a paint, so it cannot show #99.
67+
- Not defects: Coin's stipple pattern for alpha 0.498 has 16 more set bits
68+
per 1024 (1.6% more lobe pixels; the ~1650 isolated pixels in mo5);
69+
SORTED_OBJECT_BLEND orders two equidistant lobes differently (ESP maps are
70+
one opaque surface); the vendored build shows no normal-mode vectors on the
71+
first paint after `nmvect` (stale until the next camera change) and Coin
72+
does, so Coin is right.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#ifndef COINEGLOFFSCREEN_H
2+
#define COINEGLOFFSCREEN_H
3+
4+
// Coin3D only (#166): offscreen GL contexts for SoOffscreenRenderer from Mesa
5+
// EGL instead of GLX, so thumbnails render under Xvfb and with no X server.
6+
// Call once, before the first SoOffscreenRenderer is created (SoWx::init does).
7+
// Returns false, leaving Coin's own GLX path in place, if EGL is unusable.
8+
namespace CoinEglOffscreen {
9+
bool install();
10+
}
11+
12+
#endif

‎include/inv/SoWx/SoWxRenderArea.H‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,15 @@ protected:
176176
// single/double buffer swapping.
177177
virtual void redraw();
178178
virtual void actualRedraw();
179+
180+
public:
181+
// Called after each scene render, before the buffers are swapped; lets
182+
// a test read the finished frame back (tools/coin/compare.sh).
183+
typedef void (*FrameCallback)(void *userData);
184+
void setFrameCallback(FrameCallback cb, void *userData)
185+
{ p_frameCB = cb; p_frameCBData = userData; }
186+
187+
protected:
179188
virtual void redrawOverlay();
180189
virtual void actualOverlayRedraw();
181190

@@ -280,6 +289,9 @@ private:
280289
// dropped with no retry, which is what made geometry-trace/vibration
281290
// step-through and looped animation update unreliably (see OnPaint()).
282291
bool p_redrawPending;
292+
293+
FrameCallback p_frameCB;
294+
void *p_frameCBData;
283295
};
284296

285297
#endif

0 commit comments

Comments
 (0)