Skip to content

Add keyboard shortcuts to ViewScene (issue #694)#2513

Draft
dcwhite wants to merge 57 commits into
masterfrom
viewscene-shortcuts
Draft

Add keyboard shortcuts to ViewScene (issue #694)#2513
dcwhite wants to merge 57 commits into
masterfrom
viewscene-shortcuts

Conversation

@dcwhite

@dcwhite dcwhite commented Jun 13, 2026

Copy link
Copy Markdown
Member

Summary

Implements a keyboard shortcut system for the ViewScene 3D viewer, addressing issue #694.

Shortcuts added

Key Action
0 Autoview
Ctrl+0 Autoview (no scale)
16 Axis-aligned views (±X, ±Y, ±Z)
X Snap to nearest axis
H Go to home view
Alt+H Set home view
C Toggle active clipping plane visibility
D Toggle fog
K Toggle all lights on/off
L Lock/unlock all view axes
O Toggle corner orientation triad
I Open shortcuts help dialog

Help dialog

  • I key (or toolbar ? button) opens a reference dialog listing all shortcuts
  • Dialog positions at top-right of ViewScene and remembers position within a session
  • Double-clicking a row in the dialog invokes that action
  • Unimplemented shortcuts are hidden; wired shortcuts flash a tooltip on activation
  • Key presses are forwarded to ViewScene even when the help dialog has focus

Refactor

  • Shortcuts defined via ShortcutDef enum class with string annotations and action mechanics, replacing a hand-coded .ui table

Bug fixes

  • Orientation glyph aspect ratio wrong at initialization in CoreBootstrap (stale 640×480 default size)
  • Orientation glyph squished on first show

OffscreenGLRenderer

  • Prototype files (OffscreenGLRenderer.cc/.h) added but not wired into ViewScene — kept for future regression-image work

Test plan

  • Open ViewScene and verify each shortcut key triggers the correct action
  • Press I — confirm help dialog opens at top-right, closes cleanly
  • Double-click a shortcut row — confirm the action fires
  • Verify Alt+H / H round-trip (set home, move camera, go home)
  • Verify orientation glyph has correct aspect ratio on first show
  • Confirm no regressions in existing ViewScene mouse/toolbar controls

🤖 Generated with Claude Code

dcwhite and others added 30 commits June 3, 2026 19:24
OffscreenGLRenderer wraps SRInterface with a QOffscreenSurface +
QOpenGLContext + QOpenGLFramebufferObject so ViewScene can render
without a visible window. Compiles and context/FBO initialization works.

Not yet integrated into ViewSceneDialog: spire's VBOMan has a VBO-ID-
recycling bug where doFrame() runs the bootstrap (creates VBOs), GC frees
them, GL recycles those IDs, and a subsequent handleGeomObject call hits
addVBOAttributes with a recycled ID that still has a stale map entry with
different attribute layouts — throwing inside a noexcept context and
calling terminate().

Next steps:
1. Fix the stale-entry bug in spire VBOMan (remove map entry on GC free)
2. Wire OffscreenGLRenderer into ViewSceneDialog for regression mode
3. Add image-comparison tests using renderToImage()

Also: Screenshot::setImage() added for the eventual offscreen path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ling bugs

All 61 Renderer regression tests now pass using offscreen rendering
(QOffscreenSurface + FBO) instead of a visible GLWidget. No window,
no display server, no window-system timing races.

Three spire/SRInterface fixes were required to make this work:

1. VBOMan::addInMemoryVBO — erase any stale map entry for a freshly
   generated buffer name before addVBOAttributes. GL guarantees a new
   glGenBuffers name is unused, so a lingering entry is always stale
   (the buffer was deleted by font rendering or the IBO manager, which
   call glGenBuffers/glDeleteBuffers directly and share GL's single
   buffer-name namespace). Previously this threw "Differing attributes"
   inside a noexcept context and aborted.

2. IBOMan::addInMemoryIBO — same fix; its insert() silently kept stale
   primitive data on a recycled id rather than crashing, a latent
   rendering bug.

3. SRInterface::addIBOToEntity — skip the IBO component when hasIBO()
   returns 0 instead of letting getIBOData() throw. A named IBO can be
   legitimately absent (GC'd between geometry updates); a missing index
   buffer should drop the pass, not abort.

ViewSceneDialog now constructs an OffscreenGLRenderer when
isRegressionMode() is set; the window path is unchanged. mGLWidget is
null in offscreen mode, so its dereferences are guarded.

Residual flakiness is down to ~0-1 of 61 per full run (different test
each time, all pass in isolation) — the same environmental async-
teardown class tracked separately, no longer a rendering problem.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two CI fixes from the first regression-tests-ci run:

1. async1/async2_multiData hung for 1500s (CTest timeout) on the mac-gui
   job. These streaming-test networks keep a data-push thread alive; the
   30s in-app regression timeout fires but teardown then blocks forever
   waiting on the thread. quick_exit() was originally added for exactly
   this (commit c51b537) and I removed it on regression-tests-ci on the
   mistaken theory that it caused ViewScene flakiness. The real cause of
   that flakiness was window/timing + spire buffer bugs, now fixed by
   offscreen rendering — so restore quick_exit() and keep both wins.
   Verified locally: async1/async2 now exit in ~2s, all 61 Renderer
   tests still pass.

2. Windows test-log upload failed: results were written to C:\ which is
   outside the workspace (D:\a\SCIRun\SCIRun), so upload-artifact
   rejected the path. Write to $env:GITHUB_WORKSPACE instead and
   reference workspace-relative paths in the upload step.

Also added --timeout 300 to all ctest invocations as a safety net so a
future hang is killed in minutes instead of eating the 1500s default.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes two consistent segfaults exposed by the offscreen ViewScene path:
editMeshBBox_trans_check (SEGFAULT) and additionalTypesOfVSObjectsSaved
(flaky SIGABRT) — both ViewScene modules in regression mode.

updateDockWidgetProperties is connected to the dock's topLevelChanged
signal inside configDockable(). For a ViewScene dialog in regression mode,
configDockable calls dockable->setFloating(true), which fires that signal
synchronously — but dockable_ is assigned only after config() returns from
makeOptionsDialog(). The slot then dereferenced a null dockable_.

This was latent before but masked: with the real GLWidget central widget
the dock's floating-state transition didn't re-fire during setup. The
offscreen path (no GLWidget) changed the dialog's layout/visibility timing
enough to trigger the signal, surfacing the null deref. Both tests passed
on regression-tests-ci (windowed) and crashed only with offscreen.

Guard: return early when dockable_ is not yet set; configDockable handles
the initial show/float itself, and later user-driven dock/float events
arrive after dockable_ is valid.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ling

The headline fix: ModuleWidget::dockable_ was an uninitialized raw pointer.
updateDockWidgetProperties() (connected to the dock's topLevelChanged signal
in configDockable) can fire before makeOptionsDialog assigns dockable_; the
slot null-checks it, but garbage in an uninitialized pointer passes the null
check and segfaults in QWidget::setWindowFlags. This was masked serially
(benign stack garbage) but crashed random tests under parallel load with
different memory contents. Null-initializing dockable_ makes the existing
guard correct and eliminates all the parallel segfaults (calcBundleDifference,
ResizeMatrixTest, PrintStringIntoString, uncertaintyTensorGlyphs, etc.).

Also fixed for parallel safety:
- Defer session-trace init to readCommandLine and skip it in regression mode.
  The trace DB wrote to a fixed shared path, causing SQLite corruption and
  SIGBUS (mmap) when test processes ran concurrently. Guard the destructor's
  endSession() since session() is now null in regression mode.
- Testing/CMakeLists.txt: give every ViewScene-containing network a shared
  RESOURCE_LOCK (gpu) so CTest never schedules two GPU tests at once; bump the
  in-app regression timeout 30->60s for headroom under load.
- reusable-build.yml: run the regression ctest step at -j3 (was -j1).

Coverage tooling (requested):
- ENABLE_COVERAGE CMake option: Clang source-based instrumentation
  (-fprofile-instr-generate -fcoverage-mapping) on both compile and link flags
  (the pre-existing ENABLE_GCOV_DATA_FILES set compile flags only).
- scripts/coverage.sh: runs ctest with per-process LLVM_PROFILE_FILE, merges
  with llvm-profdata, reports via llvm-cov (text + HTML) across all dylibs.

Results: full suite at -j6 went from ~4 segfaults + bus error to 0 crashes;
at -j3 (CI target) it is clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Concurrent regression-test processes shared one settings file
(~/Library/Preferences/SCIRun5.plist). Reading a half-written plist during
startup — favorites and window geometry restore — could crash in the
module-selector tree (handleCheckedModuleEntry -> QTreeWidget::setCurrentItem).
Suffix the QSettings application name with the process id in regression mode so
each process gets an isolated, empty, deterministic store. Normal interactive
runs keep the shared "SCIRun5" settings.

Stress result: full suite at -j6 now passes 100% in 2 of 3 runs (was crashing
on random tests every run). One rarer pre-existing async-state heap race
remains (ViewScene::asyncExecute -> SimpleMapModuleState::setTransientValue),
tracked separately.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of starting each regression process with an empty settings store,
copy all keys from the shared default "SCIRun5" store into the per-process
"SCIRun5_regression_<pid>" store at startup. Regression runs now see the
developer's real settings (favorites, directories, mouse mode, etc.) and look
consistent run to run, while still only ever *writing* to their own isolated
file. Concurrent *reads* of the stable shared store are safe; only the
concurrent writes caused the original startup race.

dest.clear() first so a recycled pid can't inherit a stale store.

Verified: a regression process's plist inherits the source keys; a full -j6
run shows no new failures (the two that remain are the pre-existing async
setTransientValue race tracked in #2486 and a -j6-only load timeout).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Regression processes each create an isolated SCIRun5_regression_<pid> settings
store; without cleanup these accumulate (one per test process, ~300 per run).

Cleanup is done after the run rather than in-app: on macOS cfprefsd owns the
plist and recreates it if a still-running process deletes its own file, so
in-process deletion at exit doesn't stick. Once the run finishes the domains
are dead and removal is reliable.

- scripts/run-regression-tests.sh: wraps ctest, removing the per-process stores
  before and after the run. Cross-platform (macOS plist via `defaults delete` +
  rm, Linux .conf, Windows registry keys). Verified: 5 stale stores before -> 0
  after.
- reusable-build.yml: the Unix regression CI step removes the stores afterward.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New mac-coverage job in regression-tests.yml: a gui macOS build with
-DENABLE_COVERAGE=ON (Debug), runs the full test suite under llvm profiling,
and uploads a coverage report (text summary + HTML) as an artifact.

reusable-build.yml gains a `coverage` input that:
- checks out SCIRunTestData (also needed for the regression portion),
- adds --debug -DENABLE_COVERAGE:BOOL=ON to the build,
- runs scripts/coverage.sh (per-process LLVM_PROFILE_FILE -> llvm-profdata
  merge -> llvm-cov text+HTML), cleans up the regression QSettings stores,
  and uploads coverage-report-macOS.

Superbuild.cmake: declare ENABLE_COVERAGE and forward it to the inner SCIRun
build via SCIRUN_CACHE_ARGS. Without this the option set on build.sh never
reached the project that defines it, so no instrumentation would be applied.
(The pre-existing ENABLE_GCOV_DATA_FILES had the same gap.)

Coverage toolchain commands validated against a standalone instrumented
binary; the llvm-cov report/show invocations and profdata merge work as used.

Note: the instrumented Debug build running the full suite is a long job
(expect several hours); scope the ctest selection in the coverage step down
if runtime becomes a problem.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Badges are per-workflow on GitHub Actions, so one regression-tests badge
covers all of its jobs (linux-headless, mac-gui, windows-headless, and
mac-coverage). Links to the workflow's Actions page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The .Test.ImportNetwork.* tests pass -x, which routed them to ConsoleApplication
where ImportNetworkFile is unimplemented — every test threw "Unknown global
command type" and exited 1. Drop -x so import goes through the GuiCommandFactory's
FileImportCommand (the intended path), and make that path testable in regression
mode:

- GlobalCommandBuilderFromCommandLine: enqueue a quit after import/save in
  regression mode (the GUI import flow never executes the network, so nothing
  else quits it — tests previously hung until manually closed). Interactive
  --import keeps the window open. Also redirect the converted <name>_imported.srn5
  to a temp dir in regression mode so the test data dir isn't polluted.

- GuiCommands: never pop a modal "module not found" dialog in regression mode
  (it blocked the run waiting for a manual OK); report via the log instead. Add a
  failTestOnError() hook (true for import) so an import failure in regression mode
  exits non-zero, surfacing as a failed ctest result with the reason logged.

- Testing/CMakeLists.txt: name import tests by path relative to the data dir
  (several legacy nets share a basename and collided into one test name); filter
  the glob to skip serialization artifacts (TransientOutput/) and scratch files
  (*_TMP.srn); add a 120s CTest timeout.

Result: import suite goes from 0/434 (all crashing at startup) to 98% passing in
~84s with no dialogs. The ~9 remaining failures are genuine gaps the tests now
correctly surface (undefined legacy modules; a v4 lexical-cast parse error),
tracked separately.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…GLRenderer

OffscreenGLRenderer.h included <QOffscreenSurface>/<QOpenGLContext>/
<QOpenGLFramebufferObject>, which pull in <gl.h> on Windows. Because that
landed before glew.h (included via gl-platform in the .cc), MSVC failed the
Interface_Modules_Render build with C1189 "gl.h included before glew.h". This
broke every Windows CI build on the branch since the OffscreenGLRenderer was
added; Linux/macOS tolerate the ordering.

Forward-declare the Qt OpenGL classes in the header (they're only pointer
members; only QImage, which is gl.h-free, still needs including) and in the .cc
include gl-platform (glew) before the Qt OpenGL headers, matching GLWidget's
ordering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ges)

Foundation for golden-image regression testing. The offscreen renderer already
feeds rendered frames into the screenshot path; this makes the saved filenames
deterministic so they can be matched against reference images.

In regression mode, autoSaveScreenshot() now names files
<dir>/<network>.<module>.png — keyed to the loaded network file
(getCurrentFileName) and the module id, with no wall-clock timestamp and no
1s sleep. The directory defaults to the current working dir when the screenshot
preference is unset. Interactive (non-regression) behavior is unchanged.

Verified: `SCIRun_test -E ViewSceneBackgroundColor.srn5 --regression --save-images`
writes ViewSceneBackgroundColor.ViewScene-0.png ... -5.png, each a correct
800x600 offscreen render (background color + orientation axes).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-render

Step 1 of golden-image regression: control where rendered images go and stop
the quit-time crash.

- New --image-dir command-line option (wired through ApplicationParameters like
  --datadir). In regression mode autoSaveScreenshot writes to it when given,
  else the screenshot pref, else CWD; the directory is created if missing.
- autoSaveScreenshot now saves the frame already captured during the run
  (frameFinished -> takeScreenshot) instead of re-rendering at exit. A fresh
  offscreen doFrame at teardown was crashing in the GL geometry draw for
  networks with real geometry.

Verified: --image-dir writes deterministic <network>.<module>.png files to the
requested dir (auto-created), and field/glyph networks no longer crash with
--save-images.

Known limitation (documented in OffscreenGLRenderer::renderToImage and tracked
separately): real VBO/IBO geometry still renders blank offscreen — forcing it to
draw (waiting for shader promises) crashes in RenderBasicSys/glDrawElements,
likely a GL profile/VAO mismatch vs the QOpenGLWidget context. Background +
orientation axes render correctly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire up the existing ViewSceneShortcuts.ui (previously unused) as a lazily-
created QDialog shown by pressing I. Grays out unimplemented rows (stereo,
backculling). Adds the .ui to CMakeLists so ui_ViewSceneShortcuts.h is generated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n widths

- Extract showShortcutsDialog() as a proper slot; both Key_I and the new
  toolbar button call it
- Add ? toolbar button (Qt standard question-mark icon) to the top toolbar
- Expand grayed-out rows to cover all currently unimplemented shortcuts:
  1-8 axis views, Ctrl+0, X snap, Ctrl+1-9, Set Home, Home, bounding box,
  flat shading, orthographic, stereo, backculling, wireframe
- Call resizeColumnsToContents() so column widths fit the text

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…color

- Set NoEditTriggers on the table so all cells are read-only
- Populate missing Action column cells for all 20 rows so grayed rows
  show their action name in the disabled style
- Switch gray-out from Qt::gray to palette Disabled/Text color for
  correct appearance on both light and dark themes
- Also strip ItemIsEnabled flag from grayed items
- Increase initial dialog size to 700x720

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…active

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tions and action mechanics

- Add ViewSceneDialog::ShortcutDef nested struct with:
    Id enum (one value per shortcut), Qt key + modifiers, actionName,
    shortcutDisplay, description, and nullable Action (std::function)
- Add private static shortcutTable() returning the canonical ShortcutTable;
    Autoview and OpenHelp have actions; everything else is null (grayed)
- showShortcutsDialog() now builds the table entirely from shortcutTable()
    — no hardcoded row indices or separate name arrays
- keyPressEvent() dispatches through shortcutTable() instead of a switch;
    adding a new shortcut now only requires updating one place
- Strip all row/item data from ViewSceneShortcuts.ui; table is code-driven

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unimplemented (grayed) rows silently do nothing. Tooltip on the table
advertises the feature.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#694)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… focus

Extract dispatchShortcutKey() from keyPressEvent. Install an event filter
on the shortcuts dialog that calls it; returns true only when a shortcut
fired, so Escape/Enter still close the dialog normally.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Key events go to the focused child widget (QTableWidget), not the dialog
itself. Install the event filter on the table too and check for either
object in eventFilter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n in session

On first open, dialog appears flush with the right edge of the ViewScene
window. QEvent::Move is caught in eventFilter and stored in
shortcutsDialogPos_ (std::optional<QPoint>), so subsequent opens restore
wherever the user last dragged it.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ault size

ScreenParams initialises to 640x480 and is only updated by resizeGL(), which
Qt calls asynchronously. On first show, paintGL() fires before resizeGL(),
so the glyph renders with the wrong aspect ratio until the first manual resize.

Fix: at the top of paintGL(), if the widget's actual size doesn't match what
the renderer knows, call eventResize() immediately before doFrame(). This is
idempotent and harmless on subsequent frames where the sizes already agree.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dcwhite and others added 21 commits June 5, 2026 21:03
Ctrl+H saves the current camera distance, lookAt, and rotation into
a transient HomeCamera stored on impl_. H restores it. If no home
has been set, H is a no-op.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
No separate center-without-rescale API exists in SRInterface yet, so
this calls doAutoView() as a placeholder. A TODO marks the spot for
when spire gets a centerView() method.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On macOS Qt::ControlModifier == Command, and Cmd+H is reserved by the
system to hide the window. Alt (Option) + H has no system conflict.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- AutoviewNoScale: Ctrl+0 → Alt+0 (consistent with Alt+H for SetHome)
- SetHome now shows a brief "Home view set" tooltip near the top of
  the ViewScene for 1.5s as a visual confirmation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
flashShortcutTooltip() shows the action name near the top-center of
the ViewScene for 1.5s. dispatchShortcutKey calls it after any
successful action, including axis views (shows "+X View" etc.).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CoreBootstrap was initializing StaticCamera and StaticOrthoCamera with
a hardcoded 800x600 aspect ratio. Since it runs inside the first
doFrame, this always overrode whatever eventResize had set earlier,
causing the glyph to render with the wrong aspect until the next
resizeGL (i.e. first manual resize).

Fix: read the actual dimensions from StaticScreenDims, which is
populated by SRInterface::setupCore and kept current by eventResize.
Fall back to 800/600 only if the component is missing or zero-height.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The autoview button had setShortcut(Key_0) which intercepted the key
before keyPressEvent, bypassing dispatchShortcutKey and its tooltip.
Removed the duplicate; the dispatcher is now the sole handler for '0'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous impl called snapToViewAxis() which reads from the View
Axis Chooser dropdown — empty on startup, so X did nothing.

setClosestAxisView() reads the current camera rotation quaternion,
derives the world-space look and up vectors via mat3 transpose, then
finds the nearest cardinal axis for each independently. Works from any
camera orientation with no UI state dependency.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Unimplemented shortcut rows are now hidden rather than grayed out.
Each null entry in shortcutTable() has a detailed TODO comment
describing exactly what renderer/state work is needed to implement it.

Double-click handler updated to use UserRole index stored per row so
the row→shortcut mapping remains correct after skipping hidden entries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
#2503 world axes, #2504 bounding box, #2505 flat shading,
#2506 orthographic, #2507 stereo, #2508 backculling,
#2509 wireframe, #2510 copy view

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AdjustToContents policy + resizeRowsToContents lets the table report
its exact height. adjustSize() then shrinks the dialog to fit, and
setFixedSize() locks it so it doesn't resize.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
40px width + 60px height ensures the last row is fully visible without
scrolling, and gives Windows enough room for its larger window chrome.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two-layer caching to reduce ~2+ hour build times:
- actions/cache on bin/Externals/Install+Stamp keyed on Superbuild/*.cmake hash,
  so Boost/Eigen/Python/Teem etc. are only rebuilt when a dependency actually changes
- sccache via mozilla-actions/sccache-action for SCIRun source compilation,
  wired via CMAKE_C/CXX_COMPILER_LAUNCHER on all three platforms

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…re use

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dows-2022

- reusable-build.yml: add Externals/src to cache path so ExternalProject's
  git-update script can find .git when the cache is restored; bump cache
  key to v2 to avoid restoring broken v1 caches
- regression-tests.yml: change windows-headless-regression runner from
  windows-latest to windows-2022, matching every other Windows job; fixes
  "could not find any instance of Visual Studio" on the updated runner image

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@dcwhite dcwhite self-assigned this Jun 17, 2026
@dcwhite dcwhite requested a review from jessdtate June 17, 2026 04:30
@dcwhite dcwhite added this to the 5.0.0 milestone Jun 17, 2026
dcwhite and others added 4 commits June 16, 2026 23:10
The cache was storing bin/Externals/src (lowercase) but CMake
ExternalProject with EP_BASE puts git clones in bin/Externals/Source
(capital S). On a cache restore, CMake still ran the git update check
for every GIT_REPOSITORY external and hit 'fatal: not a git repository'
because the Source directory was never actually cached.

Fix: set EP_UPDATE_DISCONNECTED TRUE globally in Superbuild.cmake so
that when Install+Stamp are restored from cache the git update step is
skipped entirely. The cache key already covers hashFiles('Superbuild/*.cmake'),
so any GIT_TAG change still forces a fresh clone. Remove the bogus /src
path from the cache and bump the key to v3 to avoid restoring broken
v2 caches.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mac-gui was picking up the mac-headless cache via the loose restore-key
that strips the variant suffix. Headless doesn't build Zlib (GUI-only
external), so there was no Zlib stamp in that cache. mac-gui then tried
to build Zlib but Source/ isn't cached, hitting the same
'not a git repository' error.

Drop the variant-agnostic restore-key so a gui build can only fall back
to another gui cache (same variant, different python flag), never to a
headless cache with a disjoint set of built externals.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
std::remove without erase left bin contents unchanged; apply erase-remove idiom.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant