From 319ef715d2c44f51059c932afc31b0caa06ff333 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sun, 12 Jul 2026 09:47:32 +0100 Subject: [PATCH 1/8] feat(viewer): add page thumbnails sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New Pages tab in the viewer sidebar showing clickable page thumbnails rendered from the PDF via fitz. Click a thumbnail to jump to that page; the current page is highlighted with ACCENT and stays in sync as the user scrolls the canvas. Thumbnails render in a background QThread (a private fitz.Document copy so we don't share pymupdf state with the canvas) at 40 DPI, then smooth-scale to 120x160 on paint. Cache is capped at CACHE_MAX (200) with LRU eviction so long PDFs don't leak memory. The sidebar container is now a QTabWidget with two tabs: - Contents (existing TOC) — hidden when the PDF has no outline - Pages (new thumbnails) — always available once a doc is loaded The bookmark button on the header now toggles the whole sidebar. It becomes visible on any PDF load (previously only when TOC existed) because the Pages tab is always useful. - New: app/viewer/thumbnails.py * ThumbnailWorker (QThread) — background render + password support * ThumbnailModel (QAbstractListModel) — page slots + LRU cache * ThumbnailDelegate (QStyledItemDelegate) — custom paint with current-page ACCENT highlight and hover feedback * ThumbnailPanel (QWidget) — sidebar container + view lifecycle - Modified: app/viewer/panel.py * Wrap sidebar in QTabWidget with Contents + Pages tabs * Wire page_requested to canvas scroll_to_page * Sync current page from _update_page_label * Clear thumbnails on doc replace and closeEvent * Forward theme changes to thumbnail delegate - New i18n keys (8 languages): viewer.sidebar.contents, viewer.sidebar.pages, viewer.thumbnails.loading - Tests: 18 (source-level guards + ThumbnailModel behavioural + i18n parity + panel integration) Co-Authored-By: Claude Opus 4.7 --- app/translations.json | 24 +++ app/viewer/panel.py | 114 ++++++++++-- app/viewer/thumbnails.py | 376 +++++++++++++++++++++++++++++++++++++++ tests/test_thumbnails.py | 182 +++++++++++++++++++ 4 files changed, 680 insertions(+), 16 deletions(-) create mode 100644 app/viewer/thumbnails.py create mode 100644 tests/test_thumbnails.py diff --git a/app/translations.json b/app/translations.json index 1e3b071..232ec7a 100644 --- a/app/translations.json +++ b/app/translations.json @@ -416,6 +416,9 @@ "viewer.presentation": "Presentation", "viewer.fullscreen": "Fullscreen", "viewer.toc": "Bookmarks", + "viewer.sidebar.contents": "Contents", + "viewer.sidebar.pages": "Pages", + "viewer.thumbnails.loading": "Rendering thumbnails...", "viewer.toc_empty": "No bookmarks in this PDF", "viewer.night_mode": "Night reading mode (invert colors)", "update.changes": "What's new in this version:", @@ -1028,6 +1031,9 @@ "viewer.presentation": "Apresentação", "viewer.fullscreen": "Ecrã inteiro", "viewer.toc": "Marcadores", + "viewer.sidebar.contents": "Índice", + "viewer.sidebar.pages": "Páginas", + "viewer.thumbnails.loading": "A renderizar miniaturas...", "viewer.toc_empty": "Este PDF não tem marcadores", "viewer.night_mode": "Modo noturno (inverter cores)", "update.changes": "Novidades nesta versão:", @@ -1640,6 +1646,9 @@ "viewer.presentation": "Presentación", "viewer.fullscreen": "Pantalla completa", "viewer.toc": "Marcadores", + "viewer.sidebar.contents": "Índice", + "viewer.sidebar.pages": "Páginas", + "viewer.thumbnails.loading": "Renderizando miniaturas...", "viewer.toc_empty": "Este PDF no tiene marcadores", "viewer.night_mode": "Modo nocturno (invertir colores)", "update.changes": "Novedades en esta versión:", @@ -2252,6 +2261,9 @@ "viewer.presentation": "Présentation", "viewer.fullscreen": "Plein écran", "viewer.toc": "Signets", + "viewer.sidebar.contents": "Sommaire", + "viewer.sidebar.pages": "Pages", + "viewer.thumbnails.loading": "Génération des miniatures...", "viewer.toc_empty": "Ce PDF n'a pas de signets", "viewer.night_mode": "Mode nuit (inverser les couleurs)", "update.changes": "Nouveautés de cette version :", @@ -2864,6 +2876,9 @@ "viewer.presentation": "Präsentation", "viewer.fullscreen": "Vollbild", "viewer.toc": "Lesezeichen", + "viewer.sidebar.contents": "Inhalt", + "viewer.sidebar.pages": "Seiten", + "viewer.thumbnails.loading": "Miniaturansichten werden erstellt...", "viewer.toc_empty": "Dieses PDF hat keine Lesezeichen", "viewer.night_mode": "Nachtmodus (Farben invertieren)", "update.changes": "Neuigkeiten in dieser Version:", @@ -3476,6 +3491,9 @@ "viewer.presentation": "演示", "viewer.fullscreen": "全屏", "viewer.toc": "书签", + "viewer.sidebar.contents": "目录", + "viewer.sidebar.pages": "页面", + "viewer.thumbnails.loading": "生成缩略图...", "viewer.toc_empty": "此PDF没有书签", "viewer.night_mode": "夜间阅读模式（反转颜色）", "update.changes": "此版本的新功能：", @@ -4088,6 +4106,9 @@ "viewer.presentation": "Presentazione", "viewer.fullscreen": "Schermo intero", "viewer.toc": "Segnalibri", + "viewer.sidebar.contents": "Sommario", + "viewer.sidebar.pages": "Pagine", + "viewer.thumbnails.loading": "Generazione miniature...", "viewer.toc_empty": "Questo PDF non ha segnalibri", "viewer.night_mode": "Modalità notturna (inverti colori)", "update.changes": "Novità in questa versione:", @@ -4700,6 +4721,9 @@ "viewer.presentation": "Presentatie", "viewer.fullscreen": "Volledig scherm", "viewer.toc": "Bladwijzers", + "viewer.sidebar.contents": "Inhoud", + "viewer.sidebar.pages": "Pagina's", + "viewer.thumbnails.loading": "Miniaturen genereren...", "viewer.toc_empty": "Deze PDF heeft geen bladwijzers", "viewer.night_mode": "Nachtmodus (kleuren inverteren)", "update.changes": "Wat is er nieuw in deze versie:", diff --git a/app/viewer/panel.py b/app/viewer/panel.py index fc7f082..ffba993 100644 --- a/app/viewer/panel.py +++ b/app/viewer/panel.py @@ -6,7 +6,7 @@ from PySide6.QtWidgets import ( QWidget, QVBoxLayout, QHBoxLayout, QLabel, QPushButton, QScrollArea, QFrame, QFileDialog, QMessageBox, QDialog, - QLineEdit, QSplitter, QTreeWidget, QTreeWidgetItem, + QLineEdit, QSplitter, QTabWidget, QTreeWidget, QTreeWidgetItem, ) from PySide6.QtGui import QKeySequence, QShortcut from shiboken6 import isValid @@ -16,6 +16,7 @@ from app.utils import _paint_bg, normalize_password from app.i18n import t from app.viewer.canvas import _SelectCanvas +from app.viewer.thumbnails import ThumbnailPanel class PdfViewerPanel(QWidget): @@ -143,13 +144,29 @@ def _a11y(btn, tip): self._placeholder = ph_widget layout.addWidget(self._placeholder, 1) - # ── TOC tree (left of canvas) ────────────────────────────────── + # ── TOC tree (Contents tab of the sidebar) ───────────────────── self._toc_tree = QTreeWidget() self._toc_tree.setObjectName("toc_tree") self._toc_tree.setHeaderHidden(True) self._toc_tree.setMinimumWidth(180) self._toc_tree.itemClicked.connect(self._on_toc_clicked) + # ── Thumbnails (Pages tab of the sidebar) ─────────────────────── + self._thumbnails = ThumbnailPanel(self) + self._thumbnails.page_requested.connect(self._on_thumbnail_clicked) + + # Sidebar tab widget: [Contents | Pages]. Individual tabs are + # shown/hidden dynamically: Contents only when the PDF has a + # non-empty TOC, Pages always once a doc is loaded. + self._sidebar_tabs = QTabWidget() + self._sidebar_tabs.setObjectName("viewer_sidebar_tabs") + self._sidebar_tabs.setDocumentMode(True) + self._sidebar_tabs.setMinimumWidth(180) + self._toc_tab_idx = self._sidebar_tabs.addTab( + self._toc_tree, t("viewer.sidebar.contents")) + self._pages_tab_idx = self._sidebar_tabs.addTab( + self._thumbnails, t("viewer.sidebar.pages")) + # ── Canvas with continuous scroll of all pages ────────────────── self._canvas = _SelectCanvas() self._canvas.zoom_changed.connect(self._on_zoom_changed) @@ -162,9 +179,9 @@ def _a11y(btn, tip): self._canvas_scroll.viewport().installEventFilter(self) self._canvas_scroll.verticalScrollBar().valueChanged.connect(self._on_scroll) - # Splitter: TOC | canvas + # Splitter: sidebar | canvas self._viewer_splitter = QSplitter(Qt.Orientation.Horizontal) - self._viewer_splitter.addWidget(self._toc_tree) + self._viewer_splitter.addWidget(self._sidebar_tabs) self._viewer_splitter.addWidget(self._canvas_scroll) self._viewer_splitter.setStretchFactor(0, 0) self._viewer_splitter.setStretchFactor(1, 1) @@ -172,7 +189,7 @@ def _a11y(btn, tip): self._viewer_splitter.setCollapsible(0, True) self._viewer_splitter.setCollapsible(1, False) self._viewer_splitter.setVisible(False) - self._toc_tree.setVisible(False) # hidden until a PDF with TOC is loaded + self._sidebar_tabs.setVisible(False) # hidden until a PDF is loaded layout.addWidget(self._viewer_splitter, 1) # ── Search bar (Ctrl+F) ─────────────────────────────────────────── @@ -300,6 +317,8 @@ def update_theme(self, dark: bool) -> None: link.setStyleSheet(link_style) for btn in self._recent_del_btns: btn.setIcon(qta.icon("fa5s.trash-alt", color=c)) + # Thumbnail delegate re-reads dark flag to pick hover colour. + self._thumbnails.update_theme(dark) # Drag & drop is handled at the MainWindow level (see window.py). @@ -405,8 +424,30 @@ def current_path(self) -> str: return self._current_path # ── TOC / Bookmarks ───────────────────────────────────────────────── + def _set_toc_tab_visible(self, visible: bool) -> None: + """Show/hide the Contents tab without removing it from the tab + widget. Qt has no first-class ``setTabVisible`` on older + PySide6 releases, but 6.11 does — guard with hasattr so we + keep working on legacy bindings by inserting/removing instead. + """ + if hasattr(self._sidebar_tabs, "setTabVisible"): + self._sidebar_tabs.setTabVisible(self._toc_tab_idx, visible) + return + # Legacy path: remove/insert. Best-effort; safe if already gone. + present = self._sidebar_tabs.indexOf(self._toc_tree) != -1 + if visible and not present: + self._sidebar_tabs.insertTab( + 0, self._toc_tree, t("viewer.sidebar.contents")) + self._toc_tab_idx = 0 + # Pages moved to index 1 after re-insert. + self._pages_tab_idx = self._sidebar_tabs.indexOf(self._thumbnails) + elif not visible and present: + self._sidebar_tabs.removeTab(self._toc_tab_idx) + self._pages_tab_idx = self._sidebar_tabs.indexOf(self._thumbnails) + def _populate_toc(self, doc): - """Read the PDF outline and build the tree. Hides the panel if empty. + """Read the PDF outline and build the tree. Hides the Contents + tab (but keeps Pages) if the outline is empty. Wrapped in try/except: a malformed outline (cyclic refs, bad page indexes, unexpected entry shape) used to leave the TOC @@ -424,8 +465,12 @@ def _populate_toc(self, doc): "Failed to read TOC for %s: %s", self._current_path, exc) toc = [] if not toc: - self._toc_tree.setVisible(False) - self._toc_btn.setVisible(False) + self._set_toc_tab_visible(False) + # Pages tab is still there → activate it so the user sees + # something useful when they open the sidebar. + pages_idx = self._sidebar_tabs.indexOf(self._thumbnails) + if pages_idx >= 0: + self._sidebar_tabs.setCurrentIndex(pages_idx) return try: # toc is a list of [level, title, page] (page is 1-indexed) @@ -439,9 +484,7 @@ def _populate_toc(self, doc): item.setToolTip(0, title) stack.append((level, item)) self._toc_tree.expandToDepth(1) - self._toc_tree.setVisible(True) - self._toc_btn.setVisible(True) - self._toc_btn.setEnabled(True) + self._set_toc_tab_visible(True) except Exception as exc: import logging logging.getLogger(__name__).warning( @@ -450,8 +493,7 @@ def _populate_toc(self, doc): # Reset to a known-empty state so a partial build does not # leave dangling QTreeWidgetItems pointing at invalid pages. self._toc_tree.clear() - self._toc_tree.setVisible(False) - self._toc_btn.setVisible(False) + self._set_toc_tab_visible(False) def _on_toc_clicked(self, item, column): page_idx = item.data(0, Qt.ItemDataRole.UserRole) @@ -460,11 +502,24 @@ def _on_toc_clicked(self, item, column): y = self._canvas.scroll_to_page(int(page_idx)) self._canvas_scroll.verticalScrollBar().setValue(y) + def _on_thumbnail_clicked(self, page_idx: int) -> None: + """User clicked a page thumbnail — scroll the canvas there.""" + y = self._canvas.scroll_to_page(int(page_idx)) + self._canvas_scroll.verticalScrollBar().setValue(y) + def _toggle_toc(self): - visible = self._toc_tree.isVisible() - self._toc_tree.setVisible(not visible) + """Toggle the whole sidebar (Contents + Pages tab widget). + + The button still lives on the header with the bookmark icon — + the semantics widened when thumbnails joined the sidebar, but + renaming the attribute would break existing accessibility hooks + and translation strings on this release. + """ + visible = self._sidebar_tabs.isVisible() + self._sidebar_tabs.setVisible(not visible) if not visible: - self._viewer_splitter.setSizes([220, max(800, self._viewer_splitter.width() - 220)]) + self._viewer_splitter.setSizes( + [220, max(800, self._viewer_splitter.width() - 220)]) # Re-layout pages after splitter change if self._canvas._doc and self._canvas._zoom_factor == 1.0: from PySide6.QtCore import QTimer @@ -492,6 +547,9 @@ def load(self, path: str): if self._fitz_doc: self._canvas.close_doc() self._fitz_doc = None + # Stop the thumbnail worker and drop cached pixmaps of the + # previous doc before we point the panel at a new file. + self._thumbnails.clear() # Forget the previous file's password before we start the # new one's prompt flow (R5/D2). _clear_pdf_password is a # best-effort wipe — Python str immutability blocks a true @@ -527,12 +585,27 @@ def load(self, path: str): self._canvas_scroll.verticalScrollBar().setValue(0) self._placeholder.setVisible(False) self._viewer_splitter.setVisible(True) + # Sidebar is always available once a PDF loads — thumbnails + # don't depend on the PDF having a TOC. + self._sidebar_tabs.setVisible(True) self._sel_status.setVisible(True) self._name_lbl.setText(os.path.basename(path)) self._zoom_lbl.setText(t("zoom.fit")) for btn in (self._zoom_out_btn, self._zoom_in_btn, self._fit_btn, self._print_btn, self._night_btn): btn.setEnabled(True) + # The sidebar toggle used to live behind a "PDF has TOC" gate; + # now that Pages is a first-class tab the button is always + # meaningful. Keep the icon/tooltip — see _toggle_toc note. + self._toc_btn.setVisible(True) + self._toc_btn.setEnabled(True) + # Populate thumbnails BEFORE the TOC so the worker starts + # rendering while _populate_toc walks the outline on the UI + # thread. + self._thumbnails.set_document( + path, doc.page_count, + password=getattr(self, "_pdf_password", "")) + self._thumbnails.set_current_page(0) self._populate_toc(doc) # ── Search ────────────────────────────────────────────────────────── @@ -643,6 +716,9 @@ def _update_page_label(self): self._page_lbl.setText(f"{idx + 1} / {total}") self._prev_btn.setEnabled(idx > 0) self._next_btn.setEnabled(idx < total - 1) + # Keep the thumbnail sidebar highlight in sync with whichever + # page the user is currently reading in the canvas. + self._thumbnails.set_current_page(idx) def _prev_page(self): if not self._canvas._entries: @@ -769,4 +845,10 @@ def closeEvent(self, event): # Wipe cached password before the C++ widget is destroyed so a # heap dump after teardown no longer surfaces it. self._clear_pdf_password() + # Cancel + join the thumbnail render thread so tearing the + # panel down doesn't leak a QThread past the C++ deletion. + try: + self._thumbnails.clear() + except Exception: + pass super().closeEvent(event) diff --git a/app/viewer/thumbnails.py b/app/viewer/thumbnails.py new file mode 100644 index 0000000..a9964b9 --- /dev/null +++ b/app/viewer/thumbnails.py @@ -0,0 +1,376 @@ +"""PDFApps – PDF page thumbnails panel for the viewer sidebar. + +A lightweight ``QListView`` that shows one clickable thumbnail per +page. Thumbnails are rendered off the UI thread by a ``QThread`` worker +(pymupdf's ``fitz.open`` is not thread-safe across the same +``Document`` handle, so the worker opens its own copy) and cached in an +insertion-ordered dict capped at ``CACHE_MAX`` entries — long documents +never blow up memory. + +Public API +---------- +``ThumbnailPanel.set_document(path, page_count, password="")`` + Load a document. Resets the model and kicks off rendering. + +``ThumbnailPanel.clear()`` + Unload the current document. + +``ThumbnailPanel.set_current_page(idx)`` + Highlight + scroll to a page. + +``ThumbnailPanel.page_requested (Signal[int])`` + User clicked / keyed a thumbnail. The viewer should scroll canvas + there. + +``ThumbnailPanel.update_theme(dark)`` + Repaint delegate so ACCENT / TEXT_SEC track the theme. +""" + +from __future__ import annotations + +import contextlib + +from PySide6.QtCore import ( + QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, Signal, +) +from PySide6.QtGui import QColor, QImage, QPainter, QPixmap +from PySide6.QtWidgets import ( + QAbstractItemView, QListView, QStyle, QStyledItemDelegate, + QVBoxLayout, QWidget, +) + +from app.constants import ACCENT, TEXT_SEC + + +THUMB_WIDTH = 120 # px – target thumbnail width before aspect-fit +THUMB_HEIGHT = 160 # px – target thumbnail height (portrait ~ 1.33) +THUMB_PADDING = 12 # px around the thumbnail inside a row +PAGE_NUM_HEIGHT = 20 # px reserved for the page-number label +CACHE_MAX = 200 # LRU-ish cap on cached QPixmaps + + +# ── Worker ──────────────────────────────────────────────────────────── + + +class ThumbnailWorker(QThread): + """Render a batch of page thumbnails in a background thread. + + The worker opens its own ``fitz.Document`` copy — pymupdf documents + are NOT thread-safe across the main-thread handle used by the + canvas. Password is applied when set. ``cancel()`` sets a flag the + render loop polls between pages; wait 2 s in the caller for a + graceful shutdown before dropping the reference. + """ + + thumbnail_ready = Signal(int, QPixmap) # (page_index, thumbnail) + + def __init__(self, doc_path: str, page_indices: list[int], + password: str = "", parent=None) -> None: + super().__init__(parent) + self._doc_path = doc_path + self._pages = list(page_indices) + self._password = password + self._cancelled = False + + def cancel(self) -> None: + self._cancelled = True + + def run(self) -> None: + import fitz # local — keeps import cost off UI startup path + + try: + doc = fitz.open(self._doc_path) + except Exception: + return + try: + if self._password and doc.needs_pass: + with contextlib.suppress(Exception): + doc.authenticate(self._password) + page_count = doc.page_count + for idx in self._pages: + if self._cancelled: + break + if idx < 0 or idx >= page_count: + continue + try: + page = doc[idx] + # 40 DPI ≈ 330×467 px for A4 — plenty of detail + # for a 120×160 downscale with smooth transform. + pix = page.get_pixmap(dpi=40, alpha=False, annots=False) + if pix.n != 3: + pix = fitz.Pixmap(fitz.csRGB, pix) + # ``QImage`` views ``pix.samples`` directly; the + # Pixmap is freed on the next loop iteration, so + # ``.copy()`` an eager copy of the pixels BEFORE + # emitting — otherwise the receiver sees a + # dangling buffer (same fix pattern as the print + # loop and OCR round 3). + img = QImage( + pix.samples, pix.width, pix.height, + pix.stride, QImage.Format.Format_RGB888, + ).copy() + self.thumbnail_ready.emit(idx, QPixmap.fromImage(img)) + except Exception: + # Skip page — bad object / partial doc, keep going. + continue + finally: + with contextlib.suppress(Exception): + doc.close() + + +# ── Model ───────────────────────────────────────────────────────────── + + +class ThumbnailModel(QAbstractListModel): + """List model: one row per page. Decoration = QPixmap thumbnail.""" + + def __init__(self, parent=None) -> None: + super().__init__(parent) + self._page_count = 0 + self._cache: dict[int, QPixmap] = {} + self._doc_path = "" + + def rowCount(self, parent=QModelIndex()) -> int: # noqa: B008 + return 0 if parent.isValid() else self._page_count + + def data(self, index: QModelIndex, role=Qt.ItemDataRole.DisplayRole): + row = index.row() + if row < 0 or row >= self._page_count: + return None + if role == Qt.ItemDataRole.DecorationRole: + return self._cache.get(row) # None → delegate paints placeholder + if role == Qt.ItemDataRole.DisplayRole: + return str(row + 1) + return None + + def set_document(self, doc_path: str, page_count: int) -> None: + self.beginResetModel() + self._doc_path = doc_path + self._page_count = max(0, int(page_count)) + self._cache.clear() + self.endResetModel() + + def clear(self) -> None: + self.set_document("", 0) + + def cache_pixmap(self, page_idx: int, pix: QPixmap) -> None: + # Refresh insertion order for LRU eviction: pop + re-add. + if page_idx in self._cache: + self._cache.pop(page_idx, None) + self._cache[page_idx] = pix + # Evict oldest entries until we're back under the cap. ``dict`` + # preserves insertion order since Python 3.7, so the first + # iterator element is the oldest. + while len(self._cache) > CACHE_MAX: + oldest = next(iter(self._cache)) + del self._cache[oldest] + idx = self.index(page_idx) + self.dataChanged.emit(idx, idx, [Qt.ItemDataRole.DecorationRole]) + + # Expose the cache size for tests / debug. + def cache_size(self) -> int: + return len(self._cache) + + +# ── Delegate ────────────────────────────────────────────────────────── + + +class ThumbnailDelegate(QStyledItemDelegate): + """Paint each row: thumbnail + page number, with current-page + highlight in ACCENT and hover feedback.""" + + def __init__(self, parent=None) -> None: + super().__init__(parent) + self._current_page = -1 + self._dark = True + + def set_current_page(self, page_idx: int) -> int: + old = self._current_page + self._current_page = page_idx + return old + + def set_dark(self, dark: bool) -> None: + self._dark = bool(dark) + + def sizeHint(self, option, index): # noqa: D401 + return QSize( + THUMB_WIDTH + 2 * THUMB_PADDING, + THUMB_HEIGHT + 2 * THUMB_PADDING + PAGE_NUM_HEIGHT, + ) + + def paint(self, painter: QPainter, option, index: QModelIndex) -> None: + page_idx = index.row() + rect = option.rect + pix = index.data(Qt.ItemDataRole.DecorationRole) + + painter.save() + painter.setRenderHint(QPainter.RenderHint.Antialiasing) + + # Row background — current selection or hover feedback. + if page_idx == self._current_page: + accent = QColor(ACCENT) + accent.setAlpha(60) + painter.setBrush(accent) + painter.setPen(Qt.PenStyle.NoPen) + painter.drawRoundedRect(rect.adjusted(4, 4, -4, -4), 6, 6) + elif option.state & QStyle.StateFlag.State_MouseOver: + hover = QColor(255, 255, 255, 20) if self._dark \ + else QColor(0, 0, 0, 20) + painter.setBrush(hover) + painter.setPen(Qt.PenStyle.NoPen) + painter.drawRoundedRect(rect.adjusted(4, 4, -4, -4), 6, 6) + + # Thumbnail region. + thumb_x = rect.x() + THUMB_PADDING + thumb_y = rect.y() + THUMB_PADDING + thumb_rect = QRect(thumb_x, thumb_y, THUMB_WIDTH, THUMB_HEIGHT) + + if pix is not None and not pix.isNull(): + scaled = pix.scaled( + THUMB_WIDTH, THUMB_HEIGHT, + Qt.AspectRatioMode.KeepAspectRatio, + Qt.TransformationMode.SmoothTransformation, + ) + cx = thumb_x + (THUMB_WIDTH - scaled.width()) // 2 + cy = thumb_y + (THUMB_HEIGHT - scaled.height()) // 2 + painter.drawPixmap(cx, cy, scaled) + # 1 px hairline so a white-page thumbnail is still visible + # against a light background. + painter.setPen(QColor(0, 0, 0, 60)) + painter.setBrush(Qt.BrushStyle.NoBrush) + painter.drawRect(cx, cy, scaled.width() - 1, scaled.height() - 1) + else: + # Placeholder while the worker catches up. + painter.setPen(QColor(TEXT_SEC)) + painter.setBrush(Qt.BrushStyle.NoBrush) + painter.drawRect(thumb_rect) + painter.drawText(thumb_rect, Qt.AlignmentFlag.AlignCenter, "…") + + # Page number label under the thumbnail. + if page_idx == self._current_page: + painter.setPen(QColor(ACCENT)) + else: + painter.setPen(QColor(TEXT_SEC)) + num_rect = QRect( + rect.x(), thumb_y + THUMB_HEIGHT + 2, + rect.width(), PAGE_NUM_HEIGHT, + ) + painter.drawText(num_rect, Qt.AlignmentFlag.AlignCenter, + str(page_idx + 1)) + + painter.restore() + + +# ── Panel ───────────────────────────────────────────────────────────── + + +class ThumbnailPanel(QWidget): + """Sidebar container hosting the QListView of thumbnails. + + Emits :pyattr:`page_requested` when the user clicks / activates a + thumbnail. Owns the current ``ThumbnailWorker`` and shuts it down + cleanly on doc change / close event. + """ + + page_requested = Signal(int) # 0-based page index + + def __init__(self, parent=None) -> None: + super().__init__(parent) + self.setObjectName("thumbnail_panel") + self._doc_path = "" + self._password = "" + self._worker: ThumbnailWorker | None = None + + layout = QVBoxLayout(self) + layout.setContentsMargins(0, 0, 0, 0) + layout.setSpacing(0) + + self._model = ThumbnailModel(self) + self._delegate = ThumbnailDelegate(self) + + self._view = QListView(self) + self._view.setModel(self._model) + self._view.setItemDelegate(self._delegate) + self._view.setViewMode(QListView.ViewMode.ListMode) + self._view.setUniformItemSizes(True) + self._view.setMouseTracking(True) + self._view.setEditTriggers( + QAbstractItemView.EditTrigger.NoEditTriggers) + self._view.setSelectionMode( + QAbstractItemView.SelectionMode.SingleSelection) + self._view.setVerticalScrollMode( + QAbstractItemView.ScrollMode.ScrollPerPixel) + self._view.setHorizontalScrollBarPolicy( + Qt.ScrollBarPolicy.ScrollBarAlwaysOff) + self._view.clicked.connect(self._on_activated) + self._view.activated.connect(self._on_activated) # keyboard Enter + layout.addWidget(self._view) + + # ── Public API ──────────────────────────────────────────────── + + def set_document(self, doc_path: str, page_count: int, + password: str = "") -> None: + """Reset model + kick off background rendering.""" + self._doc_path = doc_path + self._password = password or "" + self._stop_worker() + self._model.set_document(doc_path, page_count) + self._delegate.set_current_page(-1) + if page_count > 0 and doc_path: + self._start_worker(list(range(page_count))) + + def clear(self) -> None: + """Unload the current document.""" + self._doc_path = "" + self._password = "" + self._stop_worker() + self._model.clear() + self._delegate.set_current_page(-1) + + def set_current_page(self, page_idx: int) -> None: + """Highlight given page and scroll to it (if visible).""" + if not (0 <= page_idx < self._model.rowCount()): + return + old = self._delegate.set_current_page(page_idx) + idx = self._model.index(page_idx) + self._view.setCurrentIndex(idx) + self._view.scrollTo( + idx, QAbstractItemView.ScrollHint.EnsureVisible) + # Repaint both the previous and the new row so the highlight + # tracks correctly even though the model data didn't change. + vp = self._view.viewport() + if old >= 0: + old_rect = self._view.visualRect(self._model.index(old)) + vp.update(old_rect) + vp.update(self._view.visualRect(idx)) + + def update_theme(self, dark: bool) -> None: + """Repaint so ACCENT / TEXT_SEC track the theme.""" + self._delegate.set_dark(dark) + self._view.viewport().update() + + # ── Internals ───────────────────────────────────────────────── + + def _on_activated(self, index: QModelIndex) -> None: + if index.isValid(): + self.page_requested.emit(index.row()) + + def _start_worker(self, page_indices: list[int]) -> None: + self._worker = ThumbnailWorker( + self._doc_path, page_indices, self._password, self) + self._worker.thumbnail_ready.connect(self._model.cache_pixmap) + self._worker.start() + + def _stop_worker(self) -> None: + if self._worker is None: + return + self._worker.cancel() + # 2 s is generous — a single page render at 40 DPI is well + # under 100 ms; the cancel flag is polled between pages so + # worst case we wait for the current page to finish. + self._worker.wait(2000) + self._worker = None + + def closeEvent(self, event): # noqa: D401 + self._stop_worker() + super().closeEvent(event) diff --git a/tests/test_thumbnails.py b/tests/test_thumbnails.py new file mode 100644 index 0000000..419bbac --- /dev/null +++ b/tests/test_thumbnails.py @@ -0,0 +1,182 @@ +"""Regression tests for the viewer thumbnails sidebar. + +Blends source-level guards (cheap, no Qt event loop needed) with +behavioural tests on the ``ThumbnailModel`` (which is pure Qt data, no +threading). Full end-to-end rendering requires a real ``QApplication`` +and is exercised by the smoke import + manual QA on the release +candidate — see the parent task's manual test plan. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT)) + + +THUMBS_SRC = (ROOT / "app" / "viewer" / "thumbnails.py").read_text( + encoding="utf-8") +PANEL_SRC = (ROOT / "app" / "viewer" / "panel.py").read_text( + encoding="utf-8") + + +# ── Import / API surface ────────────────────────────────────────────── + + +def test_module_imports(): + from app.viewer import thumbnails + assert hasattr(thumbnails, "ThumbnailPanel") + assert hasattr(thumbnails, "ThumbnailModel") + assert hasattr(thumbnails, "ThumbnailDelegate") + assert hasattr(thumbnails, "ThumbnailWorker") + + +def test_public_signal_present(): + """Guards against a rename of the signal the viewer wires into.""" + assert "page_requested" in THUMBS_SRC + assert "Signal(int)" in THUMBS_SRC + + +def test_worker_uses_qthread(): + assert "class ThumbnailWorker(QThread)" in THUMBS_SRC + + +def test_worker_pixmap_lifetime_uses_copy(): + """The QImage-from-Pixmap pattern requires .copy() so pixels don't + disappear when the fitz Pixmap is freed on the next loop iteration + (same pattern as the print loop and OCR Round 3).""" + assert ".copy()" in THUMBS_SRC + + +def test_worker_passes_password_to_fitz(): + """Encrypted PDFs would raise on every page render without this.""" + assert "authenticate" in THUMBS_SRC + + +def test_delegate_highlights_current_page(): + assert "set_current_page" in THUMBS_SRC + assert "_current_page" in THUMBS_SRC + + +def test_cache_has_size_limit(): + """The dict cache must be capped so long PDFs don't leak memory.""" + assert "CACHE_MAX" in THUMBS_SRC + + +# ── Model behaviour ─────────────────────────────────────────────────── + + +@pytest.fixture(scope="module") +def qapp(): + """Session-scoped QApplication for the model tests. QListView is + not built here, but QAbstractListModel / QModelIndex constructors + dereference the Qt runtime, so an app must exist.""" + from PySide6.QtWidgets import QApplication + app = QApplication.instance() or QApplication(sys.argv) + yield app + + +def test_model_starts_empty(qapp): + from app.viewer.thumbnails import ThumbnailModel + m = ThumbnailModel() + assert m.rowCount() == 0 + assert m.cache_size() == 0 + + +def test_model_row_count_matches_page_count(qapp): + from app.viewer.thumbnails import ThumbnailModel + m = ThumbnailModel() + m.set_document("/nonexistent.pdf", 10) + assert m.rowCount() == 10 + m.clear() + assert m.rowCount() == 0 + assert m.cache_size() == 0 + + +def test_model_cache_evicts_lru(qapp): + """Insert CACHE_MAX + 5 pixmaps and confirm the oldest 5 are + evicted so the cache stays at the cap.""" + from PySide6.QtGui import QPixmap + from app.viewer.thumbnails import CACHE_MAX, ThumbnailModel + m = ThumbnailModel() + m.set_document("/x.pdf", CACHE_MAX + 5) + for i in range(CACHE_MAX + 5): + m.cache_pixmap(i, QPixmap(1, 1)) + assert m.cache_size() == CACHE_MAX + + +def test_model_data_returns_none_out_of_range(qapp): + from PySide6.QtCore import Qt + from app.viewer.thumbnails import ThumbnailModel + m = ThumbnailModel() + m.set_document("/x.pdf", 3) + bad = m.index(99) + assert m.data(bad, Qt.ItemDataRole.DecorationRole) is None + good = m.index(0) + # No pixmap cached yet → decoration is None (delegate paints ... + # placeholder), display returns 1-based page number. + assert m.data(good, Qt.ItemDataRole.DecorationRole) is None + assert m.data(good, Qt.ItemDataRole.DisplayRole) == "1" + + +# ── Panel integration ──────────────────────────────────────────────── + + +def test_panel_wires_thumbnail_panel(): + """Viewer must instantiate ThumbnailPanel and route its signal.""" + assert "ThumbnailPanel" in PANEL_SRC + assert "page_requested" in PANEL_SRC + assert "_on_thumbnail_clicked" in PANEL_SRC + + +def test_panel_syncs_current_page_on_scroll(): + """Scrolling the canvas must update the thumbnail highlight.""" + assert "self._thumbnails.set_current_page(idx)" in PANEL_SRC + + +def test_panel_loads_thumbnails_on_open(): + assert "self._thumbnails.set_document(" in PANEL_SRC + + +def test_panel_clears_thumbnails_on_close_and_replace(): + """Both the closeEvent and the doc-replace path must stop the + worker so a stale QThread doesn't outlive the panel.""" + assert PANEL_SRC.count("self._thumbnails.clear()") >= 2 + + +def test_panel_updates_thumbnails_theme(): + assert "self._thumbnails.update_theme(dark)" in PANEL_SRC + + +# ── i18n ────────────────────────────────────────────────────────────── + + +def test_i18n_keys_present_in_all_langs(): + with open(ROOT / "app" / "translations.json", encoding="utf-8") as f: + data = json.load(f) + langs = ["en", "pt", "es", "fr", "de", "it", "nl", "zh"] + for lang in langs: + assert lang in data, f"language {lang} missing" + for key in ("viewer.sidebar.pages", + "viewer.sidebar.contents", + "viewer.thumbnails.loading"): + assert key in data[lang], f"{lang}: {key} missing" + assert data[lang][key].strip(), f"{lang}: {key} empty" + + +def test_i18n_key_parity(): + """Every language dict must expose exactly the same keys.""" + with open(ROOT / "app" / "translations.json", encoding="utf-8") as f: + data = json.load(f) + reference = set(data["en"].keys()) + for lang, entries in data.items(): + assert set(entries.keys()) == reference, ( + f"{lang} diverges: " + f"+{set(entries) - reference} -{reference - set(entries)}" + ) From cdf3efcdca72b1e135295407e7d1702888dfb5ec Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sun, 12 Jul 2026 11:30:20 +0100 Subject: [PATCH 2/8] fix(viewer): construct QPixmap in main thread, not worker Qt requires QPixmap to be created/mutated only in the main GUI thread. The initial ThumbnailWorker created QPixmap.fromImage() in the worker thread, which silently failed (or would crash on some platforms) - no thumbnails ever appeared in the sidebar. Now: - Worker emits QImage (thread-safe) via thumbnail_ready signal - ThumbnailPanel._on_image_ready slot converts to QPixmap on the main thread, then hands it to the model - Connection uses explicit Qt.ConnectionType.QueuedConnection to respect Py3.14 PySide6's stricter cross-thread routing (see memory/project_compress_freeze_py314.md) - @Slot decorator on the receiver for the same reason Also adds debug logging in worker + panel for future diagnostics. --- app/viewer/thumbnails.py | 48 ++++++++++++++++++++++++++++++++++++---- tests/test_thumbnails.py | 26 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/app/viewer/thumbnails.py b/app/viewer/thumbnails.py index a9964b9..0e4f592 100644 --- a/app/viewer/thumbnails.py +++ b/app/viewer/thumbnails.py @@ -29,9 +29,10 @@ from __future__ import annotations import contextlib +import logging from PySide6.QtCore import ( - QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, Signal, + QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, Signal, Slot, ) from PySide6.QtGui import QColor, QImage, QPainter, QPixmap from PySide6.QtWidgets import ( @@ -42,6 +43,9 @@ from app.constants import ACCENT, TEXT_SEC +_log = logging.getLogger(__name__) + + THUMB_WIDTH = 120 # px – target thumbnail width before aspect-fit THUMB_HEIGHT = 160 # px – target thumbnail height (portrait ~ 1.33) THUMB_PADDING = 12 # px around the thumbnail inside a row @@ -62,7 +66,12 @@ class ThumbnailWorker(QThread): graceful shutdown before dropping the reference. """ - thumbnail_ready = Signal(int, QPixmap) # (page_index, thumbnail) + # Emits QImage (thread-safe) rather than QPixmap: Qt requires + # QPixmap to be constructed / mutated only in the main GUI thread, + # so the receiver slot ``ThumbnailPanel._on_image_ready`` performs + # the ``QPixmap.fromImage()`` conversion on the main thread before + # handing the pixmap to the model. + thumbnail_ready = Signal(int, QImage) # (page_index, thumbnail) def __init__(self, doc_path: str, page_indices: list[int], password: str = "", parent=None) -> None: @@ -93,6 +102,10 @@ def run(self) -> None: if idx < 0 or idx >= page_count: continue try: + _log.debug( + "ThumbnailWorker: rendering page %d/%d", + idx + 1, page_count, + ) page = doc[idx] # 40 DPI ≈ 330×467 px for A4 — plenty of detail # for a 120×160 downscale with smooth transform. @@ -105,11 +118,16 @@ def run(self) -> None: # emitting — otherwise the receiver sees a # dangling buffer (same fix pattern as the print # loop and OCR round 3). + # + # Emit QImage (thread-safe) — the main-thread slot + # converts to QPixmap. Constructing QPixmap here in + # the worker thread is invalid per Qt's threading + # model (silent no-op or crash on some platforms). img = QImage( pix.samples, pix.width, pix.height, pix.stride, QImage.Format.Format_RGB888, ).copy() - self.thumbnail_ready.emit(idx, QPixmap.fromImage(img)) + self.thumbnail_ready.emit(idx, img) except Exception: # Skip page — bad object / partial doc, keep going. continue @@ -358,9 +376,31 @@ def _on_activated(self, index: QModelIndex) -> None: def _start_worker(self, page_indices: list[int]) -> None: self._worker = ThumbnailWorker( self._doc_path, page_indices, self._password, self) - self._worker.thumbnail_ready.connect(self._model.cache_pixmap) + # Explicit QueuedConnection so the slot runs on this (main) + # thread rather than the worker — required for QPixmap + # construction, and matches the Py3.14 PySide6 routing rules + # documented in memory/project_compress_freeze_py314.md. + self._worker.thumbnail_ready.connect( + self._on_image_ready, + Qt.ConnectionType.QueuedConnection, + ) self._worker.start() + @Slot(int, QImage) + def _on_image_ready(self, page_idx: int, img: QImage) -> None: + """Runs on the main GUI thread. Convert the worker's QImage + into a QPixmap and hand it to the model. + + QPixmap construction is only valid on the main thread; doing it + here (instead of in the worker) is why the parent signal + carries QImage.""" + _log.debug( + "Thumbnail received for page %d (%dx%d)", + page_idx + 1, img.width(), img.height(), + ) + pix = QPixmap.fromImage(img) + self._model.cache_pixmap(page_idx, pix) + def _stop_worker(self) -> None: if self._worker is None: return diff --git a/tests/test_thumbnails.py b/tests/test_thumbnails.py index 419bbac..52d2b48 100644 --- a/tests/test_thumbnails.py +++ b/tests/test_thumbnails.py @@ -54,6 +54,32 @@ def test_worker_pixmap_lifetime_uses_copy(): assert ".copy()" in THUMBS_SRC +def test_worker_emits_qimage_not_qpixmap(): + """QPixmap construction is main-thread-only in Qt. The worker must + emit QImage; the panel converts to QPixmap in a main-thread slot.""" + # Signal should carry QImage, not QPixmap. + assert "Signal(int, QImage)" in THUMBS_SRC, ( + "ThumbnailWorker.thumbnail_ready must emit QImage, not QPixmap, " + "since QPixmap construction requires the main GUI thread" + ) + # The main-thread slot converts image -> pixmap. + assert "QPixmap.fromImage" in THUMBS_SRC + + +def test_signal_uses_queued_connection(): + """Py3.14 PySide6 requires explicit QueuedConnection for + cross-thread signals so slots run on the receiver's thread + (see memory/project_compress_freeze_py314.md).""" + assert "QueuedConnection" in THUMBS_SRC + + +def test_image_ready_slot_decorated(): + """@Slot decorator on the receiver keeps Py3.14 PySide6 routing + predictable for cross-thread signal delivery.""" + assert "@Slot(int, QImage)" in THUMBS_SRC + assert "_on_image_ready" in THUMBS_SRC + + def test_worker_passes_password_to_fitz(): """Encrypted PDFs would raise on every page render without this.""" assert "authenticate" in THUMBS_SRC From dac96d0c5b5968144484b6b23c98b9a5fac6ab50 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Sun, 12 Jul 2026 14:07:36 +0100 Subject: [PATCH 3/8] fix: harden store-update version handling, fitz auth check, and thumbnail epoch guard - i18n: validate store version format and UInt16 component magnitude; scrub corrupt dismissed-version keys so stale values no longer suppress update notifications - base: verify _open_fitz authenticate() return and raise on wrong password - viewer/thumbnails: add epoch counter to discard stale cross-document thumbnails - remove unused variables (F841); add requirements-dev.txt; ignore thumb_repro*.png Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 2 + app/base.py | 7 ++- app/i18n.py | 58 +++++++++++++++++++-- app/tools/_pdf_extract.py | 1 - app/viewer/panel.py | 2 - app/viewer/thumbnails.py | 83 ++++++++++++++++++++++++++--- app/window.py | 9 ++-- requirements-dev.txt | 14 +++++ tests/test_store_updater.py | 101 +++++++++++++++++++++++++++++++++++- tests/test_thumbnails.py | 8 +-- 10 files changed, 264 insertions(+), 21 deletions(-) create mode 100644 requirements-dev.txt diff --git a/.gitignore b/.gitignore index 5153297..0734393 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,8 @@ test_*.pdf test_*.xlsx *.tmp *.bak +# Debug repro artifacts for the thumbnails sidebar +thumb_repro*.png # Build logs locais pyinstaller.log diff --git a/app/base.py b/app/base.py index 81e501b..bf5a95d 100644 --- a/app/base.py +++ b/app/base.py @@ -382,7 +382,12 @@ def _open_fitz(self, path: str): import fitz doc = fitz.open(path) if doc.needs_pass and self._pdf_password: - doc.authenticate(self._nfc(self._pdf_password)) + # PyMuPDF's authenticate() returns a falsy value (0) on a + # wrong password and leaves the document locked — mirror + # _open_reader and raise instead of handing back a Document + # whose pages can't be read. + if not doc.authenticate(self._nfc(self._pdf_password)): + raise ValueError(t("tool.err.wrong_password")) return doc def _clear_pdf_password(self) -> None: diff --git a/app/i18n.py b/app/i18n.py index e669e5a..ba3799b 100644 --- a/app/i18n.py +++ b/app/i18n.py @@ -5,6 +5,7 @@ import locale import logging import os +import re import shutil import sys import threading @@ -208,6 +209,43 @@ def _save_config_language(lang: str): # `check_for_store_update` can suppress further nags until a NEWER # version is published. +# A dismissed version must be a plain dotted-numeric string (1 to 4 +# components, e.g. "1", "1.13", "1.13.17", "1.13.17.0"). Anything else — +# a bare integer serialized without dots (e.g. "281530812399616") or +# junk — is treated as corrupt and ignored. Without this guard a bogus +# huge value like "281530812399616" parses (via _parse_version) to a +# tuple that dwarfs every real Store version, so `check_for_store_update` +# would suppress ALL future update notifications forever. +# +# The format regex alone is NOT enough: a single gigantic component like +# "281530812399616" is a valid dotted-numeric string (0 dots), so it +# passes the regex and would still poison the comparison. MSIX / Windows +# package versions are Major.Minor.Build.Revision where every component +# is a UInt16 (0-65535), so we additionally reject any component that +# exceeds that ceiling. This rejects "281530812399616" while keeping +# real versions like "1.13.17", "1.13.17.0" and small bare ints like +# "12345". +_STORE_VERSION_RE = re.compile(r"^\d+(\.\d+){0,3}$") +_STORE_VERSION_COMPONENT_MAX = 65535 # UInt16 ceiling per MSIX spec + + +def _is_valid_store_version(value) -> bool: + """True if ``value`` is a well-formed dotted-numeric version string. + + Beyond matching the dotted-numeric *format*, each integer component + must fit in a UInt16 (<= 65535) — the MSIX/Windows package-version + range. This rejects an oversized bare integer (e.g. + ``"281530812399616"``) that would otherwise pass the format check + and permanently suppress update notifications. + """ + if not isinstance(value, str) or not _STORE_VERSION_RE.match(value): + return False + # Regex guarantees every split part is a non-empty run of digits, so + # int() cannot raise here; the only remaining check is magnitude. + return all(int(part) <= _STORE_VERSION_COMPONENT_MAX + for part in value.split(".")) + + def get_dismissed_store_version() -> str | None: """Return the Store version the user last dismissed, or ``None``. @@ -215,6 +253,10 @@ def get_dismissed_store_version() -> str | None: version; do not nag again unless the Store publishes something newer". Stored under the ``dismissed_store_version`` key in config.json. + + If the persisted value is malformed (not a dotted-numeric version), + it is treated as "nothing dismissed" AND scrubbed from config so the + corrupt key can't keep suppressing notifications on every launch. """ try: with open(_CONFIG_PATH, "r", encoding="utf-8") as f: @@ -224,8 +266,13 @@ def get_dismissed_store_version() -> str | None: if not isinstance(cfg, dict): return None val = cfg.get("dismissed_store_version") - if isinstance(val, str) and val: + if val is None: + return None + if _is_valid_store_version(val): return val + # Corrupt value: drop it so it stops masking real updates. + with contextlib.suppress(Exception): + _update_config(lambda c: c.pop("dismissed_store_version", None)) return None @@ -235,12 +282,17 @@ def set_dismissed_store_version(version: str | None) -> None: Pass a version string (e.g. ``"1.13.17"``) to record the dismiss. Pass ``None`` to clear the flag (so the next available-update notification is shown unconditionally). + + Malformed version strings are rejected (never persisted) to avoid + writing a value that would later suppress all notifications. """ def _mutate(cfg: dict) -> None: - if version: + if version and _is_valid_store_version(version): cfg["dismissed_store_version"] = version - else: + elif not version: cfg.pop("dismissed_store_version", None) + # An invalid non-empty version is silently ignored: neither + # written nor used to clear an existing valid value. _update_config(_mutate) diff --git a/app/tools/_pdf_extract.py b/app/tools/_pdf_extract.py index ab2813b..a998ae2 100644 --- a/app/tools/_pdf_extract.py +++ b/app/tools/_pdf_extract.py @@ -1180,7 +1180,6 @@ def _split_component_on_y_gaps( drawing_idx_list: list[int] = [] any_text = False consumed_text: list[int] = [] - seen_text: set[int] = set() # Precompute cell bboxes so we can pre-assign each text block # to whichever cell contains its centroid (handles the common # PyMuPDF case where adjacent columns of one table row land in diff --git a/app/viewer/panel.py b/app/viewer/panel.py index ffba993..44c05a5 100644 --- a/app/viewer/panel.py +++ b/app/viewer/panel.py @@ -683,13 +683,11 @@ def _update_search_highlight(self): all_highlights = [] flat_idx = 0 current_page = 0 - current_rect_idx = 0 for page_idx, rects in self._search_results: for rect in rects: all_highlights.append((page_idx, rect)) if flat_idx == self._search_current: current_page = page_idx - current_rect_idx = flat_idx flat_idx += 1 self._canvas.set_search_highlights(all_highlights, self._search_current) # Scroll to current match diff --git a/app/viewer/thumbnails.py b/app/viewer/thumbnails.py index 0e4f592..9f6f469 100644 --- a/app/viewer/thumbnails.py +++ b/app/viewer/thumbnails.py @@ -32,7 +32,8 @@ import logging from PySide6.QtCore import ( - QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, Signal, Slot, + QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, QTimer, + Signal, Slot, ) from PySide6.QtGui import QColor, QImage, QPainter, QPixmap from PySide6.QtWidgets import ( @@ -71,14 +72,18 @@ class ThumbnailWorker(QThread): # so the receiver slot ``ThumbnailPanel._on_image_ready`` performs # the ``QPixmap.fromImage()`` conversion on the main thread before # handing the pixmap to the model. - thumbnail_ready = Signal(int, QImage) # (page_index, thumbnail) + thumbnail_ready = Signal(int, QImage, int) # (page_index, thumbnail, epoch) def __init__(self, doc_path: str, page_indices: list[int], - password: str = "", parent=None) -> None: + password: str = "", epoch: int = 0, parent=None) -> None: super().__init__(parent) self._doc_path = doc_path self._pages = list(page_indices) self._password = password + # Generation this worker was launched for. Emitted alongside + # each thumbnail so the panel can discard images produced for a + # document that has since been replaced (see ThumbnailPanel). + self._epoch = epoch self._cancelled = False def cancel(self) -> None: @@ -127,7 +132,7 @@ def run(self) -> None: pix.samples, pix.width, pix.height, pix.stride, QImage.Format.Format_RGB888, ).copy() - self.thumbnail_ready.emit(idx, img) + self.thumbnail_ready.emit(idx, img, self._epoch) except Exception: # Skip page — bad object / partial doc, keep going. continue @@ -298,6 +303,11 @@ def __init__(self, parent=None) -> None: self._doc_path = "" self._password = "" self._worker: ThumbnailWorker | None = None + # Monotonic generation counter. Bumped on every set_document / + # clear so late thumbnails from a superseded worker (still in the + # queued-connection event queue) can be identified and dropped + # instead of being cached against the NEW document's indices. + self._epoch = 0 layout = QVBoxLayout(self) layout.setContentsMargins(0, 0, 0, 0) @@ -332,6 +342,9 @@ def set_document(self, doc_path: str, page_count: int, self._doc_path = doc_path self._password = password or "" self._stop_worker() + # New generation: any thumbnail still queued from a prior worker + # now carries a stale epoch and will be dropped in _on_image_ready. + self._epoch += 1 self._model.set_document(doc_path, page_count) self._delegate.set_current_page(-1) if page_count > 0 and doc_path: @@ -342,6 +355,8 @@ def clear(self) -> None: self._doc_path = "" self._password = "" self._stop_worker() + # New generation — see set_document. + self._epoch += 1 self._model.clear() self._delegate.set_current_page(-1) @@ -375,7 +390,7 @@ def _on_activated(self, index: QModelIndex) -> None: def _start_worker(self, page_indices: list[int]) -> None: self._worker = ThumbnailWorker( - self._doc_path, page_indices, self._password, self) + self._doc_path, page_indices, self._password, self._epoch, self) # Explicit QueuedConnection so the slot runs on this (main) # thread rather than the worker — required for QPixmap # construction, and matches the Py3.14 PySide6 routing rules @@ -386,14 +401,24 @@ def _start_worker(self, page_indices: list[int]) -> None: ) self._worker.start() - @Slot(int, QImage) - def _on_image_ready(self, page_idx: int, img: QImage) -> None: + @Slot(int, QImage, int) + def _on_image_ready(self, page_idx: int, img: QImage, epoch: int) -> None: """Runs on the main GUI thread. Convert the worker's QImage into a QPixmap and hand it to the model. QPixmap construction is only valid on the main thread; doing it here (instead of in the worker) is why the parent signal carries QImage.""" + # Drop thumbnails from a superseded document: a queued emission + # from the previous worker can still be delivered after + # set_document/clear bumped the epoch, and caching it would paint + # the old document's page into the new document's index. + if epoch != self._epoch: + _log.debug( + "Dropping stale thumbnail page %d (epoch %d != %d)", + page_idx + 1, epoch, self._epoch, + ) + return _log.debug( "Thumbnail received for page %d (%dx%d)", page_idx + 1, img.width(), img.height(), @@ -405,12 +430,56 @@ def _stop_worker(self) -> None: if self._worker is None: return self._worker.cancel() + # Disconnect BEFORE waiting so a thumbnail emitted between the + # cancel flag check and the worker unwinding can't sneak into + # the model for the old document. The epoch guard in + # _on_image_ready is the belt; this disconnect is the braces. + with contextlib.suppress(RuntimeError, TypeError): + self._worker.thumbnail_ready.disconnect(self._on_image_ready) # 2 s is generous — a single page render at 40 DPI is well # under 100 ms; the cancel flag is polled between pages so # worst case we wait for the current page to finish. self._worker.wait(2000) self._worker = None + def showEvent(self, event): # noqa: D401 + """Force a viewport repaint when the panel becomes visible. + + The worker fills the model's pixmap cache in the background even + while this widget is hidden (e.g. the sidebar is on the + "Contents" tab). Qt normally coalesces / drops repaints for + hidden widgets, so ``dataChanged`` signals emitted during that + window never reach the viewport. Calling ``update()`` here on + first show — and on every subsequent show — makes the QListView + redraw all visible rows against the current cache so any + pixmaps that arrived while hidden appear immediately instead of + remaining as ``…`` placeholders. + """ + super().showEvent(event) + if self._view is not None: + vp = self._view.viewport() + if vp is not None: + vp.update() + # Layout can still be incomplete when showEvent fires + # (e.g. the tab was just switched — viewport size is + # still the collapsed placeholder value). Schedule a + # second update after the current event-loop iteration + # so it runs against the final resized geometry. Without + # this, the first update() marks the wrong (tiny) area + # dirty and Qt does not repaint the newly-resized + # region. + QTimer.singleShot(0, vp.update) + + def resizeEvent(self, event): # noqa: D401 + """Force viewport repaint on resize so newly-visible rows paint + against the current cache (not against stale/degenerate + geometry from before the layout settled).""" + super().resizeEvent(event) + if self._view is not None: + vp = self._view.viewport() + if vp is not None: + vp.update() + def closeEvent(self, event): # noqa: D401 self._stop_worker() super().closeEvent(event) diff --git a/app/window.py b/app/window.py index 8d2710c..1db8f43 100644 --- a/app/window.py +++ b/app/window.py @@ -1606,7 +1606,10 @@ def _notify_store_update(self, latest: str, deep_link: str): t("update.store.btn.open"), QMessageBox.ButtonRole.AcceptRole, ) - later_btn = box.addButton( + # "Later" button: its handle isn't needed — clickedButton() is + # compared against open/dismiss only, and anything else (Later or + # the close-box) is a no-op — but the button must still exist. + box.addButton( t("update.store.btn.later"), QMessageBox.ButtonRole.RejectRole, ) @@ -1632,8 +1635,8 @@ def _notify_store_update(self, latest: str, deep_link: str): with contextlib.suppress(Exception): if latest: set_dismissed_store_version(latest) - # "Later" (later_btn) and the close-box: do nothing; dialog - # will reappear on the next startup. + # "Later" and the close-box: do nothing; dialog will reappear on + # the next startup. def _show_update_dialog(self): if self._update_release: diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..c03757e --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,14 @@ +# Development / CI tooling for PDFApps. +# +# Runtime dependencies live in requirements.txt; this file is only the +# static-analysis / security / lint toolchain used locally and by the +# CI workflows (e.g. security-deps.yml runs pip-audit; the lint gate +# runs ruff). Install with: pip install -r requirements-dev.txt +# +# Lower bounds mirror the >= style of requirements.txt so a fresh venv +# picks up current releases while staying reproducible enough. +mypy>=1.11.0 +bandit>=1.7.9 +pip-audit>=2.7.3 +pyyaml>=6.0.2 +ruff>=0.6.0 diff --git a/tests/test_store_updater.py b/tests/test_store_updater.py index 442398d..efc4cdb 100644 --- a/tests/test_store_updater.py +++ b/tests/test_store_updater.py @@ -181,7 +181,11 @@ def test_get_store_version_returns_none_when_no_packages(): def test_check_for_store_update_detects_newer(): from app import store_updater - with patch.object(store_updater, "get_store_version", return_value="99.0.0"): + # Patch the dismiss lookup so the test is hermetic — otherwise a + # real ~/.pdfapps_config.json with a dismissed_store_version could + # suppress the update and make this assertion machine-dependent. + with patch.object(store_updater, "get_store_version", return_value="99.0.0"), \ + patch("app.i18n.get_dismissed_store_version", return_value=None): has_update, latest = store_updater.check_for_store_update() assert has_update is True assert latest == "99.0.0" @@ -301,6 +305,101 @@ def test_dismissed_store_version_round_trip(tmp_path, monkeypatch): assert i18n.get_dismissed_store_version() is None +def test_is_valid_store_version_magnitude_guard(): + """A dotted-numeric string with an oversized (> UInt16) component must + be rejected even though it matches the format regex, while real + versions and small bare integers stay valid. + + Regression: the corrupt value "281530812399616" that broke update + notifications is a valid dotted-numeric string (0 dots) and slipped + past the format-only check.""" + from app import i18n + # Corrupt: single gigantic component exceeds 65535. + assert i18n._is_valid_store_version("281530812399616") is False + # Still valid: real dotted versions. + assert i18n._is_valid_store_version("1.13.17") is True + assert i18n._is_valid_store_version("1.13.17.0") is True + # Still valid: small bare integer within UInt16 range. + assert i18n._is_valid_store_version("12345") is True + # Boundary: exactly the UInt16 ceiling is accepted, one over is not. + assert i18n._is_valid_store_version("65535") is True + assert i18n._is_valid_store_version("65536") is False + # A per-component overflow inside a dotted version is also rejected. + assert i18n._is_valid_store_version("1.99999") is False + # Non-string junk stays invalid. + assert i18n._is_valid_store_version(281530812399616) is False + assert i18n._is_valid_store_version(None) is False + + +def test_dismissed_store_version_scrubs_oversized_string(tmp_path, + monkeypatch): + """A persisted STRING "281530812399616" must be treated as "nothing + dismissed" AND scrubbed from config, so it can't keep suppressing + every future update notification.""" + from app import i18n + cfg = tmp_path / "config.json" + cfg.write_text(json.dumps({ + "language": "pt", + "dismissed_store_version": "281530812399616", + }), encoding="utf-8") + monkeypatch.setattr(i18n, "_CONFIG_PATH", str(cfg)) + + # Corrupt value is ignored on read. + assert i18n.get_dismissed_store_version() is None + # And scrubbed from disk (self-heal), without wiping other keys. + on_disk = json.loads(cfg.read_text(encoding="utf-8")) + assert "dismissed_store_version" not in on_disk + assert on_disk["language"] == "pt" + + +def test_dismissed_store_version_scrubs_oversized_number(tmp_path, + monkeypatch): + """Same guard for the JSON-number subcase (not a string).""" + from app import i18n + cfg = tmp_path / "config.json" + cfg.write_text(json.dumps({ + "dismissed_store_version": 281530812399616, + }), encoding="utf-8") + monkeypatch.setattr(i18n, "_CONFIG_PATH", str(cfg)) + + assert i18n.get_dismissed_store_version() is None + on_disk = json.loads(cfg.read_text(encoding="utf-8")) + assert "dismissed_store_version" not in on_disk + + +def test_set_dismissed_store_version_rejects_oversized(tmp_path, + monkeypatch): + """set_dismissed_store_version must not persist an oversized value, + and must not clear an existing valid dismiss when handed junk.""" + from app import i18n + cfg = tmp_path / "config.json" + monkeypatch.setattr(i18n, "_CONFIG_PATH", str(cfg)) + + # Seed a valid dismiss. + i18n.set_dismissed_store_version("1.13.17") + assert i18n.get_dismissed_store_version() == "1.13.17" + + # An oversized value is silently rejected: not written, and the + # existing valid value is preserved. + i18n.set_dismissed_store_version("281530812399616") + assert i18n.get_dismissed_store_version() == "1.13.17" + + +def test_dismissed_store_version_no_scrub_on_unreadable_json(tmp_path, + monkeypatch): + """If config.json is unreadable, get_dismissed_store_version returns + None WITHOUT rewriting (scrubbing) the file — we must never touch a + file we couldn't parse.""" + from app import i18n + cfg = tmp_path / "config.json" + cfg.write_text("not json at all", encoding="utf-8") + monkeypatch.setattr(i18n, "_CONFIG_PATH", str(cfg)) + + assert i18n.get_dismissed_store_version() is None + # File left untouched (not overwritten with "{}"). + assert cfg.read_text(encoding="utf-8") == "not json at all" + + # ── store_deep_link ──────────────────────────────────────────────────── diff --git a/tests/test_thumbnails.py b/tests/test_thumbnails.py index 52d2b48..6d18bcd 100644 --- a/tests/test_thumbnails.py +++ b/tests/test_thumbnails.py @@ -57,8 +57,10 @@ def test_worker_pixmap_lifetime_uses_copy(): def test_worker_emits_qimage_not_qpixmap(): """QPixmap construction is main-thread-only in Qt. The worker must emit QImage; the panel converts to QPixmap in a main-thread slot.""" - # Signal should carry QImage, not QPixmap. - assert "Signal(int, QImage)" in THUMBS_SRC, ( + # Signal should carry QImage, not QPixmap. It also carries the + # generation/epoch (trailing int) so stale thumbnails from a + # superseded document can be dropped by the panel. + assert "Signal(int, QImage, int)" in THUMBS_SRC, ( "ThumbnailWorker.thumbnail_ready must emit QImage, not QPixmap, " "since QPixmap construction requires the main GUI thread" ) @@ -76,7 +78,7 @@ def test_signal_uses_queued_connection(): def test_image_ready_slot_decorated(): """@Slot decorator on the receiver keeps Py3.14 PySide6 routing predictable for cross-thread signal delivery.""" - assert "@Slot(int, QImage)" in THUMBS_SRC + assert "@Slot(int, QImage, int)" in THUMBS_SRC assert "_on_image_ready" in THUMBS_SRC From 230e89ba88374d9b9f535ac71483e175fe15f083 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Mon, 13 Jul 2026 19:00:18 +0100 Subject: [PATCH 4/8] fix(thumbnails): render sidebar thumbnails lazily and fix TOC tab - Replace eager whole-document render with viewport-based lazy rendering (visible window +/- buffer; hidden-tab warm window around the anchor page), fixing the LRU cache eviction that left the top pages of large PDFs (>CACHE_MAX pages, e.g. ebooks with a TOC) stuck on placeholders. - Reconcile in-flight pages on worker finish/failure so a page is never stranded in the in-flight set and can be re-rendered when scrolled back. - Render DPR-aware (fitz.Matrix sized to THUMB*devicePixelRatio, smooth downscale) so thumbnails are crisp on HiDPI screens; DPR read live, never hardcoded. - Re-activate the Contents (TOC) tab by default when a PDF has bookmarks, fixing the sidebar getting stuck on the Pages tab after opening a no-TOC document. - Move the opt-in debug log (PDFAPPS_THUMB_DEBUG) to a per-user data dir instead of a shared temp path. - Expand tests/test_thumbnails.py (31 passing) incl. a discriminative large-doc eviction regression test and DPR/TOC behavioural tests. --- app/viewer/panel.py | 8 + app/viewer/thumbnails.py | 565 +++++++++++++++++++++++++++++++++------ tests/test_thumbnails.py | 443 ++++++++++++++++++++++++++++++ 3 files changed, 937 insertions(+), 79 deletions(-) diff --git a/app/viewer/panel.py b/app/viewer/panel.py index 44c05a5..a16b0f8 100644 --- a/app/viewer/panel.py +++ b/app/viewer/panel.py @@ -485,6 +485,14 @@ def _populate_toc(self, doc): stack.append((level, item)) self._toc_tree.expandToDepth(1) self._set_toc_tab_visible(True) + # Make Contents the active tab by default when the PDF has a + # TOC — the original (pre-sidebar-tabs) behaviour showed the + # outline as the sidebar itself. Without this, a prior no-TOC + # document that switched the active tab to "Pages" (see the + # empty-TOC branch above) leaves the sidebar parked on Pages, + # so re-showing the Contents tab button here never actually + # surfaces the outline: the tab appears absent to the user. + self._sidebar_tabs.setCurrentIndex(self._toc_tab_idx) except Exception as exc: import logging logging.getLogger(__name__).warning( diff --git a/app/viewer/thumbnails.py b/app/viewer/thumbnails.py index 9f6f469..970025c 100644 --- a/app/viewer/thumbnails.py +++ b/app/viewer/thumbnails.py @@ -30,10 +30,11 @@ import contextlib import logging +import os from PySide6.QtCore import ( - QAbstractListModel, QModelIndex, QRect, QSize, Qt, QThread, QTimer, - Signal, Slot, + QAbstractListModel, QModelIndex, QRect, QSize, QStandardPaths, Qt, + QThread, QTimer, Signal, Slot, ) from PySide6.QtGui import QColor, QImage, QPainter, QPixmap from PySide6.QtWidgets import ( @@ -47,11 +48,98 @@ _log = logging.getLogger(__name__) +class _FlushFileHandler(logging.FileHandler): + """FileHandler that flushes + fsyncs after every record. + + The project's hang-debugging notes warn that PowerShell terminal + redirection is unreliable for capturing the moments right before a + freeze / kill. Writing to a dedicated file with line-buffering plus + an explicit ``fsync`` guarantees the thumbnail diagnostics survive a + hard process kill so the user can re-send the log and we can close + the diagnosis without guessing (see memory/feedback_diag_logging.md). + """ + + def emit(self, record): # noqa: D401 + super().emit(record) + with contextlib.suppress(Exception): + self.flush() + os.fsync(self.stream.fileno()) + + +# Basename of the kill-proof diagnostic log. Only written when the env +# var PDFAPPS_THUMB_DEBUG is set (any value) so production runs stay +# quiet; the resolved path is stable so the user can be told exactly +# where to look. +THUMB_LOG_NAME = "pdfapps_thumbnails.log" + + +def _thumb_log_path() -> str: + """Resolve the diagnostic log path inside a per-user data directory. + + The old implementation dropped the log in a FIXED, predictable name + under the *shared* system temp dir (``/tmp`` on Linux/macOS). That is + exactly the symlink-following / attacker-pre-created-file pattern the + project's security notes warn about (see memory/feedback_security.md): + another user can pre-create or symlink ``/tmp/pdfapps_thumbnails.log`` + and our append-mode open would follow it. + + Writing under a per-user location owned by the current user (Windows: + ``%LOCALAPPDATA%/PDFApps``; Linux: ``~/.local/share/PDFApps``; macOS: + ``~/Library/Application Support/PDFApps``) removes the shared-dir + race: the directory is not world-writable, so a plain append open is + safe. A fixed ``PDFApps`` subfolder keeps the path deterministic + regardless of the (deliberately blank) QApplication name. + """ + base = QStandardPaths.writableLocation( + QStandardPaths.StandardLocation.GenericDataLocation) + if not base: + # No usable data location (headless / unset HOME): fall back to a + # per-user dotdir in the home directory rather than shared temp. + base = os.path.join(os.path.expanduser("~"), ".local", "share") + d = os.path.join(base, "PDFApps") + os.makedirs(d, exist_ok=True) + return os.path.join(d, THUMB_LOG_NAME) + + +def _install_debug_log() -> None: + """Attach the kill-proof file handler when PDFAPPS_THUMB_DEBUG is set. + + Idempotent — safe to call from every ThumbnailPanel constructor. + """ + if not os.environ.get("PDFAPPS_THUMB_DEBUG"): + return + for h in _log.handlers: + if isinstance(h, _FlushFileHandler): + return + with contextlib.suppress(Exception): + log_path = _thumb_log_path() + handler = _FlushFileHandler(log_path, mode="a", encoding="utf-8") + handler.setFormatter(logging.Formatter( + "%(asctime)s %(levelname)s %(message)s")) + handler.setLevel(logging.DEBUG) + _log.addHandler(handler) + _log.setLevel(logging.DEBUG) + _log.info("── thumbnail debug log opened (pid=%d) ──", os.getpid()) + + THUMB_WIDTH = 120 # px – target thumbnail width before aspect-fit THUMB_HEIGHT = 160 # px – target thumbnail height (portrait ~ 1.33) THUMB_PADDING = 12 # px around the thumbnail inside a row PAGE_NUM_HEIGHT = 20 # px reserved for the page-number label CACHE_MAX = 200 # LRU-ish cap on cached QPixmaps +# How many rows above/below the viewport to pre-render so scrolling a +# little doesn't reveal a wall of "…" placeholders before the worker +# catches up. +VISIBLE_BUFFER = 4 +# When the panel is hidden (e.g. parked behind the "Contents" tab for a +# PDF that has a TOC) the viewport has no usable geometry, so we render a +# window of this many pages around the anchor (the current reading page) +# up-front. When the tab is later revealed, showEvent recomputes the +# real visible range and fills any gaps. This is the core of the fix: +# we NEVER eagerly render the whole document (which, for a doc longer +# than CACHE_MAX, evicted its own early pages before the user could see +# them and left the top of the list on placeholders forever). +HIDDEN_WINDOW = 12 # ── Worker ──────────────────────────────────────────────────────────── @@ -73,13 +161,25 @@ class ThumbnailWorker(QThread): # the ``QPixmap.fromImage()`` conversion on the main thread before # handing the pixmap to the model. thumbnail_ready = Signal(int, QImage, int) # (page_index, thumbnail, epoch) + # Emitted once when the worker finishes having rendered ZERO pages + # despite pages being requested (import failure, open failure, or + # every page raising). Carries (epoch, reason) so the panel can log + # it against the right generation instead of failing silently. + render_failed = Signal(int, str) # (epoch, reason) def __init__(self, doc_path: str, page_indices: list[int], - password: str = "", epoch: int = 0, parent=None) -> None: + password: str = "", epoch: int = 0, dpr: float = 1.0, + parent=None) -> None: super().__init__(parent) self._doc_path = doc_path self._pages = list(page_indices) self._password = password + # Device-pixel ratio of the screen the panel is on. Thumbnails + # are rendered at target_size * dpr real pixels and tagged with + # this ratio so a HiDPI display shows crisp (not upscaled-blurry) + # pages. Never hardcoded — the panel reads the live value off its + # widget/screen and passes it in (see memory/feedback_icons.md). + self._dpr = float(dpr) if dpr and dpr > 0 else 1.0 # Generation this worker was launched for. Emitted alongside # each thumbnail so the panel can discard images produced for a # document that has since been replaced (see ThumbnailPanel). @@ -90,13 +190,24 @@ def cancel(self) -> None: self._cancelled = True def run(self) -> None: - import fitz # local — keeps import cost off UI startup path - - try: - doc = fitz.open(self._doc_path) - except Exception: - return + # ``import fitz`` lives INSIDE the try: a failed import (missing + # binary wheel, broken install) must surface as a warning + a + # render_failed signal, NOT silently kill the QThread with an + # unhandled ImportError that leaves the sidebar stuck on + # placeholders forever. + requested = len(self._pages) + rendered = 0 + doc = None try: + import fitz # local — keeps import cost off UI startup path + try: + doc = fitz.open(self._doc_path) + except Exception as exc: + _log.warning( + "ThumbnailWorker: failed to open %r: %s", + self._doc_path, exc, + ) + return if self._password and doc.needs_pass: with contextlib.suppress(Exception): doc.authenticate(self._password) @@ -112,9 +223,26 @@ def run(self) -> None: idx + 1, page_count, ) page = doc[idx] - # 40 DPI ≈ 330×467 px for A4 — plenty of detail - # for a 120×160 downscale with smooth transform. - pix = page.get_pixmap(dpi=40, alpha=False, annots=False) + # Render at the ACTUAL device-pixel size the thumbnail + # occupies: target box (THUMB_WIDTH×THUMB_HEIGHT) times + # the screen's device-pixel ratio. A fixed 40 DPI used + # to render ~331×468 for A4 and then the delegate + # downscaled to a 120×160 *logical* pixmap with no DPR + # set — so on a 2× HiDPI screen Qt upscaled that 120 px + # image to fill 240 device px: visibly soft/blurry. + # Computing zoom from the page rect keeps the aspect + # ratio (KeepAspectRatio via min) and hits the exact + # pixel budget, so the pixmap is drawn ~1:1 and sharp. + rect = page.rect + tw = THUMB_WIDTH * self._dpr + th = THUMB_HEIGHT * self._dpr + if rect.width > 0 and rect.height > 0: + zoom = min(tw / rect.width, th / rect.height) + else: + zoom = self._dpr + mat = fitz.Matrix(zoom, zoom) + pix = page.get_pixmap(matrix=mat, alpha=False, + annots=False) if pix.n != 3: pix = fitz.Pixmap(fitz.csRGB, pix) # ``QImage`` views ``pix.samples`` directly; the @@ -132,13 +260,46 @@ def run(self) -> None: pix.samples, pix.width, pix.height, pix.stride, QImage.Format.Format_RGB888, ).copy() + # Tag with the device-pixel ratio so the receiving + # QPixmap paints at its device-independent (logical) + # size while carrying full HiDPI detail. Preserved by + # QPixmap.fromImage on the main thread. + img.setDevicePixelRatio(self._dpr) + rendered += 1 self.thumbnail_ready.emit(idx, img, self._epoch) - except Exception: - # Skip page — bad object / partial doc, keep going. + except Exception as exc: + # Skip page — bad object / partial doc, keep going — + # but log it: a wall of these is the difference + # between "one bad page" and "the whole doc failed". + _log.warning( + "ThumbnailWorker: failed to render page %d: %s", + idx + 1, exc, + ) continue + except Exception as exc: + # import fitz / unexpected fatal — don't let the thread die + # mute; the panel needs to know rendering never happened. + _log.error( + "ThumbnailWorker: fatal error, no thumbnails produced: %s", + exc, + ) finally: - with contextlib.suppress(Exception): - doc.close() + if doc is not None: + with contextlib.suppress(Exception): + doc.close() + # Zero pages rendered when pages were requested and we + # weren't cancelled is a hard failure — make it loud and + # give the panel a chance to reflect it. + if requested and rendered == 0 and not self._cancelled: + _log.warning( + "ThumbnailWorker: rendered 0/%d pages for %r", + requested, self._doc_path, + ) + with contextlib.suppress(RuntimeError): + self.render_failed.emit( + self._epoch, + f"rendered 0/{requested} pages", + ) # ── Model ───────────────────────────────────────────────────────────── @@ -194,6 +355,33 @@ def cache_pixmap(self, page_idx: int, pix: QPixmap) -> None: def cache_size(self) -> int: return len(self._cache) + def has_pixmap(self, page_idx: int) -> bool: + """True if ``page_idx`` currently has a cached (non-evicted) + pixmap. Used by the panel's lazy renderer to avoid re-requesting + pages that are already cached, while still allowing a page that + was LRU-evicted to be re-rendered when scrolled back into view.""" + return page_idx in self._cache + + def refresh_decorations(self) -> None: + """Re-emit ``dataChanged`` for every row's decoration. + + The worker fills ``self._cache`` while the panel may be hidden + (sidebar parked on the Contents tab). Qt drops/coalesces the + per-row ``dataChanged`` repaints for a hidden viewport, so those + pixmaps never reach the screen. When the panel becomes visible + again the view must be told to re-query DecorationRole for the + rows it now shows — a bare ``viewport().update()`` repaints but + does NOT force the view to re-read the model, so on some + platforms the cached-but-never-delivered pixmaps stay invisible. + Emitting dataChanged here guarantees a fresh read against the + now-populated cache. + """ + if self._page_count <= 0: + return + top = self.index(0) + bottom = self.index(self._page_count - 1) + self.dataChanged.emit(top, bottom, [Qt.ItemDataRole.DecorationRole]) + # ── Delegate ────────────────────────────────────────────────────────── @@ -249,19 +437,28 @@ def paint(self, painter: QPainter, option, index: QModelIndex) -> None: thumb_rect = QRect(thumb_x, thumb_y, THUMB_WIDTH, THUMB_HEIGHT) if pix is not None and not pix.isNull(): + # Scale in DEVICE pixels (target × DPR) then tag the result + # with the same ratio, so the pixmap paints at its logical + # THUMB box size while retaining full HiDPI detail — no soft + # upscaling on a 2× screen. Geometry below uses the logical + # (device-independent) size. + dpr = pix.devicePixelRatio() or 1.0 scaled = pix.scaled( - THUMB_WIDTH, THUMB_HEIGHT, + round(THUMB_WIDTH * dpr), round(THUMB_HEIGHT * dpr), Qt.AspectRatioMode.KeepAspectRatio, Qt.TransformationMode.SmoothTransformation, ) - cx = thumb_x + (THUMB_WIDTH - scaled.width()) // 2 - cy = thumb_y + (THUMB_HEIGHT - scaled.height()) // 2 + scaled.setDevicePixelRatio(dpr) + lw = round(scaled.width() / dpr) + lh = round(scaled.height() / dpr) + cx = thumb_x + (THUMB_WIDTH - lw) // 2 + cy = thumb_y + (THUMB_HEIGHT - lh) // 2 painter.drawPixmap(cx, cy, scaled) # 1 px hairline so a white-page thumbnail is still visible # against a light background. painter.setPen(QColor(0, 0, 0, 60)) painter.setBrush(Qt.BrushStyle.NoBrush) - painter.drawRect(cx, cy, scaled.width() - 1, scaled.height() - 1) + painter.drawRect(cx, cy, lw - 1, lh - 1) else: # Placeholder while the worker catches up. painter.setPen(QColor(TEXT_SEC)) @@ -299,10 +496,27 @@ class ThumbnailPanel(QWidget): def __init__(self, parent=None) -> None: super().__init__(parent) + _install_debug_log() self.setObjectName("thumbnail_panel") self._doc_path = "" self._password = "" - self._worker: ThumbnailWorker | None = None + # Multiple short-lived workers may run concurrently: as the user + # scrolls we spin one up per newly-visible batch rather than + # cancelling / blocking on the previous one. Finished workers are + # pruned lazily. The epoch guard drops any results for a + # superseded document. + self._workers: list[ThumbnailWorker] = [] + # Pages that have been handed to a live worker but whose pixmap + # has not yet arrived. Prevents re-requesting an in-flight page + # on every scroll tick. Cleared per-page in _on_image_ready and + # wholesale on set_document / clear. A page that is LRU-evicted + # from the model cache is NOT in this set, so it is re-rendered + # when scrolled back into view — that is what keeps long + # documents from stranding their early pages on placeholders. + self._inflight: set[int] = set() + # Page to centre the pre-render window on while the panel is + # hidden (tracks the current reading page via set_current_page). + self._anchor = 0 # Monotonic generation counter. Bumped on every set_document / # clear so late thumbnails from a superseded worker (still in the # queued-connection event queue) can be identified and dropped @@ -332,31 +546,50 @@ def __init__(self, parent=None) -> None: Qt.ScrollBarPolicy.ScrollBarAlwaysOff) self._view.clicked.connect(self._on_activated) self._view.activated.connect(self._on_activated) # keyboard Enter + # Debounce scroll-driven render requests so a flick doesn't spawn + # a worker per pixel. The timer coalesces bursts into one render + # of the settled visible range. + self._scroll_timer = QTimer(self) + self._scroll_timer.setSingleShot(True) + self._scroll_timer.setInterval(120) + self._scroll_timer.timeout.connect(self._render_visible) + sb = self._view.verticalScrollBar() + if sb is not None: + sb.valueChanged.connect(lambda _=0: self._scroll_timer.start()) layout.addWidget(self._view) # ── Public API ──────────────────────────────────────────────── def set_document(self, doc_path: str, page_count: int, password: str = "") -> None: - """Reset model + kick off background rendering.""" + """Reset model + kick off background rendering of the pages the + user is about to see (NOT the whole document).""" self._doc_path = doc_path self._password = password or "" - self._stop_worker() + self._stop_all_workers() # New generation: any thumbnail still queued from a prior worker # now carries a stale epoch and will be dropped in _on_image_ready. self._epoch += 1 + self._inflight.clear() + self._anchor = 0 self._model.set_document(doc_path, page_count) self._delegate.set_current_page(-1) + _log.debug( + "set_document: %r page_count=%d epoch=%d visible=%s", + doc_path, page_count, self._epoch, self.isVisible(), + ) if page_count > 0 and doc_path: - self._start_worker(list(range(page_count))) + self._render_visible() def clear(self) -> None: """Unload the current document.""" self._doc_path = "" self._password = "" - self._stop_worker() + self._stop_all_workers() # New generation — see set_document. self._epoch += 1 + self._inflight.clear() + self._anchor = 0 self._model.clear() self._delegate.set_current_page(-1) @@ -364,6 +597,10 @@ def set_current_page(self, page_idx: int) -> None: """Highlight given page and scroll to it (if visible).""" if not (0 <= page_idx < self._model.rowCount()): return + # Remember the reading position so a hidden panel pre-renders the + # right window (the pages the user will land on when they open + # the Pages tab) instead of always page 1. + self._anchor = page_idx old = self._delegate.set_current_page(page_idx) idx = self._model.index(page_idx) self._view.setCurrentIndex(idx) @@ -376,6 +613,10 @@ def set_current_page(self, page_idx: int) -> None: old_rect = self._view.visualRect(self._model.index(old)) vp.update(old_rect) vp.update(self._view.visualRect(idx)) + # If the panel is hidden the scrollbar signal above won't have + # fired a render; make sure the window around the new anchor is + # requested so switching to the tab shows real thumbnails. + self._render_visible() def update_theme(self, dark: bool) -> None: """Repaint so ACCENT / TEXT_SEC track the theme.""" @@ -388,18 +629,98 @@ def _on_activated(self, index: QModelIndex) -> None: if index.isValid(): self.page_requested.emit(index.row()) + def _row_height(self) -> int: + """Uniform row height from the delegate's size hint.""" + return (self._delegate.sizeHint(None, self._model.index(0)).height() + or 1) + + def _visible_range(self) -> list[int]: + """Compute which page rows should be rendered right now. + + When the panel has real geometry we return the rows in the + viewport padded by VISIBLE_BUFFER. When it is hidden / not yet + laid out (viewport height 0 — exactly the state the Pages tab is + in while parked behind the Contents tab of a TOC PDF) the + viewport can't tell us anything, so we fall back to a window + around the anchor. This is the crux of the fix: we render the + pages the user will actually look at instead of the whole doc. + """ + total = self._model.rowCount() + if total <= 0: + return [] + row_h = self._row_height() + vp = self._view.viewport() + vp_h = vp.height() if vp is not None else 0 + if vp_h <= 0 or not self._view.isVisible(): + first = max(0, self._anchor - VISIBLE_BUFFER) + last = min(total - 1, self._anchor + HIDDEN_WINDOW) + return list(range(first, last + 1)) + sb = self._view.verticalScrollBar() + top = sb.value() if sb is not None else 0 + first = top // row_h + last = (top + vp_h) // row_h + first = max(0, first - VISIBLE_BUFFER) + last = min(total - 1, last + VISIBLE_BUFFER) + return list(range(first, last + 1)) + + def _render_visible(self) -> None: + """Request rendering of the currently-visible (or hidden-window) + pages that aren't already cached or in flight.""" + if not self._doc_path or self._model.rowCount() <= 0: + return + # Prune workers that have finished so the list can't grow without + # bound over a long scrolling session. + self._workers = [w for w in self._workers if w.isRunning()] + want = self._visible_range() + missing = [p for p in want + if p not in self._inflight and not self._model.has_pixmap(p)] + if not missing: + return + self._inflight.update(missing) + _log.debug( + "render_visible: want=[%d..%d] missing=%d inflight=%d " + "cache=%d workers=%d", + want[0] if want else -1, want[-1] if want else -1, + len(missing), len(self._inflight), + self._model.cache_size(), len(self._workers), + ) + self._start_worker(missing) + def _start_worker(self, page_indices: list[int]) -> None: - self._worker = ThumbnailWorker( - self._doc_path, page_indices, self._password, self._epoch, self) + # Read the LIVE device-pixel ratio off the view (falls back to the + # panel, then 1.0) so thumbnails render sharp on HiDPI screens. + # Never hardcoded — see memory/feedback_icons.md. + dpr = 1.0 + with contextlib.suppress(Exception): + dpr = (self._view.devicePixelRatioF() + or self.devicePixelRatioF() or 1.0) + worker = ThumbnailWorker( + self._doc_path, page_indices, self._password, self._epoch, + dpr, self) # Explicit QueuedConnection so the slot runs on this (main) # thread rather than the worker — required for QPixmap # construction, and matches the Py3.14 PySide6 routing rules # documented in memory/project_compress_freeze_py314.md. - self._worker.thumbnail_ready.connect( + worker.thumbnail_ready.connect( self._on_image_ready, Qt.ConnectionType.QueuedConnection, ) - self._worker.start() + worker.render_failed.connect( + self._on_render_failed, + Qt.ConnectionType.QueuedConnection, + ) + # When the worker's run() returns, reconcile _inflight on the + # main thread: release any requested page it did NOT deliver so a + # later scroll can re-request it (see _on_worker_finished). Queued + # so the slot runs here, not in the worker thread. + worker.finished.connect( + self._on_worker_finished, + Qt.ConnectionType.QueuedConnection, + ) + self._workers.append(worker) + _log.debug("start_worker: %d pages epoch=%d", + len(page_indices), self._epoch) + worker.start() @Slot(int, QImage, int) def _on_image_ready(self, page_idx: int, img: QImage, epoch: int) -> None: @@ -419,67 +740,153 @@ def _on_image_ready(self, page_idx: int, img: QImage, epoch: int) -> None: page_idx + 1, epoch, self._epoch, ) return - _log.debug( - "Thumbnail received for page %d (%dx%d)", - page_idx + 1, img.width(), img.height(), - ) + # This page is no longer in flight; a future scroll that re-visits + # it will now see has_pixmap() True and skip it (or, if evicted, + # re-request it). + self._inflight.discard(page_idx) pix = QPixmap.fromImage(img) + # fromImage propagates the image's devicePixelRatio, but re-assert + # it explicitly so the delegate can always read the correct HiDPI + # scale off the pixmap regardless of binding quirks. + pix.setDevicePixelRatio(img.devicePixelRatio()) self._model.cache_pixmap(page_idx, pix) + _log.debug( + "image_ready: page %d (%dx%d) cache=%d", + page_idx + 1, img.width(), img.height(), self._model.cache_size(), + ) + + @Slot(int, str) + def _on_render_failed(self, epoch: int, reason: str) -> None: + """Runs on the main thread. Surface a worker that produced no + thumbnails instead of leaving the sidebar silently stuck on + placeholders. Stale generations are ignored.""" + # A worker that rendered zero pages left ALL its requested pages + # pinned in _inflight; release them (idempotent with the finished + # handler) so they aren't stranded on placeholders forever. + worker = self.sender() + if isinstance(worker, ThumbnailWorker): + self._release_inflight_for(worker) + if epoch != self._epoch: + return + _log.warning( + "Thumbnail rendering failed for %r: %s", + self._doc_path, reason, + ) + + @Slot() + def _on_worker_finished(self) -> None: + """Runs on the main thread when a worker's run() returns. - def _stop_worker(self) -> None: - if self._worker is None: + Reconcile bookkeeping so a page can never be stranded in + ``_inflight`` by a worker that failed to deliver it, and drop the + finished QThread so they don't accumulate over a long session. + """ + worker = self.sender() + if not isinstance(worker, ThumbnailWorker): + return + self._release_inflight_for(worker) + with contextlib.suppress(ValueError): + self._workers.remove(worker) + # run() has returned by the time finished fires and we've already + # reconciled its inflight pages, so it's safe to schedule deletion + # (MINOR-1: don't leak finished QThreads parented to the panel). + worker.deleteLater() + + def _release_inflight_for(self, worker: ThumbnailWorker) -> None: + """Release from ``_inflight`` every page this worker was asked to + render but did NOT deliver (i.e. is not in the model cache). + + Called when a worker finishes or fails. Without it, a page the + worker skipped — out of range, a per-page render exception, or a + whole-document open failure — would stay pinned in ``_inflight`` + forever, and ``_render_visible`` (which excludes in-flight pages) + would never request it again: placeholder-forever by failed page. + Delivered pages were already discarded from ``_inflight`` in + ``_on_image_ready`` and remain cached, so they are left alone. + Not re-triggering a render here is deliberate: a genuinely broken + page is retried only when the user scrolls it back into view, + never in a tight busy-loop. + """ + # Stale generation: set_document/clear already cleared _inflight + # for the new document, so this worker's page numbers no longer + # refer to the current in-flight set. Touching it would corrupt + # the new generation's bookkeeping. + if worker._epoch != self._epoch: return - self._worker.cancel() - # Disconnect BEFORE waiting so a thumbnail emitted between the - # cancel flag check and the worker unwinding can't sneak into - # the model for the old document. The epoch guard in - # _on_image_ready is the belt; this disconnect is the braces. - with contextlib.suppress(RuntimeError, TypeError): - self._worker.thumbnail_ready.disconnect(self._on_image_ready) - # 2 s is generous — a single page render at 40 DPI is well - # under 100 ms; the cancel flag is polled between pages so - # worst case we wait for the current page to finish. - self._worker.wait(2000) - self._worker = None + for page in worker._pages: + if not self._model.has_pixmap(page): + self._inflight.discard(page) + + def _stop_all_workers(self) -> None: + """Cancel + join every live worker. Called on doc change / close + so a stale QThread can't outlive the panel or leak thumbnails + into the next document.""" + workers, self._workers = self._workers, [] + for worker in workers: + worker.cancel() + # Disconnect BEFORE waiting so a thumbnail emitted between the + # cancel flag check and the worker unwinding can't sneak into + # the model for the old document. The epoch guard in + # _on_image_ready is the belt; this disconnect is the braces. + with contextlib.suppress(RuntimeError, TypeError): + worker.thumbnail_ready.disconnect(self._on_image_ready) + with contextlib.suppress(RuntimeError, TypeError): + worker.render_failed.disconnect(self._on_render_failed) + # 2 s is generous — a single page render at 40 DPI is well + # under 100 ms; the cancel flag is polled between pages so + # worst case we wait for the current page to finish. + worker.wait(2000) def showEvent(self, event): # noqa: D401 - """Force a viewport repaint when the panel becomes visible. - - The worker fills the model's pixmap cache in the background even - while this widget is hidden (e.g. the sidebar is on the - "Contents" tab). Qt normally coalesces / drops repaints for - hidden widgets, so ``dataChanged`` signals emitted during that - window never reach the viewport. Calling ``update()`` here on - first show — and on every subsequent show — makes the QListView - redraw all visible rows against the current cache so any - pixmaps that arrived while hidden appear immediately instead of - remaining as ``…`` placeholders. + """Render + repaint the now-visible rows when the panel appears. + + The Pages tab is typically hidden when the document loads (parked + behind the Contents tab for a PDF that has a TOC), so this is + where the real visible range first becomes knowable. We (1) kick + the lazy renderer for the pages now on screen and (2) force the + view to re-read DecorationRole against the cache so any pixmaps + that arrived while hidden are painted instead of staying on ``…`` + placeholders. """ super().showEvent(event) - if self._view is not None: - vp = self._view.viewport() - if vp is not None: - vp.update() - # Layout can still be incomplete when showEvent fires - # (e.g. the tab was just switched — viewport size is - # still the collapsed placeholder value). Schedule a - # second update after the current event-loop iteration - # so it runs against the final resized geometry. Without - # this, the first update() marks the wrong (tiny) area - # dirty and Qt does not repaint the newly-resized - # region. - QTimer.singleShot(0, vp.update) + if self._view is None: + return + _log.debug("showEvent: visible=%s cache=%d", + self.isVisible(), self._model.cache_size()) + self._render_visible() + vp = self._view.viewport() + if vp is not None: + self._model.refresh_decorations() + vp.update() + + # Layout can still be incomplete when showEvent fires (the tab + # was just switched — viewport size is still the collapsed + # placeholder value). Re-run after the current event-loop + # iteration against the final geometry so we render/repaint the + # rows that are actually on screen. + def _refresh_again(): + if self._view is None: + return + self._render_visible() + self._model.refresh_decorations() + v = self._view.viewport() + if v is not None: + v.update() + QTimer.singleShot(0, _refresh_again) def resizeEvent(self, event): # noqa: D401 - """Force viewport repaint on resize so newly-visible rows paint - against the current cache (not against stale/degenerate - geometry from before the layout settled).""" + """Render + repaint newly-visible rows on resize so growing the + panel (or the first real layout after a collapsed one) fills in + thumbnails instead of leaving fresh rows on placeholders.""" super().resizeEvent(event) - if self._view is not None: - vp = self._view.viewport() - if vp is not None: - vp.update() + if self._view is None: + return + self._render_visible() + vp = self._view.viewport() + if vp is not None: + self._model.refresh_decorations() + vp.update() def closeEvent(self, event): # noqa: D401 - self._stop_worker() + self._stop_all_workers() super().closeEvent(event) diff --git a/tests/test_thumbnails.py b/tests/test_thumbnails.py index 6d18bcd..bf2dd46 100644 --- a/tests/test_thumbnails.py +++ b/tests/test_thumbnails.py @@ -10,6 +10,7 @@ from __future__ import annotations import json +import os import sys from pathlib import Path @@ -19,6 +20,11 @@ ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(ROOT)) +# Headless-safe: the end-to-end render test builds real QWidgets, so a +# QApplication on the offscreen platform must exist before Qt classes +# are dereferenced (matches tests/test_atomic_pdf_write.py). +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + THUMBS_SRC = (ROOT / "app" / "viewer" / "thumbnails.py").read_text( encoding="utf-8") @@ -97,6 +103,30 @@ def test_cache_has_size_limit(): assert "CACHE_MAX" in THUMBS_SRC +def test_worker_renders_dpr_aware(): + """Thumbnails must render at target_size * devicePixelRatio and tag + the image with the ratio, otherwise HiDPI screens show a soft, + upscaled image (regression fix). The DPR must be read live, never + hardcoded.""" + # Worker takes a dpr and sets it on the emitted image. + assert "dpr" in THUMBS_SRC + assert "setDevicePixelRatio" in THUMBS_SRC + # Render must be matrix-driven (fit to the target box) rather than a + # fixed low DPI that then gets upscaled on paint. + assert "fitz.Matrix(zoom, zoom)" in THUMBS_SRC + assert "dpi=40" not in THUMBS_SRC, ( + "fixed 40 DPI render is the blurry-thumbnail regression" + ) + # DPR is read off the live widget, not a constant. + assert "devicePixelRatioF()" in THUMBS_SRC + + +def test_delegate_scales_dpr_aware(): + """The paint path must scale in device pixels (target * dpr) and + re-tag the pixmap so the drawn thumbnail is crisp on HiDPI.""" + assert "pix.devicePixelRatio()" in THUMBS_SRC + + # ── Model behaviour ─────────────────────────────────────────────────── @@ -153,6 +183,355 @@ def test_model_data_returns_none_out_of_range(qapp): assert m.data(good, Qt.ItemDataRole.DisplayRole) == "1" +# ── End-to-end render (real fitz + QThread worker) ──────────────────── + + +def _make_real_pdf(path: Path, pages: int = 2) -> Path: + """Write a small multi-page PDF with visible text via fitz.""" + import fitz + doc = fitz.open() + for i in range(pages): + page = doc.new_page(width=595, height=842) + page.insert_text((72, 144), f"Thumbnail test page {i + 1}", + fontsize=24) + doc.save(str(path)) + doc.close() + return path + + +def test_end_to_end_worker_populates_cache(qapp, tmp_path): + """Real reproduction of the sidebar bug: build a genuine PDF, + hand it to a ThumbnailPanel, spin the event loop, and assert the + background worker actually delivered pixmaps into the model cache. + + If this fails with cache_size() == 0 the render pipeline is broken + (import fitz, get_pixmap, epoch guard, or main-thread QPixmap + conversion) — exactly the placeholder-forever symptom.""" + from PySide6.QtTest import QTest + from app.viewer.thumbnails import ThumbnailPanel + + pdf = _make_real_pdf(tmp_path / "e2e.pdf", pages=2) + + panel = ThumbnailPanel() + try: + panel.set_document(str(pdf), 2) + # Wait until the worker finishes (or a generous timeout). + deadline = 5000 + waited = 0 + while panel._model.cache_size() < 2 and waited < deadline: + QTest.qWait(50) + waited += 50 + assert panel._model.cache_size() >= 1, ( + "worker delivered no thumbnails — render pipeline is broken" + ) + assert panel._model.cache_size() == 2, ( + f"expected 2 thumbnails, got {panel._model.cache_size()}" + ) + # The delivered pixmap must be a real, non-null image. + from PySide6.QtCore import Qt + pix = panel._model.data( + panel._model.index(0), Qt.ItemDataRole.DecorationRole) + assert pix is not None and not pix.isNull() + finally: + panel.clear() + panel.deleteLater() + + +def test_worker_hidpi_render_is_higher_res_and_tagged(qapp, tmp_path): + """A dpr=2 render must produce ~2× the raw pixels of a dpr=1 render + and carry devicePixelRatio==2, while keeping the same logical + (device-independent) size. This is the crux of the sharpness fix: + the widget draws it at THUMB size but with HiDPI detail instead of + upscaling a low-res image.""" + from PySide6.QtTest import QTest + from app.viewer.thumbnails import ThumbnailWorker + + pdf = _make_real_pdf(tmp_path / "hidpi.pdf", pages=1) + + def _render(dpr: float): + got: list = [] + w = ThumbnailWorker(str(pdf), [0], epoch=1, dpr=dpr) + w.thumbnail_ready.connect(lambda i, img, e: got.append(img)) + w.start() + w.wait(3000) + QTest.qWait(50) + assert got, f"no thumbnail delivered at dpr={dpr}" + return got[0] + + img1 = _render(1.0) + img2 = _render(2.0) + + assert img1.devicePixelRatio() == 1.0 + assert img2.devicePixelRatio() == 2.0 + # Roughly double the raw pixels along each axis. + assert img2.width() >= int(img1.width() * 1.7) + assert img2.height() >= int(img1.height() * 1.7) + # Same logical footprint (device-independent size) within rounding. + log1_w = img1.width() / img1.devicePixelRatio() + log2_w = img2.width() / img2.devicePixelRatio() + assert abs(log1_w - log2_w) <= 2 + + +def test_worker_signals_render_failed_on_bad_path(qapp, tmp_path): + """A worker that can open nothing must not die silently: it emits + render_failed so the panel can log/reflect instead of leaving the + sidebar stuck on placeholders forever.""" + from PySide6.QtTest import QTest + from app.viewer.thumbnails import ThumbnailWorker + + bad = str(tmp_path / "does_not_exist.pdf") + worker = ThumbnailWorker(bad, [0, 1, 2], epoch=7) + got: list[tuple[int, str]] = [] + worker.render_failed.connect(lambda e, r: got.append((e, r))) + + worker.start() + waited = 0 + while worker.isRunning() and waited < 3000: + QTest.qWait(20) + waited += 20 + worker.wait(2000) + # Let the queued signal drain on the main thread. + QTest.qWait(50) + assert got, "worker did not emit render_failed on an unopenable path" + assert got[0][0] == 7 # epoch propagated + + +def test_worker_no_render_failed_when_cancelled(qapp, tmp_path): + """Cancellation is not a failure — no render_failed should fire.""" + from PySide6.QtTest import QTest + from app.viewer.thumbnails import ThumbnailWorker + + pdf = _make_real_pdf(tmp_path / "cancel.pdf", pages=1) + worker = ThumbnailWorker(str(pdf), [0], epoch=1) + got: list = [] + worker.render_failed.connect(lambda e, r: got.append((e, r))) + worker.cancel() # cancel before start → loop breaks immediately + worker.start() + worker.wait(2000) + QTest.qWait(50) + assert got == [] + + +def test_hidden_tab_thumbnails_paint_when_shown(qapp, tmp_path): + """Exercise the real app scenario: the ThumbnailPanel lives as the + second ("Pages") tab of a QTabWidget while another tab is active, so + the panel is HIDDEN when the document is loaded — mirroring + ``ViewerPage`` where the "Contents" tab (index 0) stays active for a + PDF that has a TOC while ``set_document`` runs on the parked Pages + tab (see app/viewer/panel.py:161-168, 605). + + What this test actually validates + --------------------------------- + * The background worker fills the model cache even while the panel is + hidden (``cache_size == pages``) — rendering does not depend on + visibility. + * While the panel is the *inactive* tab it receives ZERO delegate + paints (the "before" snapshot), so the "after" assertion is + genuinely discriminating rather than trivially true. + * Once the QTabWidget switches to the Pages tab, the delegate is + painted with NON-NULL pixmaps and ``data(DecorationRole)`` returns + a real pixmap for the visible rows — the cached-while-hidden + thumbnails reach the paint path instead of staying "…" placeholders. + + What this test does NOT prove + ----------------------------- + The ``offscreen`` QPA backend does not reproduce the *field* bug: a + counterfactual run with ``ThumbnailModel.refresh_decorations`` + neutered still paints every row with a non-null pixmap the moment the + tab is revealed, because offscreen Qt re-reads the model on show + anyway. So this is a regression test for the hidden→visible FLOW; it + cannot, on this platform, confirm or refute that the ``showEvent`` + ``refresh_decorations`` repaint fix is what resolves the on-device + placeholder-forever symptom. That distinction is intentional — see + the parent task notes — rather than a trivially-green assert that + would give false confidence in the fix. + """ + from PySide6.QtCore import Qt + from PySide6.QtTest import QTest + from PySide6.QtWidgets import QLabel, QTabWidget + from app.viewer.thumbnails import ThumbnailDelegate, ThumbnailPanel + + pages = 3 + pdf = _make_real_pdf(tmp_path / "hidden_tab.pdf", pages=pages) + + # Instrument the delegate: count paints and how many carried a real + # (non-null) pixmap. Subclassing (not monkeypatching the instance) so + # PySide6's C++ virtual dispatch reliably reaches our override. + paint_log = {"total": 0, "with_pixmap": 0} + + class CountingDelegate(ThumbnailDelegate): + def paint(self, painter, option, index): + paint_log["total"] += 1 + pix = index.data(Qt.ItemDataRole.DecorationRole) + if pix is not None and not pix.isNull(): + paint_log["with_pixmap"] += 1 + super().paint(painter, option, index) + + panel = ThumbnailPanel() + delegate = CountingDelegate(panel) + panel._view.setItemDelegate(delegate) + panel._delegate = delegate + + tabs = QTabWidget() + other = QLabel("contents") + try: + tabs.addTab(other, "Contents") # index 0 – active + tabs.addTab(panel, "Pages") # index 1 – starts hidden + tabs.setCurrentIndex(0) + tabs.resize(400, 600) + tabs.show() + QTest.qWait(50) + + assert not panel.isVisible(), ( + "precondition failed: the Pages tab must start hidden so the " + "hidden→visible transition is actually exercised" + ) + + # Load the document while the panel is the inactive tab. + panel.set_document(str(pdf), pages) + waited = 0 + while panel._model.cache_size() < pages and waited < 5000: + QTest.qWait(50) + waited += 50 + + # The worker renders regardless of visibility. + assert panel._model.cache_size() == pages, ( + f"worker did not fill cache while hidden: " + f"{panel._model.cache_size()}/{pages}" + ) + + # A genuinely-hidden inactive tab must not have been painted yet; + # this is what makes the post-switch assertion meaningful. + before = dict(paint_log) + assert before["with_pixmap"] == 0, ( + "expected zero delegate paints while the panel was hidden, " + f"got {before}" + ) + + # Reveal the Pages tab; give the showEvent + its queued + # singleShot(0) second refresh time to run. + tabs.setCurrentIndex(1) + QTest.qWait(50) + QTest.qWait(100) + + assert panel.isVisible(), "panel should be visible after switching" + + after = dict(paint_log) + painted_with_pixmap = after["with_pixmap"] - before["with_pixmap"] + assert painted_with_pixmap > 0, ( + "after revealing the Pages tab the delegate was never painted " + f"with a real pixmap (before={before}, after={after}) — the " + "cached-while-hidden thumbnails never reached the paint path" + ) + + # And the model still serves a real pixmap for the first row. + pix = panel._model.data( + panel._model.index(0), Qt.ItemDataRole.DecorationRole) + assert pix is not None and not pix.isNull(), ( + "DecorationRole returned no pixmap for row 0 after the tab " + "became visible" + ) + finally: + panel.clear() + tabs.deleteLater() + panel.deleteLater() + + +def test_large_doc_top_pages_not_evicted_when_shown(qapp, tmp_path): + """Regression for the ebook placeholder-forever bug. + + A PDF longer than ``CACHE_MAX`` used to be rendered eagerly, page by + page, into an LRU cache smaller than the document. By the time the + worker finished, the EARLY pages (the top of the list — exactly what + the user sees when they open the Pages tab) had been evicted by the + later ones, so row 0 held no pixmap and nothing ever re-rendered it: + placeholders forever. It only reproduced with big docs, and it was + *permanent* (rather than merely flickering) when the Pages tab was + parked behind the Contents tab of a TOC PDF — hidden while the worker + ran, so the early pages were never even painted before eviction. + + This is a cache/data assertion (not a paint one) so the offscreen QPA + backend reproduces it deterministically: the fix renders only the + visible window lazily, so the on-screen rows are always populated and + the cache never exceeds a small sliding window. + + Discriminating power + -------------------- + The old eager code rendered ``range(page_count)`` up-front into an LRU + cache smaller than the document, so it (a) evicted row 0 (leaving it + ``None``) and (b) ended with ``cache_size() == CACHE_MAX`` (200). Both + final assertions below therefore FAIL against the old code and only + PASS against the lazy-window fix: + + * we join the real workers (no fixed short sleep that could sample the + cache *before* eviction happens) — under the old code the join + settles with row 0 evicted; + * we require ``cache_size()`` to be a SMALL window (``< 30``), which + the eager 200-entry cache can never satisfy. + """ + from PySide6.QtCore import Qt + from PySide6.QtTest import QTest + from PySide6.QtWidgets import QLabel, QTabWidget + from app.viewer.thumbnails import CACHE_MAX, ThumbnailPanel + + pages = CACHE_MAX + 40 # longer than the cache → old code evicted top + pdf = _make_real_pdf(tmp_path / "ebook.pdf", pages=pages) + + panel = ThumbnailPanel() + tabs = QTabWidget() + other = QLabel("contents") + try: + tabs.addTab(other, "Contents") # index 0 – active (TOC case) + tabs.addTab(panel, "Pages") # index 1 – hidden during load + tabs.setCurrentIndex(0) + tabs.resize(320, 600) + tabs.show() + QTest.qWait(30) + assert not panel.isVisible() + + panel.set_document(str(pdf), pages) + # Reveal the Pages tab and JOIN the real render workers until they + # settle (row 0 delivered, nothing in flight, no thread running) — + # a deterministic wait rather than a short fixed sleep that could + # sample the cache before the old eager code had a chance to evict + # the top of the list. + tabs.setCurrentIndex(1) + m = panel._model + + def _settled() -> bool: + row0 = m.data(m.index(0), Qt.ItemDataRole.DecorationRole) + workers_done = all(not w.isRunning() for w in panel._workers) + return (row0 is not None + and not panel._inflight + and workers_done) + + waited = 0 + while not _settled() and waited < 5000: + QTest.qWait(50) + waited += 50 + + # The row the user is looking at (top of the list) must have a + # real thumbnail — the whole point of the fix. Under the old eager + # code it has been LRU-evicted by the trailing pages: None. + pix0 = m.data(m.index(0), Qt.ItemDataRole.DecorationRole) + assert pix0 is not None and not pix0.isNull(), ( + "top page has no thumbnail — eager-render-then-evict " + "regression is back" + ) + # The cache must be a SMALL sliding window, not the whole document + # capped at CACHE_MAX. The old eager code ends at CACHE_MAX (200), + # so this fails there and only passes with lazy windowing. + assert m.cache_size() < 30, ( + f"cache holds {m.cache_size()} pages — expected a small " + "visible window; the whole document was rendered eagerly " + "(lazy windowing is broken / regressed)" + ) + finally: + panel.clear() + tabs.deleteLater() + panel.deleteLater() + + # ── Panel integration ──────────────────────────────────────────────── @@ -182,6 +561,70 @@ def test_panel_updates_thumbnails_theme(): assert "self._thumbnails.update_theme(dark)" in PANEL_SRC +def test_panel_activates_contents_tab_when_toc_present(): + """Guard the TOC regression: the outline branch must re-select the + Contents tab so it doesn't stay parked on Pages after a prior no-TOC + document switched the active tab.""" + assert "self._sidebar_tabs.setCurrentIndex(self._toc_tab_idx)" in PANEL_SRC + + +def test_toc_tab_active_by_default_across_reload(qapp, tmp_path): + """Behavioural regression: after loading a no-TOC PDF (which hides the + Contents tab and activates Pages) then a PDF WITH a TOC, the Contents + tab must be visible AND active — the original behaviour. Before the + fix it stayed on the Pages tab, so the outline appeared to be gone.""" + import fitz + from PySide6.QtTest import QTest + from app.viewer.panel import PdfViewerPanel + + def _make(name, toc): + p = tmp_path / name + d = fitz.open() + for i in range(6): + pg = d.new_page(width=595, height=842) + pg.insert_text((72, 144), f"P{i + 1}", fontsize=20) + if toc: + d.set_toc(toc) + d.save(str(p)) + d.close() + return str(p) + + toc_pdf = _make("toc.pdf", [[1, "Ch1", 1], [1, "Ch2", 4]]) + plain_pdf = _make("plain.pdf", None) + + panel = PdfViewerPanel() + try: + panel.resize(900, 600) + panel.show() + QTest.qWait(30) + tabs = panel._sidebar_tabs + + panel.load(toc_pdf) + QTest.qWait(60) + assert tabs.isTabVisible(panel._toc_tab_idx) + assert tabs.currentIndex() == panel._toc_tab_idx + + panel.load(plain_pdf) + QTest.qWait(60) + # No TOC → Contents hidden, Pages active. + assert not tabs.isTabVisible(panel._toc_tab_idx) + assert tabs.currentIndex() == panel._pages_tab_idx + + panel.load(toc_pdf) + QTest.qWait(60) + # TOC back → both tabs present and Contents active by default. + assert tabs.isTabVisible(panel._toc_tab_idx) + assert tabs.isTabVisible(panel._pages_tab_idx) + assert tabs.currentIndex() == panel._toc_tab_idx, ( + "Contents tab must be active by default when the PDF has a TOC" + ) + finally: + panel._thumbnails.clear() + panel.close() + panel.deleteLater() + QTest.qWait(30) + + # ── i18n ────────────────────────────────────────────────────────────── From c15f32c5fe7c861b75beaf449649c3cef02af42c Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Mon, 13 Jul 2026 19:57:20 +0100 Subject: [PATCH 5/8] docs(website): document thumbnails panel and fix version-bump drift Extend release.yml so a single release run rewrites the version in every docs page (download/features/privacy/docs) via the same v{old}->v{new} pattern already used for index/changelog, eliminating the drift that left those pages pinned to an old tag. Repair the 5 stale v1.13.16 references to match the current 1.13.17. Backfill the missing v1.13.16 changelog entry and document the new viewer page-thumbnails panel in features.html and docs.html. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/release.yml | 17 +++++++++++++++++ docs/changelog.html | 17 +++++++++++++++++ docs/docs.html | 9 ++++++++- docs/download.html | 4 ++-- docs/features.html | 9 ++++++++- docs/privacy.html | 2 +- 6 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3a2d8ec..5528a86 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -83,6 +83,23 @@ jobs: ("docs/changelog.html", [ (rf'v{old_re}', f'v{new}'), ], False), + # Every remaining marketing page carries the version in an + # eyebrow badge and/or the footer changelog link. They use the + # same ``v{old}`` -> ``v{new}`` rewrite as index/changelog so a + # single release run keeps all pages in lock-step (prevents the + # docs version drift where these stayed pinned to an old tag). + ("docs/download.html", [ + (rf'v{old_re}', f'v{new}'), + ], False), + ("docs/features.html", [ + (rf'v{old_re}', f'v{new}'), + ], False), + ("docs/privacy.html", [ + (rf'v{old_re}', f'v{new}'), + ], False), + ("docs/docs.html", [ + (rf'v{old_re}', f'v{new}'), + ], False), ("snap/snapcraft.yaml", [ (rf"version:\s*'{ANY}'", f"version: '{new}'"), ], True), diff --git a/docs/changelog.html b/docs/changelog.html index 283937f..e0f7c09 100644 --- a/docs/changelog.html +++ b/docs/changelog.html @@ -85,6 +85,23 @@

Editor audit fixes, atomic saves, and streaming OCR

+
+
+ v1.13.16 + June 18, 2026 +
+

Audit hardening rounds 10 & 11

+
    +
  • Editor: 13 audit findings resolved (1 critical, 7 high, 5 medium plus low-severity polish).
    • +
  • Round 10: 12 findings fixed (2 critical, 5 high, 3 medium, 2 low).
    • +
  • Round 11: 8 findings fixed (3 high, 5 medium).
    • +
  • Polish: 10 low-severity fixes plus a further cluster of 20 medium and low findings.
    • +
  • Snap Store: permanent version-bump fix that unfroze the Snap publication pipeline at v1.13.15.
    • +
  • CI: pinned the snapcore actions to the Node.js 24 runtime and ignored local artifacts-*/ release folders.
    • +
  • Dependencies: pypdf 6.13.0, PySide6 6.11.1, shiboken6 6.11.1.
    • +
+
+
v1.13.15 diff --git a/docs/docs.html b/docs/docs.html index a51877f..9d29866 100644 --- a/docs/docs.html +++ b/docs/docs.html @@ -74,6 +74,7 @@

Getting Started

Viewer

+
+

Page thumbnails

+

The viewer sidebar can show a panel of page miniatures. Scroll through it and click any thumbnail to jump straight to that page; the current page stays highlighted as you move around the document. Thumbnails render lazily as they scroll into view, so even long PDFs open instantly, and they stay sharp on HiDPI displays.

+

When the PDF has an outline, the panel also offers a Contents tab with the clickable table of contents, so you can switch between browsing by thumbnail and jumping by chapter. For PDFs without bookmarks the panel simply shows the thumbnails.

+
+
diff --git a/docs/download.html b/docs/download.html index 72bc8fa..efa154f 100644 --- a/docs/download.html +++ b/docs/download.html @@ -56,7 +56,7 @@
- v1.13.16 · Now on the Microsoft Store + v1.13.17 · Now on the Microsoft Store

Download PDFApps

Free forever, open source. Choose your platform below; installation takes less than a minute.

@@ -227,7 +227,7 @@

Ghostscript

© 2026 Nelson Duarte · Made with ♥ for the open web
GitHub - v1.13.16 + v1.13.17
diff --git a/docs/features.html b/docs/features.html index dab4f4f..9cbd1e5 100644 --- a/docs/features.html +++ b/docs/features.html @@ -226,6 +226,13 @@

Read PDFs the way you want

Bookmarks / TOC

Clickable side panel showing the PDF outline. Jump to any chapter or section.

+
+
+ +
+

Page thumbnails

+

A sidebar panel with miniatures of every page. Scroll and click to jump anywhere; thumbnails render as they come into view and stay crisp on HiDPI screens. A Contents tab appears when the PDF has an outline.

+
@@ -343,7 +350,7 @@

Try it for yourself

© 2026 Nelson Duarte · Made with ♥ for the open web
GitHub - v1.13.16 + v1.13.17
diff --git a/docs/privacy.html b/docs/privacy.html index 567fbef..884fbdf 100644 --- a/docs/privacy.html +++ b/docs/privacy.html @@ -193,7 +193,7 @@

10. Contact

© 2026 Nelson Duarte · Made with ♥ for the open web
GitHub - v1.13.16 + v1.13.17
From 1dff48765dae7e141969d04fd95f3b32fa512a06 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Mon, 13 Jul 2026 20:06:35 +0100 Subject: [PATCH 6/8] fix(release): anchor changelog bump to footer link to stop clobbering history The "Bump version everywhere" step rewrote every `v{old}` occurrence in docs/changelog.html, including the topmost historical release heading (``), not just the footer link. That is why v1.13.16 "disappeared" in 72f25e8 (its heading was renamed to v1.13.17). The next release would have corrupted the v1.13.17 entry the same way. - Anchor the changelog rewrite to the footer changelog link only: `()v{old}()`. Release headings and the "Latest" badge are now an editorial step done by hand. - Mark changelog + the four marketing pages (download/features/privacy/ docs) required=True so a future drift fails the release loudly instead of being silently masked. These pages carry the current version only (verified by grep), so their global `v{old}` rewrite stays safe. - Add the v1.14.0 changelog entry (page thumbnails sidebar) and move the "Latest" badge off v1.13.17, leaving that entry intact. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/release.yml | 31 ++++++++++++++++++++----------- docs/changelog.html | 17 ++++++++++++++++- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5528a86..72499e7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -80,26 +80,35 @@ jobs: (rf'"softwareVersion":\s*"{ANY}"', f'"softwareVersion": "{new}"'), (rf'v{old_re}', f'v{new}'), ], True), + # changelog.html carries the *whole* release history: every past + # release has a ``v...`` heading + # that must NEVER change. A global ``v{old}`` rewrite clobbered the + # topmost historical heading every release (this is why v1.13.16 + # "disappeared" in 72f25e8: its heading was renamed to v1.13.17). + # Anchor the bump to the footer changelog link ONLY. New release + # headings (incl. moving the "Latest" badge) are an editorial step + # done by hand in the same PR that ships the feature work. ("docs/changelog.html", [ - (rf'v{old_re}', f'v{new}'), - ], False), - # Every remaining marketing page carries the version in an - # eyebrow badge and/or the footer changelog link. They use the - # same ``v{old}`` -> ``v{new}`` rewrite as index/changelog so a - # single release run keeps all pages in lock-step (prevents the - # docs version drift where these stayed pinned to an old tag). + (rf'()v{old_re}()', rf'\g<1>v{new}\g<2>'), + ], True), + # Every remaining marketing page carries the current version only, + # in an eyebrow badge and/or the footer changelog link (no historical + # version strings — verified by grep). The global ``v{old}`` -> ``v{new}`` + # rewrite is therefore safe and keeps all pages in lock-step. required=True + # so the release fails loudly if a page ever drifts off the old tag + # (instead of silently masking the drift). ("docs/download.html", [ (rf'v{old_re}', f'v{new}'), - ], False), + ], True), ("docs/features.html", [ (rf'v{old_re}', f'v{new}'), - ], False), + ], True), ("docs/privacy.html", [ (rf'v{old_re}', f'v{new}'), - ], False), + ], True), ("docs/docs.html", [ (rf'v{old_re}', f'v{new}'), - ], False), + ], True), ("snap/snapcraft.yaml", [ (rf"version:\s*'{ANY}'", f"version: '{new}'"), ], True), diff --git a/docs/changelog.html b/docs/changelog.html index e0f7c09..cda72ab 100644 --- a/docs/changelog.html +++ b/docs/changelog.html @@ -65,11 +65,26 @@

