diff --git a/server/app/routers/device.py b/server/app/routers/device.py index 9a044a7..1350ab0 100644 --- a/server/app/routers/device.py +++ b/server/app/routers/device.py @@ -14,6 +14,7 @@ from __future__ import annotations import logging import time +from concurrent.futures import ThreadPoolExecutor from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import FileResponse, Response @@ -23,7 +24,7 @@ from sqlalchemy.orm import Session from .. import grid, mail, quiet_hours from ..auth import get_server_settings, require_device -from ..db import frame_locked, get_db +from ..db import SessionLocal, frame_locked, get_db from ..firmware import firmware_path from ..global_actions import GLOBAL_ACTIONS from ..image_pipeline import draw_widget_border, logical_render_size, render_panel, render_placeholder, resolve_border_color @@ -82,6 +83,40 @@ def _setup_placeholder(frame: Frame, request: Request, manage: dict | None = Non ) +def _render_one_widget(frame_id: int, widget_id: int, orientation: str, panel_w: int, panel_h: int, + cell: tuple[int, int, int, int], is_normal_wake: bool, + ) -> tuple[tuple[int, int, int, int], object] | None: + """Renders exactly one widget on its own DB session, so several of + these can run concurrently in a thread pool -- see app/db.py's + module docstring: handlers already run multi-threaded (sync + handlers in FastAPI's threadpool, one process), and frame_locked/ + widget_locked's per-frame threading.Lock is what makes that safe, + not anything about which Session object is in play. A SQLAlchemy + Session itself is never safe to share across threads, so each + concurrent render gets a fresh one rather than reusing the + request's. Most of a widget's render time is spent waiting on an + external call (Immich, a weather provider, CalDAV) with the DB + untouched, which is exactly the time this buys back.""" + db = SessionLocal() + try: + frame = db.get(Frame, frame_id) + widget = db.get(Widget, widget_id) + if widget is None: + return None # deleted between the listing query and this fetch -- skip it, not a 500 + module = WIDGET_TYPES.get(widget.widget_type) + if module is None: + return None # unrecognized widget_type -- shouldn't happen, skip defensively rather than 500 + px, py, pw, ph = grid.cell_to_pixels(orientation, panel_w, panel_h, cell) + img = module.render(db, frame, widget, pw, ph, is_normal_wake=is_normal_wake) + draw_widget_border( + img, widget.border_style, widget.border_thickness, + resolve_border_color(widget.border_color_index, frame.palette_rgb), + ) + return (px, py, pw, ph), img + finally: + db.close() + + def _render_widgets(db: Session, frame: Frame, manage: dict | None, is_normal_wake: bool, as_png: bool = False, capture_snapshot: bool = False) -> bytes | tuple[bytes, bytes]: """The widget-system compositor: renders every widget on this frame @@ -92,25 +127,35 @@ def _render_widgets(db: Session, frame: Frame, manage: dict | None, is_normal_wa image_pipeline.render_panel for the single shared paste/enhance/ overlay/quantize/pack pass. Replaces the old per-mode RENDERERS dict -- a frame can now show several widgets at once instead of - exactly one mode owning the whole panel.""" + exactly one mode owning the whole panel. + + Widgets render concurrently (_render_one_widget, each on its own DB + session) rather than one at a time -- a layout with several + network-backed widgets (photos, weather, calendar) previously paid + their fetch latency serially, which could push a single /frame/* + response past the firmware's fixed HTTP timeout and show a + misleading "server failed" status screen even though the server + was simply still working. Futures are submitted in sort_order and + collected in that same order (not completion order) -- overlapping + widgets must still paint in the original z-order.""" all_widgets = db.scalars( select(Widget).where(Widget.frame_id == frame.id).order_by(Widget.sort_order) ).all() panel_w, panel_h = logical_render_size(frame.orientation) regions = [] - for widget in all_widgets: - module = WIDGET_TYPES.get(widget.widget_type) - if module is None: - continue # unrecognized widget_type -- shouldn't happen, skip defensively rather than 500 - px, py, pw, ph = grid.cell_to_pixels( - frame.orientation, panel_w, panel_h, (widget.x, widget.y, widget.w, widget.h) - ) - img = module.render(db, frame, widget, pw, ph, is_normal_wake=is_normal_wake) - draw_widget_border( - img, widget.border_style, widget.border_thickness, - resolve_border_color(widget.border_color_index, frame.palette_rgb), - ) - regions.append(((px, py, pw, ph), img)) + if all_widgets: + with ThreadPoolExecutor(max_workers=min(len(all_widgets), 8)) as pool: + futures = [ + pool.submit( + _render_one_widget, frame.id, widget.id, frame.orientation, panel_w, panel_h, + (widget.x, widget.y, widget.w, widget.h), is_normal_wake, + ) + for widget in all_widgets + ] + for future in futures: + result = future.result() + if result is not None: + regions.append(result) return render_panel( regions, orientation=frame.orientation, palette_rgb=frame.palette_rgb, color_boost=frame.color_boost, contrast_boost=frame.contrast_boost, diff --git a/server/tests/test_device_widget_dispatch.py b/server/tests/test_device_widget_dispatch.py index dfd685f..531035a 100644 --- a/server/tests/test_device_widget_dispatch.py +++ b/server/tests/test_device_widget_dispatch.py @@ -144,3 +144,50 @@ def test_two_widget_frame_composites_both_and_next_targets_calendar(client, db_s photo_cfg = db_session.get(PhotoWidgetConfig, photo_widget.id) assert cal_cfg.browse_offset == 1 assert photo_cfg.current_asset_id == "" # untouched -- NEXT was never bound to it + + +def test_widgets_render_concurrently(client, db_session, monkeypatch): + """Two independent, slow widgets on one frame should render in + roughly the time of the slowest one, not the sum -- the actual fix + for the "hold to cycle layouts times out and shows a false server- + failed status screen" bug: several network-backed widgets (photos, + weather, calendar) rendering one after another could push a single + /frame/* response past the firmware's fixed HTTP timeout even though + the server was simply still working.""" + monkeypatch.setattr(widgets.photos, "immich_client_for", lambda frame: object()) + monkeypatch.setattr(widgets.photos, "list_assets", lambda client, album_id: _ASSETS) + + from PIL import Image + source = Image.new("RGB", (100, 80), (10, 20, 30)) + + def _slow_fetch(client, mode, asset_id): + time.sleep(0.25) + return source, None + + monkeypatch.setattr(widgets.photos, "fetch_source_and_faces", _slow_fetch) + + frame = Frame( + name="Concurrency Frame", device_id="112233445566", device_token="devtok-3", + manage_token="mtok-3", orientation="landscape", created_at=time.time(), + ) + db_session.add(frame) + db_session.flush() + + widget_a = Widget(frame_id=frame.id, widget_type="photos", x=0, y=0, w=4, h=5, + sort_order=0, created_at=time.time()) + widget_b = Widget(frame_id=frame.id, widget_type="photos", x=4, y=0, w=4, h=5, + sort_order=1, created_at=time.time()) + db_session.add_all([widget_a, widget_b]) + db_session.flush() + db_session.add(PhotoWidgetConfig(widget_id=widget_a.id, album_id="album-a")) + db_session.add(PhotoWidgetConfig(widget_id=widget_b.id, album_id="album-b")) + db_session.commit() + + start = time.monotonic() + resp = client.get(f"/frame/image?id={frame.device_id}&token={frame.device_token}") + elapsed = time.monotonic() - start + + assert resp.status_code == 200 + assert len(resp.content) == EXPECTED_BYTES + # Serial would be ~0.5s (2 x 0.25s); concurrent should land near 0.25s. + assert elapsed < 0.45, f"widgets rendered serially, not concurrently ({elapsed:.2f}s)"