diff --git a/docs/LAYOUT_REMEDIATION_SPEC.md b/docs/LAYOUT_REMEDIATION_SPEC.md index a75e0e7..cc347e8 100644 --- a/docs/LAYOUT_REMEDIATION_SPEC.md +++ b/docs/LAYOUT_REMEDIATION_SPEC.md @@ -28,6 +28,7 @@ It is independent of every other spec here. | [S11](#s11--partial-block-progress-is-discarded) | Partial-block progress is discarded | 0 | | [S12](#s12--background-rendering) | Background rendering | 4 | | [S13](#s13--word-spacing-and-alignment) | Word spacing and alignment | 0 | +| [S14](#s14--vertical-centring-in-buttons-and-fields) | Vertical centring in buttons and fields | 0 | ## Design invariants @@ -1158,6 +1159,58 @@ Three defects, all visible as a right edge that wobbles from line to line. --- +## S14 — Vertical centring in buttons and fields + +### Problem + +`ButtonText.render` and `FormFieldText.render` both placed the text baseline at +`box_top + box_height / 2 + descent / 2`. Centring glyphs whose visual height is +`ascent + descent` inside a box of height `H` puts the baseline at +`box_top + H/2 + (ascent - descent)/2`. The two agree only when +`ascent == 2 * descent`; DejaVu is nearer 4:1, so labels rode high against the +top edge of the control. + +`ButtonText` also sized itself as `font_size + padding`, but the text's visual +height exceeds the nominal size — DejaVu at 14px measures 17 — so the button was +too short to centre its own label in. + +### Evidence + +A 14px "Save Document" button with 6px vertical padding, measuring the label's +ink against the button rectangle: + +``` +gap above text: 5px +gap below text: 11px +``` + +### Design + +- `baseline = area_top + (area_height - (ascent + descent)) / 2 + ascent` in both + renderers. +- `ButtonText._padded_height` derives from `ascent + descent`, guarded so a mock + or unusual font object falls back to the nominal size. + +### Acceptance criteria + +- Label ink is centred within ±2px at font sizes 10, 14 and 20. +- Label ink stays inside the button rectangle. +- Button height is at least `ascent + descent + vertical padding`. +- A form field's value is centred within its input box (±3px). + +### Files + +`pyWebLayout/concrete/functional.py` + +### Note + +`docs/images/example_07_pressed_state.png` was stale — no example regenerates it; +`07_pressed_state_demo.py` writes `demo_07_pressed.png` at the repository root +and the docs copy had been placed by hand. It has been refreshed. Worth wiring +the demo to write straight to `docs/images/` so it cannot drift again. + +--- + ## Test plan Findings were reproduced with four probe scripts; each becomes a regression test diff --git a/docs/images/example_07_button_animation.gif b/docs/images/example_07_button_animation.gif index b0f87a3..15df782 100644 Binary files a/docs/images/example_07_button_animation.gif and b/docs/images/example_07_button_animation.gif differ diff --git a/docs/images/example_07_pressed_state.png b/docs/images/example_07_pressed_state.png index 1539ad1..49f83ce 100644 Binary files a/docs/images/example_07_pressed_state.png and b/docs/images/example_07_pressed_state.png differ diff --git a/docs/images/example_14_interactive_table.png b/docs/images/example_14_interactive_table.png index 573582d..997fc39 100644 Binary files a/docs/images/example_14_interactive_table.png and b/docs/images/example_14_interactive_table.png differ diff --git a/pyWebLayout/concrete/functional.py b/pyWebLayout/concrete/functional.py index 557b139..3e168b9 100644 --- a/pyWebLayout/concrete/functional.py +++ b/pyWebLayout/concrete/functional.py @@ -153,7 +153,22 @@ class ButtonText(Text, Interactable, Queriable): self, '_width', 0) if not hasattr( self._width, '__call__') else 0 self._padded_width = text_width + padding[1] + padding[3] - self._padded_height = self._style.font_size + padding[0] + padding[2] + + # Size the button from the text's visual height (ascent + descent), not + # from the nominal font size. The two differ by several pixels - DejaVu at + # 14px measures 17 - so sizing by font_size leaves the button too short to + # centre its own label in. + self._text_height = self._visual_text_height() + self._padded_height = self._text_height + padding[0] + padding[2] + + def _visual_text_height(self) -> int: + """Height of the rendered text, ascender to descender.""" + try: + ascent, descent = self._style.font.getmetrics() + return int(ascent + descent) + except (AttributeError, TypeError, ValueError): + # Mock or unusual font object; the nominal size is the best guess. + return int(getattr(self._style, 'font_size', 0) or 0) @property def button(self) -> Button: @@ -237,11 +252,18 @@ class ButtonText(Text, Interactable, Queriable): # Total button height minus top and bottom padding gives us text area height text_area_height = self._padded_height - self._padding[0] - self._padding[2] - # Center the text visual height (ascent + descent) within the text area - # The y position is where the baseline sits - # Visual center = area_height/2, baseline should be at center + descent/2 - vertical_center = text_area_height / 2 - text_y = self._origin[1] + self._padding[0] + vertical_center + (descent / 2) + # Centre the text's visual height (ascent + descent) within the text area. + # text_y is the baseline, since Text renders with anchor "ls". + # + # top of glyphs = area_top + (area_height - (ascent + descent)) / 2 + # baseline = top of glyphs + ascent + # + # The previous form, area_top + area_height/2 + descent/2, is only + # equivalent when ascent == 2 * descent. Real fonts sit nearer 4:1, so the + # label rendered several pixels above centre, against the top edge. + text_top = self._origin[1] + self._padding[0] \ + + (text_area_height - (ascent + descent)) / 2 + text_y = text_top + ascent # Temporarily set origin for text rendering original_origin = self._origin.copy() @@ -360,11 +382,12 @@ class FormFieldText(Text, Interactable, Queriable): # Get font metrics to properly center the baseline ascent, descent = value_font.font.getmetrics() - # Center the text vertically within the field - # The y coordinate is where the baseline sits (anchor="ls") - vertical_center = self._field_height / 2 + # Centre the value within the input box. As in ButtonText, the + # baseline sits at the top of the glyphs plus the ascent; centring on + # half the box height plus half the descent only works for a 2:1 + # ascent/descent ratio and otherwise rides high. value_x = field_x + 5 - value_y = field_y + vertical_center + (descent / 2) + value_y = field_y + (self._field_height - (ascent + descent)) / 2 + ascent # Draw the value text self._draw.text((value_x, value_y), value_text, diff --git a/tests/concrete/test_functional_text_centering.py b/tests/concrete/test_functional_text_centering.py new file mode 100644 index 0000000..86d4162 --- /dev/null +++ b/tests/concrete/test_functional_text_centering.py @@ -0,0 +1,124 @@ +""" +Regression tests for vertical centring of text in buttons and form fields. + +Both placed the baseline at `top + height/2 + descent/2`. Centring text whose +visual height is ascent+descent inside a box of height H puts the baseline at +`top + H/2 + (ascent-descent)/2`; the two agree only when ascent == 2*descent. +Real fonts have a much larger ratio - DejaVu is nearer 4:1 - so the text sat +several pixels high, hugging the top edge of the button. + +The button was also sized from the nominal font size rather than the text's +actual visual height, leaving it too short to centre anything in. +""" + +import numpy as np +import pytest +from PIL import Image, ImageDraw + +from pyWebLayout.abstract.functional import Button, FormField, FormFieldType +from pyWebLayout.concrete.functional import ButtonText, FormFieldText +from pyWebLayout.style import Font + + +CANVAS = (300, 120) +PADDING = (6, 10, 6, 10) # top, right, bottom, left + + +@pytest.fixture +def draw_ctx(): + image = Image.new("RGB", CANVAS, (255, 255, 255)) + return image, ImageDraw.Draw(image) + + +def ink_rows(image, box): + """ + Rows within box that carry text ink. + + Only the central columns are sampled: the button has rounded corners, so the + page background shows through at the extremes of every row and would read as + white text on all of them. + """ + x0, y0, x1, y1 = box + inset = (x1 - x0) // 4 + pixels = image.convert("RGB").load() + rows = [] + for y in range(y0, y1): + for x in range(x0 + inset, x1 - inset): + r, g, b = pixels[x, y] + # Button text is white on a blue fill; look for near-white ink. + if r > 240 and g > 240 and b > 240: + rows.append(y) + break + return rows + + +class TestButtonTextCentring: + + @pytest.mark.parametrize("font_size", [10, 14, 20]) + def test_text_is_vertically_centred(self, draw_ctx, font_size): + image, draw = draw_ctx + font = Font(font_size=font_size, colour=(255, 255, 255)) + button = ButtonText(Button(label="Save Document", callback=lambda p: None), + font, draw, padding=PADDING) + button.set_origin(np.array([20, 20])) + button.render() + + x0, y0 = 20, 20 + x1 = x0 + int(button.size[0]) + y1 = y0 + int(button.size[1]) + rows = ink_rows(image, (x0, y0, x1, y1)) + assert rows, "the button should have visible text" + + gap_above = min(rows) - y0 + gap_below = y1 - max(rows) - 1 + + assert abs(gap_above - gap_below) <= 2, ( + f"text not centred at size {font_size}: " + f"{gap_above}px above, {gap_below}px below") + + def test_button_is_tall_enough_for_its_text(self): + font = Font(font_size=14, colour=(255, 255, 255)) + image = Image.new("RGB", CANVAS, (255, 255, 255)) + draw = ImageDraw.Draw(image) + button = ButtonText(Button(label="Cancel", callback=lambda p: None), + font, draw, padding=PADDING) + + ascent, descent = font.font.getmetrics() + assert int(button.size[1]) >= ascent + descent + PADDING[0] + PADDING[2], \ + "button height must accommodate the text's visual height, not the nominal size" + + def test_text_stays_inside_the_button(self, draw_ctx): + image, draw = draw_ctx + font = Font(font_size=14, colour=(255, 255, 255)) + button = ButtonText(Button(label="Save Document", callback=lambda p: None), + font, draw, padding=PADDING) + button.set_origin(np.array([20, 20])) + button.render() + + y0, y1 = 20, 20 + int(button.size[1]) + rows = ink_rows(image, (20, y0, 20 + int(button.size[0]), y1)) + assert min(rows) >= y0, "text escaped above the button" + assert max(rows) < y1, "text escaped below the button" + + +class TestFormFieldValueCentring: + + def test_value_is_centred_in_the_input_box(self): + image = Image.new("RGB", (300, 120), (0, 0, 0)) + draw = ImageDraw.Draw(image) + font = Font(font_size=12, colour=(0, 0, 0)) + field = FormField(name="who", field_type=FormFieldType.TEXT, value="Hello") + renderable = FormFieldText(field, font, draw, field_height=28) + renderable.set_origin(np.array([10, 10])) + renderable.render() + + field_y = 10 + font.font_size + 5 + pixels = image.convert("RGB").load() + rows = [y for y in range(field_y, field_y + 28) + if any(pixels[x, y] == (0, 0, 0) for x in range(12, 200))] + assert rows, "the field value should be visible" + + gap_above = min(rows) - field_y + gap_below = (field_y + 28) - max(rows) - 1 + assert abs(gap_above - gap_below) <= 3, ( + f"field value not centred: {gap_above}px above, {gap_below}px below")