Changelog

+
+
+ v1.14.0 + July 13, 2026 + Latest +
+

Page thumbnails sidebar in the viewer

+
    +
  • Page thumbnails: the viewer sidebar now has a Pages tab showing a live thumbnail of every page. Click a thumbnail to jump straight to that page; the current page stays highlighted and in sync as you scroll.
    • +
  • Fast & crisp: thumbnails render on demand for the pages you are actually looking at, so even large documents open instantly, and they stay sharp on high-DPI displays.
    • +
  • Contents tab: when a PDF has bookmarks, the sidebar opens on a Contents tab with the document outline; the bookmark button now toggles the whole sidebar.
    • +
  • Encrypted PDFs: password-protected documents are now verified correctly when opened for viewing, with a clear error on the wrong password instead of a silent failure.
    • +
  • Store updates: hardened version handling for Microsoft Store update notifications so stale or malformed entries can no longer suppress a genuine update prompt.
    • +
+
+
v1.13.17 June 21, 2026 - Latest

Editor audit fixes, atomic saves, and streaming OCR

    From 2a47af014a3a8551801afc72ed5f55de4668e0ea Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Mon, 13 Jul 2026 20:14:49 +0100 Subject: [PATCH 7/8] test(release): update bump fixture for anchored changelog footer The release.yml bump step now anchors the changelog rewrite to the footer changelog link and marks changelog + the four marketing pages required=True. The fixture's changelog.html carried a bare "v1.13.14" (no footer link) and never created download/features/privacy/docs.html, so the bump script exited 1 (stale_required + missing_required). Align the fixture with the real HTML: add a footer changelog link (and a historical version-tag heading) and the four required marketing pages, each carrying the v{old} string. Co-Authored-By: Claude Opus 4.8 --- tests/test_release_bump_script.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_release_bump_script.py b/tests/test_release_bump_script.py index 0e4ae61..e2799e8 100644 --- a/tests/test_release_bump_script.py +++ b/tests/test_release_bump_script.py @@ -49,7 +49,18 @@ def fake_repo(tmp_path: Path) -> Path: (tmp_path / "docs" / "index.html").write_text( '"softwareVersion": "1.13.14"\nv1.13.14\n', encoding="utf-8" ) - (tmp_path / "docs" / "changelog.html").write_text("v1.13.14\n", encoding="utf-8") + # The changelog rewrite is anchored to the footer changelog link only + # (release headings carry the whole history and must never be clobbered), + # so the fixture must expose that exact footer-link shape to be matched. + (tmp_path / "docs" / "changelog.html").write_text( + 'v1.13.14\n' + 'v1.13.14\n', + encoding="utf-8", + ) + # The four remaining marketing pages carry the current version only and are + # required=True, so each must contain a v{old} string for the bump to match. + for page in ("download.html", "features.html", "privacy.html", "docs.html"): + (tmp_path / "docs" / page).write_text("v1.13.14\n", encoding="utf-8") (tmp_path / "snap").mkdir() # Frozen one minor behind app/constants.py — this is exactly the # bug we're guarding against. From de63e95903c0fc5afb8fe5f37664353cd4773365 Mon Sep 17 00:00:00 2001 From: nelsonduarte Date: Mon, 13 Jul 2026 20:47:40 +0100 Subject: [PATCH 8/8] refactor(thumbnails): drop redundant dpr assignment (CodeQL) The leading `dpr = 1.0` before the `contextlib.suppress` block was flagged by CodeQL as a dead store (redefined before use), since the suppress body is modelled as unconditional. Convert to try/except so the 1.0 fallback lives only in the except branch (reached solely on failure). Live view read -> panel fallback -> 1.0 semantics are unchanged. Co-Authored-By: Claude Opus 4.8 --- app/viewer/thumbnails.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/viewer/thumbnails.py b/app/viewer/thumbnails.py index 970025c..d7d719f 100644 --- a/app/viewer/thumbnails.py +++ b/app/viewer/thumbnails.py @@ -690,10 +690,11 @@ def _start_worker(self, page_indices: list[int]) -> None: # Read the LIVE device-pixel ratio off the view (falls back to the # panel, then 1.0) so thumbnails render sharp on HiDPI screens. # Never hardcoded — see memory/feedback_icons.md. - dpr = 1.0 - with contextlib.suppress(Exception): + try: dpr = (self._view.devicePixelRatioF() or self.devicePixelRatioF() or 1.0) + except Exception: + dpr = 1.0 worker = ThumbnailWorker( self._doc_path, page_indices, self._password, self._epoch, dpr, self)