Per os renderers - #733
Merged
Merged
Conversation
Package content is assembled by MSBuild from several unrelated mechanisms and nothing in the build asserted the result, so a stray or missing file only surfaced once it was on nuget.org. The baselines record today's output, which includes a defect worth naming: the tray package carries DiffEngineViewer.exe, its runtimeconfig and all six native renderers, but no DiffEngineViewer.dll. That apphost cannot start, and the natives are 13 MB of dead weight in a Windows only package. Fixed next, so the removal reads as a diff. Windows only via the solution file. Release-NotWindows drops DiffEngineTray, so a run there would be describing an incomplete release.
A solution level BuildDependency is treated as a reference, not just as
ordering. DiffEngine declared one on DiffEngineViewer to make sure the viewer
was built before PublishViewerHeads shelled out to dotnet publish, and the
viewer is an Exe carrying six native renderers as content, so all of it was
copied into DiffEngine's output for each of its eight target frameworks and
then on into DiffEngineTray through the ordinary project reference.
DiffEngine's own package was unaffected, because lib/{tfm} only takes the
primary assembly. DiffEngineTray's was not: PackAsTool packages the whole
publish directory, so it shipped an apphost with no assembly beside it and
13 MB of renderers for platforms it does not run on. 6,421,352 bytes to
363,032.
It takes a solution build to reproduce, which is what publish-nuget.yml runs,
so any DiffEngineTray package published since the viewer landed carries it.
The apphost was never startable, since the assembly it needs was not copied
either, so the effect was dead weight rather than a broken command.
Ordering is now a project reference in DiffEngine.csproj, where
ReferenceOutputAssembly, Private and ExcludeAssets can say that nothing is
wanted from it, and where it can be limited to the one target framework the
viewer has.
The three invariants added here are the ones a content snapshot states poorly:
an apphost with no assembly, a viewer file in the tray, and an incomplete
bundled head.
ViewerWindow was already the only type touching the native shim and already had close to the right shape, so this mostly promotes it: TryOpen becomes an OpenWindow delegate returning null with a message, Poll, SetHidden and Focus become instance members, and the class becomes NativeViewerWindow. Program splits in two. ViewerProgram.Run takes the renderer to use, so the queue semantics, the wire protocol and the loop are shared. Program.Main keeps only the choice of renderer and the NativeResolver registration that goes with it, which is the shape each platform head will take. Loop itself is untouched, which is the point: the scroll amplification, the button index lookup and the tray-running close-means-hide rule stay in one place rather than being reimplemented per platform. ViewerInput moves out of Native, having never had anything native about it.
A portable RID agnostic tool package and a per platform GUI framework cannot coexist: WinForms needs Microsoft.WindowsDesktop.App named in runtimeconfig.json, which fails to start on macOS and Linux. The single package only ever worked because the platform variation was native files that NativeResolver picks at load time. So DiffEngineViewer becomes a library, and DiffEngineViewer.Windows, .Mac and .Linux are the packages. All three still build a DiffEngineViewer apphost, so tools/viewer keeps its shape and BundledViewerDirectory, ViewerLauncher and the tool definition are all untouched. The core takes the .Core assembly name to leave the plain one for the heads, which is the name the launcher looks up. The committed renderers move to the head that loads them, which is also the statement that the Windows head is about to stop having any. Every head is still the native renderer here. Swapping the Windows one for WinForms is the next commit, so that diff is only the renderer. A user now downloads 1.0, 2.3 or 3.5 MB rather than 6.2, and DiffEngine.nupkg is unchanged apart from carrying DiffEngineViewer.Core.dll per RID.
The Screen model was already renderer agnostic, so this head consumes it directly: no marshalling layer, no flat blittable frame description, and no committed binary. The two Windows renderers, 1.9 MB, are deleted. One owner drawn surface rather than a control per pane, because ScreenBuilder has already sliced each pane to the rows that fit. A scrolling control would want to own that decision, and the text snapshots would stop describing what this draws. The footer buttons are real controls, so they keep native focus, keyboard access and theming. Pumped through DoEvents rather than inverted onto Application.Run. ViewerProgram owns the loop for all three heads, and keeping it there is what keeps the scroll amplification, the button lookup and the close-means-hide rule in one place. The usual objections to DoEvents do not apply: no modal dialogs, no nested message loops, and session state already behind its own lock. Two things this fixes rather than ports. The window reports the character grid it measured from the font instead of dividing pixels by a hardcoded 9 by 18, which is what makes it right on a scaled display. And closing is FormClosing with Cancel, rather than declaring a GLFW symbol by hand because raylib latches its close flag and offers no way to clear it. Text goes through GDI+ with grayscale antialiasing rather than GDI, which honours whatever ClearType setting the machine has. That is what lets the four baselines here be committed at all. DiffEngineViewer.Windows is 275 KB against the 6.2 MB the one portable package used to be, and DiffEngine.nupkg drops from 9.38 MB to 8.63 MB.
DeviewInput.columns and rows carried pixels, and NativeViewerWindow divided them by a hardcoded 9 by 18 measured once for JetBrains Mono at 15px. That is the whole of the viewer's DPI handling, and it is wrong on any scaled display: the grid ScreenBuilder slices to is either larger than the window or leaves it half empty. The shim is the only side that knows what font it loaded, so it measures and reports cells. The managed side keeps only the floors, which are the app's rule rather than the renderer's. DEVIEW_VERSION goes to 2, since the struct is the same shape but a field means something else, which is exactly the case a version check has to catch. The WinForms head already did this properly by measuring GDI+ metrics, so this brings the other two into line rather than introducing the idea. The committed Linux and macOS binaries are still version 1, so the native smoke job in test.yml fails until build-native regenerates them. The unix job does not: it builds the Linux shim from source, which is also why the four pixel baselines are unaffected.
build-native drops its two Windows matrix entries, and lays its artifacts out
as src/{head}/runtimes/{rid}/native so the propose job can merge them straight
into the head that loads them. The native smoke job loses win-arm64 for the
same reason. publish-nuget checks four RIDs rather than six; its push step
already enumerates whatever is in nugets, so the three viewer packages need
nothing.
Docs now say the renderer is per platform and name the three packages, with
the reason a portable one is not possible.
Implements the same eight exports as native/, so the managed side is untouched: Deview, DeviewStructs, ScreenPayload, NativeResolver and NativeViewerWindow all stay exactly as they are and neither knows which library it loaded. The header is imported rather than redeclared, because Swift does not guarantee struct layout and two copies of an ABI is the bug DEVIEW_VERSION exists to catch. deview.h grows a DEVIEW_TYPES_ONLY guard so this side can take the structs without also taking prototypes for symbols it is about to define. C# keeps the loop. deview_present drains NSApp.nextEvent up to a deadline and returns, rather than handing control to NSApplication.run, which is what keeps the scroll amplification, the button lookup and the close-means-hide rule in ViewerProgram for every platform. The deadline doubles as the frame throttle. No app bundle: setActivationPolicy plus finishLaunching is enough for a window that takes focus, the same thing GLFW does for the Linux build. Nothing is flipped either; layout is written top down and converted once, rather than fighting the text matrix to keep glyphs upright. deview_capture draws into a bitmap context of its own making rather than asking the view for one. bitmapImageRepForCachingDisplay would inherit the window's backing scale, so a committed baseline would only match on the kind of display that produced it. Scale, colour space and the six font smoothing and subpixel switches are pinned there instead, which also means capture needs no window server. Linux and macOS now render differently, so PixelTests gains UniqueForOSPlatform and the existing baselines become .Linux. The macOS set does not exist yet: it has to be produced by the pinned macos-14 runner and accepted from the received artifacts, the same way the Linux one was. Not compiled. There is no macOS here to build or run it on, so the first real feedback is CI.
swift build --arch arm64 --arch x86_64 compiles both slices but leaves a per architecture dylib in more than one place, so taking the first match gave an arm64 only binary. It shipped to both osx RIDs, and the lipo check in the native smoke job caught it, which is what that check is for. Every candidate is now checked, and if none is already universal they are merged, so the only thing collectable is a fat binary. The arch assertion moves into this step too: finding out at collect time beats finding out a job later.
DiffEngine references Microsoft.Sbom.Targets under a condition on the CI variable, so a release package carries two _manifest entries that a local pack does not. The baselines were taken locally, so the Windows job failed on the difference rather than on anything being wrong. Dropped from the entry lists so one baseline describes both. Absence would then go unnoticed, so the half that can still be checked is: on CI, the SBOM is there.
The debug bundle holds a DWARF file of the same name, and it is universal too, so the arch check would have accepted it and shipped symbols as the library. Ordering happened to favour the real product this time.
…rers Rebuild native renderer binaries
The pixel snapshots crashed with "NSWindow should only be instantiated on the main thread". TUnit runs Before(Class) on a worker, and deview_init built a window there. The app was never at risk: ViewerProgram.Run is called from Main, which is the main thread. A hidden start now builds only the renderer, which is all capture needs, since it draws into a bitmap context of its own making. The window is deferred to deview_set_hidden, so the ABI contract is unchanged rather than diverging from the Linux head, and the header says so. Test passed on macOS through all of this because NativeTests only calls deview_version, which touches no AppKit.
…rers Rebuild native renderer binaries
Produced by the pinned macos-14 runner, which is the only place they can come from: Core Text is the system text stack, so a developer machine renders correctly but not identically. Checked against the WinForms and Linux renders of the same four screens rather than accepted on the strength of the tests going green. Between them they cover the three column inline layout and the two column file one, added and modified rows, filler, the queue selection, a disabled button and a status message.
ProjectDefaults supplies System.Collections, System.Diagnostics, System.Reflection and System.Collections.Concurrent through a preprocessed content file, which NuGet materialises into obj and records in FileWrites. A rebuild deletes it and nothing puts it back in the same invocation, so the compile runs without those four namespaces and every use of Process, DebuggerDisplay, ConcurrentDictionary or the non generic IDictionary fails. Reproducible on main with dotnet build src --no-incremental, and the reason nobody noticed is that CI always builds a fresh clone and an incremental build never removes the file. Rider rebuilding is what found it. Declaring them here puts them in the generated GlobalUsings instead, which the SDK owns and a clean does not touch. Overlapping with the content file is harmless: a duplicate global using is not even a warning, and warnings are errors here.
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.
No description provided.