On 10 January 2013 12:01, Ian Romanick <i...@freedesktop.org
<mailto:i...@freedesktop.org>> wrote:
On 01/08/2013 02:27 PM, Paul Berry wrote:
In most cases, the width, height, and depth of the physical surface
used by the driver to implement a texture or renderbuffer is
equal to
the logical width, height, and depth exposed to the client through
functions such as glTexImage3D(). However, there are two
exceptions:
cube maps (which have a physical depth of 6 but a logical depth
of 1)
and multisampled renderbuffers (which have larger physical
dimensions
than logical dimensions to allow multiple samples per pixel).
Previous to this patch, we accounted for the difference between
physical and logical surface dimensions at inconsistent places
in the
call graph (multisampling was accounted for in
intel_miptree_create_for___renderbuffer(), and cubemaps were
accounted
for in intel_miptree_create_internal(__)). As a result, it wasn't
always clear, when calling a miptree creation function, whether
physical or logical dimensions were needed. Also, we weren't
consistent about storing logical dimensions in the intel_mipmap_tree
structure (we only did so in the
intel_miptree_create_for___renderbuffer() code path, and we did not
store depth).
This patch refactors things so that
intel_miptree_create_internal(__) is
responsible for converting logical to physical dimensions and for
storing both the physical and logical dimensions in the
intel_mipmap_tree structure. As a result, all miptree creation
functions interpret their arguments as logical dimensions, and both
physical and logical dimensions are always available to
functions that
work with intel_mipmap_trees.
In addition, it renames the fields in intel_mipmap_tree used to
store
the dimensions, so that it is clear from the name whether
physical or
logical dimensions are being referred to.
This should fix the following bugs:
- When creating a separate stencil surface for a depthstencil
cubemap,
we would erroneously try to convert the depth from 1 to 6 twice,
resulting in an assertion failure.
- When creating an MCS buffer for compressed multisampling, we used
physical dimensions instead of logical dimensions, resulting in
wasted memory.
In addition, this should considerably simplify the implementation of
ARB_texture_multisample, because it moves the code to compute the
physical size of multisampled surfaces out of renderbuffer-only
code.
---
src/mesa/drivers/dri/i915/__i915_tex_layout.c | 36 ++---
src/mesa/drivers/dri/i965/brw___tex_layout.c | 20 +--
src/mesa/drivers/dri/intel/__intel_fbo.c | 1 -
src/mesa/drivers/dri/intel/__intel_mipmap_tree.c | 191
+++++++++++-------------
src/mesa/drivers/dri/intel/__intel_mipmap_tree.h | 28 ++--
src/mesa/drivers/dri/intel/__intel_tex_image.c | 1 -
src/mesa/drivers/dri/intel/__intel_tex_layout.c | 18 +--
src/mesa/drivers/dri/intel/__intel_tex_validate.c | 1 -
8 files changed, 143 insertions(+), 153 deletions(-)
diff --git a/src/mesa/drivers/dri/i915/__i915_tex_layout.c
b/src/mesa/drivers/dri/i915/__i915_tex_layout.c
index 1e3cfad..90911a6 100644
--- a/src/mesa/drivers/dri/i915/__i915_tex_layout.c
+++ b/src/mesa/drivers/dri/i915/__i915_tex_layout.c
@@ -114,9 +114,9 @@ static GLint bottom_offsets[6] = {
static void
i915_miptree_layout_cube(__struct intel_mipmap_tree * mt)
{
- const GLuint dim = mt->width0;
+ const GLuint dim = mt->physical_width0;
GLuint face;
- GLuint lvlWidth = mt->width0, lvlHeight = mt->height0;
+ GLuint lvlWidth = mt->physical_width0, lvlHeight =
mt->physical_height0;
GLint level;
assert(lvlWidth == lvlHeight); /* cubemap images are square */
@@ -156,14 +156,14 @@ i915_miptree_layout_cube(__struct
intel_mipmap_tree * mt)
static void
i915_miptree_layout_3d(struct intel_mipmap_tree * mt)
{
- GLuint width = mt->width0;
- GLuint height = mt->height0;
- GLuint depth = mt->depth0;
+ GLuint width = mt->physical_width0;
+ GLuint height = mt->physical_height0;
+ GLuint depth = mt->physical_depth0;
GLuint stack_height = 0;
GLint level;
/* Calculate the size of a single slice. */
- mt->total_width = mt->width0;
+ mt->total_width = mt->physical_width0;
/* XXX: hardware expects/requires 9 levels at minimum. */
for (level = mt->first_level; level <= MAX2(8,
mt->last_level); level++) {
@@ -178,7 +178,7 @@ i915_miptree_layout_3d(struct
intel_mipmap_tree * mt)
}
/* Fixup depth image_offsets: */
- depth = mt->depth0;
+ depth = mt->physical_depth0;
for (level = mt->first_level; level <= mt->last_level;
level++) {
GLuint i;
for (i = 0; i < depth; i++) {
@@ -193,18 +193,18 @@ i915_miptree_layout_3d(struct
intel_mipmap_tree * mt)
* remarkable how wasteful of memory the i915 texture layouts
* are. They are largely fixed in the i945.
*/
- mt->total_height = stack_height * mt->depth0;
+ mt->total_height = stack_height * mt->physical_depth0;
}
static void
i915_miptree_layout_2d(struct intel_mipmap_tree * mt)
{
- GLuint width = mt->width0;
- GLuint height = mt->height0;
+ GLuint width = mt->physical_width0;
+ GLuint height = mt->physical_height0;
GLuint img_height;
GLint level;
- mt->total_width = mt->width0;
+ mt->total_width = mt->physical_width0;
mt->total_height = 0;
for (level = mt->first_level; level <= mt->last_level;
level++) {
@@ -312,9 +312,9 @@ i915_miptree_layout(struct intel_mipmap_tree
* mt)
static void
i945_miptree_layout_cube(__struct intel_mipmap_tree * mt)
{
- const GLuint dim = mt->width0;
+ const GLuint dim = mt->physical_width0;
GLuint face;
- GLuint lvlWidth = mt->width0, lvlHeight = mt->height0;
+ GLuint lvlWidth = mt->physical_width0, lvlHeight =
mt->physical_height0;
GLint level;
assert(lvlWidth == lvlHeight); /* cubemap images are square */
@@ -402,17 +402,17 @@ i945_miptree_layout_cube(__struct
intel_mipmap_tree * mt)
static void
i945_miptree_layout_3d(struct intel_mipmap_tree * mt)
{
- GLuint width = mt->width0;
- GLuint height = mt->height0;
- GLuint depth = mt->depth0;
+ GLuint width = mt->physical_width0;
+ GLuint height = mt->physical_height0;
+ GLuint depth = mt->physical_depth0;
GLuint pack_x_pitch, pack_x_nr;
GLuint pack_y_pitch;
GLuint level;
- mt->total_width = mt->width0;
+ mt->total_width = mt->physical_width0;
mt->total_height = 0;
- pack_y_pitch = MAX2(mt->height0, 2);
+ pack_y_pitch = MAX2(mt->physical_height0, 2);
pack_x_pitch = mt->total_width;
pack_x_nr = 1;
diff --git a/src/mesa/drivers/dri/i965/__brw_tex_layout.c
b/src/mesa/drivers/dri/i965/__brw_tex_layout.c
index b661570..1428396 100644
--- a/src/mesa/drivers/dri/i965/__brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/__brw_tex_layout.c
@@ -47,8 +47,8 @@ brw_miptree_layout_texture___array(struct
intel_context *intel,
GLuint qpitch = 0;
int h0, h1, q;
- h0 = ALIGN(mt->height0, mt->align_h);
- h1 = ALIGN(minify(mt->height0), mt->align_h);
+ h0 = ALIGN(mt->physical_height0, mt->align_h);
+ h1 = ALIGN(minify(mt->physical___height0), mt->align_h);
if (mt->array_spacing_lod0)
qpitch = h0;
else
@@ -59,11 +59,11 @@ brw_miptree_layout_texture___array(struct
intel_context *intel,
i945_miptree_layout_2d(mt);
for (level = mt->first_level; level <= mt->last_level;
level++) {
- for (q = 0; q < mt->depth0; q++) {
+ for (q = 0; q < mt->physical_depth0; q++) {
intel_miptree_set_image___offset(mt, level, q, 0, q *
qpitch);
}
}
- mt->total_height = qpitch * mt->depth0;
+ mt->total_height = qpitch * mt->physical_depth0;
}
void
@@ -84,13 +84,13 @@ brw_miptree_layout(struct intel_context
*intel, struct intel_mipmap_tree *mt)
brw_miptree_layout_texture___array(intel, mt);
break;
}
- assert(mt->depth0 == 6);
+ assert(mt->physical_depth0 == 6);
/* FALLTHROUGH */
case GL_TEXTURE_3D: {
- GLuint width = mt->width0;
- GLuint height = mt->height0;
- GLuint depth = mt->depth0;
+ GLuint width = mt->physical_width0;
+ GLuint height = mt->physical_height0;
+ GLuint depth = mt->physical_depth0;
GLuint pack_x_pitch, pack_x_nr;
GLuint pack_y_pitch;
GLuint level;
@@ -101,8 +101,8 @@ brw_miptree_layout(struct intel_context
*intel, struct intel_mipmap_tree *mt)
mt->total_width = ALIGN(width, mt->align_w);
pack_y_pitch = (height + 3) / 4;
} else {
- mt->total_width = mt->width0;
- pack_y_pitch = ALIGN(mt->height0, mt->align_h);
+ mt->total_width = mt->physical_width0;
+ pack_y_pitch = ALIGN(mt->physical_height0, mt->align_h);
}
pack_x_pitch = width;
diff --git a/src/mesa/drivers/dri/intel/__intel_fbo.c
b/src/mesa/drivers/dri/intel/__intel_fbo.c
index 0597015..034fa8a 100644
--- a/src/mesa/drivers/dri/intel/__intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/__intel_fbo.c
@@ -939,7 +939,6 @@ intel_renderbuffer_move_to___temp(struct
intel_context *intel,
width, height, depth,
true,
irb->mt->num_samples,
- irb->mt->msaa_layout,
false /* force_y_tiling */);
intel_miptree_copy_teximage(__intel, intel_image, new_mt);
diff --git a/src/mesa/drivers/dri/intel/__intel_mipmap_tree.c
b/src/mesa/drivers/dri/intel/__intel_mipmap_tree.c
index 12b77b6..7542219 100644
--- a/src/mesa/drivers/dri/intel/__intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/__intel_mipmap_tree.c
@@ -122,8 +122,7 @@ intel_miptree_create_internal(__struct
intel_context *intel,
GLuint height0,
GLuint depth0,
bool for_region,
- GLuint num_samples,
- enum intel_msaa_layout msaa_layout)
+ GLuint num_samples)
{
struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
int compress_byte = 0;
@@ -140,18 +139,78 @@ intel_miptree_create_internal(__struct
intel_context *intel,
mt->format = format;
mt->first_level = first_level;
mt->last_level = last_level;
- mt->width0 = width0;
- mt->height0 = height0;
+ mt->logical_width0 = width0;
+ mt->logical_height0 = height0;
+ mt->logical_depth0 = depth0;
mt->cpp = compress_byte ? compress_byte :
_mesa_get_format_bytes(mt->__format);
mt->num_samples = num_samples;
mt->compressed = compress_byte ? 1 : 0;
- mt->msaa_layout = msaa_layout;
+ mt->msaa_layout = INTEL_MSAA_LAYOUT_NONE;
mt->refcount = 1;
+ if (num_samples > 1) {
+ /* Adjust width/height/depth for MSAA */
+ mt->msaa_layout = compute_msaa_layout(intel, format);
+ if (mt->msaa_layout == INTEL_MSAA_LAYOUT_IMS) {
+ /* In the Sandy Bridge PRM, volume 4, part 1, page 31,
it says:
+ *
+ * "Any of the other messages (sample*, LOD,
load4) used with a
+ * (4x) multisampled surface will in-effect
sample a surface with
+ * double the height and width as that indicated
in the surface
+ * state. Each pixel position on the
original-sized surface is
+ * replaced with a 2x2 of samples with the
following arrangement:
+ *
+ * sample 0 sample 2
+ * sample 1 sample 3"
+ *
+ * Thus, when sampling from a multisampled texture, it
behaves as
+ * though the layout in memory for (x,y,sample) is:
+ *
+ * (0,0,0) (0,0,2) (1,0,0) (1,0,2)
+ * (0,0,1) (0,0,3) (1,0,1) (1,0,3)
+ *
+ * (0,1,0) (0,1,2) (1,1,0) (1,1,2)
+ * (0,1,1) (0,1,3) (1,1,1) (1,1,3)
+ *
+ * However, the actual layout of multisampled data in
memory is:
+ *
+ * (0,0,0) (1,0,0) (0,0,1) (1,0,1)
+ * (0,1,0) (1,1,0) (0,1,1) (1,1,1)
+ *
+ * (0,0,2) (1,0,2) (0,0,3) (1,0,3)
+ * (0,1,2) (1,1,2) (0,1,3) (1,1,3)
+ *
+ * This pattern repeats for each 2x2 pixel block.
+ *
+ * As a result, when calculating the size of our
4-sample buffer for
+ * an odd width or height, we have to align before
scaling up because
+ * sample 3 is in that bottom right 2x2 block.
+ */
+ switch (num_samples) {
+ case 4:
+ width0 = ALIGN(width0, 2) * 2;
+ height0 = ALIGN(height0, 2) * 2;
+ break;
+ case 8:
+ width0 = ALIGN(width0, 2) * 4;
+ height0 = ALIGN(height0, 2) * 2;
+ break;
+ default:
+ /* num_samples should already have been quantized
to 0, 1, 4, or
+ * 8.
+ */
+ assert(false);
+ }
+ } else {
+ /* Non-interleaved */
+ depth0 *= num_samples;
+ }
+ }
+
/* array_spacing_lod0 is only used for non-IMS MSAA
surfaces. TODO: can we
* use it elsewhere?
*/
- switch (msaa_layout) {
+ switch (mt->msaa_layout) {
case INTEL_MSAA_LAYOUT_NONE:
case INTEL_MSAA_LAYOUT_IMS:
mt->array_spacing_lod0 = false;
@@ -164,30 +223,28 @@ intel_miptree_create_internal(__struct
intel_context *intel,
if (target == GL_TEXTURE_CUBE_MAP) {
assert(depth0 == 1);
- mt->depth0 = 6;
- } else {
- mt->depth0 = depth0;
+ depth0 = 6;
This is basically converting depth0 from logical to physical. We
had discussed that this could cause problems with future cubemap
arrays. I may not be following the code completely, but does this
potential future problem still loom?
I think we're ok w.r.t. cubemap arrays. Once we get around to
supporting them, all we should have to do is remove the "assert(depth0
== 1)" line and replace "depth0 = 6" with "depth0 *= 6".
I was tempted to just go ahead and do that in this patch, but I decided
that for now keeping the assertion around is a nice way to verify that
we don't accidentally feed physical dimsensions to
intel_miptree_create_internal when logical dimensions are intended.
