[Mesa-dev] [PATCH V5 7/7] intel: implement create image from texture

[Eric Anholt]

Abdiel Janulgue <abdiel.janulgue at linux.intel.com> writes:
> @@ -295,6 +301,95 @@ intel_allocate_image(int dri_format, void *loaderPrivate)
>      return image;
>  }
>  
> +/**
> + * Save appropriately-aligned base offset pointing to the image
> + * in addition to the x/y offsets from miptree's base address.
> + * The fine adjustment in pixels are derived later from the x/y
> + * offsets when the new texture resolves to our original image.
> + */
> +static void
> +intel_image_set_level_info(__DRIimage *image, struct intel_mipmap_tree *mt,
> +                           int level, int slice)
> +{
> +   unsigned int draw_x, draw_y;
> +   uint32_t mask_x, mask_y;
> +
> +   intel_region_get_tile_masks(mt->region, &mask_x, &mask_y, false);
> +   intel_miptree_get_image_offset(mt, level, slice, &draw_x, &draw_y);
> +
> +   image->width = mt->level[level].width;
> +   image->height = mt->level[level].height;
> +   image->draw_x = draw_x;
> +   image->draw_y = draw_y;
> +
> +   image->offset = intel_region_get_aligned_offset(mt->region,
> +                                                   draw_x & ~mask_x,
> +                                                   draw_y & ~mask_y,
> +                                                   false);
> +}

This still looks like it's double-counting draw_x/y.

Imagine you're looking at a 2-level miptree for a 128x128 texture, and
we're trying to export the smaller level.  The miptree looks like:

+---+
|   |
|   |
+-+-+
| |
+-+

128 pixels of 32bpp is a tile width, and 128 pixels high of that is 16
tiles.  The values I see this function having are:

mask_x = 127
mask_y = 15
draw_x = 0
draw_y = 128
image->offset = (16 * 4096)

So when we go to texture from it, we'll add 16 * 4096 to the offset, and
then we'll... Aha!  We only make use of the intra-tile offsets (0, 0)
there, and drop the inter-tile offset (0, 128).  So normal texturing
won't double-count.  If you SubImage, the mt->offset is ignored in the
mapping we do, which is broken for create_sub_image.

I think we should either

1) Rename draw_x/draw_y in the image struct to tile_x/tile_y, set them
to (draw_x & mask_x) and (draw_y & mask_y), and make
intel_miptree_image_map() add the mt->offset to its returned pointers.

2) make *_wm_surface_state also add the intel_region_get_aligned_offset
to its offset and not set image->offset in this function.

I think I prefer 1).

> +/**
> + * Sets up a DRIImage structure to point to our shared image in a region
> + */
> +static bool
> +intel_setup_image_from_mipmap_tree(struct intel_context *intel, __DRIimage *image,
> +                                   struct intel_mipmap_tree *mt, GLuint level,
> +                                   GLuint zoffset)
> +{
> +   bool has_surface_tile_offset = false;
> +   uint32_t draw_x, draw_y;
> +
> +   intel_miptree_check_level_layer(mt, level, zoffset);
> +   intel_miptree_get_tile_offsets(mt, level, zoffset, &draw_x, &draw_y);
> +
> +#ifndef I915
> +   has_surface_tile_offset = brw_context(&intel->ctx)->has_surface_tile_offset;
> +#endif
> +   if ((!has_surface_tile_offset &&
> +        (draw_x != 0 || draw_y != 0)) ||
> +       mt->stencil_mt) {
> +      /* Non-tile aligned sufaces in gen4 hw and earlier have problems resolving
> +       * back to our destination due to alignment issues. Bail-out and report error
> +       */
> +      return false;

The mt->stencil_mt failure is not about tile alignment, it's about how
we can't export that data through our current _DRIimage struct.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130122/a01fea2f/attachment.pgp>