Repository navigation
Register diagnostics last so they do not change when the app's services are created - #616
Merged
Merged
Conversation
…r factory and GPU device ahead of the app's services, and test the frame loop, console and dispose wiring
…GPU device position, fail on logging after the logger factory is disposed, and report GPU memory on dispose
…t the container's singleton creation order
stanoddly
force-pushed
the
diagnostics-registration-order
branch
from
October 5, 2026 13:33
6fd9b5b to
6ea9bc0
Compare
stanoddly
enabled auto-merge (squash)
October 5, 2026 13:34
|
📦 Development package |
|
✅ Development package validation passed on all platforms. |
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 #613, from its second review.
Change
PixelyAppBuilderconstructor, and thePerformanceReportfactory resolvedILoggerFactoryandGpuDevicebefore checking whether diagnostics were on. Singletons are created in the order their types were first registered, so the logger factory, SDL initialization and the GPU device were created ahead of every service the app registered, even with diagnostics off. An app registration that sets SDL state before the device exists would silently stop working.Build()now registersFrameTimings,IFrameTimingRecorderandPerformanceReportlast, withFunc<ServiceProvider, T?>factories. The report resolves the logger factory and the GPU device only while diagnostics are on.UseWindowRenderingfalls back toNullFrameTimingRecorderfor a provider built withoutBuild(), since the constructor no longer registers a recorder.PerformanceReporttakes it on its first update instead of at construction, so collections from building the app and loading content no longer count against the first period.docs/class-registration.mdnow states the singleton creation order this relies on: type by type, in the order each service type was first registered, with dependencies created first when needed.docs/diagnostics.mdandDECISIONS.mdno longer say that nothing is registered or that the frame pays only null checks while diagnostics are off: the frame calls a recorder that does nothing, and no report is created.Verification
PixelyAppDiagnosticsTests, which clear thePIXELY_*variables for each test and restore them after:main's builder.Build()registers the null recorder and no report while diagnostics are off, andFrameTimingsand one report while they are on (moved fromPerformanceReportTests).RunFramecalls on a fake clock record the update between the frame start and the render, and disposing the app logs the final report. The test logger records only the report's category, so SDL's messages on quit cannot come after the report, and it throws if anything is logged after its factory is disposed.InputAutomationConsoleTests: the console's reads are recorded as input wait, so they count toward neither the frame nor the update.PixelyAppDiagnosticsTestsalso pass withPIXELY_DIAGNOSTICS=1 PIXELY_HEADLESS=1set.GPU memory 1.83 MB, textures 1.83 MB.