On Tue, 7 Jun 2011 11:47:14 -0700, Eric Anholt <e...@anholt.net> wrote:
> We were mapping the renderbuffer once, then walking over all the
> buffers to map just the texture ones using the other texture mapping
> function that handled the x/y offset to the image in the region. But
> then we would go and overwrite *those* mappings with the original
> mappings for depth/stencil, which was wrong.
>
> Instead, just walk over the attachments once and map the attachments.
> Wasn't that easy?
> ---
> src/mesa/drivers/dri/intel/intel_span.c | 87 ++++++++----------------------
> 1 files changed, 23 insertions(+), 64 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/intel/intel_span.c
> b/src/mesa/drivers/dri/intel/intel_span.c
> index 8978129..92a7586 100644
> --- a/src/mesa/drivers/dri/intel/intel_span.c
> +++ b/src/mesa/drivers/dri/intel/intel_span.c
> @@ -122,10 +122,16 @@ intel_renderbuffer_map(struct intel_context *intel,
> struct gl_renderbuffer *rb)
> rb->Data = irb->region->buffer->virtual;
> rb->RowStride = irb->region->pitch;
>
> - /* Flip orientation if it's the window system buffer */
> if (!rb->Name) {
> + /* Flip orientation of the window system buffer */
> rb->Data += rb->RowStride * (irb->region->height - 1) *
> irb->region->cpp;
> rb->RowStride = -rb->RowStride;
> + } else {
> + /* Adjust the base pointer of a texture image drawbuffer to the image
> + * within the miptree region (all else has draw_x/y = 0).
> + */
> + rb->Data += irb->draw_x * irb->region->cpp;
> + rb->Data += irb->draw_y * rb->RowStride * irb->region->cpp;
> }
>
> intel_set_span_functions(intel, rb);
> @@ -148,71 +154,26 @@ intel_renderbuffer_unmap(struct intel_context *intel,
> rb->RowStride = 0;
> }
>
> -/**
> - * Map or unmap all the renderbuffers which we may need during
> - * software rendering.
> - * XXX in the future, we could probably convey extra information to
> - * reduce the number of mappings needed. I.e. if doing a glReadPixels
> - * from the depth buffer, we really only need one mapping.
> - *
> - * XXX Rewrite this function someday.
> - * We can probably just loop over all the renderbuffer attachments,
> - * map/unmap all of them, and not worry about the _ColorDrawBuffers
> - * _ColorReadBuffer, _DepthBuffer or _StencilBuffer fields.
> - */
> static void
> -intel_map_unmap_framebuffer(struct intel_context *intel,
> - struct gl_framebuffer *fb,
> - GLboolean map)
> +intel_framebuffer_map(struct intel_context *intel, struct gl_framebuffer *fb)
> {
> - GLuint i;
> -
> - /* color draw buffers */
> - for (i = 0; i < fb->_NumColorDrawBuffers; i++) {
> - if (map)
> - intel_renderbuffer_map(intel, fb->_ColorDrawBuffers[i]);
> - else
> - intel_renderbuffer_unmap(intel, fb->_ColorDrawBuffers[i]);
> - }
> -
> - /* color read buffer */
> - if (map)
> - intel_renderbuffer_map(intel, fb->_ColorReadBuffer);
> - else
> - intel_renderbuffer_unmap(intel, fb->_ColorReadBuffer);
> + int i;
>
> - /* check for render to textures */
> for (i = 0; i < BUFFER_COUNT; i++) {
> - struct gl_renderbuffer_attachment *att =
> - fb->Attachment + i;
> - struct gl_texture_object *tex = att->Texture;
> - if (tex) {
> - /* render to texture */
> - ASSERT(att->Renderbuffer);
> - if (map)
> - intel_tex_map_images(intel, intel_texture_object(tex));
> - else
> - intel_tex_unmap_images(intel, intel_texture_object(tex));
> - }
> + intel_renderbuffer_map(intel, fb->Attachment[i].Renderbuffer);
> }
>
> - /* depth buffer (Note wrapper!) */
> - if (fb->_DepthBuffer) {
> - if (map)
> - intel_renderbuffer_map(intel, fb->_DepthBuffer->Wrapped);
> - else
> - intel_renderbuffer_unmap(intel, fb->_DepthBuffer->Wrapped);
> - }
> + intel_check_front_buffer_rendering(intel);
> +}
>
> - /* stencil buffer (Note wrapper!) */
> - if (fb->_StencilBuffer) {
> - if (map)
> - intel_renderbuffer_map(intel, fb->_StencilBuffer->Wrapped);
> - else
> - intel_renderbuffer_unmap(intel, fb->_StencilBuffer->Wrapped);
> - }
> +static void
> +intel_framebuffer_unmap(struct intel_context *intel, struct gl_framebuffer
> *fb)
> +{
> + int i;
>
> - intel_check_front_buffer_rendering(intel);
> + for (i = 0; i < BUFFER_COUNT; i++) {
> + intel_renderbuffer_unmap(intel, fb->Attachment[i].Renderbuffer);
> + }
> }
>
> /**
> @@ -239,9 +200,8 @@ intelSpanRenderStart(struct gl_context * ctx)
> }
> }
>
> - intel_map_unmap_framebuffer(intel, ctx->DrawBuffer, GL_TRUE);
> - if (ctx->ReadBuffer != ctx->DrawBuffer)
> - intel_map_unmap_framebuffer(intel, ctx->ReadBuffer, GL_TRUE);
> + intel_framebuffer_map(intel, ctx->DrawBuffer);
> + intel_framebuffer_map(intel, ctx->ReadBuffer);
> }
I think this hunk should be:
@@ -239,9 +200,8 @@ intelSpanRenderStart(struct gl_context * ctx)
}
}
- intel_map_unmap_framebuffer(intel, ctx->DrawBuffer, GL_TRUE);
+ intel_framebuffer_map(intel, ctx->DrawBuffer);
- if (ctx->ReadBuffer != ctx->DrawBuffer)
- intel_map_unmap_framebuffer(intel, ctx->ReadBuffer, GL_TRUE);
+ intel_framebuffer_map(intel, ctx->ReadBuffer);
}
There's no reason to re-map the read buffer if we've already done so.
>
> /**
> @@ -263,9 +223,8 @@ intelSpanRenderFinish(struct gl_context * ctx)
> }
> }
>
> - intel_map_unmap_framebuffer(intel, ctx->DrawBuffer, GL_FALSE);
> - if (ctx->ReadBuffer != ctx->DrawBuffer)
> - intel_map_unmap_framebuffer(intel, ctx->ReadBuffer, GL_FALSE);
> + intel_framebuffer_unmap(intel, ctx->DrawBuffer);
> + intel_framebuffer_unmap(intel, ctx->ReadBuffer);
> }
Ditto for this hunk.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
