From 8243368f4cfdcdce61b611a7a466d4e4975122ff Mon Sep 17 00:00:00 2001 From: ShotaroKataoka Date: Wed, 5 Aug 2026 09:09:44 +0900 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20fix(aspect-ratio-agnostic-sl?= =?UTF-8?q?ide-canvas):=20=E3=82=A8=E3=83=B3=E3=82=B8=E3=83=B3=E3=81=AE?= =?UTF-8?q?=E5=BA=A7=E6=A8=99=E7=B3=BB=E3=82=92=E5=AE=9F=E3=82=B9=E3=83=A9?= =?UTF-8?q?=E3=82=A4=E3=83=89=E5=AF=B8=E6=B3=95=E3=81=AB=E8=BF=BD=E5=BE=93?= =?UTF-8?q?=E3=81=95=E3=81=9B=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 16:9 以外（4:3 等）のカスタムテンプレートでレイアウトが崩れる問題 (#208) を修正。 座標系の正は builder/converter に既にある「幅1920px固定・高さ可変」とし、 それに従っていなかったレイヤーを整合させる: - engine に slide_size_px() / emu_per_px() を新設し、builder と converter の インライン計算を置き換え（挙動不変） - analyzer: emu=6350 固定をやめ幅1920正規化に統一（4:3 で 1440x1080 と 誤報告していたものが 1920x1440 になる） - lint: 高さ OOB チェックを撤去しスライドサイズ非依存に純化。 4:3 の正しい配置を誤警告で押し戻すフィードバックループを解消。 幅チェックは定数として常に正しいので維持 - generate: テンプレート実寸ベースの高さ境界警告と slideSize 食い違い警告を追加 - deck.json の slideSize は新規生成時のみ engine が書く（api.init / pptx import）。 既存 deck.json は更新しない - measure: viewBox 幅基準の等方スケールに変更（4:3 で y が圧縮される問題） - grid overlay / imbalance / detect_layout の基準を統一 16:9 の挙動は完全に不変（全変更が16:9では同値）。 analyzer は既存テストが皆無だったため 16:9 pin テストを先に追加した。 SPEC: 20260805-0100_aspect-ratio-agnostic-slide-canvas Refs: #208 --- sdpm/sdpm/api.py | 44 +++- sdpm/sdpm/engine/__init__.py | 33 +++ sdpm/sdpm/engine/analyzer/__init__.py | 8 +- sdpm/sdpm/engine/builder/__init__.py | 3 +- sdpm/sdpm/engine/converter/constants.py | 3 +- sdpm/sdpm/engine/converter/pipeline.py | 4 + sdpm/sdpm/engine/converter/slide.py | 5 +- sdpm/sdpm/engine/preview/__init__.py | 8 +- sdpm/sdpm/engine/preview/measure.py | 9 +- sdpm/sdpm/engine/schema/lint.py | 7 +- sdpm/sdpm/tools/__init__.py | 7 +- tests/conftest.py | 32 +++ tests/test_analyzer.py | 126 +++++++++++ tests/test_canvas_derivation.py | 90 ++++++++ tests/test_canvas_slidesize.py | 266 ++++++++++++++++++++++++ tests/test_converter_elements.py | 1 + tests/test_converter_slide_layout.py | 65 ++++++ tests/test_lint.py | 16 +- tests/test_preview_imbalance.py | 77 +++++++ tests/test_preview_measure.py | 93 +++++++++ 20 files changed, 872 insertions(+), 25 deletions(-) create mode 100644 tests/test_analyzer.py create mode 100644 tests/test_canvas_derivation.py create mode 100644 tests/test_canvas_slidesize.py create mode 100644 tests/test_converter_slide_layout.py create mode 100644 tests/test_preview_imbalance.py create mode 100644 tests/test_preview_measure.py diff --git a/sdpm/sdpm/api.py b/sdpm/sdpm/api.py index cc22969b..af53b182 100644 --- a/sdpm/sdpm/api.py +++ b/sdpm/sdpm/api.py @@ -314,6 +314,16 @@ def init( deck_data["fonts"] = extract_fonts(template_src) except Exception: pass + # Write slideSize derived from template (new-deck only — R4) + try: + from pptx import Presentation as _Prs + from sdpm.engine import slide_size_px as _slide_size_px + + _prs = _Prs(str(template_src)) + w, h = _slide_size_px(int(_prs.slide_width), int(_prs.slide_height)) + deck_data["slideSize"] = {"width": w, "height": h} + except Exception: + pass deck_json = out_dir / "deck.json" write_json(deck_json, deck_data, suffix="\n") @@ -463,6 +473,36 @@ def _resolve_config( dtc = "#FFFFFF" if is_dark else "#333333" warnings.append(f"defaultTextColor auto-set to {dtc}") + # slideSize validation — compare deck.json cache with template reality + from pptx import Presentation as _Prs + from sdpm.engine import slide_size_px as _slide_size_px + + _prs = _Prs(str(template_file)) + actual_size = _slide_size_px(int(_prs.slide_width), int(_prs.slide_height)) + + deck_slide_size = data.get("slideSize") + if deck_slide_size: + cached = (deck_slide_size.get("width"), deck_slide_size.get("height")) + if cached != actual_size: + warnings.append( + f"slideSize mismatch: deck.json has {dict(deck_slide_size)}, " + f'template actual is {{"width": {actual_size[0]}, "height": {actual_size[1]}}}. ' + f"Please update deck.json slideSize." + ) + + # Height boundary warning (moved from lint — requires template reality) + actual_height = actual_size[1] + for si, slide in enumerate(data.get("slides", []), 1): + for ei, elem in enumerate(slide.get("elements", []), 1): + ey = elem.get("y") + eh = elem.get("height") + if isinstance(ey, (int, float)) and isinstance(eh, (int, float)): + if ey + eh > actual_height: + warnings.append( + f"slide {si} element {ei}: y({ey}) + height({eh}) = {ey + eh} " + f"exceeds slide height {actual_height}." + ) + # Lint from sdpm.engine.schema.lint import lint as lint_slides @@ -763,7 +803,9 @@ def _apply_grid_overlay(png_paths: list[str]) -> None: draw = ImageDraw.Draw(overlay) for pct in range(5, 100, 5): x, y = int(w * pct / 100), int(h * pct / 100) - px_x, px_y = int(1920 * pct / 100), int(1080 * pct / 100) + px_x = int(1920 * pct / 100) + # Derive px_y from image aspect ratio (no Presentation needed) + px_y = round(1920 * h / w * pct / 100) draw.line([(x, 0), (x, h)], fill=color, width=1) draw.line([(0, y), (w, y)], fill=color, width=1) if pct % 10 == 0: diff --git a/sdpm/sdpm/engine/__init__.py b/sdpm/sdpm/engine/__init__.py index 7c4cb4fd..bab23b77 100644 --- a/sdpm/sdpm/engine/__init__.py +++ b/sdpm/sdpm/engine/__init__.py @@ -13,3 +13,36 @@ - diff: deck diffing (roundtrip based) - analyzer: template analysis """ + +# ── Canvas derivation helpers ── +# Width is always 1920 px (design invariant D1). Height and EMU scale +# are derived from the template's physical slide dimensions. + +_CANVAS_WIDTH_PX = 1920 + + +def emu_per_px(slide_width_emu: int) -> float: + """Derive EMU-per-px scale from the slide width in EMU. + + The canvas is always 1920 px wide; this function returns the + EMU-per-pixel ratio for that basis. + + Examples: + 16:9 (12192000 EMU) → 6350.0 + 4:3 (9144000 EMU) → 4762.5 + """ + return slide_width_emu / _CANVAS_WIDTH_PX + + +def slide_size_px(slide_width_emu: int, slide_height_emu: int) -> tuple[int, int]: + """Derive canvas size in px from physical slide dimensions in EMU. + + Width is always 1920 (design invariant). Height is proportional + to the aspect ratio. + + Examples: + 16:9 (12192000, 6858000) → (1920, 1080) + 4:3 (9144000, 6858000) → (1920, 1440) + """ + scale = emu_per_px(slide_width_emu) + return (_CANVAS_WIDTH_PX, round(slide_height_emu / scale)) diff --git a/sdpm/sdpm/engine/analyzer/__init__.py b/sdpm/sdpm/engine/analyzer/__init__.py index 8730d356..9698a970 100644 --- a/sdpm/sdpm/engine/analyzer/__init__.py +++ b/sdpm/sdpm/engine/analyzer/__init__.py @@ -8,6 +8,7 @@ from pptx import Presentation from sdpm.config import CACHE_DIR +from sdpm.engine import emu_per_px, slide_size_px from sdpm.utils.io import write_json, read_json @@ -20,7 +21,6 @@ def analyze_template(template_path: Path): cache_color_usage() if missing. """ prs = Presentation(str(template_path)) - emu = 6350 # Build layout→notes mapping from slides layout_notes = {} @@ -45,8 +45,8 @@ def analyze_template(template_path: Path): fonts = extract_fonts(template_path) color_usage = _load_color_usage_cache(template_path) slide_size = { - "width": int(prs.slide_width / emu), - "height": int(prs.slide_height / emu), + "width": slide_size_px(prs.slide_width, prs.slide_height)[0], + "height": slide_size_px(prs.slide_width, prs.slide_height)[1], } return { @@ -61,7 +61,7 @@ def analyze_template(template_path: Path): def get_layout_placeholders(template_path: Path, layout_name: str): """Get placeholder details and notes for a layout directly from pptx.""" prs = Presentation(str(template_path)) - emu = 6350 + emu = emu_per_px(prs.slide_width) # Find layout layout_obj = None diff --git a/sdpm/sdpm/engine/builder/__init__.py b/sdpm/sdpm/engine/builder/__init__.py index d15bb6cd..abea75c5 100644 --- a/sdpm/sdpm/engine/builder/__init__.py +++ b/sdpm/sdpm/engine/builder/__init__.py @@ -103,7 +103,8 @@ def __init__(self, template_path: Path, custom_template: bool = False, self.theme_colors, self.is_dark = self._extract_theme_colors(template_path) self.theme_colors["text"] = default_text_color self.master_idx = 0 - self.EMU_PER_PX = int(self.prs.slide_width) / 1920 + from sdpm.engine import emu_per_px as _emu_per_px + self.EMU_PER_PX = _emu_per_px(int(self.prs.slide_width)) self.custom_template = custom_template self.fonts = fonts self.auto_spacing = auto_spacing # CJK↔Latin spacing; False = keep source text verbatim diff --git a/sdpm/sdpm/engine/converter/constants.py b/sdpm/sdpm/engine/converter/constants.py index 103da021..376263ed 100644 --- a/sdpm/sdpm/engine/converter/constants.py +++ b/sdpm/sdpm/engine/converter/constants.py @@ -40,7 +40,8 @@ def conversion_scale(slide_width_emu): exception, or nested use — so error paths and reentrant conversions cannot poison later conversions in the same process. """ - token = _CURRENT_EMU_PER_PX.set(slide_width_emu / 1920) + from sdpm.engine import emu_per_px as _emu_per_px + token = _CURRENT_EMU_PER_PX.set(_emu_per_px(slide_width_emu)) try: yield finally: diff --git a/sdpm/sdpm/engine/converter/pipeline.py b/sdpm/sdpm/engine/converter/pipeline.py index dfc724fc..28260dcf 100644 --- a/sdpm/sdpm/engine/converter/pipeline.py +++ b/sdpm/sdpm/engine/converter/pipeline.py @@ -106,6 +106,10 @@ def pptx_to_json(pptx_path: Path, output_dir: Path = None, use_layout_names: boo deck_meta["defaultTextColor"] = result["defaultTextColor"] # Imported text must roundtrip verbatim — disable CJK↔Latin auto-spacing deck_meta["autoSpacing"] = False + # Write slideSize from the source pptx (new-deck only — R4) + from sdpm.engine import slide_size_px as _slide_size_px + _w, _h = _slide_size_px(int(prs.slide_width), int(prs.slide_height)) + deck_meta["slideSize"] = {"width": _w, "height": _h} write_json(output_dir / "deck.json", deck_meta) # Write slides/slide-{NNN}.json (1-based, zero-padded, hyphen separator to match parse_outline_slugs). diff --git a/sdpm/sdpm/engine/converter/slide.py b/sdpm/sdpm/engine/converter/slide.py index 438ce439..4bcb9d95 100644 --- a/sdpm/sdpm/engine/converter/slide.py +++ b/sdpm/sdpm/engine/converter/slide.py @@ -62,8 +62,9 @@ def detect_layout(slide): pass # Check for left accent line (content layout marker) for shape in slide.shapes: - if not shape.is_placeholder and shape.left < 50 * 6350: - if shape.width < 20 * 6350 and shape.height > 500 * 6350: + emu_px = get_emu_per_px() + if not shape.is_placeholder and shape.left < 50 * emu_px: + if shape.width < 20 * emu_px and shape.height > 500 * emu_px: return "content" return "title_only" diff --git a/sdpm/sdpm/engine/preview/__init__.py b/sdpm/sdpm/engine/preview/__init__.py index 96001282..fdc7b135 100644 --- a/sdpm/sdpm/engine/preview/__init__.py +++ b/sdpm/sdpm/engine/preview/__init__.py @@ -7,6 +7,8 @@ from pptx import Presentation +from sdpm.engine import emu_per_px as _emu_per_px + from .backend import detect_backend, get_work_dir # noqa: F401 @@ -34,9 +36,9 @@ def check_layout_imbalance_data(pptx_path, slide_defs=None): """Detect slides where bbox centroid deviates from content area center.""" _THRESHOLD = 0.03 prs = Presentation(str(pptx_path)) - _SW = int(prs.slide_width / 6350) - _SH = int(prs.slide_height / 6350) - emu = 6350 + emu = _emu_per_px(prs.slide_width) + _SW = int(prs.slide_width / emu) + _SH = int(prs.slide_height / emu) _TITLE_BOTTOM = int(_SH * 0.13) _CONTENT_BOTTOM = int(_SH * 0.88) _CY = (_TITLE_BOTTOM + _CONTENT_BOTTOM) / 2 diff --git a/sdpm/sdpm/engine/preview/measure.py b/sdpm/sdpm/engine/preview/measure.py index 4e421d07..c8f72f23 100644 --- a/sdpm/sdpm/engine/preview/measure.py +++ b/sdpm/sdpm/engine/preview/measure.py @@ -45,12 +45,13 @@ def measure_from_svg( # Read viewBox for coordinate conversion vb = root.get("viewBox", "").split() if len(vb) == 4: - vb_w, vb_h = float(vb[2]), float(vb[3]) + vb_w = float(vb[2]) else: - vb_w, vb_h = 25400.0, 19050.0 + vb_w = 25400.0 - scale_x = 1920.0 / vb_w - scale_y = 1080.0 / vb_h + # Equal-aspect scale: SVG viewBox preserves the template's aspect ratio, + # so a single width-based scale is correct for all aspect ratios. + scale_x = scale_y = 1920.0 / vb_w # Find all Slide groups (skip index 0 = dummy) slides_g = root.findall(f".//{{{SVG_NS}}}g[@class='Slide']") diff --git a/sdpm/sdpm/engine/schema/lint.py b/sdpm/sdpm/engine/schema/lint.py index b6f35d97..4f8ae427 100644 --- a/sdpm/sdpm/engine/schema/lint.py +++ b/sdpm/sdpm/engine/schema/lint.py @@ -116,18 +116,15 @@ def _lint_common(si: int, ei: int, elem: dict) -> list[dict]: results.append(_diag(si, ei, "invalid-verticalAlign", f"verticalAlign '{va}' is not valid. Allowed: {sorted(_VALIGN_VALUES)}")) # out-of-bounds (bbox elements) + # Width is a D1 constant (always 1920 px). Height is template-dependent + # and validated at generate time with the real slide dimensions instead. etype = elem.get("type", "") if etype in ("shape", "textbox", "image", "chart", "table", "video", "freeform"): x = elem.get("x", 0) - y = elem.get("y", 0) w = elem.get("width", 0) - h = elem.get("height", 0) if isinstance(x, (int, float)) and isinstance(w, (int, float)) and x + w > 1920: results.append(_diag(si, ei, "out-of-bounds", f"x({x}) + width({w}) = {x+w} exceeds slide width 1920.")) - if isinstance(y, (int, float)) and isinstance(h, (int, float)) and y + h > 1080: - results.append(_diag(si, ei, "out-of-bounds", - f"y({y}) + height({h}) = {y+h} exceeds slide height 1080.")) return results diff --git a/sdpm/sdpm/tools/__init__.py b/sdpm/sdpm/tools/__init__.py index 3e184438..dcdf52a6 100644 --- a/sdpm/sdpm/tools/__init__.py +++ b/sdpm/sdpm/tools/__init__.py @@ -77,6 +77,9 @@ def init_presentation(name: str) -> dict[str, Any]: """Initialize a presentation workspace. Creates deck.json, slides/, and specs/. Call after briefing is complete, before building slides. + When a template is provided, deck.json will include slideSize + (e.g. {"width": 1920, "height": 1080} for 16:9, {"width": 1920, "height": 1440} for 4:3) + derived from the template's physical dimensions. Args: name: Presentation name (e.g. "lambda-overview"). @@ -96,7 +99,9 @@ def analyze_template(template: str, layout: str = "") -> dict[str, Any]: layout: Optional layout name for detailed placeholder info. Returns: - Dict with layouts, theme_colors, fonts, and optional layout_detail. + Dict with layouts, theme_colors, fonts, slide_size, and optional layout_detail. + slide_size is {"width": 1920, "height": H} where H depends on the template's + aspect ratio (e.g. 1080 for 16:9, 1440 for 4:3). Width is always 1920px. """ from sdpm.engine.analyzer import analyze_template as _analyze, get_layout_placeholders from sdpm.api import _find_template_in_dirs, get_templates_dirs diff --git a/tests/conftest.py b/tests/conftest.py index d858900c..b65f8442 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,39 @@ import sys from pathlib import Path +import pytest +from pptx import Presentation +from pptx.util import Emu + _root = Path(__file__).resolve().parent.parent sys.path.insert(0, str(_root)) sys.path.insert(0, str(_root / "servers" / "remote")) sys.path.insert(0, str(_root / "sdpm")) + +# Standard slide dimensions in EMU +_W_16X9 = 12192000 +_H_16X9 = 6858000 +_W_4X3 = 9144000 +_H_4X3 = 6858000 + + +@pytest.fixture() +def template_16x9() -> Path: + """Path to the bundled 16:9 blank-dark template (no copy needed).""" + return _root / "sdpm" / "templates" / "blank-dark.pptx" + + +@pytest.fixture() +def template_4x3(tmp_path: Path) -> Path: + """Generate a 4:3 template from blank-dark.pptx by resizing slide dimensions. + + The roundtrip depends on layout name "Blank" existing in the template, + so we cannot use a bare Presentation() — we must base it on blank-dark.pptx. + """ + src = _root / "sdpm" / "templates" / "blank-dark.pptx" + prs = Presentation(str(src)) + prs.slide_width = Emu(_W_4X3) + prs.slide_height = Emu(_H_4X3) + out = tmp_path / "blank-dark-4x3.pptx" + prs.save(str(out)) + return out diff --git a/tests/test_analyzer.py b/tests/test_analyzer.py new file mode 100644 index 00000000..136d3395 --- /dev/null +++ b/tests/test_analyzer.py @@ -0,0 +1,126 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for sdpm.engine.analyzer — pin existing behavior and verify aspect-ratio fix.""" + +from pathlib import Path + +from sdpm.engine.analyzer import analyze_template, get_layout_placeholders + + +# ── Phase 0: Pin 16:9 behavior (must pass BEFORE and AFTER the fix) ────────── + + +class TestAnalyzerPin16x9: + """Pin 16:9 template behavior to prove the fix does not regress.""" + + def test_slide_size_16x9(self, template_16x9: Path): + """slide_size must be 1920×1080 for 16:9.""" + result = analyze_template(template_16x9) + assert result["slide_size"] == {"width": 1920, "height": 1080} + + def test_layouts_present(self, template_16x9: Path): + """analyze_template returns at least one layout.""" + result = analyze_template(template_16x9) + assert len(result["layouts"]) > 0 + names = [layout["name"] for layout in result["layouts"]] + # blank-dark has Title Slide and Title Only (from slides) + assert "Title Slide" in names + + def test_theme_colors_present(self, template_16x9: Path): + """Theme colors dict is non-empty.""" + result = analyze_template(template_16x9) + assert "text" in result["theme_colors"] + assert "background" in result["theme_colors"] + + def test_fonts_present(self, template_16x9: Path): + """Fonts are extracted.""" + result = analyze_template(template_16x9) + assert "halfwidth" in result["fonts"] + assert "fullwidth" in result["fonts"] + + def test_placeholder_px_values_16x9(self, template_16x9: Path): + """Pin placeholder px coordinates for Title Slide layout at 16:9. + + EMU_PER_PX = 12192000 / 1920 = 6350.0 + Title placeholder (idx=0): left=682625 → int(682625/6350) = 107 + """ + ph = get_layout_placeholders(template_16x9, "Title Slide") + assert ph is not None + assert len(ph["placeholders"]) >= 2 + + title = ph["placeholders"][0] + assert title["idx"] == 0 + assert title["x"] == 107 + assert title["y"] == 335 + assert title["width"] == 1557 + assert title["height"] == 231 + + subtitle = ph["placeholders"][1] + assert subtitle["idx"] == 1 + assert subtitle["x"] == 108 + assert subtitle["y"] == 612 + assert subtitle["width"] == 1537 + assert subtitle["height"] == 122 + + def test_get_layout_placeholders_not_found(self, template_16x9: Path): + """Non-existent layout returns None.""" + result = get_layout_placeholders(template_16x9, "NonExistentLayout") + assert result is None + + def test_blank_layout_empty_placeholders(self, template_16x9: Path): + """Blank layout has no content placeholders (types 13,15,16 are filtered).""" + ph = get_layout_placeholders(template_16x9, "Blank") + assert ph is not None + assert ph["placeholders"] == [] + + +# ── Phase 1-2: Verify 4:3 uses correct D1-normalized values ───────────────── + + +class TestAnalyzer4x3: + """Verify analyzer produces correct D1-normalized values for 4:3.""" + + def test_slide_size_4x3(self, template_4x3: Path): + """4:3 slide_size must be 1920×1440 (width-1920 invariant). + + 9144000 / 1920 = 4762.5 emu_per_px + 6858000 / 4762.5 = 1440 height + """ + result = analyze_template(template_4x3) + assert result["slide_size"] == {"width": 1920, "height": 1440} + + def test_placeholder_px_values_4x3(self, template_4x3: Path): + """4:3 placeholders use emu_per_px = 4762.5 (not 6350). + + Title placeholder left=682625 EMU → int(682625/4762.5) = 143 + The coordinates are LARGER because the pixel grid is finer. + """ + ph = get_layout_placeholders(template_4x3, "Title Slide") + assert ph is not None + assert len(ph["placeholders"]) >= 2 + + title = ph["placeholders"][0] + assert title["idx"] == 0 + assert title["x"] == 143 + assert title["y"] == 447 + assert title["width"] == 2076 + assert title["height"] == 308 + + def test_slide_size_consistency_with_engine(self, template_4x3: Path): + """analyzer slide_size must match engine.slide_size_px().""" + from sdpm.engine import slide_size_px + + result = analyze_template(template_4x3) + # 4:3 EMU dimensions + expected = slide_size_px(9144000, 6858000) + assert result["slide_size"]["width"] == expected[0] + assert result["slide_size"]["height"] == expected[1] + + def test_slide_size_consistency_16x9(self, template_16x9: Path): + """analyzer slide_size must match engine.slide_size_px() for 16:9 too.""" + from sdpm.engine import slide_size_px + + result = analyze_template(template_16x9) + expected = slide_size_px(12192000, 6858000) + assert result["slide_size"]["width"] == expected[0] + assert result["slide_size"]["height"] == expected[1] diff --git a/tests/test_canvas_derivation.py b/tests/test_canvas_derivation.py new file mode 100644 index 00000000..7c133e4b --- /dev/null +++ b/tests/test_canvas_derivation.py @@ -0,0 +1,90 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for Issue #208: aspect-ratio-agnostic canvas. + +Phase 0: Analyzer 16:9 pin tests — prove current behavior is invariant. +Phase 1-1: slide_size_px() / emu_per_px() derivation functions. +""" + +from pathlib import Path + +import pytest + +from sdpm.engine import emu_per_px, slide_size_px +from sdpm.engine.analyzer import analyze_template, get_layout_placeholders + + +# ── Standard EMU dimensions ── +_W_16X9 = 12192000 +_H_16X9 = 6858000 +_W_4X3 = 9144000 +_H_4X3 = 6858000 + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 1-1: derivation function unit tests +# ═══════════════════════════════════════════════════════════════════════ + + +class TestEmuPerPx: + def test_16x9(self): + assert emu_per_px(_W_16X9) == 6350.0 + + def test_4x3(self): + assert emu_per_px(_W_4X3) == 4762.5 + + def test_arbitrary(self): + # 1920 * 5000 = 9_600_000 + assert emu_per_px(9_600_000) == 5000.0 + + +class TestSlideSizePx: + def test_16x9(self): + assert slide_size_px(_W_16X9, _H_16X9) == (1920, 1080) + + def test_4x3(self): + assert slide_size_px(_W_4X3, _H_4X3) == (1920, 1440) + + def test_width_always_1920(self): + w, _ = slide_size_px(9_600_000, 7_200_000) + assert w == 1920 + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 0: Analyzer 16:9 pin tests (immutability evidence) +# ═══════════════════════════════════════════════════════════════════════ + + +class TestAnalyzerPin16x9: + """Pin the 16:9 analyzer output so later changes provably do not regress.""" + + def test_slide_size(self, template_16x9: Path): + result = analyze_template(template_16x9) + assert result["slide_size"] == {"width": 1920, "height": 1080} + + def test_slide_size_keys(self, template_16x9: Path): + result = analyze_template(template_16x9) + assert set(result["slide_size"].keys()) == {"width", "height"} + + +class TestLayoutPlaceholdersPin16x9: + """Pin get_layout_placeholders px values for 16:9 template.""" + + def test_returns_result(self, template_16x9: Path): + result = get_layout_placeholders(template_16x9, "Blank") + # Blank layout exists in blank-dark.pptx + assert result is not None + assert result["name"] == "Blank" + + def test_placeholder_positions_use_6350_basis(self, template_16x9: Path): + """All px values must be derived at EMU/6350 (16:9 basis).""" + result = get_layout_placeholders(template_16x9, "Blank") + if not result or not result.get("placeholders"): + pytest.skip("Blank layout has no non-media placeholders") + for ph in result["placeholders"]: + # All coordinates must be non-negative integers + for key in ("x", "y", "width", "height"): + assert isinstance(ph[key], int) + assert ph[key] >= 0 + # Width must not exceed canvas width + assert ph["x"] + ph["width"] <= 1920 diff --git a/tests/test_canvas_slidesize.py b/tests/test_canvas_slidesize.py new file mode 100644 index 00000000..a5e8eab5 --- /dev/null +++ b/tests/test_canvas_slidesize.py @@ -0,0 +1,266 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for Issue #208: slideSize in deck.json and generate-time validation. + +Covers: +- Phase 1-5: _resolve_config slideSize mismatch warning / height boundary warning +- Phase 1-6: api.init writes slideSize / converter/pipeline writes slideSize +- Phase 1-7: _apply_grid_overlay derives px_y from image aspect ratio +""" + +from __future__ import annotations + +import json +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parent.parent + + +# ── Helpers ── + +def _make_deck(tmp_path: Path, *, template_path: Path, slide_size: dict | None = None, + elements: list[dict] | None = None) -> Path: + """Create a minimal deck directory for generate testing.""" + deck = tmp_path / "deck" + (deck / "slides").mkdir(parents=True) + (deck / "specs").mkdir() + deck_data: dict = { + "template": str(template_path), + "fonts": {"fullwidth": "Meiryo", "halfwidth": "Arial"}, + "defaultTextColor": "#FFFFFF", + } + if slide_size is not None: + deck_data["slideSize"] = slide_size + (deck / "deck.json").write_text(json.dumps(deck_data, ensure_ascii=False)) + (deck / "specs" / "outline.md").write_text("- [intro] Intro\n") + slide = { + "layout": "Blank", + "elements": elements or [ + {"type": "textbox", "text": "Hello", "x": 100, "y": 100, "width": 800, "height": 80}, + ], + } + (deck / "slides" / "intro.json").write_text(json.dumps(slide, ensure_ascii=False)) + return deck + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 1-5: generate-time slideSize / height-boundary warnings +# ═══════════════════════════════════════════════════════════════════════ + + +class TestResolvConfigSlideSizeWarning: + """_resolve_config emits slideSize mismatch warning when deck.json disagrees with template.""" + + def test_mismatch_warning_4x3_deck_wrong_size(self, template_4x3: Path, tmp_path: Path): + """4:3 template with wrong slideSize in deck.json triggers warning.""" + from sdpm.api import _resolve_config + + deck = _make_deck(tmp_path, template_path=template_4x3, + slide_size={"width": 1920, "height": 1080}) + config = _resolve_config(deck) + mismatch_warnings = [w for w in config.warnings if "slideSize mismatch" in w] + assert len(mismatch_warnings) == 1 + assert "1440" in mismatch_warnings[0] + + def test_no_warning_when_correct_size(self, template_4x3: Path, tmp_path: Path): + """4:3 template with correct slideSize emits no mismatch warning.""" + from sdpm.api import _resolve_config + + deck = _make_deck(tmp_path, template_path=template_4x3, + slide_size={"width": 1920, "height": 1440}) + config = _resolve_config(deck) + mismatch_warnings = [w for w in config.warnings if "slideSize mismatch" in w] + assert len(mismatch_warnings) == 0 + + def test_no_warning_when_no_slide_size_key(self, template_16x9: Path, tmp_path: Path): + """No slideSize in deck.json → no mismatch warning (key is optional).""" + from sdpm.api import _resolve_config + + deck = _make_deck(tmp_path, template_path=template_16x9, slide_size=None) + config = _resolve_config(deck) + mismatch_warnings = [w for w in config.warnings if "slideSize mismatch" in w] + assert len(mismatch_warnings) == 0 + + def test_16x9_correct_no_warning(self, template_16x9: Path, tmp_path: Path): + """16:9 template with correct slideSize emits no warning.""" + from sdpm.api import _resolve_config + + deck = _make_deck(tmp_path, template_path=template_16x9, + slide_size={"width": 1920, "height": 1080}) + config = _resolve_config(deck) + mismatch_warnings = [w for w in config.warnings if "slideSize mismatch" in w] + assert len(mismatch_warnings) == 0 + + +class TestResolveConfigHeightBoundaryWarning: + """_resolve_config emits height boundary warning for out-of-bounds elements.""" + + def test_height_oob_4x3(self, template_4x3: Path, tmp_path: Path): + """Element exceeding 4:3 height (1440) triggers height boundary warning.""" + from sdpm.api import _resolve_config + + elements = [ + {"type": "textbox", "text": "A", "x": 0, "y": 1400, "width": 200, "height": 100}, + ] + deck = _make_deck(tmp_path, template_path=template_4x3, elements=elements) + config = _resolve_config(deck) + height_warnings = [w for w in config.warnings if "exceeds slide height" in w] + assert len(height_warnings) == 1 + assert "1440" in height_warnings[0] + + def test_no_height_warning_within_bounds_4x3(self, template_4x3: Path, tmp_path: Path): + """Element within 4:3 height (y+h<=1440) does not trigger warning.""" + from sdpm.api import _resolve_config + + elements = [ + {"type": "textbox", "text": "A", "x": 0, "y": 1300, "width": 200, "height": 100}, + ] + deck = _make_deck(tmp_path, template_path=template_4x3, elements=elements) + config = _resolve_config(deck) + height_warnings = [w for w in config.warnings if "exceeds slide height" in w] + assert len(height_warnings) == 0 + + def test_no_false_positive_16x9(self, template_16x9: Path, tmp_path: Path): + """16:9: element at y=900 h=180 → 1080 exactly, no warning.""" + from sdpm.api import _resolve_config + + elements = [ + {"type": "textbox", "text": "A", "x": 0, "y": 900, "width": 200, "height": 180}, + ] + deck = _make_deck(tmp_path, template_path=template_16x9, elements=elements) + config = _resolve_config(deck) + height_warnings = [w for w in config.warnings if "exceeds slide height" in w] + assert len(height_warnings) == 0 + + def test_height_oob_16x9(self, template_16x9: Path, tmp_path: Path): + """16:9: element at y=1000 h=100 → 1100 > 1080, triggers warning.""" + from sdpm.api import _resolve_config + + elements = [ + {"type": "textbox", "text": "A", "x": 0, "y": 1000, "width": 200, "height": 100}, + ] + deck = _make_deck(tmp_path, template_path=template_16x9, elements=elements) + config = _resolve_config(deck) + height_warnings = [w for w in config.warnings if "exceeds slide height" in w] + assert len(height_warnings) == 1 + assert "1080" in height_warnings[0] + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 1-6: api.init writes slideSize +# ═══════════════════════════════════════════════════════════════════════ + + +class TestInitSlidesSize: + """api.init writes slideSize to deck.json when template is provided.""" + + def test_init_16x9_template(self, template_16x9: Path, tmp_path: Path): + from sdpm.api import init + + result = init(name="test", template=str(template_16x9), output_dir=str(tmp_path / "out")) + deck_json = json.loads(Path(result["deck_json"]).read_text()) + assert deck_json["slideSize"] == {"width": 1920, "height": 1080} + + def test_init_4x3_template(self, template_4x3: Path, tmp_path: Path): + from sdpm.api import init + + result = init(name="test", template=str(template_4x3), output_dir=str(tmp_path / "out")) + deck_json = json.loads(Path(result["deck_json"]).read_text()) + assert deck_json["slideSize"] == {"width": 1920, "height": 1440} + + def test_init_no_template_no_slide_size(self, tmp_path: Path): + from sdpm.api import init + + result = init(name="test", output_dir=str(tmp_path / "out")) + deck_json = json.loads(Path(result["deck_json"]).read_text()) + assert "slideSize" not in deck_json + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 1-6: converter/pipeline writes slideSize +# ═══════════════════════════════════════════════════════════════════════ + + +class TestPipelineSlidesSize: + """pptx_to_json writes slideSize in deck.json.""" + + def test_16x9_import(self, template_16x9: Path, tmp_path: Path): + from sdpm.engine.converter import pptx_to_json + + # Need a pptx with at least one slide + from pptx import Presentation + prs = Presentation(str(template_16x9)) + layout = prs.slide_layouts[0] + prs.slides.add_slide(layout) + src = tmp_path / "src.pptx" + prs.save(str(src)) + + out_dir = tmp_path / "out" + pptx_to_json(src, output_dir=out_dir) + deck = json.loads((out_dir / "deck.json").read_text()) + assert deck["slideSize"] == {"width": 1920, "height": 1080} + + def test_4x3_import(self, template_4x3: Path, tmp_path: Path): + from sdpm.engine.converter import pptx_to_json + from pptx import Presentation + + prs = Presentation(str(template_4x3)) + layout = prs.slide_layouts[0] + prs.slides.add_slide(layout) + src = tmp_path / "src.pptx" + prs.save(str(src)) + + out_dir = tmp_path / "out" + pptx_to_json(src, output_dir=out_dir) + deck = json.loads((out_dir / "deck.json").read_text()) + assert deck["slideSize"] == {"width": 1920, "height": 1440} + + +# ═══════════════════════════════════════════════════════════════════════ +# Phase 1-7: _apply_grid_overlay px_y derivation +# ═══════════════════════════════════════════════════════════════════════ + + +class TestGridOverlayAspectRatio: + """_apply_grid_overlay derives px_y from image dimensions, not fixed 1080.""" + + def test_16x9_image_px_y_is_1080_based(self, tmp_path: Path): + """16:9 image: px_y at 50% should be 540 (= 1920 * 540/960 * 50/100 = 540).""" + from PIL import Image + from sdpm.api import _apply_grid_overlay + + # 1920x1080 image (or proportional: 960x540) + img = Image.new("RGB", (960, 540), (255, 255, 255)) + p = tmp_path / "slide.png" + img.save(str(p)) + + _apply_grid_overlay([str(p)]) + + # Verify the file was modified (overlay applied) + result = Image.open(str(p)) + assert result.size == (960, 540) + + def test_4x3_image_px_y_is_1440_based(self, tmp_path: Path): + """4:3 image (1920x1440 proportional): px_y at 50% should be 720.""" + from PIL import Image + from sdpm.api import _apply_grid_overlay + + # 4:3 proportional image: 960x720 + img = Image.new("RGB", (960, 720), (255, 255, 255)) + p = tmp_path / "slide.png" + img.save(str(p)) + + _apply_grid_overlay([str(p)]) + + # Just verify it runs without error (assertion on pixel content is fragile) + result = Image.open(str(p)) + assert result.size == (960, 720) + + def test_px_y_formula_correctness(self): + """Verify formula: round(1920 * h / w * pct / 100) gives correct results.""" + # 16:9 (1920x1080 equivalent: w=960, h=540) at 50% + assert round(1920 * 540 / 960 * 50 / 100) == 540 + # 4:3 (1920x1440 equivalent: w=960, h=720) at 50% + assert round(1920 * 720 / 960 * 50 / 100) == 720 + # 4:3 at 100% + assert round(1920 * 720 / 960 * 100 / 100) == 1440 diff --git a/tests/test_converter_elements.py b/tests/test_converter_elements.py index 3fd41739..a3f467e4 100644 --- a/tests/test_converter_elements.py +++ b/tests/test_converter_elements.py @@ -534,6 +534,7 @@ def test_standard_16x9_corpus(self, tmp_path): "fonts": {"halfwidth": "Calibri", "fullwidth": ""}, "defaultTextColor": "#000000", "autoSpacing": False, + "slideSize": {"width": 1920, "height": 1080}, } assert (out_dir / "slides" / "slide-001.json").exists() assert sorted(p.name for p in (out_dir / "images").iterdir()) == ["slide1_image1.png"] diff --git a/tests/test_converter_slide_layout.py b/tests/test_converter_slide_layout.py new file mode 100644 index 00000000..94ba5d74 --- /dev/null +++ b/tests/test_converter_slide_layout.py @@ -0,0 +1,65 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for converter/slide.py — detect_layout with emu_per_px (Phase 1-7).""" + +from pathlib import Path + +from pptx import Presentation + +from sdpm.engine.converter.constants import conversion_scale +from sdpm.engine.converter.slide import detect_layout + + +class TestDetectLayoutEmuPerPx: + """Ensure detect_layout uses get_emu_per_px() for left-accent-line detection.""" + + def _make_slide_with_accent_line(self, prs, left_px, width_px, height_px): + """Add a slide using 'Title Only' layout with a thin left accent line. + + The accent line check only fires when has_title=True and has_content=False, + which is the 'Title Only' layout (index 5). + """ + layout = prs.slide_layouts[5] # "Title Only" — has TITLE but no BODY/OBJECT + slide = prs.slides.add_slide(layout) + from pptx.util import Emu as EmuUtil + emu_px = prs.slide_width / 1920 + slide.shapes.add_shape( + 1, # rectangle + EmuUtil(round(left_px * emu_px)), + EmuUtil(0), + EmuUtil(round(width_px * emu_px)), + EmuUtil(round(height_px * emu_px)), + ) + return slide + + def test_16x9_detects_accent_line(self, template_16x9: Path): + """16:9: left accent line (< 50px wide, > 500px tall) detected as content.""" + prs = Presentation(str(template_16x9)) + slide = self._make_slide_with_accent_line(prs, left_px=10, width_px=5, height_px=800) + + # Within default conversion scope (16:9), detect should find the accent line + result = detect_layout(slide) + assert result == "content" + + def test_4x3_detects_accent_line(self, template_4x3: Path): + """4:3: left accent line detected correctly with proper emu_per_px scope.""" + prs = Presentation(str(template_4x3)) + slide = self._make_slide_with_accent_line(prs, left_px=10, width_px=5, height_px=800) + + # Must be within 4:3 conversion scope for correct threshold + with conversion_scale(prs.slide_width): + result = detect_layout(slide) + assert result == "content" + + def test_4x3_accent_line_without_scope_uses_default(self, template_4x3: Path): + """4:3 without scope: default emu_per_px (6350) is more permissive on left threshold.""" + prs = Presentation(str(template_4x3)) + # Place accent line at 10px in 4:3 coords (emu = 10 * 4762.5 = 47625). + # Default scope threshold: 50 * 6350 = 317500. 47625 < 317500 passes. + # Width threshold: 20 * 6350 = 127000. shape width = 5 * 4762.5 = 23812. passes. + # Height threshold: 500 * 6350 = 3175000. shape height = 800 * 4762.5 = 3810000. passes. + slide = self._make_slide_with_accent_line(prs, left_px=10, width_px=5, height_px=800) + # In default scope, the EMU values from 4:3 are smaller for width/left + # but the thresholds (with larger emu_per_px) are also larger — still passes. + result = detect_layout(slide) + assert result == "content" diff --git a/tests/test_lint.py b/tests/test_lint.py index b117b794..95e1bb4b 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -369,11 +369,13 @@ def test_out_of_bounds_x(self): ]}]) assert any(d["rule"] == "out-of-bounds" for d in diags) - def test_out_of_bounds_y(self): + def test_height_not_checked(self): + """Height OOB is not checked by lint — it is template-dependent and + validated at generate time with real slide dimensions (R2/D2).""" diags = lint([{"elements": [ - {"type": "shape", "shape": "rectangle", "x": 0, "y": 1000, "width": 100, "height": 100} + {"type": "shape", "shape": "rectangle", "x": 0, "y": 1000, "width": 100, "height": 500} ]}]) - assert any(d["rule"] == "out-of-bounds" for d in diags) + assert not any(d["rule"] == "out-of-bounds" for d in diags) def test_within_bounds(self): diags = lint([{"elements": [ @@ -381,6 +383,14 @@ def test_within_bounds(self): ]}]) assert not any(d["rule"] == "out-of-bounds" for d in diags) + def test_height_exceeds_4x3_no_warning(self): + """A 4:3 canvas is 1920x1440 — elements beyond 1080 should NOT trigger + out-of-bounds since lint is slide-size agnostic.""" + diags = lint([{"elements": [ + {"type": "shape", "shape": "rectangle", "x": 0, "y": 1100, "width": 800, "height": 300} + ]}]) + assert not any(d["rule"] == "out-of-bounds" for d in diags) + # presentation dict format def test_presentation_dict(self): diags = lint({"slides": [{"elements": [ diff --git a/tests/test_preview_imbalance.py b/tests/test_preview_imbalance.py new file mode 100644 index 00000000..b9a849f8 --- /dev/null +++ b/tests/test_preview_imbalance.py @@ -0,0 +1,77 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for preview/__init__.py — imbalance check with emu_per_px (Phase 1-7).""" + +from pathlib import Path + +from pptx import Presentation +from pptx.util import Emu + +from sdpm.engine.preview import check_layout_imbalance_data + + +class TestImbalanceEmuPerPx: + """Ensure check_layout_imbalance_data uses emu_per_px from slide width.""" + + def test_16x9_canvas_size(self, template_16x9: Path, tmp_path: Path): + """16:9 template: canvas reports as 1920 x 1080.""" + prs = Presentation(str(template_16x9)) + slide = prs.slides.add_slide(prs.slide_layouts[0]) + shape = slide.shapes.add_textbox(Emu(6350 * 960), Emu(6350 * 540), Emu(6350 * 200), Emu(6350 * 100)) + shape.text = "Center" + out = tmp_path / "test_16x9.pptx" + prs.save(str(out)) + + # Centered element should not trigger imbalance + alerts = check_layout_imbalance_data(str(out)) + # Element is roughly centered (540 + 50 = cy=590 vs _CY=~560), may or may not alert + # The key assertion: no crash and it runs with real emu_per_px + assert isinstance(alerts, list) + + def test_4x3_canvas_size(self, template_4x3: Path, tmp_path: Path): + """4:3 template: canvas reports as 1920 x 1440 (not 1440 x 1080).""" + prs = Presentation(str(template_4x3)) + slide = prs.slides.add_slide(prs.slide_layouts[0]) + # Place an element at the vertical center of a 4:3 slide + # 4:3 emu_per_px = 9144000 / 1920 = 4762.5 + emu_px = 4762.5 + # Vertically centered: y = ~720 (half of 1440) + cy_target = 720 + elem_h = 200 + elem_y = cy_target - elem_h // 2 # y = 620 + shape = slide.shapes.add_textbox( + Emu(round(100 * emu_px)), + Emu(round(elem_y * emu_px)), + Emu(round(400 * emu_px)), + Emu(round(elem_h * emu_px)), + ) + shape.text = "4:3 centered" + out = tmp_path / "test_4x3.pptx" + prs.save(str(out)) + + alerts = check_layout_imbalance_data(str(out)) + # With correct emu_per_px, a centered element should produce + # minimal or no imbalance alert + assert isinstance(alerts, list) + + def test_4x3_bbox_reports_correct_dimensions(self, template_4x3: Path, tmp_path: Path): + """4:3: bbox string should reference '1920x1440' (not '1440x1080').""" + prs = Presentation(str(template_4x3)) + slide = prs.slides.add_slide(prs.slide_layouts[0]) + emu_px = 4762.5 + # Place element at top-left so it triggers imbalance (top-heavy) + shape = slide.shapes.add_textbox( + Emu(round(100 * emu_px)), + Emu(round(50 * emu_px)), + Emu(round(400 * emu_px)), + Emu(round(100 * emu_px)), + ) + shape.text = "Top element" + out = tmp_path / "test_4x3_top.pptx" + prs.save(str(out)) + + alerts = check_layout_imbalance_data(str(out)) + # Should have an alert (element is way at the top) + assert len(alerts) >= 1 + # The bbox should reference 1920x1440 + assert "1920x1440" in alerts[0]["bbox"] diff --git a/tests/test_preview_measure.py b/tests/test_preview_measure.py new file mode 100644 index 00000000..729edcd4 --- /dev/null +++ b/tests/test_preview_measure.py @@ -0,0 +1,93 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +"""Tests for preview/measure.py — viewBox scaling (Phase 1-4).""" + +from pathlib import Path + +import pytest + +from sdpm.engine.preview.measure import measure_from_svg + + +def _make_svg(vb_w: float, vb_h: float, rect_x: float, rect_y: float, rect_w: float, rect_h: float) -> str: + """Create a minimal SVG with one text shape for measurement. + + viewBox is ``0 0 {vb_w} {vb_h}`` — LibreOffice uses EMU-scale values here. + """ + return f"""\ + + + + + + + + + + Hello + + + + +""" + + +class TestViewBoxScaling: + """Ensure measure uses width-based equal-aspect scale (not separate x/y).""" + + def test_16x9_scale_unchanged(self, tmp_path: Path): + """16:9 viewBox: behaviour is identical to before the fix.""" + # 16:9 EMU viewBox: 25400 x 14287.5 (ratio preserving at 6350 emu/px) + vb_w, vb_h = 25400.0, 14287.5 + # Shape at viewBox coords (2540, 1428.75) with size (5080, 2857.5) + svg = _make_svg(vb_w, vb_h, 2540, 1428.75, 5080, 2857.5) + svg_path = tmp_path / "test.svg" + svg_path.write_text(svg) + + results = measure_from_svg(svg_path) + assert 1 in results + bbox = results[1][0] + # scale = 1920 / 25400 ≈ 0.07559 + assert bbox.x_px == pytest.approx(192.0, abs=0.1) + assert bbox.y_px == pytest.approx(108.0, abs=0.1) + assert bbox.w_px == pytest.approx(384.0, abs=0.1) + assert bbox.h_px == pytest.approx(216.0, abs=0.1) + + def test_4x3_not_compressed(self, tmp_path: Path): + """4:3 viewBox: y/h must NOT be 0.75x compressed (the bug this fixes).""" + # 4:3 EMU viewBox: 19050 x 14287.5 (9144000/480 x 6858000/480) + vb_w, vb_h = 19050.0, 14287.5 + # Shape at viewBox center-ish: (9525, 7143.75) = half of each dimension + svg = _make_svg(vb_w, vb_h, 9525.0, 7143.75, 1905.0, 1428.75) + svg_path = tmp_path / "test.svg" + svg_path.write_text(svg) + + results = measure_from_svg(svg_path) + assert 1 in results + bbox = results[1][0] + # scale = 1920 / 19050 ≈ 0.10079 + # x_px = 9525 * 0.10079 = 960.0 + # y_px = 7143.75 * 0.10079 = 720.0 (NOT 540.0 which old code would give) + # w_px = 1905 * 0.10079 = 192.0 + # h_px = 1428.75 * 0.10079 = 144.0 (NOT 108.0 which old code would give) + assert bbox.x_px == pytest.approx(960.0, abs=0.1) + assert bbox.y_px == pytest.approx(720.0, abs=0.1) + assert bbox.w_px == pytest.approx(192.0, abs=0.1) + assert bbox.h_px == pytest.approx(144.0, abs=0.1) + + def test_4x3_y_not_compressed_explicit(self, tmp_path: Path): + """Explicit check: with old code (scale_y=1080/vb_h) the y would be 0.75x.""" + # 4:3: viewBox 19050 x 14287.5 + # A shape at y=14287.5 (bottom edge) should map to y=1440 (4:3 canvas height) + # Old code would give: 14287.5 * (1080 / 14287.5) = 1080 (wrong, cuts 25%) + # New code gives: 14287.5 * (1920 / 19050) = 1440 (correct) + vb_w, vb_h = 19050.0, 14287.5 + svg = _make_svg(vb_w, vb_h, 0, 14287.5, 19050.0, 0) + svg_path = tmp_path / "test.svg" + svg_path.write_text(svg) + + results = measure_from_svg(svg_path) + assert 1 in results + bbox = results[1][0] + # y_px should be 1440 (full 4:3 height), not 1080 + assert bbox.y_px == pytest.approx(1440.0, abs=0.1) From 30b33f0cf4c5912b99dbd25ff22268921c638b8c Mon Sep 17 00:00:00 2001 From: ShotaroKataoka Date: Wed, 5 Aug 2026 09:09:56 +0900 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9A=20docs(aspect-ratio-agnostic-s?= =?UTF-8?q?lide-canvas):=20=E3=82=AD=E3=83=A3=E3=83=B3=E3=83=90=E3=82=B9?= =?UTF-8?q?=E5=89=8D=E6=8F=90=E8=A8=98=E8=BF=B0=E3=82=92=E3=83=91=E3=83=A9?= =?UTF-8?q?=E3=83=A1=E3=83=BC=E3=82=BF=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit references が「キャンバスは1920×1080、描画領域 y=173–950」と固定値で教えて いたため、4:3 テンプレート（実キャンバス 1920×1440px）でも AI が y≤1080 で 設計し下部が空白になっていた (#208 のメイン症状)。 - キャンバス寸法は「幅1920固定・高さは slideSize 参照」に書き換え - 派生値（描画領域など）は導出式を教え、16:9 の具体値は例示として残す。 比率スケールにしないのは y=173 が title.y2+margin 由来の絶対値、 y=950 が「下端から130px」由来の絶対マージンであり、 4:3 でタイトル帯を1.33倍に太らせるのは誤りだから - art-direction / vibe に deck.json への slideSize 記録手順を追加 （vibe は analyze_template を呼ばないため deck.json が唯一の伝達経路） - composer に「幅1920固定・高さ可変、slideSize を必ず参照」の規則を追加 - スタイルデモは 16:9 固定キャンバスであることを明記 - arch 図の box 自動高さが非16:9で過小になる既知の制約と回避策 （box.height の明示指定）を arch-layout-engine.md に記載 SPEC: 20260805-0100_aspect-ratio-agnostic-slide-canvas Refs: #208 --- personas/composer.md | 8 ++++++++ personas/style.md | 2 +- personas/vibe.md | 8 ++++++-- sdpm/references/guides/arch-elements.md | 4 ++-- sdpm/references/guides/arch-layout-engine.md | 9 +++++++++ sdpm/references/guides/chart-bar.md | 2 ++ sdpm/references/guides/chart-line.md | 2 ++ sdpm/references/guides/chart-pie.md | 2 ++ sdpm/references/guides/design-rules.md | 3 ++- sdpm/references/guides/grid.md | 5 ++++- .../workflows/create-new-1-art-direction.md | 5 ++++- sdpm/references/workflows/create-style.md | 2 +- sdpm/references/workflows/slide-json-spec.md | 16 ++++++++++------ 13 files changed, 53 insertions(+), 15 deletions(-) diff --git a/personas/composer.md b/personas/composer.md index b9386195..5cfdff53 100644 --- a/personas/composer.md +++ b/personas/composer.md @@ -142,6 +142,14 @@ style's `:root` (`--fs-*`, color vars) in `specs/art-direction.html`. The style FROZEN for you — if a needed token genuinely doesn't exist, report it in your summary rather than inventing an ad-hoc value. +### Canvas dimensions + +The canvas is **width 1920px fixed, height variable** depending on the template. +Always read `deck.json` `slideSize` to determine the actual slide height (H). +- Content area: y = title bottom + margin to H−130 +- For 16:9 (H=1080): y=173–950. For 4:3 (H=1440): y=173–1310 +- Never assume H=1080 — derive from `slideSize` + ## Consistency Review Mode If the instruction is `"Consistency review."` (or asks for a consistency review), you diff --git a/personas/style.md b/personas/style.md index bf5644fb..c7966938 100644 --- a/personas/style.md +++ b/personas/style.md @@ -55,7 +55,7 @@ Write incrementally via `run_style_python`: - All colors via `var()` references, never hardcoded in elements - Text style classes (`.t-title`, `.t-body`, etc.) reference CSS variables - Inline style only for position/size (`left`, `top`, `width`, `height`) -- Coordinate system: 1920×1080 absolute positioning +- Coordinate system: 1920×1080 absolute positioning (style demos always use 16:9 fixed canvas) - Font sizes: pt units only - `body { zoom: 0.7; }` for display scaling - 5–6 slides maximum (cover + design areas) diff --git a/personas/vibe.md b/personas/vibe.md index dee23be6..35ceacf2 100644 --- a/personas/vibe.md +++ b/personas/vibe.md @@ -94,12 +94,16 @@ Rules: 3. Call `apply_style(deck_id, style)` to set art direction 4. If the user specified a style or tone, honor that instead of inferring 5. Read `specs/art-direction.html` via `run_python` (`read_text("specs/art-direction.html")`), - extract the `:root` CSS variables, then update `deck.json` via `write_json`: + extract the `:root` CSS variables, then update `deck.json` via `write_json`. + Also record the template's `slideSize` — call `analyze_template(template)` to get + `slide_size`, or use the known default `{"width": 1920, "height": 1080}` for standard + 16:9 templates: ```json { "template": "{template}.pptx", "fonts": {"fullwidth": "{fullwidth font}", "halfwidth": "{halfwidth font}"}, - "defaultTextColor": "{--color-text value}" + "defaultTextColor": "{--color-text value}", + "slideSize": {"width": 1920, "height": (from analyze_template or 1080 for 16:9)} } ``` diff --git a/sdpm/references/guides/arch-elements.md b/sdpm/references/guides/arch-elements.md index 77500e5f..dd02b874 100644 --- a/sdpm/references/guides/arch-elements.md +++ b/sdpm/references/guides/arch-elements.md @@ -45,8 +45,8 @@ Choose scale based on architecture complexity: ## Drawing Area ``` -Slide: 1920 x 1080px -Recommended area: x=60–1860, y=200–900 (title_only layout) +Slide: 1920 x H px (H = slide height from slideSize; 1080 for 16:9, 1440 for 4:3) +Recommended area: x=60–1860, y=200–(H−180) (title_only layout) ``` --- diff --git a/sdpm/references/guides/arch-layout-engine.md b/sdpm/references/guides/arch-layout-engine.md index 895a38b9..09774d8b 100644 --- a/sdpm/references/guides/arch-layout-engine.md +++ b/sdpm/references/guides/arch-layout-engine.md @@ -31,6 +31,15 @@ arch_diagram(spec="", x=100, y=180, width=1720, height=800, theme="dark") ``` +Default target area (x=100, y=180, width=1720, height=800) assumes 16:9 canvas. +For non-16:9 templates, derive height from slideSize: e.g. `height = H - 180 - 130` +(H from `deck.json` `slideSize`; 4:3 H=1440 → height=1130). + +> **Known constraint (non-16:9):** The layout engine's text-to-box sizing uses a +> fixed px-to-pt ratio calibrated for 16:9. On taller canvases (e.g. 4:3), boxes with +> long `description` text may overflow. **Workaround:** explicitly set `"height"` on +> boxes with multi-line descriptions instead of relying on auto-calculation. + **CLI** — same engine, for SKILL.md/script hosts: ```bash diff --git a/sdpm/references/guides/chart-bar.md b/sdpm/references/guides/chart-bar.md index fa9da60d..52db35ab 100644 --- a/sdpm/references/guides/chart-bar.md +++ b/sdpm/references/guides/chart-bar.md @@ -58,6 +58,8 @@ These can be set either as top-level `style` object or individually: ## JSON: Vertical bar (comparison) +(Example coordinates assume 16:9 canvas H=1080. Adjust y/height for other ratios.) + ```json { "slides": [ diff --git a/sdpm/references/guides/chart-line.md b/sdpm/references/guides/chart-line.md index 196957f3..8cca2d15 100644 --- a/sdpm/references/guides/chart-line.md +++ b/sdpm/references/guides/chart-line.md @@ -47,6 +47,8 @@ These can be set as a `style` object: ## JSON: Load test trend +(Example coordinates assume 16:9 canvas H=1080. Adjust y/height for other ratios.) + ```json { "slides": [ diff --git a/sdpm/references/guides/chart-pie.md b/sdpm/references/guides/chart-pie.md index 658accee..f74d2d97 100644 --- a/sdpm/references/guides/chart-pie.md +++ b/sdpm/references/guides/chart-pie.md @@ -35,6 +35,8 @@ These can be set as a `style` object: ## JSON: Cost breakdown +(Example coordinates assume 16:9 canvas H=1080. Adjust y/height for other ratios.) + ```json { "slides": [ diff --git a/sdpm/references/guides/design-rules.md b/sdpm/references/guides/design-rules.md index 43494307..f9fef72b 100644 --- a/sdpm/references/guides/design-rules.md +++ b/sdpm/references/guides/design-rules.md @@ -80,7 +80,8 @@ Let the distance between elements reflect their relationship — related items c ## Layout Balance Balance elements vertically within the content area. Do not cluster at the top unless intentional (e.g. hero title). -Sample template (1920×1080): content area y=143–950. +Content area: y = title bottom + margin to H−130 (H = slide height from `slideSize`). +For 16:9 (H=1080): content area y=143–950. Custom templates: refer to slide size and placeholder positions from `analyze-template`. diff --git a/sdpm/references/guides/grid.md b/sdpm/references/guides/grid.md index aecb8ec9..7c32a7b2 100644 --- a/sdpm/references/guides/grid.md +++ b/sdpm/references/guides/grid.md @@ -97,7 +97,8 @@ When `items` is specified, matched cells get an additional `"item"` key with cen The starting point of grid is "which region to divide." This decision determines layout quality. -- **Full slide**: Use analyze-template to get the title bottom edge and calculate the content area +- **Full slide**: Use analyze-template to get the title bottom edge and calculate the content area. + Content area = title.y2 + margin to H−130 (H = slide height from `slideSize`). - **Output from a parent grid**: First split the slide coarsely, then use the output coordinates as the next area - **Inside a component**: Use a card or section's coordinates as the area and subdivide its contents - **Partial region**: You don't have to use the full area — reserve space above for description text, below for a flow diagram, etc. Narrow the area to fit the content @@ -107,6 +108,7 @@ The starting point of grid is "which region to divide." This decision determines uv run python3 scripts/pptx_builder.py analyze-template template.pptx --layout "Title Only" # → TITLE: {x:64, y:47, w:1803, h:95, y2:142, ...} # → area_y = title.y2 + margin = 142 + 31 = 173 +# → area_h = H - 130 - area_y (H from slideSize; 16:9 H=1080 → h=777) ``` ### Step 2: Divide with grid @@ -132,6 +134,7 @@ Use when left and right sides have different row counts, or regions have differe These are samples showing how to use grid. They are not canonical layout patterns. The combinations of columns/rows/gap/areas are open-ended — invent freely to match the content. +(All y/h values below assume 16:9 H=1080. For other ratios, recalculate from slideSize.) ### Funnel diff --git a/sdpm/references/workflows/create-new-1-art-direction.md b/sdpm/references/workflows/create-new-1-art-direction.md index 74bd2057..44cea108 100644 --- a/sdpm/references/workflows/create-new-1-art-direction.md +++ b/sdpm/references/workflows/create-new-1-art-direction.md @@ -62,11 +62,14 @@ uv run python3 scripts/pptx_builder.py analyze-template templates/{selected_temp Update `deck.json` with the template name and fonts from the analyze output. When `specs/art-direction.html` exists, read `:root` CSS variables and use `--color-text` as `defaultTextColor`. If the style HTML specifies font-family, ask the user which to use — the style's fonts or the template's fonts. + +Also record the template's slide size — check the `slide_size` field from the analyze output: ```json { "template": "{selected_template}.pptx", "fonts": {"fullwidth": "(style or template)", "halfwidth": "(style or template)"}, - "defaultTextColor": "(use --color-text from art-direction.html :root)" + "defaultTextColor": "(use --color-text from art-direction.html :root)", + "slideSize": {"width": 1920, "height": (from analyze_template slide_size)} } ``` diff --git a/sdpm/references/workflows/create-style.md b/sdpm/references/workflows/create-style.md index 9ff2c7ae..4ad53241 100644 --- a/sdpm/references/workflows/create-style.md +++ b/sdpm/references/workflows/create-style.md @@ -205,7 +205,7 @@ Use these as starting points. The style HTML doesn't need to match any specific exactly, but staying in this range ensures the design translates well to actual slides. **Critical rules — do NOT deviate:** -- Coordinate system: 1920×1080 absolute positioning (same as slides.json) +- Coordinate system: 1920×1080 absolute positioning (style demos use a fixed 16:9 canvas regardless of target template) - Display scaling: `body { zoom: 0.7 }`. Do NOT use `transform: scale()` (breaks background sizing) - Layout: `position: absolute` on all elements via `.el` class. Do NOT use flexbox or grid for slide layout (coordinates won't match slides.json) - Font sizes: pt units only (same as slides.json). Do NOT use px, em, or rem diff --git a/sdpm/references/workflows/slide-json-spec.md b/sdpm/references/workflows/slide-json-spec.md index e1c285bb..5f971825 100644 --- a/sdpm/references/workflows/slide-json-spec.md +++ b/sdpm/references/workflows/slide-json-spec.md @@ -203,14 +203,16 @@ Note: `notes` omitted for brevity. In actual slides, write them before `elements ## Positioning - Coordinates and sizes are in pixels (px) -- Sample template: 1920×1080 basis, recommended drawing area x=58–1862, y=173–950 -- Custom templates: use slide size and placeholder positions from `analyze-template` +- Canvas width is always 1920px. Height depends on the template's aspect ratio + (check `deck.json` `slideSize` or `analyze-template` `slide_size`) +- Recommended drawing area: x=58–1862, y = title bottom + margin to H−130 + (H = slide height from slideSize; for 16:9 H=1080 → y=173–950) - Bottom-most element: aim for `y + height ≥ 80% of slide height` -### Coordinate quick reference +### Coordinate quick reference (16:9 example, H=1080) -| % | x (horizontal) | y (vertical) | -|---|----------------|--------------| +| % | x (horizontal) | y (vertical, 16:9) | +|---|----------------|---------------------| | 5 | 96 | 54 | | 10 | 192 | 108 | | 25 | 480 | 270 | @@ -218,6 +220,8 @@ Note: `notes` omitted for brevity. In actual slides, write them before `elements | 75 | 1440 | 810 | | 100 | 1920 | 1080 | +For other aspect ratios, y values scale with H (e.g. 4:3 H=1440: 100% y=1440). + **Common sizes**: card width 400–600px, 2-column 900px each, 3-column 600px each, 4-column 450px each. Icon size is relative to context — see design-rules. ## Elements @@ -302,7 +306,7 @@ Height includes the language label (22px). Code body height is `height - 22`. - `autoWidth`: true → word_wrap disabled (width fits text) - `line`: border color. Omit or `"none"` for no border - `lineWidth`: border thickness (pt, default 1) -- `margin*`: px (same 1920×1080 basis as other coordinates) +- `margin*`: px (same 1920-wide canvas basis as other coordinates) - `verticalAlign`: `top`, `middle`, `bottom` (default: top for textbox) - **Line breaks**: `\n` creates a line break (internally split into paragraphs) From d4d6faa3b2f68b9904b0f02860b7c8da33f7c1d5 Mon Sep 17 00:00:00 2001 From: ShotaroKataoka Date: Wed, 5 Aug 2026 09:10:14 +0900 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=90=9B=20fix(aspect-ratio-agnostic-sl?= =?UTF-8?q?ide-canvas):=20=E3=83=97=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC?= =?UTF-8?q?=E3=82=92=E3=82=A2=E3=82=B9=E3=83=9A=E3=82=AF=E3=83=88=E6=AF=94?= =?UTF-8?q?=E8=BF=BD=E5=BE=93=E3=81=AB=E3=81=97=E3=83=A9=E3=82=A4=E3=83=96?= =?UTF-8?q?=E3=83=97=E3=83=AC=E3=83=93=E3=83=A5=E3=83=BC=E3=81=AE3?= =?UTF-8?q?=E8=AA=B2=E9=A1=8C=E3=82=92=E4=BF=AE=E6=AD=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #208 のプレビュー見切れ修正に加え、実装中に発覚した2つの既存問題も解消。 3つとも「異なる意味の状態を1つの変数で兼用していた」ことが原因。 1. プレビューのアスペクト比追従（#208 の症状5） aspect-[16/9] 固定枠 + object-cover で 4:3 が上下25%クロップされていた。 スライドプレビュー（SlideThumbnail / WorkspaceView / SlideCarousel / AnimatedSlidePreview）を実比率追従にする。デッキ一覧カードは グリッド行高が 16:9/4:3 混在で揃わなくなるため意図的に現状維持。 幅上限は「高さ制約を幅に変換する」既存の構造を保ち、比率のみ子から 持ち上げて可変化（w-fit だと親幅←子幅←親幅の循環で幅0に崩壊する） 2. 最初の数枚がアニメーションされない（既存問題） slides タブ表示から3秒間の抑制タイマーがあり、新規デッキでは スライド出現でタブが自動切替されるため最初の数枚が必ず即時表示に なっていた。時間ベースの近似をやめ、マウント時の事実 (hadSlidesOnMount) で判定する。これは 2026-04-13 の SPEC で既に 到達していた設計で、settled タイマーは設計意図が文書化されていない 後付けだった。新規デッキでは knownUrl を渡さない 3. PNG フォールバックが機能しない（既存問題） - リトライごとに setError(false) していたためフォールバックが毎秒 アンマウントされ実質表示されなかった → fetch 成功確定時のみ false に - container null 時に無言 return して URL を処理済み扱いしていたため 恒久的に黒箱になっていた → lastComposeUrlRef をリセットしリトライ可能に - bgSvg も components も無い場合を失敗扱いに（背景のみのスライドは正当） - SlideThumbnail に onError と previewUrl 不在時の明示プレースホルダを追加 なお skipThisUpdate は「アニメーションなしで即時描画」の意味であり 「描画しない」ではない（early return にすると既存デッキが永久に 描画されなくなる）。 SPEC: 20260805-0100_aspect-ratio-agnostic-slide-canvas Refs: #208 --- .../components/deck/AnimatedSlidePreview.tsx | 23 +++- web-ui/src/components/deck/SlideCarousel.tsx | 36 +++-- .../components/deck/SlideThumbnail.test.tsx | 125 ++++++++++++++++++ web-ui/src/components/deck/SlideThumbnail.tsx | 45 +++++-- web-ui/src/components/deck/WorkspaceView.tsx | 77 +++++++---- 5 files changed, 254 insertions(+), 52 deletions(-) create mode 100644 web-ui/src/components/deck/SlideThumbnail.test.tsx diff --git a/web-ui/src/components/deck/AnimatedSlidePreview.tsx b/web-ui/src/components/deck/AnimatedSlidePreview.tsx index 9efea801..c288fef4 100644 --- a/web-ui/src/components/deck/AnimatedSlidePreview.tsx +++ b/web-ui/src/components/deck/AnimatedSlidePreview.tsx @@ -80,6 +80,7 @@ interface AnimatedSlidePreviewProps { knownUrl?: string | null onAnimate?: () => void onComplete?: () => void + onAspectRatio?: (ratio: number) => void fallback?: React.ReactNode } @@ -92,13 +93,14 @@ function assignAgent(comp: ComposeComponent, agents: ResolvedAgent[]) { return agents[4] } -export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, knownUrl, onAnimate, onComplete, fallback }: AnimatedSlidePreviewProps) { +export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, knownUrl, onAnimate, onComplete, onAspectRatio, fallback }: AnimatedSlidePreviewProps) { const containerRef = useRef(null) const timersRef = useRef[]>([]) const intervalsRef = useRef([]) const lastComposeUrlRef = useRef("") const animatingRef = useRef(false) const [error, setError] = useState(false) + const [aspectRatio, setAspectRatio] = useState("16/9") const reducedMotion = useRef( typeof window !== "undefined" && window.matchMedia("(prefers-reduced-motion: reduce)").matches ) @@ -134,7 +136,6 @@ export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, if (animatingRef.current) return // defer until animation completes const skipThisUpdate = skipRef.current || compUrlBase === knownUrlRef.current lastComposeUrlRef.current = compUrlBase - setError(false) ;(async () => { try { @@ -157,8 +158,18 @@ export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, setError(true); return } + // Empty content = nothing to render → treat as failure (fallback to PNG) + if (!data.bgSvg && data.components.length === 0) { + lastComposeUrlRef.current = "" + setError(true); return + } + const container = containerRef.current - if (!container || cancelled) return + if (!container || cancelled) { + // Container not mounted — reset so polling can retry once mounted + lastComposeUrlRef.current = "" + return + } cleanup() setError(false) @@ -184,6 +195,10 @@ export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, // --- Build SVG --- const vb = data.viewBox.split(" ").map(Number) + if (vb[2] > 0 && vb[3] > 0) { + setAspectRatio(`${vb[2]}/${vb[3]}`) + onAspectRatio?.(vb[2] / vb[3]) + } container.innerHTML = "" container.parentElement?.querySelectorAll(".asp-overlay").forEach(el => el.remove()) @@ -322,7 +337,7 @@ export function AnimatedSlidePreview({ defsUrl, composeUrl, slug, skipAnimation, }, [composeUrl]) return ( -
+
{error && fallback &&
{fallback}
}
diff --git a/web-ui/src/components/deck/SlideCarousel.tsx b/web-ui/src/components/deck/SlideCarousel.tsx index 3647bc19..da5ddeb2 100644 --- a/web-ui/src/components/deck/SlideCarousel.tsx +++ b/web-ui/src/components/deck/SlideCarousel.tsx @@ -71,10 +71,20 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo const { viewMode, setViewMode } = usePreferences() const containerRef = useRef(null) + /* ── Aspect ratio reported by the first child (deck is uniform) ── */ + const [deckAr, setDeckAr] = useState(16 / 9) + const arReported = useRef(false) + const handleAspectRatio = useCallback((ratio: number) => { + if (!arReported.current && ratio > 0) { + arReported.current = true + setDeckAr(ratio) + } + }, []) + /* ── Compose update detection → auto-scroll to changed slide ── */ const prevComposeKeys = useRef>(new Map()) const scrollTargetRef = useRef(undefined) - const [hadSlidesOnMount] = useState(slides.length > 0) + const hadSlidesOnMount = useRef(slides.length > 0) const [firstComposeSeen, setFirstComposeSeen] = useState(false) const [knownComposeUrls, setKnownComposeUrls] = useState>(new Map()) @@ -89,7 +99,7 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo } // Mark first compose seen (skip animation for existing decks) if (!firstComposeSeen && slides.some(s => s.composeUrl)) { - if (hadSlidesOnMount) { + if (hadSlidesOnMount.current) { // Existing deck: suppress animation for this first batch anyChanged = false } @@ -97,6 +107,7 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo } if (anyChanged) scrollTargetRef.current = null // arm scroll for next onAnimate setKnownComposeUrls(new Map(prevComposeKeys.current)) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [slides]) const handleAnimate = useCallback((slug: string) => { @@ -137,13 +148,6 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo /* ── Spec tab state + auto-focus ── */ const [specTab, setSpecTab] = useState("brief") const prevSpecsRef = useRef(null) - // Suppress animation for 3s after slides tab becomes visible - const [settled, setSettled] = useState(false) - useEffect(() => { - if (specTab !== "slides") { setSettled(false); return } - const t = setTimeout(() => setSettled(true), 3000) - return () => clearTimeout(t) - }, [specTab]) /** * Auto-focus: when a spec file transitions from null to non-null, @@ -346,9 +350,10 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo ))}
) : ( - /* Full view: cap width so one slide always fits the viewport height - (100vh minus header + paddings, converted to width at 16:9). */ -
+ /* Full view: cap height so one slide always fits the viewport + (100vh minus header + paddings). Width follows aspect ratio. */ +
{slidesWithPreview.map((slide, i) => ( slide.composeUrl && defsUrl ? ( handleAnimate(slide.slug)} + onAspectRatio={handleAspectRatio} fallback={ onSlideClick?.(i + 1)} + onAspectRatio={handleAspectRatio} className="slide-shadow w-full cursor-pointer hover:ring-2 hover:ring-primary/50 transition-shadow" /> } @@ -379,6 +386,7 @@ export function SlideCarousel({ slides, defsUrl, deckId, deckName, pptxUrl, isLo slug={slide.slug} onClick={() => onSlideClick?.(i + 1)} updated={updatedIds.has(slide.slug)} + onAspectRatio={handleAspectRatio} className="slide-shadow w-full cursor-pointer hover:ring-2 hover:ring-primary/50 transition-shadow" /> ) diff --git a/web-ui/src/components/deck/SlideThumbnail.test.tsx b/web-ui/src/components/deck/SlideThumbnail.test.tsx new file mode 100644 index 00000000..15e9902a --- /dev/null +++ b/web-ui/src/components/deck/SlideThumbnail.test.tsx @@ -0,0 +1,125 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: MIT-0 +/** + * Tests for aspect-ratio-agnostic slide canvas (Phase 3). + * + * Verifies that: + * 1. SlideThumbnail does NOT use hardcoded aspect-[16/9] or object-cover + * 2. SlideThumbnail adapts aspect ratio from image natural dimensions + * 3. AnimatedSlidePreview does NOT use hardcoded aspect-[16/9] + */ + +import { describe, it, expect, afterEach, vi } from "vitest" +import { render, cleanup, fireEvent } from "@testing-library/react" +import { SlideThumbnail } from "./SlideThumbnail" + +afterEach(cleanup) + +describe("SlideThumbnail — aspect ratio agnostic", () => { + it("does not use hardcoded aspect-[16/9] class", () => { + const { container } = render( + + ) + const outerDiv = container.firstChild as HTMLElement + expect(outerDiv.className).not.toContain("aspect-[16/9]") + }) + + it("does not use object-cover on the image", () => { + const { container } = render( + + ) + const img = container.querySelector("img") + expect(img).toBeTruthy() + expect(img!.className).not.toContain("object-cover") + expect(img!.className).toContain("object-contain") + }) + + it("defaults to 16/9 aspect ratio before image loads", () => { + const { container } = render( + + ) + const outerDiv = container.firstChild as HTMLElement + expect(outerDiv.style.aspectRatio).toBe("16/9") + }) + + it("updates aspect ratio from image naturalWidth/naturalHeight on load", () => { + const { container } = render( + + ) + const img = container.querySelector("img")! + + // jsdom does not set naturalWidth/naturalHeight, so mock them + Object.defineProperty(img, "naturalWidth", { value: 1920, configurable: true }) + Object.defineProperty(img, "naturalHeight", { value: 1440, configurable: true }) + + fireEvent.load(img) + + const outerDiv = container.firstChild as HTMLElement + expect(outerDiv.style.aspectRatio).toBe("1920/1440") + }) + + it("updates aspect ratio to 16:9 dimensions on load", () => { + const { container } = render( + + ) + const img = container.querySelector("img")! + + Object.defineProperty(img, "naturalWidth", { value: 1920, configurable: true }) + Object.defineProperty(img, "naturalHeight", { value: 1080, configurable: true }) + + fireEvent.load(img) + + const outerDiv = container.firstChild as HTMLElement + expect(outerDiv.style.aspectRatio).toBe("1920/1080") + }) + + it("maintains absolute inset-0 structure for skeleton contract", () => { + const { container } = render( + + ) + // Skeleton uses absolute inset-0 + const skeleton = container.querySelector(".slide-skeleton") + expect(skeleton).toBeTruthy() + expect(skeleton!.className).toContain("absolute") + expect(skeleton!.className).toContain("inset-0") + + // Image uses absolute inset-0 + const img = container.querySelector("img") + expect(img).toBeTruthy() + expect(img!.className).toContain("absolute") + expect(img!.className).toContain("inset-0") + }) + + it("does not render image when src is null", () => { + const { container } = render( + + ) + expect(container.querySelector("img")).toBeNull() + }) + + it("shows explicit placeholder when src is null instead of skeleton", () => { + const { container } = render( + + ) + expect(container.querySelector(".slide-skeleton")).toBeNull() + expect(container.querySelector("[data-placeholder]")).toBeTruthy() + }) + + it("shows skeleton when src is provided but image has not loaded", () => { + const { container } = render( + + ) + expect(container.querySelector(".slide-skeleton")).toBeTruthy() + expect(container.querySelector("[data-placeholder]")).toBeNull() + }) + + it("calls onError when image fails to load", () => { + const onError = vi.fn() + const { container } = render( + + ) + const img = container.querySelector("img")! + fireEvent.error(img) + expect(onError).toHaveBeenCalledTimes(1) + }) +}) diff --git a/web-ui/src/components/deck/SlideThumbnail.tsx b/web-ui/src/components/deck/SlideThumbnail.tsx index 2b4d63d7..8375434b 100644 --- a/web-ui/src/components/deck/SlideThumbnail.tsx +++ b/web-ui/src/components/deck/SlideThumbnail.tsx @@ -3,10 +3,14 @@ /** * SlideThumbnail — Skeleton → reveal transition for a single slide preview. * - * Shows a shimmer skeleton placeholder at 16:9 aspect ratio until the image - * loads, then reveals with a staggered scale+fade animation. On src change - * (measure/generate update), resets to skeleton to maintain layout height - * and prevent scroll position shifts. + * Shows a shimmer skeleton placeholder until the image loads, then reveals + * with a staggered scale+fade animation. On src change (measure/generate + * update), resets to skeleton to maintain layout height and prevent scroll + * position shifts. + * + * The aspect ratio adapts to the actual image dimensions (detected via + * onLoad naturalWidth/naturalHeight). Falls back to 16/9 before the first + * image has loaded. */ "use client" @@ -22,11 +26,16 @@ interface SlideThumbnailProps { updated?: boolean /** data-slide-id for scroll-to-slide targeting. */ slug?: string + /** Report detected aspect ratio to parent. */ + onAspectRatio?: (ratio: number) => void + /** Called when the image fails to load (e.g. 403). */ + onError?: () => void children?: React.ReactNode } -export function SlideThumbnail({ src, alt, index, onClick, className, updated, slug, children }: SlideThumbnailProps) { +export function SlideThumbnail({ src, alt, index, onClick, className, updated, slug, onAspectRatio, onError, children }: SlideThumbnailProps) { const [loaded, setLoaded] = useState(false) + const [aspectRatio, setAspectRatio] = useState("16/9") const prevSrc = useRef(src) // Reset loaded state when src changes (triggers skeleton re-display) @@ -39,23 +48,39 @@ export function SlideThumbnail({ src, alt, index, onClick, className, updated, s return (
{ if (e.key === "Enter" || e.key === " ") { e.preventDefault(); onClick() } } : undefined} data-slide-id={slug} > - {/* Skeleton layer */} - {!loaded &&
} + {/* Skeleton layer — shown only while loading an actual src */} + {!loaded && src &&
} + + {/* Explicit placeholder when no src is available */} + {!src && ( +
+ Preview unavailable +
+ )} {/* Image layer */} {src && ( {alt} setLoaded(true)} - className="absolute inset-0 w-full h-full object-cover slide-reveal" + onLoad={(e) => { + const img = e.currentTarget + if (img.naturalWidth > 0 && img.naturalHeight > 0) { + setAspectRatio(`${img.naturalWidth}/${img.naturalHeight}`) + onAspectRatio?.(img.naturalWidth / img.naturalHeight) + } + setLoaded(true) + }} + onError={() => onError?.()} + className="absolute inset-0 w-full h-full object-contain slide-reveal" style={{ "--reveal-delay": `${index * 60}ms` } as React.CSSProperties} data-loaded={loaded} /> diff --git a/web-ui/src/components/deck/WorkspaceView.tsx b/web-ui/src/components/deck/WorkspaceView.tsx index 5cad042e..ddcc6cac 100644 --- a/web-ui/src/components/deck/WorkspaceView.tsx +++ b/web-ui/src/components/deck/WorkspaceView.tsx @@ -13,6 +13,7 @@ "use client" +import { useState } from "react" import { DeckDetail } from "@/services/deckService" import { Share2, Download, Layers } from "lucide-react" import { PreviewImage } from "@/components/ui/PreviewImage" @@ -74,31 +75,14 @@ export function WorkspaceView({ deck, onShare, onDownload }: WorkspaceViewProps) {slideCount > 0 ? (
{deck.slides.map((slide, i) => ( -
-
- {slide.previewUrl ? ( - - ) : ( -
- -
- )} -
- {i + 1} -
-
-
+ slide={slide} + index={i} + deckId={deck.deckId} + idToken={auth.user?.id_token} + t={t} + /> ))}
) : ( @@ -113,3 +97,48 @@ export function WorkspaceView({ deck, onShare, onDownload }: WorkspaceViewProps)
) } + +/** Per-slide card that detects natural image dimensions for aspect ratio. */ +function WorkspaceSlideCard({ slide, index, deckId, idToken, t }: { + slide: { slug: string; previewUrl?: string | null } + index: number + deckId: string + idToken?: string + // eslint-disable-next-line @typescript-eslint/no-explicit-any + t: any +}) { + const [aspectRatio, setAspectRatio] = useState("16/9") + + return ( +
+
+ {slide.previewUrl ? ( + ) => { + const img = e.currentTarget + if (img.naturalWidth > 0 && img.naturalHeight > 0) { + setAspectRatio(`${img.naturalWidth}/${img.naturalHeight}`) + } + }} + /> + ) : ( +
+ +
+ )} +
+ {index + 1} +
+
+
+ ) +} From 392f833d05073e3772b567d5e76a50084abdcba8 Mon Sep 17 00:00:00 2001 From: ShotaroKataoka Date: Wed, 5 Aug 2026 09:29:31 +0900 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=93=9A=20docs(aspect-ratio-agnostic-s?= =?UTF-8?q?lide-canvas):=20=E9=9D=9E16:9=E3=83=86=E3=83=B3=E3=83=97?= =?UTF-8?q?=E3=83=AC=E3=83=BC=E3=83=88=E5=AF=BE=E5=BF=9C=E3=82=92=E3=83=89?= =?UTF-8?q?=E3=82=AD=E3=83=A5=E3=83=A1=E3=83=B3=E3=83=88=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - docs/en/custom-template.md に「Slide size (aspect ratio)」節を追加。 座標系（幅1920固定・高さは比率追従）の対応表と、既知の制約 （arch 図の box 自動高さ、スタイルデモは16:9固定、 デッキ一覧サムネイルのクロップ）を明記 - CHANGELOG の [Unreleased] に #208 とライブプレビュー2件を追記 SPEC: 20260805-0100_aspect-ratio-agnostic-slide-canvas Refs: #208 --- CHANGELOG.md | 24 ++++++++++++++++++++++++ docs/en/custom-template.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c7a897..83930177 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,30 @@ Entries before v0.5.0 were written retroactively as summaries. ### Fixed +- **Custom templates with non-16:9 slide sizes (4:3 etc.) now lay out correctly** — + the engine's px coordinate system followed the template width but assumed a + fixed height of 1080, so 4:3 decks left the bottom quarter of every slide + empty, reported false out-of-bounds warnings, mismatched placeholder + coordinates, mis-measured text overflow, and rendered cropped previews in the + Web UI. The canvas is now derived from the template's real dimensions + (1920 px wide, height following the aspect ratio — 4:3 becomes 1920×1440), + and `analyze_template` / `deck.json` carry the canvas size so slide + composition uses the full slide. Behaviour for 16:9 templates is unchanged. + Known limitation: architecture diagram boxes with an omitted `box.height` + can still under-estimate text height on non-16:9 templates — specify + `box.height` explicitly. (#208) +- **Live slide preview: the first few slides now animate** — animation was + suppressed for 3 seconds after the slides tab appeared, and because a new + deck switches to that tab as soon as slides arrive, the first slides were + always shown instantly. Suppression is now based on whether the slides + already existed when the view mounted, instead of a timer. +- **Live slide preview: the PNG fallback now actually appears** — the error + state was reset on every 1-second poll, so the fallback was unmounted before + it could be seen; a failure to find the render container also marked the + slide as permanently processed, leaving an empty black box. Slides with + nothing to draw now fall back to the rendered PNG, and the fallback image + retries expired signed URLs. + - **AWS: uploaded custom templates now apply to PPTX generation** — the remote server's template resolution only searched builtin templates, so a deck referencing an uploaded user template silently fell back to diff --git a/docs/en/custom-template.md b/docs/en/custom-template.md index 99401a42..40d9d924 100644 --- a/docs/en/custom-template.md +++ b/docs/en/custom-template.md @@ -59,6 +59,34 @@ Design your template in PowerPoint, Google Slides, or Keynote (export as .pptx): - Ensure background-to-text color contrast ratio of at least 4.5:1 - Test your template by running `analyze_template` and reviewing the output +### Slide size (aspect ratio) + +Any slide size works — 16:9, 4:3, 16:10, and other ratios are all supported. +The engine derives the drawing canvas from your template's actual dimensions. + +The coordinate system is **1920 px wide, with height following the aspect ratio**: + +| Template slide size | Canvas in slide JSON | +|---|---| +| 16:9 (13.33 × 7.5 in) | 1920 × 1080 px | +| 4:3 (10 × 7.5 in) | 1920 × 1440 px | +| 16:10 | 1920 × 1200 px | + +`analyze_template` reports the canvas size as `slide_size`, and the agent records +it in `deck.json` as `slideSize` so that slide composition uses the full canvas. + +Known limitations for non-16:9 templates: + +- **Architecture diagram boxes** — when `box.height` is omitted, the engine + estimates it from the text, and that estimate is calibrated for 16:9. On other + aspect ratios it can be up to ~25% too small, causing text to overflow. + Specify `box.height` explicitly to avoid this. +- **Style demos** — the bundled style gallery HTML files use a fixed 16:9 canvas. + This does not affect generated slides (only the design tokens are consumed). +- **Deck list thumbnails** — the Web UI deck list crops thumbnails to a fixed + ratio so that card heights stay aligned in the grid. Slide previews + (workspace, carousel) follow the real aspect ratio. + --- ## Analyzing a Template