On Sat, 5 Nov 2011 13:15:41 -0600, Brian Paul <bri...@vmware.com> wrote: > Most drivers have been creating fake/wrapper renderbuffers when a > texture image is attached to an FBO. But that's not a requirement > of core Mesa. > > So an FBO attachment that points into a texture may not have a > (fake) renderbuffer. This caused the new glReadPixels code to fail > when reading from a buffer that's actually a texture. > > Now, we check the attachment point to see if it's a real renderbuffer > or a texture image and call the corresponding MapRenderbuffer() or > MapTextureImage() function. > > This fixes a bunch of piglit fbo failures when using the non-DRI > swrast driver.
I'm not excited about needing getters for the attachment's format -- I
wish we just had one structure with the Format in it we could reference
when talking about an attachment, since we need that field so often.
On the other hand, I like the idea of forcing using a helper function
for doing the mappings, instead of going straight to ctx->Driver. With
all the hardware drivers needing to privately store x/y/w/h/mode/some
other pointer, it means that we can't recursively map a buffer (which
would be easy to accidentally do when doing packed depth/stencil or
CopyPixels). I've been thinking it would be nice to make MTI/MRB
helpers that stored those right in the GL struct, and used those fields
to assert that someone testing on a software driver doesn't produce
recursive mappings.
> +
> +/**
> + * Map a framebuffer attachment. An attachment may either be a renderbuffer
> + * or a texture image. Check the attachment type and map the corresponding
> + * image.
> + * Note: This might be moved out of swrast someday.
> + */
> +static void
> +map_attachment(struct gl_context *ctx,
> + struct gl_renderbuffer_attachment *att,
> + GLuint x, GLuint y, GLuint w, GLuint h,
> + GLbitfield mode,
> + GLubyte **mapOut, GLint *rowStrideOut)
> +{
> + if (att->Type == GL_TEXTURE) {
> + struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
> + assert(texImage);
> + ctx->Driver.MapTextureImage(ctx, texImage,
> + att->Zoffset,
> + x, y, w, h, mode,
> + mapOut, rowStrideOut);
> + }
> + else if (att->Type == GL_RENDERBUFFER) {
> + assert(att->Renderbuffer);
> + ctx->Driver.MapRenderbuffer(ctx, att->Renderbuffer,
> + x, y, w, h, mode,
> + mapOut, rowStrideOut);
> + }
> + else {
> + *mapOut = NULL;
> + *rowStrideOut = 0;
> + _mesa_warning(NULL, "invalid attachment in map_attachment()");
> + }
> +}
> +
> +/**
> + * Unmap a framebuffer attachment.
> + * Note: This might be moved out of swrast someday.
> + */
> +static void
> +unmap_attachment(struct gl_context *ctx,
> + struct gl_renderbuffer_attachment *att)
> +{
> + if (att->Type == GL_TEXTURE) {
> + struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
> + assert(texImage);
> + ctx->Driver.UnmapTextureImage(ctx, texImage, att->Zoffset);
> + }
> + else if (att->Type == GL_RENDERBUFFER) {
> + assert(att->Renderbuffer);
> + ctx->Driver.UnmapRenderbuffer(ctx, att->Renderbuffer);
> + }
> + else {
> + _mesa_warning(NULL, "invalid attachment in unmap_attachment()");
> + }
> +}
I'd like to see this moved to core up front -- if nothing else, I'm
going to need to use it elsewhere in swrast this week or so.
pgpGzTXA6Wot8.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
