On 10 January 2013 13:23, Ian Romanick <i...@freedesktop.org
<mailto:i...@freedesktop.org>> wrote:
On 01/10/2013 01:03 PM, Paul Berry wrote:
On 10 January 2013 12:01, Ian Romanick <i...@freedesktop.org
<mailto:i...@freedesktop.org>
<mailto: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.
Okay... and the 6 (or 6*depth0) created here can't get fed back
around in the cubemap array case because the receiver of depth0
(below) knows it's the physical size, and it will feed the logical
size (still 1) back around. Right?
Yes, correct--after this patch is applied, of course. (Before this
patch the 6 would occasionally get fed back around causing an assertion
failure--I think that contributed to one of our GLES3 conformance failures).
