Skip to content

Commit d8a342f

Browse files
author
andy
committed
Merge wip/mac-fixes: Retina viewer viewport/picking, Terminal.app and open(1) on macOS (#133)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFPK14wBcLD1s4dYUowpP4 # Conflicts: # .github/workflows/build.yml
2 parents 0308fe0 + 56a7a5b commit d8a342f

21 files changed

Lines changed: 502 additions & 28 deletions

File tree

‎.github/workflows/build.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,10 @@ jobs:
354354
brew install gromacs
355355
tests/gromacs_md/run_tests.py -v
356356
357+
# The editor and terminal commands macOS gets by default (#133).
358+
- name: Editor and terminal commands
359+
run: cd build-cmake && ctest -R '^(editor_command|macos_terminal)$' --output-on-failure
360+
357361
# Does it run? (#133) Starts the installed apps on the runner's
358362
# logged-in desktop and screenshots them. Allowed to fail; the
359363
# verdict is in the summary and the "macos-run" artifact.

‎CMakeLists.txt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,14 @@ install(PROGRAMS ${CMAKE_SOURCE_DIR}/scripts/ecce-askpass
10801080
DESTINATION scripts
10811081
COMPONENT client)
10821082

1083+
# scripts/ecce-macos-terminal: the default terminal on macOS (UserEditor),
1084+
# which runs xterm's "-e command" in Terminal.app.
1085+
if(APPLE)
1086+
install(PROGRAMS ${CMAKE_SOURCE_DIR}/scripts/ecce-macos-terminal
1087+
DESTINATION scripts
1088+
COMPONENT client)
1089+
endif()
1090+
10831091
# scripts/ecce-csh2sh and its rules module CshToSh.pm: converts the csh snippets
10841092
# in submit.site / CONFIG.* to the sh gensub now writes. gensub `use`s
10851093
# CshToSh.pm from its own directory, so it travels with it; the launcher runs
@@ -1888,6 +1896,14 @@ if(PYTHON3_EXECUTABLE)
18881896
${CMAKE_BINARY_DIR}/viewer-scenes)
18891897
set_tests_properties(look_nm_sticks PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 300)
18901898

1899+
# At content scale 2 the viewer fills the canvas and wx clicks pick the
1900+
# atom aimed at (#133, Retina). Needs `ninja viewer-scenes` and Xvfb.
1901+
add_test(NAME look_hidpi
1902+
COMMAND ${PYTHON3_EXECUTABLE}
1903+
${CMAKE_CURRENT_SOURCE_DIR}/tests/look/hidpi.py
1904+
${CMAKE_BINARY_DIR}/viewer-scenes)
1905+
set_tests_properties(look_hidpi PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 300)
1906+
18911907
# The saved DefaultStyle moves from the old default once (#227).
18921908
add_test(NAME look_default_style
18931909
COMMAND ${PYTHON3_EXECUTABLE}
@@ -2054,6 +2070,19 @@ if(PYTHON3_EXECUTABLE)
20542070
add_test(NAME transport_local_terminal COMMAND testLocalTerminal)
20552071
set_tests_properties(transport_local_terminal PROPERTIES TIMEOUT 60)
20562072

2073+
# The editor command (UserEditor) per platform, and macOS open(1) waiting
2074+
# for its own copy; ecce-macos-terminal with a stub open (#133).
2075+
add_executable(testEditorCommand tests/transport/testEditorCommand.C)
2076+
target_link_libraries(testEditorCommand PRIVATE
2077+
eccecomm eccercmd eccedsi eccedav eccecipc eccefaces eccexml
2078+
eccetdat ecceutil ${XercesC_LIBRARIES})
2079+
add_test(NAME editor_command COMMAND testEditorCommand)
2080+
set_tests_properties(editor_command PROPERTIES TIMEOUT 60)
2081+
add_test(NAME macos_terminal
2082+
COMMAND sh ${CMAKE_SOURCE_DIR}/tests/transport/macos_terminal_test.sh
2083+
${CMAKE_SOURCE_DIR})
2084+
set_tests_properties(macos_terminal PROPERTIES TIMEOUT 60)
2085+
20572086
# Run Management's Tail (TailSource) on a local machine; the same program
20582087
# runs against the test sshd from tests/transport/sshd/tail_test.sh.
20592088
add_executable(testTail tests/transport/testTail.C)

‎docs/claude/services/index.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,4 @@ what the deployment mode needs (GETTING_STARTED, "Deployment modes"):
5151
- [A remote view rides ECCE's own login, not an ssh of its own (Tail)](remote-views-use-ecces-own-login.md)
5252
- [A calculation's molecule and basis properties are stored below it](calculation-properties-live-below-it.md)
5353
- [Xvfb's stderr must not go to a pipe nobody reads (CI display wedge, #127)](xvfb-stderr-must-not-be-a-pipe.md)
54+
- [macOS: Terminal.app takes no command, and open(1) returns at once (#133)](macos-terminal-and-editor.md)

‎docs/claude/services/macos-terminal-and-editor.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
type: pitfall
3+
title: "macOS: Terminal.app takes no command, and open(1) returns at once (#133)"
4+
area: services
5+
paths: [scripts/ecce-macos-terminal, src/util/genutil/UserEditor.C, src/wxgui/wxtools/WxEditSessionMgr.C, src/comm/commtools/EcceShell.C, tests/transport/macos_terminal_test.sh, tests/transport/testEditorCommand.C]
6+
issues: [133]
7+
---
8+
ECCE starts every terminal as `TERM [opts] -e command args` (EcceShell,
9+
UserEditor). macOS has no xterm and Terminal.app has no `-e`; the default
10+
terminal there is `$ECCE_HOME/scripts/ecce-macos-terminal`, which writes the
11+
command to a `.command` file, opens it with `open -a Terminal` and waits for
12+
a marker the script touches on exit, so an editor session lasts as long as
13+
the editor. `ECCE_TERMINAL_DRYRUN=1` prints the file instead.
14+
15+
An edit session ends when the editor process exits (WxEditProcess), and
16+
`finishSession` then deletes the file. `open -e file` exits at once, so the
17+
file was gone before TextEdit read it. UserEditor adds `-W -n` to `open`
18+
(wait for a copy of its own); the session then ends when that TextEdit is
19+
quit, not when its window is closed. Default editor on macOS: `open -t`.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,4 @@ Read this before changing any dialog, panel, sizer, grid, ewx control or the Ope
4646
- [Every `ewxFrame`/`ewxDialog` is capped to its display and scrolls what does not fit (`fitToScreen()`); codereg dialogs scroll in `EccePanel` (#189)](windows-fit-small-screens-via-fittodisplay.md)
4747

4848
- [A failed live property update must not free the cached property; a panel's timer must die with the panel (#217)](property-cache-and-panel-timers-outlive-their-objects.md)
49+
- [The Inventor viewer works in framebuffer pixels; wx sizes and mouse positions are logical units (#133)](viewer-sizes-are-framebuffer-pixels.md)

‎docs/claude/wx-viewer/viewer-sizes-are-framebuffer-pixels.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
type: pitfall
3+
title: "The Inventor viewer works in framebuffer pixels; wx sizes and mouse positions are logical units (#133)"
4+
area: wx-viewer
5+
paths: [src/inv/wxinv/SoWxRenderArea.C, include/inv/SoWx/SoWxDevice.H, src/inv/wxinv/SoWxMouse.C, src/inv/wxinv/SoWxKeyboard.C, src/inv/wxinv/SoWxExaminerViewer.C, src/wxviz/viewer/SGViewer.C, tests/look/hidpi.py]
6+
issues: [133]
7+
---
8+
On a Retina Mac (and under GDK_SCALE=2 on Linux) a wxGLCanvas's framebuffer
9+
is `GetContentScaleFactor()` times its client size. `SoWxRenderArea::OnSize`
10+
used the client size as Inventor's window size, so the scene was drawn into
11+
the lower-left quarter of a black canvas (glClear still filled all of it).
12+
`getGlxSize()` is now framebuffer pixels, and every wx mouse/key position
13+
goes through `soWxToPixels()` (SoWxDevice.H) before it becomes an SoEvent or
14+
a viewer locator. Any new code that mixes a wx position with `getGlxSize()`,
15+
a viewport region or a pick must convert the same way. wxMSW reports scale 1.
16+
Still pixel-sized, so half as large on Retina: the corner text (`glBitmap`
17+
8x13 font in SGViewer) and the examiner's feedback axes.
18+
Test: `look_hidpi` (viewer-scenes at GDK_SCALE=1/2; `wxpick` sends real wx
19+
mouse events). Water's hydrogens are not hit at their projected centre
20+
after `viewall` by either pick path; the test clicks benzene atoms.

‎include/inv/SoWx/SoWxDevice.H‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,22 @@
44
#include <wx/window.h>
55
#include <wx/event.h>
66
#include "inv/events/SoEvent.H"
7+
#include <cmath>
8+
9+
10+
// Framebuffer pixels per wx logical unit of the window an event came from:
11+
// 2 on a Retina display or under GDK_SCALE=2, 1 on Windows (wxMSW logical
12+
// units are pixels). The viewport and every SoEvent position are pixels.
13+
inline double soWxPixelScale(const wxEvent *e)
14+
{
15+
wxWindow *w = e ? wxDynamicCast(e->GetEventObject(), wxWindow) : 0;
16+
return w ? w->GetContentScaleFactor() : 1.0;
17+
}
18+
19+
inline int soWxToPixels(const wxEvent *e, int v)
20+
{
21+
return (int)std::lround(v * soWxPixelScale(e));
22+
}
723

824

925
class SoWxDevice {
@@ -20,8 +36,13 @@ public:
2036
{ return winSize; }
2137

2238
protected:
23-
inline void setEventPosition(SoEvent *event, int x, int y) const
24-
{ event->setPosition(SbVec2s(x, (winSize[1] - 1) - y)); }
39+
inline void setEventPosition(SoEvent *event, const wxEvent *src,
40+
int x, int y) const
41+
{
42+
x = soWxToPixels(src, x);
43+
y = soWxToPixels(src, y);
44+
event->setPosition(SbVec2s(x, (winSize[1] - 1) - y));
45+
}
2546

2647
private:
2748
// size of the window this device works in

‎include/wxviz/SceneScript.H‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class SceneScript
6666

6767
private:
6868
bool exec(const vector<string>& w, const string& rest);
69-
bool pickAtoms(const string& name, const vector<string>& atoms);
69+
bool pickAtoms(const string& name, const vector<string>& atoms,
70+
bool viaWx = false);
7071
bool dragAtom(const string& name, int atom, int dx, int dy);
7172
bool countRedraws(const string& name, const vector<string>& steps);
7273
bool snapshot(const string& name, int size, float r, float g, float b);

‎scripts/ecce-macos-terminal‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#!/bin/sh
2+
# ECCE's default terminal on macOS. Terminal.app takes no command on its
3+
# command line, so this takes xterm's form, [options] -e command [args],
4+
# writes the command to a .command file, opens that in Terminal and waits
5+
# until it has ended: an editor session must last as long as the editor.
6+
# -title/-T sets the window title; other options before -e are ignored.
7+
# ECCE_TERMINAL_DRYRUN=1 prints the file and the open command instead.
8+
9+
q() { printf "'%s'" "$(printf '%s' "$1" | sed "s/'/'\\\\''/g")"; }
10+
11+
title=
12+
while [ $# -gt 0 ] && [ "$1" != -e ]; do
13+
case $1 in
14+
-title|-T) [ $# -gt 1 ] && { title=$2; shift; } ;;
15+
esac
16+
shift
17+
done
18+
[ "${1:-}" = -e ] && shift
19+
[ $# -gt 0 ] || set -- "${SHELL:-/bin/sh}" -l
20+
21+
dir=$(mktemp -d "${TMPDIR:-/tmp}/ecce-term.XXXXXX") || exit 1
22+
script=$dir/run.command
23+
done=$dir/done
24+
25+
line=
26+
for a in "$@"; do line="$line $(q "$a")"; done
27+
{
28+
printf '%s\n' '#!/bin/sh'
29+
printf '%s\n' "trap \"touch $(q "$done")\" EXIT HUP INT TERM"
30+
[ -z "$title" ] || printf '%s\n' "printf '\\033]0;%s\\007' $(q "$title")"
31+
printf '%s\n' "cd $(q "$PWD") 2>/dev/null"
32+
printf '%s\n' "$line"
33+
} > "$script"
34+
chmod 700 "$script"
35+
36+
if [ -n "${ECCE_TERMINAL_DRYRUN:-}" ]; then
37+
cat "$script"
38+
echo "open -a Terminal $script"
39+
rm -rf "$dir"
40+
exit 0
41+
fi
42+
43+
open -a Terminal "$script" || { rm -rf "$dir"; exit 1; }
44+
while [ ! -e "$done" ]; do sleep 1; done
45+
rm -rf "$dir"

‎src/inv/wxinv/SoWxExaminerViewer.C‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "inv/actions/SoSearchAction.H"
1919

2020
#include "inv/SoWx/SoWxExaminerViewer.H"
21+
#include "inv/SoWx/SoWxDevice.H"
2122
#include "inv/SoWx/SoWx.H"
2223
#include "inv/SoWx/SoWxRenderArea.H"
2324

@@ -470,8 +471,8 @@ void SoWxExaminerViewer::processEvent( wxEvent * event )
470471

471472
wxMouseEvent * mouseEvent = dynamic_cast<wxMouseEvent *>(event);
472473
if (mouseEvent != 0) {
473-
int x = mouseEvent->GetX();
474-
int y = mouseEvent->GetY();
474+
int x = soWxToPixels(mouseEvent, mouseEvent->GetX());
475+
int y = soWxToPixels(mouseEvent, mouseEvent->GetY());
475476
int timeStamp = mouseEvent->GetTimestamp();
476477

477478
if (mouseEvent->ButtonDown() || mouseEvent->ButtonUp()) {

0 commit comments

Comments
 (0)