FIX: round copy_from_bbox regions out to whole pixels by larsoner · Pull Request #32264 · matplotlib/matplotlib · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions doc/api/next_api_changes/behavior/copy_from_bbox_rounding.rst
5 changes: 4 additions & 1 deletion lib/matplotlib/animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1664,7 +1664,10 @@ def init_func() -> iterable_of_artists
Whether blitting is used to optimize drawing. Note: when using
blitting, any animated artists will be drawn according to their zorder;
however, they will be drawn on top of any previous artists, regardless
of their zorder.
of their zorder. In particular, an animated artist that reaches the
edge of the Axes is drawn over the spines, whereas a full redraw would
draw the spines (which have a *zorder* of 2.5) on top of it. Giving
the animated artist a *zorder* above the spines avoids the difference.

cache_frame_data : bool, default: True
Whether frame data is cached. Disabling cache might be helpful when
Expand Down
7 changes: 1 addition & 6 deletions lib/matplotlib/backends/_backend_tk.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import weakref
from contextlib import contextmanager
import logging
import math
import os.path
import pathlib
import sys
Expand Down Expand Up @@ -114,11 +113,7 @@ def blit(photoimage, aggimage, offsets, bbox=None):
data = np.asarray(aggimage)
height, width = data.shape[:2]
if bbox is not None:
(x1, y1), (x2, y2) = bbox.__array__()
x1 = max(math.floor(x1), 0)
x2 = min(math.ceil(x2), width)
y1 = max(math.floor(y1), 0)
y2 = min(math.ceil(y2), height)
x1, y1, x2, y2 = bbox._pixel_bounds(clip=(width, height))
if (x1 > x2) or (y1 > y2):
return
bboxptr = (x1, x2, y1, y2)
Expand Down
12 changes: 5 additions & 7 deletions lib/matplotlib/backends/backend_cairo.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import functools
import gzip
import math
import logging
from collections import namedtuple

Expand Down Expand Up @@ -529,12 +528,11 @@ def copy_from_bbox(self, bbox):
"copy_from_bbox only works when rendering to an ImageSurface")
sw = surface.get_width()
sh = surface.get_height()
x0 = math.ceil(bbox.x0)
x1 = math.floor(bbox.x1)
y0 = math.ceil(sh - bbox.y1)
y1 = math.floor(sh - bbox.y0)
if not (0 <= x0 and x1 <= sw and bbox.x0 <= bbox.x1
and 0 <= y0 and y1 <= sh and bbox.y0 <= bbox.y1):
# Round outwards (as Agg does) so that a fractional bbox keeps the
# edge pixels it only partially covers, then flip into buffer rows.
x0, ylo, x1, yhi = bbox._pixel_bounds(clip=(sw, sh))
y0, y1 = sh - yhi, sh - ylo
if not (x0 <= x1 and y0 <= y1):
raise ValueError("Invalid bbox")
sls = slice(y0, y0 + max(y1 - y0, 0)), slice(x0, x0 + max(x1 - x0, 0))
data = (np.frombuffer(surface.get_data(), np.uint32)
Expand Down
20 changes: 9 additions & 11 deletions lib/matplotlib/backends/backend_gtk3agg.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ def on_draw_event(self, widget, ctx):
bbox_queue = self._bbox_queue

for bbox in bbox_queue:
x = int(bbox.x0)
y = h - int(bbox.y1)
width = int(bbox.x1) - int(bbox.x0)
height = int(bbox.y1) - int(bbox.y0)
region = self.copy_from_bbox(bbox)
# The region is rounded out to whole pixels; take its extents
# (in buffer coordinates) rather than rounding the bbox again.
x, y, x2, y2 = region.get_extents()
width = x2 - x
height = y2 - y

buf = cbook._unmultiplied_rgba8888_to_premultiplied_argb32(
np.asarray(self.copy_from_bbox(bbox)))
np.asarray(region))
image = cairo.ImageSurface.create_for_data(
buf.ravel().data, cairo.FORMAT_ARGB32, width, height)
image.set_device_scale(scale, scale)
Expand All @@ -58,15 +60,11 @@ def blit(self, bbox=None):
if bbox is None:
bbox = self.figure.bbox

scale = self.device_pixel_ratio
allocation = self.get_allocation()
x = int(bbox.x0 / scale)
y = allocation.height - int(bbox.y1 / scale)
width = (int(bbox.x1) - int(bbox.x0)) // scale
height = (int(bbox.y1) - int(bbox.y0)) // scale
x0, y0, x1, y1 = bbox._pixel_bounds(scale=self.device_pixel_ratio)

self._bbox_queue.append(bbox)
self.queue_draw_area(x, y, width, height)
self.queue_draw_area(x0, allocation.height - y1, x1 - x0, y1 - y0)


@_BackendGTK3.export
Expand Down
5 changes: 2 additions & 3 deletions lib/matplotlib/backends/backend_qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,8 @@ def blit(self, bbox=None):
if bbox is None and self.figure:
bbox = self.figure.bbox # Blit the entire canvas if bbox is None.
# repaint uses logical pixels, not physical pixels like the renderer.
l, b, w, h = (int(pt / self.device_pixel_ratio) for pt in bbox.bounds)
t = b + h
self.repaint(l, self.rect().height() - t, w, h)
l, b, r, t = bbox._pixel_bounds(scale=self.device_pixel_ratio)
self.repaint(l, self.rect().height() - t, r - l, t - b)

def _draw_idle(self):
with self._idle_draw_cntx():
Expand Down
6 changes: 3 additions & 3 deletions lib/matplotlib/backends/backend_wxagg.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ def blit(self, bbox=None):
else:
srcDC = wx.MemoryDC(bitmap)
destDC = wx.MemoryDC(self.bitmap)
x = int(bbox.x0)
y = int(self.bitmap.GetHeight() - bbox.y1)
destDC.Blit(x, y, int(bbox.width), int(bbox.height), srcDC, x, y)
x0, y0, x1, y1 = bbox._pixel_bounds()
y = self.bitmap.GetHeight() - y1
destDC.Blit(x0, y, x1 - x0, y1 - y0, srcDC, x0, y)
destDC.SelectObject(wx.NullBitmap)
srcDC.SelectObject(wx.NullBitmap)
self.gui_repaint()
Expand Down
39 changes: 39 additions & 0 deletions lib/matplotlib/tests/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import weakref

import numpy as np
from numpy.testing import assert_array_equal
import pytest

import matplotlib as mpl
Expand Down Expand Up @@ -116,6 +117,44 @@ def test_frame_size():
assert writer.frame_size == fig.canvas.get_width_height()


def test_blit_fractional_bbox():
# A blitted frame must be identical to an unblitted one: with a fractional
# Axes bbox the cached background has to cover the partially covered edge
# pixels, or they are never restored and the damage accumulates.
frames = []
for blit in [True, False]:
fig = plt.figure(figsize=(2, 2), dpi=100)
# Fractional bbox edges, landing near the top of a pixel so that the
# pixels they partially cover are mostly inside the Axes.
ax = fig.add_axes([0.12, 0.14, 0.7845, 0.739])
ax.set(xlim=(0, 1), ylim=(0, 1), xticks=[], yticks=[])
assert ax.bbox.x1 % 1 == pytest.approx(0.9)
assert ax.bbox.y1 % 1 == pytest.approx(0.8)
# Blitting draws animated artists over the background regardless of
# zorder, so keep this one above the spines to test only the restore.
line = ax.axvline(0, color='red', zorder=5)

def animate(i):
line.set_xdata([i / 4, i / 4])
return [line]

anim = animation.FuncAnimation(
fig, animate, frames=5, blit=blit, cache_frame_data=False)
fig.canvas.draw() # triggers Animation._start

rendered = []
for _ in range(5):
anim._step() # no GUI event loop, so step the timer by hand
rendered.append(np.asarray(fig.canvas.buffer_rgba()).copy())
frames.append(rendered)
plt.close(fig)

# NB: check_figures_equal() cannot be used here, as it compares the figures
# via savefig(), which redraws them in full and so discards the blitting.
blitted, unblitted = frames
assert_array_equal(blitted, unblitted)


@pytest.mark.parametrize('anim', [dict(klass=dict)], indirect=['anim'])
def test_animation_delete(anim):
if platform.python_implementation() == 'PyPy':
Expand Down
27 changes: 26 additions & 1 deletion lib/matplotlib/tests/test_backend_cairo.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import math

import numpy as np
from numpy.testing import assert_array_equal

import pytest

Expand All @@ -8,7 +11,9 @@
collections as mcollections, patches as mpatches, path as mpath)


@pytest.mark.backend('cairo')
pytestmark = pytest.mark.backend('cairo')


