Skip to content

Conversation

greglucas
Copy link
Contributor

@greglucas greglucas commented Oct 10, 2024

PR summary

I'm getting copy_agg_buffer failed and no plots to show up with the macosx backend.

The _renderer attribute has changed with the pybind11 update and we aren't able to access it as a buffer anymore. We can use the buffer_rgba method to get the direct memoryview of the buffer instead.

git bisect shows me it is this commit: 96dd843
ping @QuLogic

The _renderer attribute has changed with the pybind11 update and
we aren't able to access it as a buffer anymore. We can use the
buffer_rgba method to get the direct memoryview of the buffer
instead.
@greglucas greglucas added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. GUI: MacOSX labels Oct 10, 2024
@greglucas greglucas added this to the v3.10.0 milestone Oct 10, 2024
@anntzer
Copy link
Contributor

anntzer commented Oct 10, 2024

(Seems close enough to #18993 which did the same for tk.)


if (!(renderer = PyObject_CallMethod(canvas, "get_renderer", ""))
|| !(renderer_buffer = PyObject_GetAttrString(renderer, "_renderer"))) {
|| !(renderer_buffer = PyObject_CallMethod(renderer, "buffer_rgba", ""))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the implementation of RendererAgg, it's:

    def buffer_rgba(self):
        return memoryview(self._renderer)

so this looks like a reasonable change. I'm a bit uncertain why PyObject_GetBuffer is not sufficient with pybind11's version of buffers, but we can investigate that for later.

@QuLogic QuLogic merged commit fbf1611 into matplotlib:main Oct 10, 2024
49 of 51 checks passed
@greglucas greglucas deleted the macos-agg-buffer branch October 10, 2024 19:47
@QuLogic
Copy link
Member

QuLogic commented Oct 11, 2024

So in the original implementation:

int PyRendererAgg_get_buffer(PyRendererAgg *self, Py_buffer *buf, int flags)
{
Py_INCREF(self);
buf->obj = (PyObject *)self;
buf->buf = self->x->pixBuffer;
buf->len = (Py_ssize_t)self->x->get_width() * (Py_ssize_t)self->x->get_height() * 4;
buf->readonly = 0;
buf->format = (char *)"B";
buf->ndim = 3;
self->shape[0] = self->x->get_height();
self->shape[1] = self->x->get_width();
self->shape[2] = 4;
buf->shape = self->shape;
self->strides[0] = self->x->get_width() * 4;
self->strides[1] = 4;
self->strides[2] = 1;
buf->strides = self->strides;
buf->suboffsets = NULL;
buf->itemsize = 1;
buf->internal = NULL;
return 1;
}

we always filled out all fields regardless of flags. The macosx backend asks for a contiguous read-only buffer:
if (PyObject_GetBuffer(renderer, buffer, PyBUF_CONTIG_RO) == -1) {

which is an alias for PyBUF_ND.

In pybind11 however, it does attempt to respect flags, but unfortunately the implementation was wrong as it only filled out ndim and shape if the flags were PyBUF_STRIDES (a higher condition that PyBUF_ND). As it defaulted to ndim=1 otherwise, macosx would then fail the shape check:

if (buffer->ndim != 3 || buffer->shape[2] != 4) {

I've opened pybind/pybind11#5407 to fix pybind11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: MacOSX Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants