fix(gui): keep the log tail-scroll at the true bottom with wrapping - #549
Merged
Merged
Conversation
The deferred/tail scroll called ScrollToEnd() directly. With word-wrap on -- where Scintilla lays out wrapped lines incrementally over several idles -- two problems fell out: - switching to a hidden log pane after the backlog loaded landed a few lines short (the single scroll ran before the tail was wrapped); and - switching to it mid-load landed at ~90%, because the batch tail-scroll and the idle re-scroll loop both moved the view and the loop misread its own batch scroll as a manual scroll and gave up. Make OnInternalIdle() the sole scroller: ScrollToBottom() only sets a pending flag, and the idle loop re-applies ScrollToEnd() until the first-visible line stops moving (wrap settled at the true bottom), bailing only when the view moves on its own (a real manual scroll). Appends never move the first-visible line, so the loop follows the growing log through the whole replay. Lives in the base CMuleLogCtrl, so all three panes (aMule Log, aMuleGUI Log, server info) share it. Follow-up to amule-project#548 (issue amule-project#547).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #548, which moved the amuleGUI log panes to
wxStyledTextCtrl(Scintilla). This fixes the auto-scroll-to-bottom landing short when log lines wrap.Background
With word-wrap on, Scintilla lays out wrapped lines incrementally over several idle cycles (wrapping tens of thousands of lines is expensive, so it doesn't block). The tail-scroll ran once, before that layout finished, so it computed against a display-line count that didn't yet include the still-unwrapped tail and stopped short. It only reproduced when lines actually wrap — @ghysler pinned it to window width (short at 1280px where his lines wrap two-up, correct at 2560px where they don't).
A first attempt (re-scroll on idle until the position settles) fixed the switch-in-after-load case but not switch-in-mid-load: while the log was still replaying, each poll's batch also scrolled directly, and the idle loop — which watches the scroll position to tell a manual scroll from an append — misread the batch's own scroll as a manual scroll and gave up, so the final one-shot scroll landed at ~90%.
Fix
Make
OnInternalIdle()the sole scroller:ScrollToBottom()only sets a pending flag, and the idle loop re-appliesScrollToEnd()until the first-visible line stops moving (wrap settled at the true bottom), bailing only when the view moves on its own (a genuine manual scroll). Since appends never move the first-visible line, the loop follows the growing log through the whole replay. It lives in the baseCMuleLogCtrl, so all three panes (aMule Log, aMuleGUI Log, server info) share it.Also corrects a few comments from #548 that the rework left inaccurate (comment-only).
Test plan
Verified against a live remote daemon with a large multi-day log (~16k lines), on a narrow window so lines wrap:
Lint:
clang-formatclean; clang-tidy Tier-1 + Tier-2 diff checks both clean.