On 02/14/2014 03:00 PM, Eric Anholt wrote:
> Since only window system renderbuffers can have a singlesample_mt, this
> lets us drop a bunch of sanity checking to make sure that we're just a
> renderbuffer-like thing.
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 20 ++-
> src/mesa/drivers/dri/i965/intel_fbo.c | 91 ++++++++++-
> src/mesa/drivers/dri/i965/intel_fbo.h | 47 +++++-
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 209
> +++-----------------------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 59 +-------
> 5 files changed, 165 insertions(+), 261 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index ba2f971..1071f9f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -997,7 +997,7 @@ intel_resolve_for_dri2_flush(struct brw_context *brw,
> if (rb->mt->num_samples <= 1)
> intel_miptree_resolve_color(brw, rb->mt);
> else
> - intel_miptree_downsample(brw, rb->mt);
> + intel_renderbuffer_downsample(brw, rb);
> }
> }
>
> @@ -1270,10 +1270,9 @@ intel_process_dri2_buffer(struct brw_context *brw,
> rb->mt->region->name == buffer->name)
> return;
> } else {
> - if (rb->mt &&
> - rb->mt->singlesample_mt &&
> - rb->mt->singlesample_mt->region &&
> - rb->mt->singlesample_mt->region->name == buffer->name)
> + if (rb->singlesample_mt &&
> + rb->singlesample_mt->region &&
> + rb->singlesample_mt->region->name == buffer->name)
> return;
> }
>
> @@ -1308,7 +1307,7 @@ intel_process_dri2_buffer(struct brw_context *brw,
> (buffer->attachment == __DRI_BUFFER_FRONT_LEFT ||
> buffer->attachment == __DRI_BUFFER_FAKE_FRONT_LEFT) &&
> rb->Base.Base.NumSamples > 1) {
> - intel_miptree_upsample(brw, rb->mt);
> + intel_renderbuffer_upsample(brw, rb);
> }
>
> assert(rb->mt);
> @@ -1355,10 +1354,9 @@ intel_update_image_buffer(struct brw_context *intel,
> rb->mt->region->bo == region->bo)
> return;
> } else {
> - if (rb->mt &&
> - rb->mt->singlesample_mt &&
> - rb->mt->singlesample_mt->region &&
> - rb->mt->singlesample_mt->region->bo == region->bo)
> + if (rb->singlesample_mt &&
> + rb->singlesample_mt->region &&
> + rb->singlesample_mt->region->bo == region->bo)
> return;
> }
>
> @@ -1367,7 +1365,7 @@ intel_update_image_buffer(struct brw_context *intel,
> if (intel->is_front_buffer_rendering &&
> buffer_type == __DRI_IMAGE_BUFFER_FRONT &&
> rb->Base.Base.NumSamples > 1) {
> - intel_miptree_upsample(intel, rb->mt);
> + intel_renderbuffer_upsample(intel, rb);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> b/src/mesa/drivers/dri/i965/intel_fbo.c
> index cd148f0..62ccfab 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -74,11 +74,41 @@ intel_delete_renderbuffer(struct gl_context *ctx, struct
> gl_renderbuffer *rb)
> ASSERT(irb);
>
> intel_miptree_release(&irb->mt);
> + intel_miptree_release(&irb->singlesample_mt);
>
> _mesa_delete_renderbuffer(ctx, rb);
> }
>
> /**
> + * \brief Downsample a winsys renderbuffer from mt to singlesample_mt.
> + *
> + * If the miptree needs no downsample, then skip.
> + */
> +void
> +intel_renderbuffer_downsample(struct brw_context *brw,
> + struct intel_renderbuffer *irb)
> +{
> + if (!irb->need_downsample)
> + return;
> + intel_miptree_updownsample(brw, irb->mt, irb->singlesample_mt);
> + irb->need_downsample = false;
> +}
> +
> +/**
> + * \brief Upsample a winsys renderbuffer from singlesample_mt to mt.
> + *
> + * The upsample is done unconditionally.
> + */
> +void
> +intel_renderbuffer_upsample(struct brw_context *brw,
> + struct intel_renderbuffer *irb)
> +{
> + assert(!irb->need_downsample);
> +
> + intel_miptree_updownsample(brw, irb->singlesample_mt, irb->mt);
> +}
> +
> +/**
> * \see dd_function_table::MapRenderbuffer
> */
> static void
> @@ -92,6 +122,7 @@ intel_map_renderbuffer(struct gl_context *ctx,
> struct brw_context *brw = brw_context(ctx);
> struct swrast_renderbuffer *srb = (struct swrast_renderbuffer *)rb;
> struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> + struct intel_mipmap_tree *mt;
> void *map;
> int stride;
>
> @@ -106,6 +137,39 @@ intel_map_renderbuffer(struct gl_context *ctx,
>
> intel_prepare_render(brw);
>
I'm having a little trouble parsing this comment. A few suggestions...
> + /* The MapRenderbuffer API should always be returning a single-sampled
"should always return"?
> + * mapping. The case we get mapping of multisampled RBs are in
"The case where we're requested to map a multisample RB is in
glReadPixels (or swrast paths like glCopyTexImage()) from a window-
system MSAA buffer."
> + * glReadPixels() (or swrast paths like glCopyTexImage()) from a
> + * window-system MSAA buffer.
> + *
> + * If it's a color miptree, there is a ->singlesample_mt which wraps the
> + * actual window system renderbuffer (which we may resolve to at any
> time),
> + * while the miptree itself is our driver-private allocation. If it's a
> + * depth or stencil miptree, we have a private MSAA buffer and no shared
> + * singlesample buffer, and since we don't expect anybody to ever actually
> + * resolve it, we just make a temporary singlesample buffer now when we
> + * have to.
> + */
Thank you for adding this comment! I like this better than the long one
that used to be in the header file.
> + if (rb->NumSamples > 1) {
> + if (!irb->singlesample_mt) {
> + irb->singlesample_mt =
> + intel_miptree_create_for_renderbuffer(brw, irb->mt->format,
> + rb->Width, rb->Height,
> + 0 /*num_samples*/);
> + if (!irb->singlesample_mt)
> + goto fail;
> + irb->singlesample_mt_is_tmp = true;
> + irb->need_downsample = true;
> + }
> +
> + intel_renderbuffer_downsample(brw, irb);
> + mt = irb->singlesample_mt;
> +
> + irb->need_map_upsample = mode & GL_MAP_WRITE_BIT;
> + } else {
> + mt = irb->mt;
> + }
> +
> /* For a window-system renderbuffer, we need to flip the mapping we
> receive
> * upside-down. So we need to ask for a rectangle on flipped vertically,
> and
> * we then return a pointer to the bottom of it with a negative stride.
> @@ -114,7 +178,7 @@ intel_map_renderbuffer(struct gl_context *ctx,
> y = rb->Height - y - h;
> }
>
> - intel_miptree_map(brw, irb->mt, irb->mt_level, irb->mt_layer,
> + intel_miptree_map(brw, mt, irb->mt_level, irb->mt_layer,
> x, y, w, h, mode, &map, &stride);
>
> if (rb->Name == 0) {
> @@ -128,6 +192,11 @@ intel_map_renderbuffer(struct gl_context *ctx,
>
> *out_map = map;
> *out_stride = stride;
> + return;
> +
> +fail:
> + *out_map = NULL;
> + *out_stride = 0;
> }
>
> /**
> @@ -140,6 +209,7 @@ intel_unmap_renderbuffer(struct gl_context *ctx,
> struct brw_context *brw = brw_context(ctx);
> struct swrast_renderbuffer *srb = (struct swrast_renderbuffer *)rb;
> struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> + struct intel_mipmap_tree *mt;
>
> DBG("%s: rb %d (%s)\n", __FUNCTION__,
> rb->Name, _mesa_get_format_name(rb->Format));
> @@ -150,7 +220,21 @@ intel_unmap_renderbuffer(struct gl_context *ctx,
> return;
> }
>
> - intel_miptree_unmap(brw, irb->mt, irb->mt_level, irb->mt_layer);
> + if (rb->NumSamples > 1) {
> + mt = irb->singlesample_mt;
> + } else {
> + mt = irb->mt;
> + }
> +
> + intel_miptree_unmap(brw, mt, irb->mt_level, irb->mt_layer);
> +
> + if (irb->need_map_upsample) {
> + intel_renderbuffer_upsample(brw, irb);
> + irb->need_map_upsample = false;
> + }
> +
> + if (irb->singlesample_mt_is_tmp)
> + intel_miptree_release(&irb->singlesample_mt);
> }
>
>
> @@ -793,8 +877,7 @@ intel_blit_framebuffer(struct gl_context *ctx,
> void
> intel_renderbuffer_set_needs_downsample(struct intel_renderbuffer *irb)
> {
> - if (irb->mt && irb->mt->singlesample_mt)
> - irb->mt->need_downsample = true;
> + irb->need_downsample = true;
> }
This function is a bit...silly...now. Maybe just replace:
intel_renderbuffer_set_needs_downsample(irb)
with
irb->needs_downsample = true;
More obvious and less typing. (This could be a follow-on patch...)
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
