Skip to content

FIX macos: Use the agg buffer_rgba rather than private attribute #28964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 10, 2024

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.)

@@ -1236,7 +1236,7 @@ -(void)drawRect:(NSRect)rect
CGContextRef cr = [[NSGraphicsContext currentContext] CGContext];

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