Skip to content

desktop: device scale never re-synced after a cross-display drag — window keeps rendering at the old display's scale (blurry / oversized) #172

Description

@AnyCPU

Summary

Drag a window from a Retina (2×) display onto a non-Retina (1×) display — or the reverse — and it keeps rendering at the old display's scale: soft/blurry output after moving to the higher-density display, an oversized 2× raster scaled down after moving to the lower-density one.

The wrong scale does not recover on its own. The canvas device scale is captured exactly once, at canvas creation, and no later code path re-reads it — ggcanvas.Canvas.Resize is a pure width/height operation — so even manually resizing the window afterwards keeps rasterizing at the stale scale. The window renders wrong for its remaining lifetime.

Root cause

All references are to the current main (v0.1.44-era), verified by reading:

  • desktop/desktop.go:907-918initCanvas calls ggcanvas.New(provider, w, h). The device scale is auto-detected once inside New (gg v0.50.4, integration/ggcanvas/canvas.go:82-91: wp.ScaleFactor() at construction only).
  • Pre-fix desktop/desktop.go contains no ScaleFactor() read and no SetDeviceScale call anywhere — after construction the render loop never looks at the scale again.
  • The render loop's resize branch (desktop/desktop.go:166-174) compares logical dimensions only: gogpu's Context.Width()/Height() return logical points (gogpu v0.44.1 context.go:93-102, physical divided by scale). A cross-display drag on macOS keeps the logical size constant and changes only the physical backing store, so this branch never fires. And even when it does fire (a real resize), ggcanvas.Canvas.Resize (gg v0.50.4 canvas.go:422-446) never re-reads the scale — hence no self-correction.
  • A stale scale poisons two caches:
    1. Boundary textures — allocated in physical pixels: ensureBoundaryTexture (desktop/desktop.go:660) sizes them via scaleToPhysical(bw, bh, cc.DeviceScale()) (desktop/desktop.go:644), so after a flip every texture is wrong-sized.
    2. Boundary scenes — SVG icons are rasterized into cached scenes as fixed-resolution bitmaps at record-time device scale (internal/render/scene_canvas.go:668-723 FillSVGPath, :749 RenderSVG). A clean boundary re-flushed from its cached scene keeps the old-scale raster indefinitely.

No gogpu-side change is needed for the fix below to engage: gogpu's darwin layer already emits a resize/redraw when the physical framebuffer size changes (windowDidChangeScreen: wake + physical-size check in checkResize), so a frame reliably fires once the window settles on the new display. gogpu v0.44.1 also began passing ScaleFactor() into its per-frame surface reconfiguration, but nothing on the ui side ever propagates the new scale into the canvas, boundary textures, or cached scenes — that is what this fix adds.

Fix

Two additions, no behavior change on the unchanged-scale path:

  1. desktop/desktop.go — poll the frame's device scale at frame entry: rl.syncDeviceScale(dc.ScaleFactor()), placed after the logical-resize branch and before the needsAnyWork frame-skip gate, so a pure scale flip with no other dirty state still forces a frame. syncDeviceScale(scale) bool no-ops on scale <= 0 or unchanged scale (O(1)); on a flip it:

    • canvas.SetDeviceScale(scale) — gg reallocates its pixmap at logical×scale synchronously inside the call;
    • releaseBoundaryTextures() — every texture is a physical-pixel allocation and now wrong-sized; a scale flip is a rare one-shot event that invalidates literally all of them, so wholesale release is appropriate. This also nils the layer tree; both are rebuilt later the same frame (texture map at the pre-render nil check, layer tree via UpdateLayerTree's first-frame path — both nil-safe);
    • fullRedrawNeeded = true — the flip frame takes the full-render blit path, so no stale old-scale damage rects survive;
    • Window.InvalidateAllScenes() — required in addition to the texture release: releasing textures alone re-flushes cached scenes, whose SVG rasters were recorded at the old scale, leaving icons inside clean boundaries blurry indefinitely.

    The scale source is deliberately dc.ScaleFactor() (the frame's own snapshot) rather than a live gogpuApp.ScaleFactor() query: gogpu captures the snapshot in the same main-thread pass as the frame's logical w/h, so a mid-drag flip can never pair a new scale with the frame's old logical dimensions (a live query could, producing a one-frame quarter-/quadruple-size glitch plus a redundant second full re-render).

  2. app/window.go — new Window.InvalidateAllScenes(): a direct InvalidateScene walk over every RepaintBoundary in the main tree plus all overlay content widgets, then needsRedraw = true. It deliberately does not ride widget.MarkRedrawInTree: SetNeedsRedraw's O(1) already-dirty guard skips scene invalidation for a boundary that is already dirty (e.g. a spinner mid-animation), which would leave exactly that boundary's scene at the stale scale.

Platform scope: macOS is the primary target (the repro). Windows (WM_DPICHANGED produces resize events during a modal drag) and Wayland are covered by the same per-frame poll. X11 is not covered: gogpu caches ScaleFactor() at init, so a runtime RandR DPI change is never observable through the poll — a pre-existing upstream limitation, unchanged by this patch.

Verification

Automated (included in the patch, both headless — no GPU or window required):

  • desktop/device_scale_sync_test.goTestSyncDeviceScale_Flip builds a render loop around a real (headless) ggcanvas at scale 2.0 with three boundary-texture entries and a boundary root, calls syncDeviceScale(1.0), and asserts: the canvas reports the new scale; all three textures were released and the texture map + layer tree are nil (rebuilt-next-walk state); fullRedrawNeeded is set; and the root boundary's scene was invalidated (texture release alone leaves cached SVG rasters at the old scale — this assertion fails for a texture-only fix). TestSyncDeviceScale_NoOps asserts same/zero/negative scale leaves every piece of state untouched.
  • app/invalidate_scenes_test.goTestInvalidateAllScenes_AllBoundariesIncludingOverlaysAndAlreadyDirty asserts scenes are invalidated for the root boundary, for overlay content, and — the load-bearing case — for a boundary that is already needsRedraw-dirty with a clean scene (the spinner-mid-animation state); a MarkRedrawInTree-based implementation fails that assertion via SetNeedsRedraw's already-dirty guard. A nil-root/no-overlay no-panic test is included.

Each test was checked against compiling mutations of the fix (inert syncDeviceScale / texture-release-only sync / MarkRedrawInTree-based walk) and fails the corresponding target assertion, so the assertions discriminate the real implementation.

CGO_ENABLED=0 go build ./..., go test ./desktop/... ./app/..., go vet, golangci-lint run (0 new issues), and gofmt -l are all clean with the patch applied to current main.

Honest status of hardware verification: the on-hardware symptom and the fix's end-to-end behavior are predicted from source and covered by the unit tests above; an actual two-display drag session (Retina + non-Retina pair, both directions, with SVG icons inside clean child boundaries visible — a text-only scene would pass even without the scene-invalidation half) has not yet been run and is still pending on my side. Happy to follow up with screenshots once done, or if a maintainer with a mixed-density setup can try it sooner: drag examples/gallery between the displays and compare text/icon sharpness in both directions.

Patch

git apply-able against current main (v0.1.44), 373 lines: the frame-entry sync + syncDeviceScale in desktop, Window.InvalidateAllScenes in app, and both test files.

device-scale-cross-display-sync.diff
diff --git a/app/invalidate_scenes_test.go b/app/invalidate_scenes_test.go
new file mode 100644
index 0000000..a9cd7c1
--- /dev/null
+++ b/app/invalidate_scenes_test.go
@@ -0,0 +1,92 @@
+package app
+
+import (
+	"testing"
+
+	"github.com/gogpu/ui/event"
+	"github.com/gogpu/ui/geometry"
+	"github.com/gogpu/ui/widget"
+)
+
+// InvalidateAllScenes must invalidate the cached scene
+// of EVERY RepaintBoundary — main tree and overlay content alike — including
+// boundaries that are already needsRedraw-dirty. The last case is the
+// load-bearing one: widget.MarkRedrawInTree rides on SetNeedsRedraw, whose
+// O(1) already-dirty guard (widget/base.go) skips InvalidateScene for a
+// boundary that is already dirty (e.g. a spinner mid-animation) — which is
+// why InvalidateAllScenes walks boundaries and invalidates scenes directly.
+
+// sceneBoundaryWidget is a minimal RepaintBoundary widget for scene-dirty
+// assertions.
+type sceneBoundaryWidget struct {
+	widget.WidgetBase
+}
+
+func newSceneBoundaryWidget() *sceneBoundaryWidget {
+	w := &sceneBoundaryWidget{}
+	w.SetVisible(true)
+	w.SetEnabled(true)
+	w.SetRepaintBoundary(true)
+	return w
+}
+
+func (w *sceneBoundaryWidget) Layout(_ widget.Context, cs geometry.Constraints) geometry.Size {
+	return cs.Constrain(geometry.Sz(50, 50))
+}
+func (w *sceneBoundaryWidget) Draw(_ widget.Context, _ widget.Canvas)     {}
+func (w *sceneBoundaryWidget) Event(_ widget.Context, _ event.Event) bool { return false }
+
+func TestInvalidateAllScenes_AllBoundariesIncludingOverlaysAndAlreadyDirty(t *testing.T) {
+	a := New()
+	w := a.Window()
+
+	root := newSceneBoundaryWidget()
+	child := newSceneBoundaryWidget()
+	root.AddChild(child)
+	w.SetRoot(root)
+
+	overlayContent := newSceneBoundaryWidget()
+	mgr := &windowOverlayManager{window: w}
+	mgr.PushOverlay(overlayContent, nil)
+
+	// Simulate a completed render pass: every flag clean.
+	widget.ClearRedrawInTree(root)
+	for _, b := range []*sceneBoundaryWidget{root, child, overlayContent} {
+		b.ClearSceneDirty()
+	}
+
+	// Vacuity trap (the reason InvalidateAllScenes exists as a direct walk):
+	// make the child boundary ALREADY needsRedraw-dirty with a CLEAN scene —
+	// the spinner-mid-animation state. SetNeedsRedraw(true) self-invalidates
+	// a boundary's scene, so clear the scene again afterwards.
+	child.SetNeedsRedraw(true)
+	child.ClearSceneDirty()
+	if !child.NeedsRedraw() || child.IsSceneDirty() {
+		t.Fatal("test setup: child must be needsRedraw-dirty with a clean scene")
+	}
+
+	w.InvalidateAllScenes()
+
+	if !root.IsSceneDirty() {
+		t.Error("root boundary scene should be dirty after InvalidateAllScenes")
+	}
+	if !child.IsSceneDirty() {
+		t.Error("already-needsRedraw-dirty child boundary scene should be dirty after InvalidateAllScenes " +
+			"(a MarkRedrawInTree-based implementation skips it via SetNeedsRedraw's already-dirty guard)")
+	}
+	if !overlayContent.IsSceneDirty() {
+		t.Error("overlay content boundary scene should be dirty after InvalidateAllScenes")
+	}
+	if !w.NeedsRedraw() {
+		t.Error("window should need redraw after InvalidateAllScenes")
+	}
+}
+
+func TestInvalidateAllScenes_NilRootNoOverlays_NoPanic(t *testing.T) {
+	a := New()
+	w := a.Window()
+	w.InvalidateAllScenes() // must not panic with nil root and no overlays
+	if !w.NeedsRedraw() {
+		t.Error("window should need redraw even with an empty tree")
+	}
+}
diff --git a/app/window.go b/app/window.go
index 2cdf106..d46719b 100644
--- a/app/window.go
+++ b/app/window.go
@@ -1066,6 +1066,51 @@ func (w *Window) updateScale() {
 	w.ctx.SetScale(scale)
 }
 
+// InvalidateAllScenes invalidates the cached scene of every RepaintBoundary
+// in the widget tree and all overlay content widgets, forcing each boundary
+// to re-record at the next paint pass.
+//
+// Called by the desktop render loop when the device scale changes
+// (cross-display Retina ↔ 1× drag). Releasing boundary TEXTURES alone is
+// insufficient — clean boundaries would re-flush their cached scenes, whose
+// SVG rasters were recorded at the old scale, leaving icons blurry until
+// something else happens to dirty them. A scale flip is a rare one-shot
+// event whose cached scenes genuinely all contain stale-scale content, so a
+// tree-wide invalidation is warranted.
+//
+// Scenes are invalidated directly (not via widget.MarkRedrawInTree):
+// SetNeedsRedraw's O(1) already-dirty guard skips InvalidateScene for a
+// boundary that is already dirty (e.g. a spinner mid-animation), which would
+// leave exactly that boundary's scene stale.
+func (w *Window) InvalidateAllScenes() {
+	invalidateScenesInTree(w.root)
+	for _, ow := range w.OverlayContentWidgets() {
+		invalidateScenesInTree(ow)
+	}
+	w.needsRedraw = true
+}
+
+// invalidateScenesInTree walks the subtree and invalidates the scene of every
+// repaint boundary, mirroring propagateDirtyUpward's two boundary checks
+// (ADR-024 property and legacy RepaintBoundaryMarker).
+func invalidateScenesInTree(wd widget.Widget) {
+	if wd == nil {
+		return
+	}
+	type boundaryInvalidator interface {
+		IsRepaintBoundary() bool
+		InvalidateScene()
+	}
+	if bi, ok := wd.(boundaryInvalidator); ok && bi.IsRepaintBoundary() {
+		bi.InvalidateScene()
+	} else if rb, ok := wd.(widget.RepaintBoundaryMarker); ok {
+		rb.MarkBoundaryDirty()
+	}
+	for _, child := range wd.Children() {
+		invalidateScenesInTree(child)
+	}
+}
+
 // updateWindowSize reads the window size from the WindowProvider.
 func (w *Window) updateWindowSize() {
 	if w.wp != nil {
diff --git a/desktop/desktop.go b/desktop/desktop.go
index 1b2e28d..5cb65be 100644
--- a/desktop/desktop.go
+++ b/desktop/desktop.go
@@ -173,6 +173,16 @@ func (rl *renderLoop) draw(dc *gogpu.Context) { //nolint:gocyclo,cyclop,gocognit
 		rl.fullRedrawNeeded = true
 	}
 
+	// Per-frame DeviceScale sync. A cross-display drag (Retina ↔ 1×) changes
+	// the backing scale while logical size — and thus the resize branch
+	// above — stays constant. dc.ScaleFactor() (not the live
+	// gogpuApp.ScaleFactor()) keeps the scale consistent with this frame's
+	// w/h snapshot: gogpu captures both from the same main-thread pass, so
+	// a mid-drag flip can never pair a new scale with old logical dims.
+	// O(1) no-op on the unchanged path; must precede the needsAnyWork gate
+	// below so a pure scale flip (no other dirty state) still forces a frame.
+	rl.syncDeviceScale(dc.ScaleFactor())
+
 	win := rl.uiApp.Window()
 
 	// ADR-028 Phase C: O(1) frame skip using flat dirty boundary list.
@@ -892,6 +902,36 @@ func collectLiveKeys(layer compositor.Layer, keys map[uint64]bool) {
 	}
 }
 
+// syncDeviceScale applies a device-scale change and reports whether one was
+// applied. On a flip it must invalidate BOTH caches a stale scale lives in:
+//
+//  1. Boundary TEXTURES — allocated in physical pixels (scaleToPhysical), so
+//     every one is now wrong-sized. A scale flip is a rare one-shot event
+//     that invalidates literally every texture, so wholesale release is
+//     appropriate. releaseBoundaryTextures also nils layerTree; both are
+//     rebuilt later this same frame (map at the pre-render nil check,
+//     layerTree via UpdateLayerTree's first-frame path — both nil-safe).
+//  2. Boundary SCENES — SVG icons are rasterized into scenes as bitmaps at
+//     record-time scale, so a clean boundary re-flushed from its cached
+//     scene would stay blurry indefinitely. Window.InvalidateAllScenes
+//     forces every boundary to re-record at the new ctx.Scale() during this
+//     frame's paint pass (Frame()'s updateScale has already read the new
+//     value).
+//
+// The canvas itself reconfigures synchronously: gg's SetDeviceScale
+// reallocates the pixmap at logical×scale inside the call; only the ggcanvas
+// GPU-texture recreation defers to the next flush.
+func (rl *renderLoop) syncDeviceScale(scale float64) bool {
+	if scale <= 0 || rl.canvas.DeviceScale() == scale {
+		return false
+	}
+	rl.canvas.SetDeviceScale(scale)
+	rl.releaseBoundaryTextures()
+	rl.fullRedrawNeeded = true
+	rl.uiApp.Window().InvalidateAllScenes()
+	return true
+}
+
 // releaseBoundaryTextures frees all offscreen GPU textures.
 func (rl *renderLoop) releaseBoundaryTextures() {
 	for _, entry := range rl.boundaryTextures {
diff --git a/desktop/device_scale_sync_test.go b/desktop/device_scale_sync_test.go
new file mode 100644
index 0000000..0faed63
--- /dev/null
+++ b/desktop/device_scale_sync_test.go
@@ -0,0 +1,155 @@
+package desktop
+
+import (
+	"testing"
+	"unsafe"
+
+	"github.com/gogpu/gg/integration/ggcanvas"
+	"github.com/gogpu/gpucontext"
+	"github.com/gogpu/gputypes"
+	"github.com/gogpu/ui/app"
+	"github.com/gogpu/ui/compositor"
+	"github.com/gogpu/ui/event"
+	"github.com/gogpu/ui/geometry"
+	"github.com/gogpu/ui/widget"
+)
+
+// syncDeviceScale must, on a device-scale change,
+// reconfigure the canvas, release every boundary texture (they are allocated
+// in physical pixels — all wrong-sized after a flip), force a full redraw,
+// and invalidate every boundary SCENE (SVG rasters are recorded into scenes
+// at record-time scale; textures alone are insufficient). On the unchanged
+// path it must be a strict no-op.
+
+// fakeDeviceProvider satisfies gpucontext.DeviceProvider with zero values —
+// enough for ggcanvas.NewWithScale, whose only provider use at construction
+// is a non-fatal SetAcceleratorDeviceProvider attempt plus optional
+// interface probes.
+type fakeDeviceProvider struct{}
+
+func (fakeDeviceProvider) Device() gpucontext.Device { return gpucontext.Device{} }
+func (fakeDeviceProvider) Queue() gpucontext.Queue   { return gpucontext.Queue{} }
+func (fakeDeviceProvider) SurfaceFormat() gputypes.TextureFormat {
+	return gputypes.TextureFormatUndefined
+}
+func (fakeDeviceProvider) Adapter() gpucontext.Adapter         { return gpucontext.Adapter{} }
+func (fakeDeviceProvider) AdapterInfo() gpucontext.AdapterInfo { return gpucontext.AdapterInfo{} }
+
+// scaleSyncBoundary is a minimal RepaintBoundary widget used to observe
+// scene invalidation through syncDeviceScale → Window.InvalidateAllScenes.
+type scaleSyncBoundary struct {
+	widget.WidgetBase
+}
+
+func newScaleSyncBoundary() *scaleSyncBoundary {
+	w := &scaleSyncBoundary{}
+	w.SetVisible(true)
+	w.SetEnabled(true)
+	w.SetRepaintBoundary(true)
+	return w
+}
+
+func (w *scaleSyncBoundary) Layout(_ widget.Context, cs geometry.Constraints) geometry.Size {
+	return cs.Constrain(geometry.Sz(50, 50))
+}
+func (w *scaleSyncBoundary) Draw(_ widget.Context, _ widget.Canvas)     {}
+func (w *scaleSyncBoundary) Event(_ widget.Context, _ event.Event) bool { return false }
+
+// newScaleSyncRenderLoop builds a renderLoop with a real (headless) ggcanvas
+// at the given scale, a headless ui App with a boundary root, a non-nil
+// layerTree, and boundary texture entries whose release closures count into
+// *released.
+func newScaleSyncRenderLoop(t *testing.T, scale float64, released *int) (*renderLoop, *scaleSyncBoundary) {
+	t.Helper()
+	canvas, err := ggcanvas.NewWithScale(fakeDeviceProvider{}, 800, 600, scale)
+	if err != nil {
+		t.Fatalf("ggcanvas.NewWithScale: %v", err)
+	}
+	t.Cleanup(func() { _ = canvas.Close() })
+
+	uiApp := app.New()
+	root := newScaleSyncBoundary()
+	uiApp.Window().SetRoot(root)
+	root.ClearSceneDirty()
+
+	rl := &renderLoop{
+		uiApp:            uiApp,
+		canvas:           canvas,
+		layerTree:        compositor.NewOffsetLayer(geometry.Point{}),
+		boundaryTextures: make(map[uint64]*boundaryTexEntry),
+	}
+	for key := uint64(1); key <= 3; key++ {
+		dummyPtr := unsafe.Pointer(&struct{}{})
+		rl.boundaryTextures[key] = &boundaryTexEntry{
+			texture: gpucontext.NewTextureView(dummyPtr),
+			release: func() { *released++ },
+			width:   48,
+			height:  48,
+		}
+	}
+	return rl, root
+}
+
+func TestSyncDeviceScale_Flip(t *testing.T) {
+	released := 0
+	rl, root := newScaleSyncRenderLoop(t, 2.0, &released)
+
+	if !rl.syncDeviceScale(1.0) {
+		t.Fatal("syncDeviceScale(1.0) on a 2.0 canvas should report a change")
+	}
+	if got := rl.canvas.DeviceScale(); got != 1.0 {
+		t.Errorf("canvas.DeviceScale() = %v, want 1.0", got)
+	}
+	if released != 3 {
+		t.Errorf("released %d boundary textures, want 3 (all stale-scale textures must be freed)", released)
+	}
+	if rl.boundaryTextures != nil {
+		t.Error("boundaryTextures should be nil after a scale flip (rebuilt next tree walk)")
+	}
+	if rl.layerTree != nil {
+		t.Error("layerTree should be nil after a scale flip (rebuilt via UpdateLayerTree first-frame path)")
+	}
+	if !rl.fullRedrawNeeded {
+		t.Error("fullRedrawNeeded should be set after a scale flip")
+	}
+	if !root.IsSceneDirty() {
+		t.Error("root boundary scene should be invalidated on a scale flip — " +
+			"texture release alone leaves cached scenes (SVG rasters) at the old scale")
+	}
+}
+
+func TestSyncDeviceScale_NoOps(t *testing.T) {
+	tests := []struct {
+		name  string
+		scale float64
+	}{
+		{"same scale", 2.0},
+		{"zero scale", 0},
+		{"negative scale", -1},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			released := 0
+			rl, root := newScaleSyncRenderLoop(t, 2.0, &released)
+
+			if rl.syncDeviceScale(tt.scale) {
+				t.Fatalf("syncDeviceScale(%v) should report no change", tt.scale)
+			}
+			if got := rl.canvas.DeviceScale(); got != 2.0 {
+				t.Errorf("canvas.DeviceScale() = %v, want 2.0 (unchanged)", got)
+			}
+			if released != 0 {
+				t.Errorf("released %d textures, want 0 on the no-op path", released)
+			}
+			if rl.boundaryTextures == nil || rl.layerTree == nil {
+				t.Error("boundaryTextures/layerTree must be untouched on the no-op path")
+			}
+			if rl.fullRedrawNeeded {
+				t.Error("fullRedrawNeeded must stay false on the no-op path")
+			}
+			if root.IsSceneDirty() {
+				t.Error("scenes must stay clean on the no-op path")
+			}
+		})
+	}
+}

Environment

  • github.com/gogpu/ui v0.1.44 (current main)
  • Go 1.26.3, CGO_ENABLED=0
  • Deps as pinned by go.mod: gg v0.50.4, gogpu v0.44.1, gpucontext v0.21.0, wgpu v0.30.10
  • Affected platforms: macOS (repro), Windows, Wayland; X11 unaffected by the fix (scale cached at init upstream in gogpu, runtime DPI changes not observable there)

Part of the fix registry: #170

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions