Guard the global appearance behind one lock

appearance.go

@@ -0,0 +1,32 @@
+// Copyright (c) 2026, the go-widgets authors. All rights reserved.
+// Use of this source code is governed by a BSD-3-Clause license that can be
+// found in the LICENSE file.
+
+package toolkit
+
+import "sync"
+
+// appearanceMu guards the toolkit's global appearance: the metric scale, the
+// touch density, the active font and the OpenType size a host last asked for.
+//
+// A host SETS these; every widget READS them, on every metric it lays out. One
+// goroutine calling SetMetricScale while another builds a widget is therefore a
+// plain data race — found by -race from an application whose tests set the
+// scale in one test while another was constructing a list. In the wild it is
+// not a test artefact: moving a window between displays is exactly this, the
+// scale set from whatever thread the display-change event arrives on while the
+// UI is drawing at the old one. What it costs is not a crash but silence —
+// chrome laid out at one scale around type measured at another.
+//
+// It is deliberately ONE lock for the four of them. They are one fact, "how big
+// is everything"; SetMetricScale re-renders the font, so the scale and the font
+// change together; and two locks taken in two orders is how a toolkit deadlocks
+// on a display change.
+//
+// THE INVARIANT, for anyone editing these files: no call holds this lock across
+// another call that takes it. SetMetricScale drops it before rescaleText, which
+// installs a face through SetFont, which takes it again. sync.RWMutex is not
+// reentrant, and RLock is not safely reentrant either — a writer arriving
+// between two nested RLocks deadlocks both. Read what you need, unlock, then
+// compute.
+var appearanceMu sync.RWMutex

appearance_test.go

@@ -0,0 +1,79 @@
+// Copyright (c) 2026, the go-widgets authors. All rights reserved.
+// Use of this source code is governed by a BSD-3-Clause license that can be
+// found in the LICENSE file.
+
+package toolkit
+
+import (
+	"sync"
+	"testing"
+)
+
+// TestAppearanceSurvivesConcurrentUse covers the toolkit's global appearance
+// being set on one goroutine while widgets read it on another.
+//
+// That is not a contrived pairing: it is a window moving between displays. The
+// scale is set from whatever thread the display-change event arrives on, and
+// the UI is laying out at the old one. Unguarded it is a data race over four
+// globals at once — the scale, the density, the active font and the OpenType
+// size — and what a race costs here is not a crash but silence: chrome laid out
+// at one scale around type measured at another.
+//
+// Run with -race, this is the whole test. Without it, it still exercises that
+// no ordering deadlocks: SetMetricScale re-renders the font through SetFont,
+// which takes the same lock.
+func TestAppearanceSurvivesConcurrentUse(t *testing.T) {
+	scale, dens, font := MetricScale(), Density(), CurrentFont()
+	t.Cleanup(func() {
+		SetFont(nil)
+		SetMetricScale(scale)
+		SetDensity(dens)
+		if font != nil {
+			SetFont(font)
+		}
+	})
+
+	const rounds = 200
+	var wg sync.WaitGroup
+	writers := []func(int){
+		func(i int) { SetMetricScale(1 + float64(i%3)) },
+		func(i int) { SetDensity(DensityLevel(i % 3)) },
+		func(int) { SetFont(nil) },
+	}
+	readers := []func(){
+		func() { _ = scaled(10) },
+		func() { _ = dpiScaled(10) },
+		func() { _ = MetricScale() },
+		func() { _ = Density() },
+		func() { _ = CurrentFont() },
+		func() { _ = GlyphHeight() },
+	}
+	for _, w := range writers {
+		wg.Add(1)
+		go func(w func(int)) {
+			defer wg.Done()
+			for i := range rounds {
+				w(i)
+			}
+		}(w)
+	}
+	for _, r := range readers {
+		wg.Add(1)
+		go func(r func()) {
+			defer wg.Done()
+			for range rounds {
+				r()
+			}
+		}(r)
+	}
+	wg.Wait()
+
+	// The state is still coherent: whatever the last writer left, a reader
+	// gets a usable answer rather than a torn one.
+	if got := scaled(10); got <= 0 {
+		t.Errorf("scaled(10) = %d after concurrent use, want a positive metric", got)
+	}
+	if CurrentFont() == nil {
+		t.Error("there is no active font after concurrent use")
+	}
+}

defaulttext.go

@@ -89,10 +89,13 @@ var openTypeLogicalPx int
 // its text because a resize could not re-render it would be worse than one
 // whose text is briefly the wrong size.
 func rescaleText() {
-	if openTypeLogicalPx <= 0 {
+	appearanceMu.RLock()
+	logical, scale := openTypeLogicalPx, metricScale
+	appearanceMu.RUnlock()
+	if logical <= 0 {
 		return
 	}
-	px := int(float64(openTypeLogicalPx)*metricScale + 0.5)
+	px := int(float64(logical)*scale + 0.5)
 	if px < 1 {
 		px = 1
 	}
@@ -110,15 +113,23 @@ func UseOpenTypeTextSize(sizePx int) error {
 	// rendered at sizePx x MetricScale, and re-rendered whenever that scale
 	// changes. A caller asking for 16 gets type that reads the same size on
 	// every display, which is the whole point of a scale.
-	px := int(float64(sizePx)*metricScale + 0.5)
+	appearanceMu.RLock()
+	scale := metricScale
+	appearanceMu.RUnlock()
+	px := int(float64(sizePx)*scale + 0.5)
 	if px < 1 {
 		px = 1
 	}
 	f, err := DefaultOpenTypeFont(px)
 	if err != nil {
 		return err
 	}
+	// SetFont takes the lock, so it is called without it held; the size is
+	// recorded after, exactly as before, because SetFont(nil) is the one that
+	// clears it and f is not nil here.
 	SetFont(f)
+	appearanceMu.Lock()
 	openTypeLogicalPx = sizePx
+	appearanceMu.Unlock()
 	return nil
 }

density.go

@@ -44,14 +44,21 @@ var density = DensityCompact
 // the density never lands on an undefined level (mirroring how [SetMetricScale]
 // ignores a non-positive scale).
 func SetDensity(d DensityLevel) {
-	if d >= DensityCompact && d <= DensityTouch {
-		density = d
+	if d < DensityCompact || d > DensityTouch {
+		return
 	}
+	appearanceMu.Lock()
+	density = d
+	appearanceMu.Unlock()
 }
 
 // Density returns the current global touch profile ([DensityCompact] by
 // default).
-func Density() DensityLevel { return density }
+func Density() DensityLevel {
+	appearanceMu.RLock()
+	defer appearanceMu.RUnlock()
+	return density
+}
 
 // densityFactor is the spacing multiplier a level applies to every base metric
 // on top of [MetricScale]. Compact is exactly 1.0 (byte-identical default);

font.go

@@ -82,6 +82,8 @@ var activeFont Font
 // whatever the current [MetricScale] is. All subsequent layout (GlyphHeight /
 // GlyphAdvance) and DrawText use it.
 func SetFont(f Font) {
+	appearanceMu.Lock()
+	defer appearanceMu.Unlock()
 	activeFont = f
 	if f == nil {
 		// Back to the bitmap means back to the bitmap: the OpenType size a
@@ -100,22 +102,30 @@ func SetFont(f Font) {
 // font chose its size too, so that one is left alone: the same rule Menu and
 // Browser follow for their own Scale fields.
 func CurrentFont() Font {
-	if activeFont != nil {
-		return activeFont
+	appearanceMu.RLock()
+	f, scale := activeFont, metricScale
+	appearanceMu.RUnlock()
+	if f != nil {
+		return f
 	}
-	return scaledDefaultFont()
+	return scaledDefaultFont(scale)
 }
 
 // scaledDefaultFont is the built-in bitmap at the current metric scale, cached
 // so the common path allocates nothing.
-func scaledDefaultFont() Font {
-	n := int(MetricScale() + 0.5)
+func scaledDefaultFont(scale float64) Font {
+	n := int(scale + 0.5)
 	if n < 1 {
 		n = 1
 	}
 	if n == 1 {
 		return defaultFont
 	}
+	// The scale is passed in rather than read again: the caller already holds
+	// it, and taking the read lock a second time inside one call is the nested
+	// RLock the invariant in appearance.go forbids.
+	appearanceMu.Lock()
+	defer appearanceMu.Unlock()
 	if cachedScaledFont == nil || cachedScaledFont.Scale != n {
 		cachedScaledFont = &bitmapFont{Scale: n}
 	}

metrics.go

@@ -27,10 +27,15 @@ func SetMetricScale(f float64) {
 	if f <= 0 {
 		return
 	}
-	if f == metricScale {
+	appearanceMu.Lock()
+	changed := f != metricScale
+	if changed {
+		metricScale = f
+	}
+	appearanceMu.Unlock()
+	if !changed {
 		return
 	}
-	metricScale = f
 	// The text follows. Every other metric here is multiplied by the scale and
 	// the one thing a person actually reads was not, so an application on a
 	// HiDPI display got labels at half the size it asked for and had to
@@ -40,7 +45,11 @@ func SetMetricScale(f float64) {
 }
 
 // MetricScale returns the current global metric scale (1.0 by default).
-func MetricScale() float64 { return metricScale }
+func MetricScale() float64 {
+	appearanceMu.RLock()
+	defer appearanceMu.RUnlock()
+	return metricScale
+}
 
 // scaled rounds a base (logical-pixel) metric to device pixels at the current
 // HiDPI scale AND touch density. It is the single seam every widget's pixel
@@ -53,13 +62,23 @@ func MetricScale() float64 { return metricScale }
 // Under the default [DensityCompact] the factor is exactly 1.0, so this reduces
 // to the pure HiDPI form and every metric is byte-identical to a density-less
 // toolkit.
-func scaled(v int) int { return int(float64(v)*metricScale*densityFactor(density) + 0.5) }
+func scaled(v int) int {
+	appearanceMu.RLock()
+	s, d := metricScale, density
+	appearanceMu.RUnlock()
+	return int(float64(v)*s*densityFactor(d) + 0.5)
+}
 
 // dpiScaled rounds a base (logical-pixel) length to device pixels at the current
 // HiDPI [MetricScale] ONLY — it does NOT apply the touch [Density] factor. It
 // backs [MinHitTarget], whose floor is an absolute reachability guarantee in
 // logical pixels that must grow with panel DPI but not with spacing density.
-func dpiScaled(v int) int { return int(float64(v)*metricScale + 0.5) }
+func dpiScaled(v int) int {
+	appearanceMu.RLock()
+	s := metricScale
+	appearanceMu.RUnlock()
+	return int(float64(v)*s + 0.5)
+}
 
 // Scaled is the exported form of [scaled]: it rounds a base (logical-pixel)
 // metric to device pixels at the current [MetricScale]. Sibling packages that