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 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) 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) 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} +
+
+
+ ) +}