Skip to content

fix(listctrl): delegate scroll to base; HandleOnScroll private since wx 3.3.3 - #348

Merged
got3nks merged 1 commit into
amule-org:masterfrom
got3nks:fix/listctrl-scroll-wx333
Jul 8, 2026
Merged

fix(listctrl): delegate scroll to base; HandleOnScroll private since wx 3.3.3#348
got3nks merged 1 commit into
amule-org:masterfrom
got3nks:fix/listctrl-scroll-wx333

Conversation

@got3nks

@got3nks got3nks commented Jul 8, 2026

Copy link
Copy Markdown

Problem

wxWidgets 3.3.3 made wxScrollHelperBase::HandleOnScroll a private member. The vendored src/extern/wxWidgets/listctrl.cpp calls it from wxListMainWindow::OnScroll, so that file no longer compiles once the toolchain ships wx 3.3.3 — which currently breaks the macOS CI builds (the GitHub runner's Homebrew wx moved 3.3.2 → 3.3.3). The call already carried a FIXME.

Fix

wxListMainWindow derives from wxScrolledWindow and binds EVT_SCROLLWIN(OnScroll), so its OnScroll overrides the base scroll handling and has to drive the scroll itself. Rather than call the now-private HandleOnScroll(), it hands the event back to the base with event.Skip()wxScrolledWindow then performs the scroll through its own handler, while we keep the list-specific bookkeeping (recompute the visible-line range, refresh the header on horizontal scroll).

event.Skip() is public API on every supported version (min wx 3.2.0), so this needs no #ifdef version guards and does not change behaviour.

Testing

Built and run on macOS against wx 3.3.3 — a clean build of the whole app (GUI, remote GUI, daemon, amulecmd, webserver), not just this file, compiles with zero errors, so listctrl was the only 3.3.3 break. Scrolling verified across the transfer, shared-files (including horizontal), search, server and preferences lists with no regression. Also builds clean on wx 3.3.2.

…wx 3.3.3

wxWidgets 3.3.3 made wxScrollHelperBase::HandleOnScroll a private member, so
the vendored wxListMainWindow::OnScroll no longer compiles there -- this
breaks the macOS CI builds once the runner picks up wx 3.3.3.

wxListMainWindow derives from wxScrolledWindow and binds EVT_SCROLLWIN, so
its OnScroll overrides the base scroll handling and must drive the scroll
itself. Instead of calling the now-private HandleOnScroll(), hand the event
back to the base with event.Skip(): wxScrolledWindow then performs the
scroll through its own handler, while we keep the list-specific bookkeeping
(visible-line range + header refresh). event.Skip() is public on every
supported version (min wx 3.2.0), so this needs no version guards and does
not change behaviour. Verified with a clean build and manual scroll testing
on both wx 3.2.x and 3.3.3.
@got3nks
got3nks merged commit 25f8c11 into amule-org:master Jul 8, 2026
13 checks passed
@got3nks
got3nks deleted the fix/listctrl-scroll-wx333 branch July 8, 2026 09:36
got3nks added a commit that referenced this pull request Jul 14, 2026
…ist rows blanking on scroll/keyboard (#478) (#477)

* fix(remote-gui): stop the log view blanking on Windows by dropping Freeze/Thaw (#445)

#471 tried to fix the "log view goes blank on every new line" Windows
regression by thawing before scrolling, but it made no difference: the
AppendText still ran while the control was frozen. Appending to a frozen
wxTE_RICH2 (RichEdit) on Windows leaves its line/scroll metrics stale, so on
Thaw the view renders blank with the newest line pinned to the top until a
manual scroll forces a recompute. The Freeze()/Thaw() that #451 wrapped the
per-poll appends in is the actual culprit, not the scroll order.

Drop the Freeze()/Thaw() and append on a live control (as the pre-#451 code
did). Keep the two real wins from #451: the daemon's 5000-lines-per-poll cap
and the conditional SetDefaultStyle, plus one coalesced ShowPosition per poll
instead of per line (m_logBatching still suppresses the per-line scroll). The
per-line SetDefaultStyle was the dominant first-sync cost, so responsiveness is
retained without the frozen-append rendering corruption.

macOS/GTK recompute metrics regardless and were unaffected either way.

* fix(remote-gui): repaint list rows on scroll & keyboard nav on Windows (#478)

#348 replaced the vendored wxListMainWindow::OnScroll's synchronous
HandleOnScroll(event) with event.Skip() -- HandleOnScroll became a private
member in wx 3.3.3, so the macOS/Linux CI (on wx 3.3.x) stopped compiling. On
wxMSW the resulting deferred-scroll path (wxScrollHelperBase::HandleOnScroll ->
ScrollWindow) blits the retained rows and only invalidates the newly exposed
strip, which is then left unpainted: rows go blank on mouse-wheel, scrollbar and
pagination scrolling until a redraw is forced. Verified in the wx 3.2.10 and
3.3.x sources that the scroll+repaint mechanism is identical, so this is a wxMSW
platform behavior, not a wx-version one.

Rather than gate on platform or wx version (both fragile -- the former needs the
now-private HandleOnScroll, the latter breaks once Windows ships wx 3.3.3),
reimplement the synchronous scroll using only the public wxScrollHelper API
(GetViewStart / GetScrollLines / GetScrollPageSize / Scroll), mirroring
HandleOnScroll()/CalcScrollInc(): translate the scroll event into a target
position in scroll units and scroll to it.

Factor the "scroll + repaint" sequence into a shared ScrollListTo(x, y) helper
(Update() to flush pending paints so the blit is clean, Scroll(), then
ResetVisibleLinesRange() so the exposed rows repaint) and route both OnScroll
(scrollbar/wheel) and MoveToItem (keyboard nav) through it. This also fixes the
keyboard HOME/END/PgUp/PgDn blanking, which was the same bug from a different
path: MoveToItem only reset the visible-line range after Scroll() under
__WXMAC__, so on Windows the range stayed stale and keyboard scrolls came up
blank. The reset now runs on every platform via the shared helper, and the old
__WXMAC__-only workaround is gone.

Not skipping the event means the base scroll helper won't also scroll, so no
double-scroll. Works on every supported wx (>= 3.2.0) and every platform with no
version or platform guard; compiles against wx 3.3.3.

Refs #478
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant