Fix the Windows head items from the review - #890
Merged
Merged
Conversation
- Glyph positions (the selection highlight, the column a click lands in, the subtitle's width) use the unrounded advance GDI+ draws at, not the whole-pixel cell the grid is laid out in: at 96 DPI 8.8 against 9, a character off by column 22 and more than three at 175%. - The first window is sized for the display it opens on, and kept inside the working area. Unscaled, a pane held four characters at 200%. - Keys, clicks and menu events are queued and handed over one per frame, in the order they happened. A slot per kind lost the second of two keys, and applied a click before the key pressed ahead of it - d then a click discarded an entry the reader never looked at. The frame does not wait while input is queued. That also reopens the menu on a right click of the row it was open on. - user32's modal loops - the scroll bar's while the thumb is dragged, the frame's while the window is moved or sized - run frames on a timer, through ILoopHooks.Frame, so the panes follow the thumb instead of freezing until the release. - Losing mouse capture, or a move with the button up, ends a selection or splitter drag. - At logoff the form stops arrivals and stages the queue itself, inside WM_ENDSESSION, through ILoopHooks.SessionEnding: Windows may end the process once that returns, before the loop reaches its own shutdown. - ImageCache keeps only what is on screen, and keys a picture by the content hash as well as its stamp. Pending files are read shared with writers and deleters, so a read overlapping an accept or a re-run cannot fail it.
SimonCropp
added a commit
that referenced
this pull request
Sep 23, 2026
A top level window is held to the screen's size. The windows build agent's screen is 1024 by 768, so every canvas CanvasHost docked came out 1028 by 749 whatever was asked for: HighlightAtColumn66CoversItsGlyph drew no column 66, and WhatTheFirstWindowHoldsAtEachScale measured a 200% window at 1028 wide. Both have failed the windows job on main since #890. A child control has no such limit.
SimonCropp
added a commit
that referenced
this pull request
Sep 23, 2026
* Fix the native items from the review
- Both native renderers drop decoded pictures the frame just drawn did not
use. An entry used to go only when its own path was asked for again and had
changed, so every image reviewed in a session stayed decoded until it ended.
- A macOS drag clamps to the rows the pane drew, as Linux's RowAt does, rather
than to the body's capacity. A drag past the end reached rows the managed
side holds back, so the status line counted, and a copy took, rows nobody
saw highlighted.
- The Linux queue header says "Pending (N)", as every other renderer does.
- RenderDrawData flips scissor rectangles with the draw data's height rather
than the window's, which a capture's render texture need not share.
- The Linux gutter takes one width for every row of a frame, wide enough for
the longest line number drawn. Formatted per row, a five digit number moved
its own row's text a cell right of the rows above, where the hit test did not
look.
- Every macOS export runs in an autorelease pool. With no NSApplication.run to
drain one per event, everything a frame autoreleased stayed until the
process exited.
- An app delegate turns a Dock Quit or logout into a close, which the managed
loop answers by persisting an owned queue and exiting. terminate: exited from
inside the pump, so that cleanup never ran.
- NativeResolver probes a musl RID and nothing else, as BundledViewerDirectory
does. The linux-{arch} candidate after it names the glibc build.
* Rebuild native renderer binaries
* Size the test canvas itself rather than docking it to a form
A top level window is held to the screen's size. The windows build agent's
screen is 1024 by 768, so every canvas CanvasHost docked came out 1028 by 749
whatever was asked for: HighlightAtColumn66CoversItsGlyph drew no column 66,
and WhatTheFirstWindowHoldsAtEachScale measured a 200% window at 1028 wide.
Both have failed the windows job on main since #890. A child control has no
such limit.
* Temporarily drop the Linux pixel baselines to recapture them
* Re-approve the Linux queue baselines for the pending count
---------
Co-authored-by: SimonCropp <122666+SimonCropp@users.noreply.github.com>
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.
The "Windows head" group from
todo.md. The repros from the review are nowFormsHeadTests, which drive a real form and canvas with real window messages. The image cache test moved toImageCacheTests.MonoFont.Advance, the unrounded advance, places the highlight,ColumnAtand the subtitle width. The integer cell stays for the gridImageCachenever evictsKeepdrops every picture that isn't on the current screenInitialClientSizescales it to the display's DPI once, when the handle is created, and keeps it inside the working areaILoopHooksin core. The loop hands the WinForms head a frame to run on a timer during user32's modal loops: thumb tracking, andWM_ENTERSIZEMOVEtoWM_EXITSIZEMOVEOnMouseCaptureChangedends the drag, and so does a move with the left button upPersistOwnedILoopHooks.SessionEnding, called fromOnFormClosedfor a session end, setsClosingand stages the queue insideWM_ENDSESSIONClosingis set whenWM_ENDSESSIONreturnsImageCacheignores the hash and holds filesFileShare.ReadFileSidereads pending files through aReadWrite | Deleteshared stream, and the cache reads through it tooNot covered by a test: the move and resize half of the modal-loop fix, which needs a real title-bar drag. It uses the same timer as the thumb.
Ten
WindowsPixelTestsbaselines changed. The subtitle moved a few pixels, because its width is now the true advance, and inSelectionthe highlight now lines up exactly with its glyphs.The full solution passes locally in Release: 2,078 passed, 24 skipped.