From e96284472af0872b807a46b6b28c5b797636286c Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Tue, 5 May 2026 21:56:00 -0400 Subject: [PATCH 1/7] Fix several bugs in the background-painting logic Previously the background was painted or erased several times in separate parts of the code when rendering a PDF page. This was partly my own fault, as I did not fully understand the rendering logic when I first wrote the QtPDF backend (the previous Poppler backend I used as a reference also overrode significant parts of it). This new code only paints the background once, and only when necessary to do so. For example, it's not necessary or desirable to paint the background when printing. Another consequence of this fix is that it's possible now to render an image with a transparent background, for example in Frescobaldi's copy-to-image dialog. I believe this was how it worked in older versions of qpageview, and was also the intended behavior here all along. --- qpageview/pdf.py | 4 ---- qpageview/render.py | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/qpageview/pdf.py b/qpageview/pdf.py index a765ea8..9822a7d 100644 --- a/qpageview/pdf.py +++ b/qpageview/pdf.py @@ -303,10 +303,6 @@ def draw(self, page, painter, key, tile, paperColor=None): Qt.AspectRatioMode.IgnoreAspectRatio, Qt.TransformationMode.SmoothTransformation) - # Erase the target area and draw the image - painter.eraseRect(target) - if paperColor: - painter.fillRect(target, paperColor) painter.drawImage(target, image, QRectF(image.rect())) diff --git a/qpageview/render.py b/qpageview/render.py index 89d4b7a..fc53019 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -219,17 +219,15 @@ def render(self, page, key, tile, paperColor=None): The default implementation prepares the image, a painter and then calls draw() to actually draw the contents. - If the paperColor is not specified, it will be read from the Page's - paperColor attribute (if not None) or else from the renderer's - paperColor attribute. + If the paperColor is not specified, the returned image will have a + transparent background, and the caller is responsible to paint or + erase the background as needed before calling this method. """ - if paperColor is None: - paperColor = page.paperColor or self.paperColor - i = QImage(tile.w, tile.h, self.imageFormat) - if paperColor: - i.fill(paperColor) + # leave the background transparent if no paperColor is specified, + # since we don't necessarily want to fill it e.g. when printing + i.fill(paperColor or Qt.GlobalColor.transparent) painter = QPainter(i) # rotate the painter accordingly @@ -333,6 +331,7 @@ def paint(self, page, painter, rect, callback=None): region = QRegion() # painted region in tile coordinates info = self.info(page, painter.device(), rect) + paperColor = page.paperColor or self.paperColor for t, image in info.images: r = QRect(*t) & info.target # part of the tile that needs to be drawn @@ -365,12 +364,14 @@ def paint(self, page, painter, rect, callback=None): else: if QRegion(info.target).subtracted(region): # paint background, still partly uncovered - painter.fillRect(rect, page.paperColor or self.paperColor) + painter.fillRect(rect, paperColor) # draw lowest quality images first for (r, image, source) in reversed(images): # scale the target rect back to the paint device target = QRectF(r.x() / info.ratio, r.y() / info.ratio, r.width() / info.ratio, r.height() / info.ratio) + # remember render() does not fill the background unless requested + painter.fillRect(target, paperColor) painter.drawImage(target, image, source) def schedule(self, page, key, tiles, callback): From 69ebd8b188d1eccabadfa2a2ffc7e3f26a9b6292 Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 08:02:32 -0400 Subject: [PATCH 2/7] Pre-fill the background when rendering the cached image This is an optimization that eliminates the need to fill the background separately before calling drawImage(), which can cause flicker. We do this in job() rather than in render() so that it only affects the cached images used to paint the widget, not images rendered for other purposes such as printing or Frescobaldi's copy-to-image feature. --- qpageview/render.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/qpageview/render.py b/qpageview/render.py index fc53019..cad42bf 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -331,7 +331,6 @@ def paint(self, page, painter, rect, callback=None): region = QRegion() # painted region in tile coordinates info = self.info(page, painter.device(), rect) - paperColor = page.paperColor or self.paperColor for t, image in info.images: r = QRect(*t) & info.target # part of the tile that needs to be drawn @@ -364,14 +363,14 @@ def paint(self, page, painter, rect, callback=None): else: if QRegion(info.target).subtracted(region): # paint background, still partly uncovered - painter.fillRect(rect, paperColor) + painter.fillRect(rect, page.paperColor or self.paperColor) # draw lowest quality images first for (r, image, source) in reversed(images): # scale the target rect back to the paint device target = QRectF(r.x() / info.ratio, r.y() / info.ratio, r.width() / info.ratio, r.height() / info.ratio) - # remember render() does not fill the background unless requested - painter.fillRect(target, paperColor) + # job() already filled the background when rendering the page + # so we don't have to do that separately, which can cause flicker painter.drawImage(target, image, source) def schedule(self, page, key, tiles, callback): @@ -399,7 +398,9 @@ def job(self, page, key, tile): exception = [] def work(): try: - return self.render(page, key, tile) + # filling the background here is an optimization for paint() + return self.render(page, key, tile, + page.paperColor or self.paperColor) except Exception: exception.extend(sys.exc_info()) return QImage() From 027b69f4a2fb2fa2c6ad1c65c83c8b56d638ffef Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 11:30:30 -0400 Subject: [PATCH 3/7] Add tiling support to the QtPDF backend This greatly improves performance at the cost of momentarily higher memory usage when rendering the page. Because of QtPDF limitations, this is implemented by rendering the entire page at once, then splitting it into tiles that are cached and displayed individually. This is less efficient than rendering tile-by-tile, as qpageview was intended to do (but which QtPDF does not currently support). However, it still provides important performance advantages of using tiles like reduced flicker when repainting. In practice, this will have the most noticeable impact at higher zoom levels, as lower zoom levels render the full page at once anyway. --- qpageview/pdf.py | 61 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 10 deletions(-) diff --git a/qpageview/pdf.py b/qpageview/pdf.py index 9822a7d..0fa7cc7 100644 --- a/qpageview/pdf.py +++ b/qpageview/pdf.py @@ -49,6 +49,8 @@ from . import locking from . import render +_jobs = render._jobs + class Link(link.Link): """A link that encapsulates QPdfLinkModel data.""" @@ -221,16 +223,6 @@ def document(self): class PdfRenderer(render.AbstractRenderer): oversampleThreshold = 96 # DPI of a standard PC screen - def tiles(self, width, height): - """Yield four-tuples Tile(x, y, w, h) describing the tiles to render. - - For the QtPdf backend, this always returns a single tile covering - the entire page because QtPdf does not support selectively rendering - a smaller area. - - """ - yield render.Tile(0, 0, width, height) - def draw(self, page, painter, key, tile, paperColor=None): """Draw a tile on the painter. @@ -305,6 +297,55 @@ def draw(self, page, painter, key, tile, paperColor=None): painter.drawImage(target, image, QRectF(image.rect())) + def schedule(self, page, key, tiles, callback): + """Schedule a new rendering job for the specified page. + + If this page has already a job pending, the callback is added to the + pending job. + + """ + import time + # render the full page now, then job() will split it into tiles later + pageTile = render.Tile(0, 0, key.width, key.height) + try: + job = _jobs[(key, pageTile)] + except KeyError: + # make a new Job for this page + job = _jobs[(key, pageTile)] = self.job(page, key, pageTile) + job.time = time.time() + job.callbacks.add(callback) + self.checkstart() + + def job(self, page, key, pageTile): + """Return a new :class:`~.backgroundjob.Job` tailored for this page.""" + from . import backgroundjob + job = backgroundjob.Job() + job.callbacks = callbacks = set() + job.mutex = page.mutex() + exception = [] + def work(): + try: + # filling the background here is an optimization for paint() + return self.render(page, key, pageTile, + page.paperColor or self.paperColor) + except Exception: + exception.extend(sys.exc_info()) + return QImage() + def finalize(image): + # split the rendered page into tiles + for tile in self.tiles(key.width, key.height): + self.cache.addtile(key, tile, + image.copy(QRect(*map(int, tile)))) + for cb in callbacks: + cb(page) + del _jobs[(key, pageTile)] + self.checkstart() + if exception: + self.exception(*exception) + job.work = work + job.finalize = finalize + return job + def load(source): """Load a PDF document. From e2b5741878c2b1f8eab666d36f86710ddb01ac88 Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 12:45:45 -0400 Subject: [PATCH 4/7] Move the render-then-divide logic into AbstractRenderer This makes it available to other backends if needed, and simplifies the code considerably besides. --- qpageview/pdf.py | 52 +-------------------------------------------- qpageview/render.py | 18 +++++++++++++++- 2 files changed, 18 insertions(+), 52 deletions(-) diff --git a/qpageview/pdf.py b/qpageview/pdf.py index 0fa7cc7..cc4ed05 100644 --- a/qpageview/pdf.py +++ b/qpageview/pdf.py @@ -49,8 +49,6 @@ from . import locking from . import render -_jobs = render._jobs - class Link(link.Link): """A link that encapsulates QPdfLinkModel data.""" @@ -222,6 +220,7 @@ def document(self): class PdfRenderer(render.AbstractRenderer): oversampleThreshold = 96 # DPI of a standard PC screen + renderFullPages = True # QtPDF doesn't support partial page rendering def draw(self, page, painter, key, tile, paperColor=None): """Draw a tile on the painter. @@ -297,55 +296,6 @@ def draw(self, page, painter, key, tile, paperColor=None): painter.drawImage(target, image, QRectF(image.rect())) - def schedule(self, page, key, tiles, callback): - """Schedule a new rendering job for the specified page. - - If this page has already a job pending, the callback is added to the - pending job. - - """ - import time - # render the full page now, then job() will split it into tiles later - pageTile = render.Tile(0, 0, key.width, key.height) - try: - job = _jobs[(key, pageTile)] - except KeyError: - # make a new Job for this page - job = _jobs[(key, pageTile)] = self.job(page, key, pageTile) - job.time = time.time() - job.callbacks.add(callback) - self.checkstart() - - def job(self, page, key, pageTile): - """Return a new :class:`~.backgroundjob.Job` tailored for this page.""" - from . import backgroundjob - job = backgroundjob.Job() - job.callbacks = callbacks = set() - job.mutex = page.mutex() - exception = [] - def work(): - try: - # filling the background here is an optimization for paint() - return self.render(page, key, pageTile, - page.paperColor or self.paperColor) - except Exception: - exception.extend(sys.exc_info()) - return QImage() - def finalize(image): - # split the rendered page into tiles - for tile in self.tiles(key.width, key.height): - self.cache.addtile(key, tile, - image.copy(QRect(*map(int, tile)))) - for cb in callbacks: - cb(page) - del _jobs[(key, pageTile)] - self.checkstart() - if exception: - self.exception(*exception) - job.work = work - job.finalize = finalize - return job - def load(source): """Load a PDF document. diff --git a/qpageview/render.py b/qpageview/render.py index cad42bf..2f7a66d 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -112,6 +112,13 @@ class AbstractRenderer: # antialias True by default (not all renderers may support this) antialiasing = True + # qpageview was designed to render pages tile-by-tile. However, some + # backends like QtPDF only support full-page rendering, so for these we + # render the page first and divide it into tiles later. This is less + # efficient, but still provides certain performance advantages of using + # tiles like reduced flicker when repainting the widget. + renderFullPages = False + def __init__(self, cache=None): if cache: self.cache = cache @@ -380,6 +387,10 @@ def schedule(self, page, key, tiles, callback): pending job. """ + if self.renderFullPages: + # ignore the caller's request and always render the full page; + # job() will do the actual division into tiles later + tiles = [Tile(0, 0, key.width, key.height)] for tile in tiles: try: job = _jobs[(key, tile)] @@ -405,7 +416,12 @@ def work(): exception.extend(sys.exc_info()) return QImage() def finalize(image): - self.cache.addtile(key, tile, image) + if self.renderFullPages: + for subtile in self.tiles(key.width, key.height): + self.cache.addtile(key, subtile, + image.copy(QRect(*map(int, subtile)))) + else: + self.cache.addtile(key, tile, image) for cb in callbacks: cb(page) del _jobs[(key, tile)] From a2709a24175c17ff2a2110315b8b5da4d9f83d42 Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 13:58:58 -0400 Subject: [PATCH 5/7] Adjust the cache size as needed to fit two full pages in memory This prevents flicker when scrolling over a page break at higher zoom levels. If two pages are displayed at once, but the cache only holds one at a time, then rendering each page purges the other's cached tiles, forcing both pages to be re-rendered each time the widget is repainted. --- qpageview/render.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qpageview/render.py b/qpageview/render.py index 2f7a66d..dcf5363 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -391,6 +391,10 @@ def schedule(self, page, key, tiles, callback): # ignore the caller's request and always render the full page; # job() will do the actual division into tiles later tiles = [Tile(0, 0, key.width, key.height)] + # ensure the cache can always hold at least two full pages to + # avoid re-rendering when scrolling over a page break + pageBytes = key.width * key.height * 4 # 32 bits == 4 bytes + self.cache.maxsize = max(cache.ImageCache.maxsize, pageBytes * 2) for tile in tiles: try: job = _jobs[(key, tile)] From db637bf4cfcb1284331841855a1f4c0bc8821994 Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 14:34:56 -0400 Subject: [PATCH 6/7] Lower the minimum cache requirement to one page (or two half-pages) When two pages are visible, we don't necessarily need all the tiles from both pages -- just the bottom tiles of the first and the top tiles of the second. We can discard the others if they don't fit in memory. This is, in fact, the entire point of tile-based rendering. --- qpageview/render.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qpageview/render.py b/qpageview/render.py index dcf5363..3f4bb6a 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -391,10 +391,10 @@ def schedule(self, page, key, tiles, callback): # ignore the caller's request and always render the full page; # job() will do the actual division into tiles later tiles = [Tile(0, 0, key.width, key.height)] - # ensure the cache can always hold at least two full pages to - # avoid re-rendering when scrolling over a page break + # ensure the cache can always hold at least one full page (or two + # half-pages) to avoid excessive re-rendering at higher zoom levels pageBytes = key.width * key.height * 4 # 32 bits == 4 bytes - self.cache.maxsize = max(cache.ImageCache.maxsize, pageBytes * 2) + self.cache.maxsize = max(cache.ImageCache.maxsize, pageBytes) for tile in tiles: try: job = _jobs[(key, tile)] From 3afc1fa421dd0c57677d263e066dd8ec81a3225d Mon Sep 17 00:00:00 2001 From: Benjamin Johnson <42893476+bmjcode@users.noreply.github.com> Date: Fri, 8 May 2026 22:14:00 -0400 Subject: [PATCH 7/7] Bump the minimum cache size back up to two full pages This prevents flicker when both page and tile breaks are visible. Even with the limitations of full-page rendering, there are probably ways to get the performance we want with a smaller cache. For example, our algorithm might take into account which tiles are currently visible when deciding what to keep in memory. But are the potential savings really worth the effort? Frescobaldi doesn't use that much memory by current standards, even under extreme conditions. For example, 800% zoom on a high-DPI screen uses about 2GB on my system, which is comparable to a couple tabs in a modern browser. I probably will find an excuse to come back and fuss with this again anyway, but for now I think the improved performance with a larger cache outweighs any potential concerns about memory usage. --- qpageview/render.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qpageview/render.py b/qpageview/render.py index 3f4bb6a..a7fd435 100644 --- a/qpageview/render.py +++ b/qpageview/render.py @@ -391,10 +391,10 @@ def schedule(self, page, key, tiles, callback): # ignore the caller's request and always render the full page; # job() will do the actual division into tiles later tiles = [Tile(0, 0, key.width, key.height)] - # ensure the cache can always hold at least one full page (or two - # half-pages) to avoid excessive re-rendering at higher zoom levels + # ensure the cache can always hold at least two full pages to + # avoid excessive re-rendering at higher zoom levels pageBytes = key.width * key.height * 4 # 32 bits == 4 bytes - self.cache.maxsize = max(cache.ImageCache.maxsize, pageBytes) + self.cache.maxsize = max(cache.ImageCache.maxsize, pageBytes * 2) for tile in tiles: try: job = _jobs[(key, tile)]