Fix the bugs and the performance items from the review: the viewer's model, the library, the inline patcher, the tray and the three heads - #922
Merged
Conversation
…nts that stopped being read The five items under Bugs in todo.md, each with tests that fail without the fix. - The viewer no longer asks for a verified file. It was declared RequiresTarget, so for a snapshot with no verified file EmptyFiles wrote a placeholder before the viewer heard of the pair, and the viewer compared against it as though it were the expected file: a blank page, or a 212 byte PDF it could not open, which also hid the received text. EmptyFiles has no file for .geojson, .gpx, .kml, .topojson, .wkt, .wkb, .fgb or .geoparquet, so for those the launch ended at NoEmptyFileForExtension and nothing was raised at all. The viewer reads a missing target as the empty side of a new snapshot, which is what it now gets. - A pair that arrives again saying what the queued one says leaves the reader where they are, as an inline patch already did. A test that keeps failing the same way sends its pair on every run, and each one opened the entry again: back at its first change and first page, fitted, with the menu closed. The entry is replaced by one that reads the same and carries the new stamps, never kept as the same object, because TrackedWatch applies what a pass found by reference and a pass that looked while the run had cleared its received file found it gone. A document is compared by its bytes, since its text is read after it arrives and the arrival may not have it yet. - DocumentWatch's loop no longer ends on a fault. A copy that could not be written into the cache, because something held the file or the disk was full, threw out of the pass, the loop said so once and returned, and no document was read or drawn again until the viewer was restarted. A turn that fails now says why, once for as long as that stays the reason, and is tried again, and the status line is given back when it gets through. Nothing is marked as started before everything that can fail has been done, so a drawing whose copy could not be written is not left as a spinner. A file that could not be read for a moment is tried again too, rather than recorded as changed. - A PDF left behind after the timeout no longer disables PDFs for the life of the window. The flag is cleared when the call that was left behind returns, and a PDF opened meanwhile waits for it and is then read, with nothing recorded against it. The timeout is counted from the last page to land rather than from the start, so a long document that keeps landing pages is not given up on part way through. - A setting one viewer put back to its default is no longer restored by another viewer's next write. A default is stored as no line at all, and each write laid everything this viewer held back over the file wherever the file had no value, so the projection one window reset came back with the other's window position. A write is now the file as it stands with the one key changed, plus any of this viewer's own whose earlier write failed.
The review this list came from was run with six others beside it, one each for the library, the inline patcher, the tray, and the Windows, Linux and macOS heads. Their findings arrived after the first list was written and are added here, unfixed, by area. Each says how far it was taken. Four inline patcher findings were reproduced by running the patcher, and the F# one by compiling what it writes with dotnet fsi. Five more were confirmed by reading the code they rest on. The rest are marked as reported, with the reviewer's own evidence, after checking that the code each one quotes is in the tree as quoted. Nothing under a Linux or macOS heading has been run.
…ow's place, and a picture composed for ever - A decode that lands for a picture no longer on screen is thrown away, and it left the path marked as on its way. A reader who came back to that picture had nothing started for it, and a spinner in each pane until the file changed: stepping past a picture before it had decoded, which holding Tab through a queue of them does to nearly every one. The mark now goes when the decode lands, whatever is done with the result. - The first window was centred for its unscaled size and then scaled in place. WinForms centres as it creates the window, before OnHandleCreated sizes it for the display, so it grew down and to the right from there: at 150% on a 1080p display the footer was under the taskbar, and that was the placement remembered for every run after. It is centred again for the size it has. - Raise put a minimised window back to normal whatever it had been minimised from, so a maximised window came back at its restored size and was then remembered as not maximised. It goes back to what it was. - The right pane's picture space was the pixel wider an odd width leaves that pane. One picture on both sides, which a page two identical documents share is, was then asked for at two sizes, and the cache keeps one composite per picture: each paint composed both, each landing throwing the other away, for as long as the entry was on screen. Both pictures are now given the same width, which also fits two pictures of one size to one size. Five pixel baselines move with it, the right hand picture one pixel to the left.
Each handler wrote its event into one field of the input the next poll reads, and the pump dispatches everything AppKit has queued before it returns. So whenever a frame was slow, the loop waiting behind an accept on InlineApplier's mutex, two presses of Down scrolled once, Tab then a accepted the entry the reader meant to skip, and d then a click on another row discarded the clicked row, which the reader had never looked at: the managed side applies a frame's click before its key. Keys, clicks and menu events now go into a queue in Runtime, and deview_poll_input hands over the one that is next, as the WinForms head does. What adds up or says where it is now, the wheel, a drag, the scroller and a close, is still whole per poll. The ABI is as it was. Two things follow from handing over one a poll. The pump does not wait out its frame while more are queued, or a fast key repeat would be handed over more slowly than it arrives. And a context menu is not popped until the queue is empty: popping holds the managed loop for as long as the menu is up, so a key pressed straight after the right-click would be applied when the menu closed, however much later that was. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it, and nothing in a capture exercises input.
…ff them The footer was one row whatever was in it. The buttons were laid out left to right with nothing to stop them at the window's edge, and the status was drawn from the right edge whatever was already there. At the 1100 points the window opens at, an image pair in a queue had "images differ" drawn over its last button, and a paged document's eleven buttons needed 1405, so Zoom out and Zoom in were off the window and could not be clicked. A button that would pass the edge now starts another row, and the body gives up the height. The status is drawn beside the last row when there is room right of the last button, as before, and otherwise on a line of its own above the buttons, losing its end if it is wider than the window. Above rather than below, and with the buttons placed from their labels and the width alone, so they stay where they are as the status comes and goes: a click is resolved by position, and the status changes while the pointer is on its way. A footer whose buttons fit one row with room for the status beside them is laid out exactly as it was. That is every scene PixelTests captures, so no macOS baseline is expected to move. The managed side still slices the body as if the footer were one row. This head's own chrome leaves it 64 points to spare, which is three rows of buttons, or two and a status line. A paged document's buttons pass that in a window under about 600 points wide, or about 740 with a status too long for the last row, and the last one or two rows of the body are then not drawn. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it.
NSScroller tracks a press on its knob in a loop of its own, inside mouseDown, until the button comes up. That is inside the pump, so deview_present did not return for the length of the drag: the knob moved, every move was reported, and the managed loop that scrolls the panes in answer ran once, at the release. The scroller is now a subclass that takes a press on the knob as three ordinary events, the way the view takes a selection, so there is a frame between one move and the next and the panes scroll as the knob is dragged. It reports where the drag has got to and leaves the knob to the frame that answers, which puts it on the row the panes show. A press in the slot is still AppKit's: what it does is the reader's setting, and it is over in a click. When the knob's own rectangle cannot be read, the press goes to AppKit as before. A window being resized is the same kind of loop, and that one cannot be stepped around. The scroller is kept against the right edge and stretched by its autoresizing mask while it runs, where it used to stay wherever the last frame had put it. The panes still keep the rows they were sliced for until the mouse comes up, drawn into the new size. Doing better takes a way to ask the managed side for a frame from inside the loop, which is a change to the ABI, and the readme now says so. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it, and a capture makes no window, so nothing there exercises a scroller.
Core Text applies a font's calt feature unless told not to, and JetBrains Mono does its code ligatures through calt. So macOS drew <>, !=, <=, =>, -> and == as one glyph each, where the Windows and Linux heads draw a glyph a character: the title of a file comparison read <> there and as one diamond here. In a tool whose job is to show which characters a snapshot holds, that is the wrong answer, and most of all for the snapshot's own text. The font is now a copy with calt and liga off, through kCTFontFeatureSettingsAttribute. This moves the macOS pixel baselines whose text holds a sequence calt substitutes, and those have to be accepted again from the macos-14 job, since they cannot be made here. Worked out from the embedded font's own GSUB table, they are the five file mode scenes, for the <> in their title: FileDiff, Images, ImagesEnlarged, Selection and Minimal, which also has ... at each end of its folded rows. The five inline scenes hold no such sequence and should come out as they were. Not compiled or run here. There is no Mac on this machine, so the macos-14 job is the first to build it.
Following a drag of the knob only does anything if the scroller has a knob. A scroll view enables and disables its own scrollers, and this one is placed by hand with no scroll view around it, so nothing here ever said it was enabled: it was whatever an NSScroller starts as, and one that is not enabled draws its slot with nothing in it. Which that is could not be checked here. If a scroller starts enabled, this changes nothing. If it does not, the pane scroller has been an empty strip since it was added, and this is what gives it a knob. A commit of its own, so it can be dropped alone. Not compiled or run: there is no Mac here, and a capture makes no window.
…ame sweep just wrote A delete for a verified file stayed tracked when a later run failed against that file and a move onto it arrived. Accept all carries out the moves and then the deletes, so the received file was moved into place and then deleted: both gone, with nothing said. DiffRunner.SettleDelete withdraws such a delete, but not from a library that predates it, not while a viewer holds the queue the settle is sent to, and not while that port is remembered as unowned. A move now withdraws the delete it finds waiting for its target, whichever port it arrives on, since a run that verified against a file is what makes its delete stale. That leaves the delete that arrived after the move, where which of the two is stale cannot be known. Both sweeps, the menu's and the one a displaying viewer asks for, now leave a delete pending when a move in the same sweep wrote its file or a move still pending is going to. It is decided by what was written rather than by what was swept, so a move whose received file has gone, which is dropped without writing anything, holds nothing back.
…hose An accept-all takes its snapshots when it starts and read the deletes when their turn came, after the snapshots had been applied. A snapshot moving inline arrives as a patch plus a delete of the verified file the patch replaces, so a pair landing while a batch was applying had its delete swept by a batch its patch was never in: the verified file went while the patch was only pending. Both sweeps now list the deletes before anything is accepted, as the viewer's own batch does. The menu's lists them at the click, and the one a displaying viewer asks for lists them ahead of its snapshots and hands the keys to ITrackedFiles.AcceptAll. A delete that arrived during the batch stays pending and is counted as neither accepted nor kept. One that was listed and has gone since is passed over. Ahead of the snapshots rather than beside them, because Verify raises the delete and then queues the patch. What is left is a batch that begins between those two sends.
…answer Accept all asked the queue what was pending and read no answer as nothing pending: RemoteInlineHost.List flattens an owner that could not be asked to an empty list, which is right for a menu. So with a viewer holding the port and a patch, cold starting or wedged, and the tray holding the paired delete, the snapshot sweep was skipped and the delete was carried out. The verified file went with the patch that replaces it never tried. IInlineHost.TryList tells the three answers apart. Nothing holding the port is still nothing pending. An owner that is there and did not say what it holds is treated as a snapshot that was not written: the deletes are held and the user is told the viewer did not answer. A program that is not a viewer holding the port is no owner either, as it is for every send DiffEngine makes, so a tray that could not bind 3493 over one still carries out its deletes. ViewerClient already records that when a reply is not this protocol, and FoundUnowned lets the host read it.
Program.Main was async Task. The entry point the runtime starts for one of those is a method the compiler writes, which carries no attribute, so the thread every tray window lives on was MTA. Clipboard.SetText throws ThreadStateException there, which the debug view's catch for a busy clipboard does not match, so every click on Copy threw and nothing was copied. Main is now a synchronous [STAThread] method that blocks on Inner, which is what the compiler written entry point did. Nothing in Inner awaits until Application.Run() has returned, and WinForms has taken its synchronization context back off the thread by then, so the awaits that follow continue on the pool instead of waiting on a loop that is no longer pumping.
A tray that owns the queue stages what is pending as it exits, but only from the unwind after Application.Run() returns. A logoff or a shutdown never gets there. Windows sends each top level window WM_QUERYENDSESSION and then WM_ENDSESSION and may end the process once they are answered. The tray has no Form to turn those into a close, only the notify icon's window, so the loop kept running until the process was ended and the queue went with it. For a tray started at login that is how it usually stops. SessionEndWindow is a hidden top level window that hears WM_ENDSESSION and calls OwnedInlineHost.SessionEnding inside the message, which stages the queue before the message is answered. Patches arriving after that are refused, as an owning viewer refuses them once it is closing, so the sender stages what a process on its way out would otherwise have acknowledged and lost. Nothing is done for the query, or for an end that was called off, since a session that carries on would then have its queue both staged and held. Confirmed outside the repo with a tray shaped process: a notify icon window and no Form answers the query and Application.Run() has not returned two seconds after WM_ENDSESSION, while a hidden window made this way is sent both messages on the UI thread. A real logoff was not run.
A move for a pair the tray already tracks was always rebuilt from what the message carried. A Move or a Diff over the viewer port carries the two paths and nothing about the tool, so the tray filled the gap as it does for a pair seen for the first time, with its own choice for the extension, over a move that had already said which tool was showing the pair. "Open diff tool" on a viewer pair does this by hand: it starts DiffEngineViewer --diff, which cannot bind the port and forwards the pair to the tray as a Diff. The pair went from the viewer, not killable and open, to whatever the tray would pick, killable and with no window, so the "Accept open" hot key passed over a pair that was on screen in the viewer. A move that names no tool now changes nothing about the one recorded: the exe, its arguments, whether it may be killed, whether it kills what locks it and whether it is the viewer all stay, and only the target is taken. The focus race test told a focus from a Diff by the exe surviving, which a Diff now leaves alone as well, so it asserts on the entry not having been replaced.
… GetAwaiter An append always went on the end of the chain in C#, so await Verify(value).ConfigureAwait(false) was given a Snapshot call on a ConfiguredTaskAwaitable, which has none (CS1061), at a call site CanAnchor had already said could host a snapshot. Only F# stopped short of the end, and only at ToTask. The three members that hand back something other than a SettingsTask now end a chain in both languages, and the call goes in front of the first of them.
An append took the first entry point the search yielded. Once an accept higher in the file leaves the hint stale, the walk starts over from the member's declaration, and the first call it meets is the one most likely to have been accepted already: the patch was refused with 'already has a Snapshot call' while the call it was for sat two lines below, and a single accept dropped the entry. A call that has one is now passed over, along with any entry point inside its arguments. The call on the recorded line still decides, since a hint that lands on a call names it, and so does the nearest call when the patch names no member to bound the walk.
The appended call and a literal on its own line were both indented one level in from the line's leading whitespace. F#'s offside rule needs them to clear the column the expression starts at, and after do!, let! x =, let x = or a binding written on one line that column is further along the line: the appended call was read as the start of the next statement and the literal as offside, FS0010 either way, in source that compiled until it was accepted into. A chain ending on a closing paren or an argument at that column did the same to a call that does start its line. Both are now measured from where the expression starts, found by walking back over the call's receivers, and FsCompilerRoundTripTests compiles the shapes with dotnet fsi.
…variable
A Remove took the call off the end of whatever it hung off and kept the receiver. That is right for a verify call and wrong for a variable: settings.Snapshot("old"); became settings; (CS0201), and VerifySettings.Snapshot is public API that Verify sends a Remove for under NotInline. Where the call hangs off a name and nothing is chained after it, the statement's lines go instead, but only where that cannot change what is around them: alone on its lines, in a block and ending in its own semicolon in C#, and followed by another statement of the same block in F#. Anything else is reported rather than left as a bare receiver.
…ne names A queue entry is keyed by its line, and accepting a snapshot moves every later call site in the file. The re-run's patches then matched no key and were queued beside the entries they should have updated, so a bulk accept applied the stale content and refused the fresh, and a passing call that had moved onto a later test's old line settled that test's entry by key. An enqueue whose key names nothing now folds into the one entry for the same member, test, mode and anchor that its framework already has content in, and takes it to the new line. A key that names another member's entry is no longer believed: a settle leaves it unless the value settles it, staged trios are cleared by the same rule, and an enqueue updates its own entry instead. The viewer keeps the reader on an entry that moved. Two existing tests gave both call sites of one member the same literal, which now reads as one call site, so they have a literal each.
…ed to it Clearing staged trios could be scoped to a framework, but never was in practice. A test run stages through InlinePatchFile.Write with a patch that names no framework, and an unlabeled trio is cleared whichever framework asks. And the caller, Verify, could not pass an origin to InlineStaging.Clear, because the moniker the labels are written with is internal. So in a multi-targeted project with no viewer, the framework that passed deleted the staged snapshot of the one still failing. Write now stamps the framework of the process staging the file where the patch carries none, as the send to a queue owner does, and InlineStaging.Settle clears for the running framework alone. Clear is unchanged, which is what a retire wants.
…urned or passed
Removing a Snapshot call from a variable now takes the whole statement, because
settings.Snapshot("old"); otherwise became settings; which is no statement. But
every call on a variable with nothing chained after it was treated that way,
and one that could not be taken with its statement was refused. So
await task.Snapshot("old"); and var kept = task.Snapshot("old"); were refused
too, where taking the call alone leaves await task; and var kept = task; which
is what each would have run without the snapshot, and what it did before.
The statement goes only where the variable would be left as the statement.
Awaited, returned, assigned, compared or passed, the call alone is taken. A
lambda's body is still refused, since _ => _ is no body for a lambda that
returns nothing. In F# what takes the value has to sit on the same line: the =
a line above is the one a whole body hangs off.
…etter typed The Linux head read letters as the characters typed, 'a' for accept and 'A' for accept all. Caps Lock types 'A' with no Shift held, so with it on the key that accepts one snapshot accepted every pending one, with no confirmation, and d, v, q, n, p, m, r and j did nothing. The letter is now folded to lower case and accept-all is chosen by whether Shift is held down, which is how the Windows and macOS heads decide it. Shift with any other letter is that letter's command, as it is on those two.
…ws only filler A drag starts only where there is text to select, and that was asked of the left pane whichever pane was pressed: where its text starts is read from the first row that draws any, and a pane of nothing but filler has none. So nothing in the right pane could be selected or copied for a pending delete, whose left side is empty, or anywhere inside a long removed block. It is now asked of the pane the press landed in. A press in a pane of filler still starts nothing.
…do not fit Every footer button was drawn on one line and the status after the last of them, wherever that was. A paged document pending in a queue has eleven buttons, 1199 pixels of them in a window with 1084, so Zoom in could not be clicked and the status line was not drawn at all; in a window of its own the same document left the status 54 pixels. The status line is where the page on screen, a page that could not be drawn, a selection and a failed accept are said. Buttons now wrap onto another row at the window's edge, and a status with no room beside the last row takes a line of its own, cut with an ellipsis if even that is too narrow. The title stops short of the subtitle in the same way rather than running on under it. A footer that fits is laid out exactly as before, so the existing baselines are unchanged. Two scenes pin it, a document in a window of its own and the same one in a queue. Both are skipped on macOS, where they have no baseline.
…Linux The embedded font was the only font the Linux head had, so CJK, Hangul, Arabic, Hebrew, Thai and emoji all drew as the replacement glyph and a snapshot holding them could not be reviewed: a line with one such character changed looked the same on both sides. The other two heads fall back to system fonts. The window's font now takes the machine's fonts as further sources, found through fontconfig in the order it falls back through them. Only the fonts a character on screen has needed are read, on a thread of their own, and fontconfig is loaded at run time rather than linked, so a machine without it loads the library as before. Each is scaled to the embedded font's em, and a glyph wider than the cells the grid gave it is cut off where the next character starts. ImGui is built with 32 bit characters so that anything past the basic plane can be drawn at all. A capture never draws with them. It uses the embedded font alone, so a baseline is the same picture whatever a runner has installed, and a new scene pins that after showing the same text in the window. It is skipped on macOS, where it has no baseline.
A launched viewer took its working directory from the test host, which is usually the test project's output folder. On Windows a directory a process is in cannot be deleted, and a viewer hidden behind a tray lives for the session, so git clean -xdf or removing a worktree failed with nothing on screen to say what was holding it. The viewer now starts in the folder it runs from, which it holds anyway. A relative path on a --delete or --diff launch meant relative to the host, so it is rooted first; a rooted one goes over as the caller spelt it, since the row is settled by a key built from that spelling. DiffRunner.LaunchProcess is left alone: a third party tool resolves relative arguments against the directory it inherits, and its window is on screen for as long as it holds one.
ViewerLaunchGate waited for the port and then reported the launch whatever had happened to the process, so a viewer that could not take the launch still read as Launched. AddInlineAsync turned that into Queued, and a caller told Queued stages nothing: the snapshot was in no queue and in no file. A copy from before 20.5.0 exits with 2 on --payload, and an apphost with no runtime exits before any viewer code runs. Each held the gate for the whole five second wait and left its payload file in the temp directory. The launcher now hands back the process, and the wait ends as Failed as soon as that process has exited with a failure. A clean exit is still waited on, since a viewer that hands its work to an owner exits with zero, and one that is running and slow is still reported as launched. A failed inline launch takes its payload file back. Two things about which copy is started, from the same item. A wildcard over folders named for versions is taken highest first rather than most recently written, so the NuGet cache no longer yields whichever DiffEngine was restored last. And a copy stamped older than 20.5.0 is passed over while a newer one is further down the search order; it is still taken when it is the only one, and one an environment variable names is never second guessed.
…m the test host DiffRunner.LaunchProcess started a tool declared UseShellExecute: false through Process.Start, which on Windows always hands the child every inheritable handle whatever is redirected. One of those is the pipe dotnet test reads the host's output from, and it reads until every writer has closed it. So a run that opened the Word or Excel comparer, Cursor or VS Code's launcher did not return until the process it had started was gone, and the Word comparer's stays until Word is closed. This is Verify issue 1229 again, for the tools ShellExecute was not the answer for. On Windows those tools are now started by CreateProcess directly, with handle inheritance off and no console. ShellExecute with a hidden window was the other way to do it and is not used: the hidden request reaches the program as how to show its first window, so a console program that opens a window of its own never appears, and nothing in the file says which kind of console program it is. No console whatever the tool declared, because a console program sharing the host's console is given the host's standard handles with it, inherited or not. Tools declared with ShellExecute are untouched, and so is everything off Windows, where nothing in a definition says which tools need the terminal's streams.
The twenty seven bugs the six per-area reviews found are fixed in the commits before this one. todo.md loses them, and gains what each fix says it did not reach: the macOS changes have not been run, Verify has a call to change before staged trios are cleared per framework, and so on by area. - docs: which copy of the viewer runs now passes over one from before 20.5.0 when a newer one is there; the tray's Accept all says which deletes it carries out and which it holds; the inline page covers the F# indentation, what ends a chain in either language, a Remove that takes its statement, InlineStaging.Settle, and a settle or a re-run asking whose entry a line names. - claude.md says the same for whoever works on this next, and counts the shim's exports correctly: eleven, not eight. - OsSettingsResolver.Resolve documented one parameter and not the others, which a Release build makes an error. The Debug builds it was written under do not. - Three comments that the fixes made untrue: ILoopHooks on the native heads having no modal loops, the tray's DiffToolLauncher on what DiffRunner starts a tool with, and the viewer definition calling itself a console program.
SimonCropp
changed the title
This was referenced Oct 5, 2026
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
1 participant
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.
Every bug in
todo.mdfrom the review ofmainat 991bc48, and then every performance item in it: the five bugs in the viewer's model, the twenty seven that six per-area reviews found (library, inline patcher, tray, and the Windows, Linux and macOS heads), and the fourteen performance items across the same areas. One commit per fix from the second round on, so any of them can be reverted alone. In the performance round a benchmark is committed ahead of each fix that replaces a path, so the earlier number can be had again from history.Needs attention before merging
macos-14job is the only build of the four macOS fixes and the two macOS performance changes. It compiles them and its suite passes, but two of the fixes are event handling, which no capture exercises, and neither performance change has been measured: a capture draws no spinner and never takes the scaled copy.todo.mdlists the check that would confirm each on a Mac.native/changed in both rounds, and both ofbuild-native's binaries PRs are merged into this branch: VerifyTests/DiffEngine#923 for the bug fixes and VerifyTests/DiffEngine#924 for the performance changes, built from 28f1528, after whichnative/has not changed. So the committed.soand.dylibare built from the sources beside them. The C ABI is unchanged throughout.InlineStaging.Settle(...)in place ofClearStaged(...)inInlineEngine.Settle(). DiffEngine's half is in here.CreateProcesswith handle inheritance off, so a test run no longer waits on them. That was proven with a console exe, a windowed exe and a.cmdstanding in; the real tools were not run.Round one: the viewer's model (5)
RequiresTarget: false), so a new snapshot gets no placeholder, and the eight map formats EmptyFiles has no template for reach the viewer at all.Round two (27)
Library (3)
UseShellExecute: falseno longer inherit the test host's handles on Windows.Inline patcher (6)
do!,let! x =) is indented from the column the expression starts at, so it compiles.FsCompilerRoundTripTestsnow covers those shapes underdotnet fsi.Snapshotcall goes in front ofConfigureAwait,ToTaskandGetAwaiter, in both languages.Appendpasses over a call in the member that already has aSnapshotcall.Removeofsettings.Snapshot("old");takes the statement, where it used to leavesettings;. Awaited, assigned, returned or passed, only the call goes.InlineStaging.Settleclears only the running one's.Tray (6)
Mainis[STAThread], so Debug view's Copy copies.MoveorDifffor a tracked pair keeps the tool it was tracked with.Windows head (4)
Raiserestores a window minimised from maximised as maximised.Linux head (4), built and run in an
ubuntu:24.04container as theunixjob isawith Shift held, so Caps Lock no longer turns accept into accept-all.macOS head (4, one of them half), compiled and run only by CI
caltandligaare off, so<>and!=are drawn as the characters they are. The five OSX baselines that moved with it were taken again from the job's artifact.Round three: performance (14)
Every performance item in
todo.md, each measured before and after by a benchmark that is now in the repository. The figures are one pass of every benchmark on the tree before this round and on the branch, a project at a time on a machine doing nothing else: BenchmarkDotNet in process, .NET 10, a Ryzen 9 5900X. The Linux head's are from anubuntu:24.04container under Xvfb with Mesa's software rasteriser on four threads, as theunixjob runs it.Viewer model (5)
ScreenCache), and the WinForms head and the native payload both stop at a screen they were handed last frame.Library (1)
Inline snapshots (2)
InlineApplier.ApplyAll), each still with an outcome of its own, and the scan rents its map.InlineStaging.Clearkeeps the list of staging directories for a second rather than walkingobjfor every passing verification.InlineStaging.Clearwith nothing stagedWindows head (2)
Linux head (2), built, run and measured in the container
macOS head (2), compiled and run only by CI, and not measured
CTLines the last two draws made. Which characters can go in one run is asked of the embedded font rather than of four ranges written down, and that half is the model's, so it is measured: segmenting a hundred rows of box drawing went from 116 µs and 319 KB to 15 µs and 3 KB.What each fix left is in
todo.mdunder Performance, with the checks that would confirm the two macOS changes on a Mac.Tests
Each fix has tests that fail without it, except where no test can reach it: keyboard and mouse handling inside the Linux shim was verified by hand in the container with
xdotool, and the macOS event handling not at all. The performance changes have tests for what they must not change (a diff past its budget is still a correct diff, a batch's outcomes are each patch's own, a frame the cache hands back is the frame a build would give) and benchmarks for what they do.Release build is clean, and
dotnet test --solution src/DiffEngine.slnx --configuration Releasepasses on Windows: 3,038 tests, 0 failed, 27 skipped. In the Linux container the branch as it stands builds with no warnings, the viewer's tests with the pixel snapshots on pass 771 with 1 skipped, and all fifteen Linux baselines reproduce byte for byte.Also in here
todo.mdloses every bug and every performance item, and gains what each fix says it did not reach. Of the smaller items, one Linux one turned out not to be a problem and is gone (GetWindowPositionis answered from a cached value in raylib 6.0), and one was run rather than left as plausible.src/DiffEngine.Benchmarks,src/DiffEngineViewer.Benchmarksandsrc/DiffEngineViewer.Windows.Benchmarks.claude.mdsays why three and why in process.docs/andclaude.mddescribe the changed behaviour.