@check_figures_equal()
def test_patch_alpha_coloring(fig_test, fig_ref):
"""
Expand Down Expand Up @@ -51,3 +56,23 @@ def test_patch_alpha_coloring(fig_test, fig_ref):
# Have pyplot manage the figures to ensure the cairo backend is used
plt.figure(fig_ref)
plt.figure(fig_test)


def test_copy_from_bbox_fractional():
# A fractional bbox must save every pixel it touches, including the edge
# pixels it only partially covers; otherwise blitting never repairs them.
fig, ax = plt.subplots(figsize=(2, 2), dpi=100, layout='constrained')
surface = fig.canvas._get_printed_image_surface()
assert ax.bbox.x1 % 1 and ax.bbox.y1 % 1 # fractional edges
sw, sh = surface.get_width(), surface.get_height()
data = np.frombuffer(surface.get_data(), np.uint32).reshape((sh, sw))
before = data.copy()

region = fig.canvas.copy_from_bbox(ax.bbox)
data[:] = 0 # scribble over the whole surface, then repair the Axes
surface.mark_dirty()
fig.canvas.restore_region(region)

sl = (slice(sh - math.ceil(ax.bbox.y1), sh - math.floor(ax.bbox.y0)),
slice(math.floor(ax.bbox.x0), math.ceil(ax.bbox.x1)))
assert_array_equal(data[sl], before[sl])
21 changes: 21 additions & 0 deletions lib/matplotlib/tests/test_backend_qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import matplotlib
from matplotlib import pyplot as plt
from matplotlib._pylab_helpers import Gcf
from matplotlib.figure import Figure
from matplotlib.transforms import Bbox
from matplotlib import _c_internal_utils

try:
Expand Down Expand Up @@ -199,6 +201,25 @@ def set_device_pixel_ratio(ratio):
assert fig.dpi == 120


@pytest.mark.backend('QtAgg', skip_on_importerror=True)
def test_blit_repaint_covers_bbox():
# The repainted region must cover every physical pixel the bbox touches;
# rounding its edges inwards leaves an edge row that blitting never updates.
from matplotlib.backends.backend_qtagg import FigureCanvasQTAgg

canvas = FigureCanvasQTAgg(Figure(figsize=(4, 2), dpi=100))
rects = []
canvas.repaint = lambda *args: rects.append(args)
bbox = Bbox.from_extents(10.5, 20.5, 30.5, 40.5)
canvas.blit(bbox)

(x, y, w, h), = rects
dpr = canvas.device_pixel_ratio
height = canvas.rect().height()
assert x * dpr <= bbox.x0 and (x + w) * dpr >= bbox.x1
assert (height - (y + h)) * dpr <= bbox.y0 and (height - y) * dpr >= bbox.y1


@pytest.mark.backend('QtAgg', skip_on_importerror=True)
def test_subplottool():
fig, ax = plt.subplots()
Expand Down
14 changes: 14 additions & 0 deletions lib/matplotlib/tests/test_transforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,20 @@ def test_bbox_frozen_copies_minpos():
assert_array_equal(frozen.minpos, bbox.minpos)


def test_bbox_pixel_bounds():
# The rounded bounds must cover every pixel the bbox touches, and must not
# grow a bbox that already lands on pixel edges.
assert mtransforms.Bbox.from_extents(1, 2, 3, 4)._pixel_bounds() == (1, 2, 3, 4)
assert mtransforms.Bbox.from_extents(
1.2, 2.8, 3.1, 4.9)._pixel_bounds() == (1, 2, 4, 5)
# *scale* divides first, so rounding happens in the scaled space.
assert mtransforms.Bbox.from_extents(
1.2, 2.8, 3.1, 4.9)._pixel_bounds(scale=2) == (0, 1, 2, 3)
# *clip* clamps to the canvas.
assert mtransforms.Bbox.from_extents(
-1.5, -1.5, 3.1, 4.9)._pixel_bounds(clip=(3, 4)) == (0, 0, 3, 4)


def test_bbox_intersection():
bbox_from_ext = mtransforms.Bbox.from_extents
inter = mtransforms.Bbox.intersection
Expand Down
32 changes: 32 additions & 0 deletions lib/matplotlib/transforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,38 @@ def count_overlaps(self, bboxes):
return count_bboxes_overlapping_bbox(
self, np.atleast_3d([np.array(x) for x in bboxes]))

def _pixel_bounds(self, *, scale=1, clip=None):
"""
Round this bbox outwards to the whole pixels it touches.

This is the rounding convention for regions that are saved, restored
and blitted: a pixel that the bbox covers only partially still has to
be included, or it is never repainted. Must match the rounding in
``RendererAgg::copy_from_bbox`` (``src/_backend_agg.cpp``).

Parameters
----------
scale : float, default: 1
Divide the bbox by this first, e.g. a device pixel ratio to
convert physical pixels to logical ones.
clip : (float, float), optional
Clamp the result to ``(0, 0, width, height)``.

Returns
-------
tuple of int
``(x0, y0, x1, y1)``, with the upper bounds exclusive: the bbox
covers every pixel with ``x0 <= col < x1`` and ``y0 <= row < y1``.
"""
x0, y0, x1, y1 = self.extents
x0, y0 = math.floor(x0 / scale), math.floor(y0 / scale)
x1, y1 = math.ceil(x1 / scale), math.ceil(y1 / scale)
if clip is not None:
width, height = clip
x0, y0, x1, y1 = (max(x0, 0), max(y0, 0),
min(x1, width), min(y1, height))
return x0, y0, x1, y1

def expanded(self, sw, sh):
"""
Construct a `Bbox` by expanding this one around its center by the
Expand Down
10 changes: 8 additions & 2 deletions src/_backend_agg.cpp
Loading