fix(zoom): quantize the step, or the round-trip guarantee is false
THE BOUNDS WERE NOT SUFFICIENT AND THE TEST COULD NOT SEE IT. `ConfigKind::Number` validates finiteness and bounds and nothing else, 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 framed guarantee, because each operation rounds independently and 16.015 and 16.005 round in OPPOSITE directions: step 0.015: 16.00 -> 16.02 -> 16.01 broken step 0.37 : 16.00 -> 16.37 -> 16.00 holds The existing round-trip test used 0.37 — centi-pixel representable — so it passed against the defect. Bitten now: with the raw value the new case lands on 16.01, while the 0.37 case still passes, which is exactly why it needed to be its own witness. QUANTIZED WHERE USED, not at `set`. 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 a precision constraint, and a validating wrapper is bypassed by a direct `pmacs.config.set` — the seam `autosave` already documents about its own interval_ms wrapper. Quantizing at the point of use cannot be bypassed. Both descriptions say "quantized to hundredths", so `describe-setting` shows it. The framing header also still said "proposed, awaiting approval" while the lane and this PR recorded it approved and implemented. Revision 5, with §3.2 recording the gap and why quantization rather than rejection closes it. Verified: fmt, clippy, diff-check, --lib 1900/0, gui_zoom 15/15. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
8fd8f585aa
commit
828f57debb
|
|
@ -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 ---
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue