fix(desktop): resync device scale after display changes - #218
Conversation
2857fda to
c9d493c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kolkov
left a comment
There was a problem hiding this comment.
APPROVE
Well-structured two-layer fix:
-
desktop/desktop.go:syncDeviceScale-- frame-level sync placed correctly before the O(1) idle gate and before boundary texture allocation. Releases boundary textures, resets canvas scale, delegates toWindow.HandleScaleChange. -
app/window.go:HandleScaleChange+invalidateScenesInTree-- widget-level invalidation covering ADR-024 boundaries, legacyRepaintBoundaryMarker, and overlay content widgets.
Test coverage is thorough: 6 tests covering boundary invalidation (including the needsRedraw+cleanScene edge case), nil root safety, non-positive scale rejection, texture release + layer tree reset, and integration-level placement in the render loop.
The reflect+unsafe usage in device_scale_sync_test.go is consistent with the existing pattern in gpu_work_test.go -- acceptable for test code that has no alternative since gogpu.Context is only fully populated during a live frame.
CI green (9/9 checks pass). No blocking issues.
Summary
Why
A window moving between displays with different scale factors can retain its logical size. In that case the resize path does not run, and
RenderDirectWithDamageRectsbypasses ggcanvas's later provider-scale check. The old device scale, boundary textures, and retained scene content can therefore survive the transition.The new frame-level synchronization updates scale-dependent state before deciding that a frame is idle, ensuring the scale-change redraw produces correctly rasterized content.
Verification
go test ./... -count=1go build ./...CGO_ENABLED=0 go build ./...go vet ./...git diff --checkThe broad
go test -race ./app ./desktoprun still reports the repository's pre-existingmockWindowProvider.RequestRedrawrace inTestWindow_AnimPumper_StartsOnInvalidation; the changed desktop tests pass under-race.Fixes #172
Local CI and Codecov preflight
main(273cc1e)go test ./... -count=1with an atomic coverage profile: passTestWindow_AnimPumper_StartsOnInvalidationmock race noted abovego build ./...,go vet ./...,gofmt, and diff checks: passapp/window.go21/21,desktop/desktop.go9/9); overall coverage 86.4%Upstream CI run approval is still external to this branch: GitHub marked the fork workflow
action_requiredwith zero jobs (run31434862113). Agogpumaintainer must approve the workflow before Actions and the configured Codecov upload/bot can run.