diff --git a/builtin/runtime/zoom.lua b/builtin/runtime/zoom.lua index 55f5990..a211520 100644 --- a/builtin/runtime/zoom.lua +++ b/builtin/runtime/zoom.lua @@ -29,7 +29,7 @@ local STATE_KEY = "gpu-zoom" pmacs.config.define { name = "ui.gpu-font-size-base", - description = "Logical-pixel size the first zoom step starts from when no font size is set.", + description = "Logical-pixel size the first zoom step starts from when no font size is set (quantized to hundredths).", type = "number", default = 16.0, min = MIN_PX, @@ -46,7 +46,7 @@ pmacs.config.define { -- only ever clamp. pmacs.config.define { name = "ui.gpu-zoom-step", - description = "Logical pixels added or removed per zoom step.", + description = "Logical pixels added or removed per zoom step (quantized to hundredths).", type = "number", default = 1.0, min = 0.01, @@ -61,6 +61,36 @@ local function quantize(px) return math.floor(px * 100 + 0.5) / 100 end +-- The configured step and base, QUANTIZED. +-- +-- The registry cannot enforce this: `ConfigKind::Number` validates +-- finiteness and bounds and nothing else (src/config_registry.rs), and +-- `on_change` listeners are notified after the fact --- they cannot +-- veto. A wrapper function would not help either, since a direct +-- `pmacs.config.set` bypasses it (the same seam `autosave` documents). +-- +-- So quantize where the value is USED. A step of 0.015 is not a +-- meaningful step in this domain: sizes live in integer hundredths of a +-- logical pixel end to end, and `validate_font_size` already +-- range-checks the original and then rounds to the nearest hundredth. +-- Rounding the step is the same operation applied one level up, not a +-- workaround for one. +-- +-- It also RESTORES the round-trip contract, which a raw step breaks: +-- with 0.015 the sequence is 16.00 -> 16.02 -> 16.01, because each +-- operation rounds independently and 16.015 and 16.005 round in +-- opposite directions. Quantizing first makes every step exact +-- addition in the quantized domain, so n in and n out returns to the +-- starting value for ANY accepted step, not only for the ones that +-- happened to be representable. +local function effective_step() + return quantize(pmacs.config.get("ui.gpu-zoom-step")) +end + +local function effective_base() + return quantize(pmacs.config.get("ui.gpu-font-size-base")) +end + -- The current size in logical px, or nil when the preference is unset. -- nil is a REAL state (the frontend's own default), never inferred from -- silence --- Q#TH7. @@ -92,7 +122,7 @@ end -- Step by `delta` logical px. Returns the new size, or nil plus a -- reason. local function step(delta) - local base = current_px() or pmacs.config.get("ui.gpu-font-size-base") + local base = current_px() or effective_base() local want = quantize(base + delta) if want < MIN_PX or want > MAX_PX then -- Reject the WHOLE step rather than pinning to the boundary. This @@ -111,11 +141,11 @@ end -- Named `increase`/`decrease` rather than `in`/`out`: `in` is a Lua -- keyword, and `in_` reads like a workaround for one. function pmacs.zoom.increase() - return step(pmacs.config.get("ui.gpu-zoom-step")) + return step(effective_step()) end function pmacs.zoom.decrease() - return step(-pmacs.config.get("ui.gpu-zoom-step")) + return step(-effective_step()) end -- Reset returns the preference to NIL --- the frontend's own default --- diff --git a/docs/gui-zoom-framing.md b/docs/gui-zoom-framing.md index cf0ce43..b7bb5eb 100644 --- a/docs/gui-zoom-framing.md +++ b/docs/gui-zoom-framing.md @@ -1,6 +1,9 @@ # GUI zoom — QoL Stage 2 -**Status: revision 4 — proposed, awaiting approval.** Q#Z1 approved as +**Status: revision 5 — APPROVED, IMPLEMENTED, in review as PR #220.** +Revision 5 closes a gap review found in the shipped code: the +round-trip guarantee in §3.1 needed the step to be **quantized where it +is used**, because the registry cannot enforce precision (§3.2). Q#Z1 approved as **(c)** (configured base, `None` preserved) and Q#Z2 as **additive**. Revision 4 fixes a self-contradiction in §5a1: the specified parser `^(%d+)$` rejects the newline-terminated format specified beside it. @@ -122,7 +125,42 @@ command does something coherent, just the opposite of its name. than the whole domain can only ever clamp or be rejected, so permitting it buys a setting that cannot be used. -**These bounds are what make the round-trip claim true.** "n steps in, +### 3.2 The bounds are not sufficient on their own + +Review found the gap in the implementation: `ConfigKind::Number` +validates **finiteness and bounds and nothing else** +(`src/config_registry.rs`), and `on_change` listeners are notified +*after* a value is stored — they cannot veto. So `0.015` is a +perfectly settable step, and nothing in the registry can refuse it. + +Used raw it breaks the guarantee below, because each operation rounds +independently and `16.015` and `16.005` round in **opposite +directions**: + +``` +step 0.015: 16.00 -> 16.02 -> 16.01 round trip broken +step 0.37 : 16.00 -> 16.37 -> 16.00 round trip holds +``` + +The original test used `0.37` — centi-pixel representable — so it could +not reach this. + +**Resolution: quantize the step and the base where they are used.** Not +a workaround: sizes live in integer hundredths end to end, and +`validate_font_size` already range-checks the original and then rounds +to the nearest hundredth. Rounding the step is that same operation one +level up. A step of `0.015` is not a finer step in this domain, it is +`0.02` written imprecisely. + +Enforcing at `set` time was considered and rejected: the registry +cannot express it, and a validating wrapper is bypassed by a direct +`pmacs.config.set` — the same seam `autosave` documents about its own +`interval_ms` wrapper. Quantizing at the point of use cannot be +bypassed. Both settings say "quantized to hundredths" in their +`description`, so `describe-setting` shows it. + +**These bounds and that quantization are what make the round-trip claim +true.** "n steps in, then n steps out, returns to exactly the starting value" holds because the step is centi-pixel representable and addition is exact in that domain — *provided no clamp occurred*, which is why §6 requires an @@ -380,6 +418,10 @@ Revision 1 left all three unspecified. As recommended in review: - **The round trip holds only where no clamp occurred**, so the out-of-range case asserts the preference is left **unmutated** rather than pinned to the boundary. +- **The round trip holds for a step that is NOT representable** — + `0.015`, which the registry accepts and which the original `0.37` + case could not reach. Bitten: with the raw value it lands on `16.01` + instead of `16.00` (§3.2). - **Zoom from the untouched state uses the configured base**, not a hardcoded 16.0 — bitten by changing the base and asserting the first step follows it. diff --git a/tests/gui_zoom_acceptance.rs b/tests/gui_zoom_acceptance.rs index 2b36acb..0d7a50d 100644 --- a/tests/gui_zoom_acceptance.rs +++ b/tests/gui_zoom_acceptance.rs @@ -122,6 +122,54 @@ fn n_steps_in_then_n_out_returns_exactly() { ); } +/// The round trip holds for a step that is NOT centi-pixel +/// representable, which is the case the 0.37 test above cannot reach. +/// +/// The registry accepts any finite number in range — `ConfigKind::Number` +/// validates finiteness and bounds and nothing else, and `on_change` +/// cannot veto — so 0.015 is a settable step. Used raw it breaks the +/// contract: each operation rounds independently, 16.015 rounds up and +/// 16.005 rounds down, giving 16.00 -> 16.02 -> 16.01. +/// +/// Quantizing the step at the point of use restores exactness for every +/// accepted step, not just the representable ones. +#[test] +fn the_round_trip_survives_a_step_that_is_not_representable() { + let (roots, _) = roots_for("round_trip_unrepresentable"); + let s = session(&roots); + exec(&s, "pmacs.gpu.set_font { size = 16.0 }"); + exec(&s, r#"pmacs.config.set("ui.gpu-zoom-step", 0.015)"#); + + exec(&s, "pmacs.zoom.increase()"); + assert_eq!( + size(&s), + Some(16.02), + "the effective step is the quantized one — 0.015 rounds to 0.02, the same operation `validate_font_size` already applies to sizes" + ); + + exec(&s, "pmacs.zoom.decrease()"); + assert_eq!( + size(&s), + Some(16.0), + "and back exactly. Used raw, 0.015 would land on 16.01 here, because each operation rounds independently" + ); +} + +/// A base that is not representable still yields a predictable origin, +/// and every step after the first is exact. +#[test] +fn an_unrepresentable_base_is_quantized_too() { + let (roots, _) = roots_for("base_quantized"); + let s = session(&roots); + exec(&s, r#"pmacs.config.set("ui.gpu-font-size-base", 20.004)"#); + exec(&s, "pmacs.zoom.increase()"); + assert_eq!( + size(&s), + Some(21.0), + "20.004 quantizes to 20.00, then + 1.0" + ); +} + /// An out-of-range step leaves the preference **unmutated** rather than /// pinning it to the boundary. Pinning would silently break the round /// trip precisely at the edges, where a user steps back and forth most.