Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images
On Thu, 17 Nov 2011 19:58:38 -0800, Chad Versace chad.vers...@linux.intel.com
wrote:
For all texture targets except GL_TEXTURE_CUBE_MAP, the 'nr_images' and
'depth' fields of intel_mipmap_level were identical. In the exceptional
case, nr_images == 6 and depth == 1.
It is simple to determine if a texture is a cube or not, so the presence
of two fields here was not helpful. Worse, it was confusing. When we
eventually implement GL_ARB_texture_cube_map_array, this mess would have
become even more confusing.
This patch removes 'nr_images' and assigns to 'depth' a consistent
meaning: depth is the number of 2D slices at each miplevel. The exact
semantics of depth varies according to the texture target:
- For GL_TEXTURE_CUBE_MAP, depth is 6.
- For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It is
identical for all miplevels in the texture.
- For GL_TEXTURE_3D, it is the texture's depth at each miplevel. It's
value, like width and height, varies with miplevel.
Its
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
b/src/mesa/drivers/dri/intel/intel_fbo.c
index 9398dbd..8c41956 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -947,8 +947,9 @@ intel_framebuffer_renderbuffer(struct gl_context * ctx,
static bool
intel_update_wrapper(struct gl_context *ctx, struct intel_renderbuffer *irb,
- struct gl_texture_image *texImage)
+ struct gl_renderbuffer_attachment *att)
{
+ struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
struct intel_texture_image *intel_image = intel_texture_image(texImage);
int width, height, depth;
@@ -993,7 +994,8 @@ intel_update_wrapper(struct gl_context *ctx, struct
intel_renderbuffer *irb,
* This will have the region info needed for hardware rendering.
*/
static struct intel_renderbuffer *
-intel_wrap_texture(struct gl_context * ctx, struct gl_texture_image
*texImage)
+intel_wrap_texture(struct gl_context * ctx,
+struct gl_renderbuffer_attachment *att)
{
const GLuint name = ~0; /* not significant, but distinct for debugging
*/
struct intel_renderbuffer *irb;
@@ -1008,7 +1010,7 @@ intel_wrap_texture(struct gl_context * ctx, struct
gl_texture_image *texImage)
_mesa_init_renderbuffer(irb-Base, name);
irb-Base.ClassID = INTEL_RB_CLASS;
- if (!intel_update_wrapper(ctx, irb, texImage)) {
+ if (!intel_update_wrapper(ctx, irb, att)) {
free(irb);
return NULL;
}
@@ -1113,7 +1115,7 @@ intel_render_texture(struct gl_context * ctx,
return;
}
else if (!irb) {
- irb = intel_wrap_texture(ctx, image);
+ irb = intel_wrap_texture(ctx, att);
if (irb) {
/* bind the wrapper to the attachment point */
_mesa_reference_renderbuffer(att-Renderbuffer, irb-Base);
@@ -1125,7 +1127,7 @@ intel_render_texture(struct gl_context * ctx,
}
}
- if (!intel_update_wrapper(ctx, irb, image)) {
+ if (!intel_update_wrapper(ctx, irb, att)) {
_mesa_reference_renderbuffer(att-Renderbuffer, NULL);
_swrast_render_texture(ctx, fb, att);
return;
The changes in this file don't seem to fit in this patch.
- /** Depth of the mipmap at this level: 1 for 1D/2D/CUBE, n for 3D. */
+
+ /**
+* \brief Number of 2D slices in this miplevel.
+*
+* The exact semantics of depth varies according to the texture target:
+*- For GL_TEXTURE_CUBE_MAP, depth is 6.
+*- For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It
is
+* identical for all miplevels in the texture.
+*- For GL_TEXTURE_3D, it is the texture's depth at this miplevel.
It's
+* value, like width and height, varies with miplevel.
Its
diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c
b/src/mesa/drivers/dri/intel/intel_tex_validate.c
index f4c1a68..8dad011 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_validate.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c
@@ -140,12 +140,13 @@ intel_tex_map_image_for_swrast(struct intel_context
*intel,
if (mt-target == GL_TEXTURE_3D ||
mt-target == GL_TEXTURE_2D_ARRAY ||
mt-target == GL_TEXTURE_1D_ARRAY) {
- int i;
/* ImageOffsets[] is only used for swrast's fetch_texel_3d, so we can't
* share code with the normal path.
*/
- for (i = 0; i mt-level[level].depth; i++) {
+ assert(face == 0);
+ int depth = mt-level[level].depth;
+ for (int i = 0; i depth; i++) {
intel_miptree_get_image_offset(mt, level, face, i, x, y);
intel_image-base.ImageOffsets[i] = x + y * mt-region-pitch;
}
Not really seeing the point of this hunk (pulling depth out of the loop)
pgptjbrHLciQo.pgp
Description: PGP signature
___
mesa-dev
Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2011 12:22 PM, Eric Anholt wrote:
On Thu, 17 Nov 2011 19:58:38 -0800, Chad Versace
chad.vers...@linux.intel.com wrote:
- For GL_TEXTURE_3D, it is the texture's depth at each miplevel. It's
value, like width and height, varies with miplevel.
Its
I'll fix the two It's.
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
b/src/mesa/drivers/dri/intel/intel_fbo.c
The changes in this file don't seem to fit in this patch.
Oops. These hunks are complete mistake. I assume they were introduced by a
rebase.
They will be removed.
diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c
b/src/mesa/drivers/dri/intel/intel_tex_validate.c
index f4c1a68..8dad011 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_validate.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c
@@ -140,12 +140,13 @@ intel_tex_map_image_for_swrast(struct intel_context
*intel,
if (mt-target == GL_TEXTURE_3D ||
mt-target == GL_TEXTURE_2D_ARRAY ||
mt-target == GL_TEXTURE_1D_ARRAY) {
- int i;
/* ImageOffsets[] is only used for swrast's fetch_texel_3d, so we
can't
* share code with the normal path.
*/
- for (i = 0; i mt-level[level].depth; i++) {
+ assert(face == 0);
+ int depth = mt-level[level].depth;
+ for (int i = 0; i depth; i++) {
intel_miptree_get_image_offset(mt, level, face, i, x, y);
intel_image-base.ImageOffsets[i] = x + y * mt-region-pitch;
}
Not really seeing the point of this hunk (pulling depth out of the loop)
This is really a sloppy commit. There is no need for this hunk either.
Other than the spurious hunks, are you ok with this patch?
-
Chad Versace
chad.vers...@linux.intel.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJOxs/XAAoJEAIvNt057x8i0YYP/1cgZSGYaug1vvgPgWkm1AUZ
V52Az/xLhDKtMEsiENuGPIZedNjk4jUyajnfxCc4vFYPtW4pcN0CefA7TYClSfXJ
7oIMvbV+MB7PLdfBKWDFcjoS/UNPyISOtc6nXISF3KAyq42ry82pNjtHP6D8r2zr
yh90bnQLEotXlsyYQKP9CG/FUXyG/tTFmh/JxEHyXOWu0zfEy/D8wKvxr9KzowoW
x+KOyR3NAQqyI20hivrwwJgqzD4H0nAZdghuTQl37LHV1RCjEKkZZ3jy7guVGwU8
bGBDYJUHdTmyzLxN+NlcZtMBbkAuCWlJmdVbtSIhHmk/OxVesCtqoKV7hqbciIsT
ofvz3QYgNqsvAaw0YEKx9GEpkqm9332saS5lBn9DLewk3fseXA+hmaGompvgl4nv
NbXW//rVuBoq1MmwUENMfXJfYdSkTMtvtpBurh/Ols57CaefD9bAaVxJGPhjdg3j
km3vrQ9qtCF+IrpCAvQ+SZkv2hU9N2C7ao2W8PRWEHfSVbOUxrJxNMQ+NAp/14XO
MLmUC+5W9nsw+3QB2kGngEosaghkcFqGn4vCqL1Vm7lZWsvx8s9uX/Ob6CU6fPeG
a7zCD6wBKocCcGhVbu1d1V3k3rrYd3pQyYqYs89YqkISOEjGte+sYr2lVeximbCE
d2DCo2vkxPU3on+IwFH0
=MDMI
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images [v2]
From: Chad Versace chad.vers...@linux.intel.com
For all texture targets except GL_TEXTURE_CUBE_MAP, the 'nr_images' and
'depth' fields of intel_mipmap_level were identical. In the exceptional
case, nr_images == 6 and depth == 1.
It is simple to determine if a texture is a cube or not, so the presence
of two fields here was not helpful. Worse, it was confusing. When we
eventually implement GL_ARB_texture_cube_map_array, this mess would have
become even more confusing.
This patch removes 'nr_images' and assigns to 'depth' a consistent
meaning: depth is the number of 2D slices at each miplevel. The exact
semantics of depth varies according to the texture target:
- For GL_TEXTURE_CUBE_MAP, depth is 6.
- For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It is
identical for all miplevels in the texture.
- For GL_TEXTURE_3D, it is the texture's depth at each miplevel. Its
value, like width and height, varies with miplevel.
- For other texture types, depth is 1.
As a consequence, parameters were removed from the following function
signatures:
intel_miptree_set_level_info
Remove 'nr_images'.
i945_miptree_layout
brw_miptree_layout_texture
brw_miptree_layout_texture_array
Remove 'slices'.
v2:
- Replace It's with Its.
- Remove all hunks in intel_fbo.c. The hunks were spurious and sneaked
in during a rebase.
- Remove unneeded hunk in intel_tex_map_image_for_swrast(). It was
a little refactor of the for-loop's upper bound.
CC: Eric Anholt e...@anholt.net
Signed-off-by: Chad Versace chad.vers...@linux.intel.com
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 17 -
src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 44 +++
src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 18 +++---
src/mesa/drivers/dri/intel/intel_tex_layout.c |4 +-
src/mesa/drivers/dri/intel/intel_tex_layout.h |3 +-
src/mesa/drivers/dri/intel/intel_tex_validate.c |2 +-
6 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index d77bf4d..ac6ade6 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -41,8 +41,7 @@
static void
brw_miptree_layout_texture_array(struct intel_context *intel,
-struct intel_mipmap_tree *mt,
-int slices)
+struct intel_mipmap_tree *mt)
{
GLuint align_w;
GLuint align_h;
@@ -58,14 +57,14 @@ brw_miptree_layout_texture_array(struct intel_context
*intel,
if (mt-compressed)
qpitch /= 4;
- i945_miptree_layout_2d(mt, slices);
+ i945_miptree_layout_2d(mt);
for (level = mt-first_level; level = mt-last_level; level++) {
- for (q = 0; q slices; q++) {
+ for (q = 0; q mt-depth0; q++) {
intel_miptree_set_image_offset(mt, level, q, 0, q * qpitch);
}
}
- mt-total_height = qpitch * slices;
+ mt-total_height = qpitch * mt-depth0;
}
void
@@ -82,7 +81,7 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
* pitch of qpitch rows, where qpitch is defined by the equation given
* in Volume 1 of the BSpec.
*/
-brw_miptree_layout_texture_array(intel, mt, 6);
+brw_miptree_layout_texture_array(intel, mt);
break;
}
/* FALLTHROUGH */
@@ -117,7 +116,7 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
GLint y = 0;
GLint q, j;
-intel_miptree_set_level_info(mt, level, nr_images,
+intel_miptree_set_level_info(mt, level,
0, mt-total_height,
width, height, depth);
@@ -170,11 +169,11 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
case GL_TEXTURE_2D_ARRAY:
case GL_TEXTURE_1D_ARRAY:
- brw_miptree_layout_texture_array(intel, mt, mt-depth0);
+ brw_miptree_layout_texture_array(intel, mt);
break;
default:
- i945_miptree_layout_2d(mt, 1);
+ i945_miptree_layout_2d(mt);
break;
}
DBG(%s: %dx%dx%d\n, __FUNCTION__,
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
index 8f10101..8b9bd19 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.c
@@ -82,11 +82,17 @@ intel_miptree_create_internal(struct intel_context *intel,
mt-last_level = last_level;
mt-width0 = width0;
mt-height0 = height0;
- mt-depth0 = depth0;
mt-cpp = compress_byte ? compress_byte :
_mesa_get_format_bytes(mt-format);
mt-compressed = compress_byte ? 1 : 0;
mt-refcount = 1;
+ if (target == GL_TEXTURE_CUBE_MAP) {
+ assert(depth0 == 1);
+ mt-depth0 = 6;
+
Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images [v2]
On 11/18/2011 01:52 PM, Chad Versace wrote:
[snip]
@@ -335,23 +338,18 @@ intel_miptree_get_image_offset(struct intel_mipmap_tree
*mt,
GLuint level, GLuint face, GLuint depth,
GLuint *x, GLuint *y)
{
- switch (mt-target) {
- case GL_TEXTURE_CUBE_MAP_ARB:
- *x = mt-level[level].slice[face].x_offset;
- *y = mt-level[level].slice[face].y_offset;
- break;
- case GL_TEXTURE_3D:
- case GL_TEXTURE_2D_ARRAY_EXT:
- case GL_TEXTURE_1D_ARRAY_EXT:
- assert(depth mt-level[level].nr_images);
- *x = mt-level[level].slice[depth].x_offset;
- *y = mt-level[level].slice[depth].y_offset;
- break;
- default:
- *x = mt-level[level].slice[0].x_offset;
- *y = mt-level[level].slice[0].y_offset;
- break;
+ int slice;
+
+ if (face 0) {
+ assert(face 6);
+ assert(depth == 0);
+ slice = face;
+ } else {
+ slice = depth;
}
I find the face 0 check confusing. For cube face 0, you're falling
through to the array case and relying the fact that depth == 0 for
cubemaps. Yes, it works, but...bizarre.
You're also relying on depth == 0 for non-cube/non-array cases, but that
seems entirely reasonable to me.
Perhaps just change the (face 0) check to (mt-target ==
GL_TEXTURE_CUBE_MAP)? That seems clear enough.
Technically you could just drop the changes in this function (they're
not necessary), but I do like the cleanup.
+ *x = mt-level[level].slice[slice].x_offset;
+ *y = mt-level[level].slice[slice].y_offset;
}
static void
@@ -429,7 +427,7 @@ intel_miptree_copy_teximage(struct intel_context *intel,
struct intel_mipmap_tree *src_mt = intelImage-mt;
int level = intelImage-base.Base.Level;
int face = intelImage-base.Base.Face;
- GLuint depth = src_mt-level[level].depth;
+ GLuint depth = intelImage-base.Base.Depth;
for (int slice = 0; slice depth; slice++) {
intel_miptree_copy_slice(intel, dst_mt, src_mt, level, face, slice);
diff --git a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
index 2cad793..8f024f9 100644
--- a/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/intel/intel_mipmap_tree.h
@@ -69,16 +69,25 @@ struct intel_mipmap_level
GLuint level_y;
GLuint width;
GLuint height;
- /** Depth of the mipmap at this level: 1 for 1D/2D/CUBE, n for 3D. */
+
+ /**
+* \brief Number of 2D slices in this miplevel.
+*
+* The exact semantics of depth varies according to the texture target:
+*- For GL_TEXTURE_CUBE_MAP, depth is 6.
+*- For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It
is
+* identical for all miplevels in the texture.
+*- For GL_TEXTURE_3D, it is the texture's depth at this miplevel. Its
+* value, like width and height, varies with miplevel.
+*- For other texture types, depth is 1.
+*/
GLuint depth;
- /** Number of images at this level: 1 for 1D/2D, 6 for CUBE, depth for 3D
*/
- GLuint nr_images;
/**
* \brief List of 2D images in this mipmap level.
*
* This may be a list of cube faces, array slices in 2D array texture, or
-* layers in a 3D texture. The list's length is \c nr_images.
+* layers in a 3D texture. The list's length is \c depth.
*/
struct intel_mipmap_slice {
/**
@@ -205,7 +214,6 @@ intel_miptree_get_dimensions_for_image(struct
gl_texture_image *image,
void intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
GLuint level,
- GLuint nr_images,
GLuint x, GLuint y,
GLuint w, GLuint h, GLuint d);
diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c
b/src/mesa/drivers/dri/intel/intel_tex_layout.c
index 64f4a70..e6324cf 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_layout.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_layout.c
@@ -50,7 +50,7 @@ intel_get_texture_alignment_unit(gl_format format,
}
}
-void i945_miptree_layout_2d(struct intel_mipmap_tree *mt, int nr_images)
+void i945_miptree_layout_2d(struct intel_mipmap_tree *mt)
{
GLuint align_h, align_w;
GLuint level;
@@ -93,7 +93,7 @@ void i945_miptree_layout_2d(struct intel_mipmap_tree *mt,
int nr_images)
for ( level = mt-first_level ; level = mt-last_level ; level++ ) {
GLuint img_height;
- intel_miptree_set_level_info(mt, level, nr_images, x, y, width,
+ intel_miptree_set_level_info(mt, level, x, y, width,
height, depth);
img_height = ALIGN(height, align_h);
diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.h
b/src/mesa/drivers/dri/intel/intel_tex_layout.h
index 257c07c..c6c865d 100644
---
Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images
On Fri, 18 Nov 2011 13:36:25 -0800, Chad Versace chad.vers...@linux.intel.com
wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2011 12:22 PM, Eric Anholt wrote:
On Thu, 17 Nov 2011 19:58:38 -0800, Chad Versace
chad.vers...@linux.intel.com wrote:
- For GL_TEXTURE_3D, it is the texture's depth at each miplevel. It's
value, like width and height, varies with miplevel.
Its
I'll fix the two It's.
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
b/src/mesa/drivers/dri/intel/intel_fbo.c
The changes in this file don't seem to fit in this patch.
Oops. These hunks are complete mistake. I assume they were introduced by a
rebase.
They will be removed.
diff --git a/src/mesa/drivers/dri/intel/intel_tex_validate.c
b/src/mesa/drivers/dri/intel/intel_tex_validate.c
index f4c1a68..8dad011 100644
--- a/src/mesa/drivers/dri/intel/intel_tex_validate.c
+++ b/src/mesa/drivers/dri/intel/intel_tex_validate.c
@@ -140,12 +140,13 @@ intel_tex_map_image_for_swrast(struct intel_context
*intel,
if (mt-target == GL_TEXTURE_3D ||
mt-target == GL_TEXTURE_2D_ARRAY ||
mt-target == GL_TEXTURE_1D_ARRAY) {
- int i;
/* ImageOffsets[] is only used for swrast's fetch_texel_3d, so we
can't
* share code with the normal path.
*/
- for (i = 0; i mt-level[level].depth; i++) {
+ assert(face == 0);
+ int depth = mt-level[level].depth;
+ for (int i = 0; i depth; i++) {
intel_miptree_get_image_offset(mt, level, face, i, x, y);
intel_image-base.ImageOffsets[i] = x + y * mt-region-pitch;
}
Not really seeing the point of this hunk (pulling depth out of the loop)
This is really a sloppy commit. There is no need for this hunk either.
Other than the spurious hunks, are you ok with this patch?
Yeah, r-b other than that.
pgpBEt4l2WVxo.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images [v2]
On Fri, 18 Nov 2011 13:52:32 -0800, a...@anholt.net wrote: From: Chad Versace chad.vers...@linux.intel.com For all texture targets except GL_TEXTURE_CUBE_MAP, the 'nr_images' and 'depth' fields of intel_mipmap_level were identical. In the exceptional case, nr_images == 6 and depth == 1. It is simple to determine if a texture is a cube or not, so the presence of two fields here was not helpful. Worse, it was confusing. When we eventually implement GL_ARB_texture_cube_map_array, this mess would have become even more confusing. This patch removes 'nr_images' and assigns to 'depth' a consistent meaning: depth is the number of 2D slices at each miplevel. The exact semantics of depth varies according to the texture target: - For GL_TEXTURE_CUBE_MAP, depth is 6. - For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It is identical for all miplevels in the texture. - For GL_TEXTURE_3D, it is the texture's depth at each miplevel. Its value, like width and height, varies with miplevel. - For other texture types, depth is 1. As a consequence, parameters were removed from the following function signatures: intel_miptree_set_level_info Remove 'nr_images'. i945_miptree_layout brw_miptree_layout_texture brw_miptree_layout_texture_array Remove 'slices'. v2: - Replace It's with Its. - Remove all hunks in intel_fbo.c. The hunks were spurious and sneaked in during a rebase. - Remove unneeded hunk in intel_tex_map_image_for_swrast(). It was a little refactor of the for-loop's upper bound. Reviewed-by: Eric Anholt e...@anholt.net pgpolKLWqgNvV.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images [v2]
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 11/18/2011 03:55 PM, Kenneth Graunke wrote:
On 11/18/2011 01:52 PM, Chad Versace wrote:
[snip]
@@ -335,23 +338,18 @@ intel_miptree_get_image_offset(struct
intel_mipmap_tree *mt,
GLuint level, GLuint face, GLuint depth,
GLuint *x, GLuint *y)
{
- switch (mt-target) {
- case GL_TEXTURE_CUBE_MAP_ARB:
- *x = mt-level[level].slice[face].x_offset;
- *y = mt-level[level].slice[face].y_offset;
- break;
- case GL_TEXTURE_3D:
- case GL_TEXTURE_2D_ARRAY_EXT:
- case GL_TEXTURE_1D_ARRAY_EXT:
- assert(depth mt-level[level].nr_images);
- *x = mt-level[level].slice[depth].x_offset;
- *y = mt-level[level].slice[depth].y_offset;
- break;
- default:
- *x = mt-level[level].slice[0].x_offset;
- *y = mt-level[level].slice[0].y_offset;
- break;
+ int slice;
+
+ if (face 0) {
+ assert(face 6);
+ assert(depth == 0);
+ slice = face;
+ } else {
+ slice = depth;
}
I find the face 0 check confusing. For cube face 0, you're falling
through to the array case and relying the fact that depth == 0 for
cubemaps. Yes, it works, but...bizarre.
You're also relying on depth == 0 for non-cube/non-array cases, but that
seems entirely reasonable to me.
Perhaps just change the (face 0) check to (mt-target ==
GL_TEXTURE_CUBE_MAP)? That seems clear enough.
If it looks bizarre to you, it will likely look bizarre to others too. I dislike
bizarre code, so I'll change this with your (mt-target = GL_TEXTURE_CUBE_MAP)
suggestion.
Technically you could just drop the changes in this function (they're
not necessary), but I do like the cleanup.
True that it's not really needed. I cleaned it up because it seemed that
the old code did a lot of special-casing for no good reason.
-
Chad Versace
chad.vers...@linux.intel.com
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJOxvWdAAoJEAIvNt057x8iq5YP/REghGtNAaYC7hKdnfeuCYU7
LGHF728d57q0wzntZT+h0bOdXZh4hhRXXFaxYHniLovBLufUOLnLG2BUrz2aFqzq
QcRWqRx6KKoe/9Fy7iq4fAS5+DEoNNidLnLkBsIKcOuEgUtXYLXRIkhtxV3ZkKyA
9ahk30aA+jbaTK7allTMX52+JLs3pkdClALOykkWhKocAPskWa4a/fW44KKq4ASq
0bYcQeb5Behu0GGCKF8hfNtblxsNYUBWMLgAgPKN8MSaMZocLbw+VZOuv7ENz3zh
CcN4BIIMXoFrwrvaSaNFmLJVwbuFK0qtTpIeNFbrVmIQh+tEgEuXhPkkXmpAYhym
0akN/eV8yNVwClhM4XGEw0firvdm6cTJKImOX+nvgJUVMP+Ll2clpYjNDPnfKPp6
+v/e6PWOjxjht/go07ED+vWuk46mr/eEQ4ZIzlkN2RF1mgd6Yc/hGpM38M3yOEet
6zNvrtfKzi0mMgvm5ZAxPpAvDUpvbzrKP/3UaNJN3NrWlVpBt/q3eb/jpd8HFEq6
w2fYSrX4g+X+PLT/k+14XXeEQxhLD9E5pv5AQyus0jlBhatbAGm7RbMmqgsiOdE/
bAFIhE4T7Ra6VQG4LUzOfl2xhewojz0g5FDQT6LDr1FkKWs/hpZx286AzCS2vDki
mGbS0DPxKXLdTD8IGKuo
=bkdP
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/41] intel: Kill intel_mipmap_level::nr_images
For all texture targets except GL_TEXTURE_CUBE_MAP, the 'nr_images' and
'depth' fields of intel_mipmap_level were identical. In the exceptional
case, nr_images == 6 and depth == 1.
It is simple to determine if a texture is a cube or not, so the presence
of two fields here was not helpful. Worse, it was confusing. When we
eventually implement GL_ARB_texture_cube_map_array, this mess would have
become even more confusing.
This patch removes 'nr_images' and assigns to 'depth' a consistent
meaning: depth is the number of 2D slices at each miplevel. The exact
semantics of depth varies according to the texture target:
- For GL_TEXTURE_CUBE_MAP, depth is 6.
- For GL_TEXTURE_2D_ARRAY, depth is the number of array slices. It is
identical for all miplevels in the texture.
- For GL_TEXTURE_3D, it is the texture's depth at each miplevel. It's
value, like width and height, varies with miplevel.
- For other texture types, depth is 1.
As a consequence, parameters were removed from the following function
signatures:
intel_miptree_set_level_info
Remove 'nr_images'.
i945_miptree_layout
brw_miptree_layout_texture
brw_miptree_layout_texture_array
Remove 'slices'.
Signed-off-by: Chad Versace chad.vers...@linux.intel.com
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 17 -
src/mesa/drivers/dri/intel/intel_fbo.c | 12 ---
src/mesa/drivers/dri/intel/intel_mipmap_tree.c | 44 +++
src/mesa/drivers/dri/intel/intel_mipmap_tree.h | 18 +++---
src/mesa/drivers/dri/intel/intel_tex_layout.c |4 +-
src/mesa/drivers/dri/intel/intel_tex_layout.h |3 +-
src/mesa/drivers/dri/intel/intel_tex_validate.c |7 ++--
7 files changed, 56 insertions(+), 49 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index d77bf4d..ac6ade6 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -41,8 +41,7 @@
static void
brw_miptree_layout_texture_array(struct intel_context *intel,
-struct intel_mipmap_tree *mt,
-int slices)
+struct intel_mipmap_tree *mt)
{
GLuint align_w;
GLuint align_h;
@@ -58,14 +57,14 @@ brw_miptree_layout_texture_array(struct intel_context
*intel,
if (mt-compressed)
qpitch /= 4;
- i945_miptree_layout_2d(mt, slices);
+ i945_miptree_layout_2d(mt);
for (level = mt-first_level; level = mt-last_level; level++) {
- for (q = 0; q slices; q++) {
+ for (q = 0; q mt-depth0; q++) {
intel_miptree_set_image_offset(mt, level, q, 0, q * qpitch);
}
}
- mt-total_height = qpitch * slices;
+ mt-total_height = qpitch * mt-depth0;
}
void
@@ -82,7 +81,7 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
* pitch of qpitch rows, where qpitch is defined by the equation given
* in Volume 1 of the BSpec.
*/
-brw_miptree_layout_texture_array(intel, mt, 6);
+brw_miptree_layout_texture_array(intel, mt);
break;
}
/* FALLTHROUGH */
@@ -117,7 +116,7 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
GLint y = 0;
GLint q, j;
-intel_miptree_set_level_info(mt, level, nr_images,
+intel_miptree_set_level_info(mt, level,
0, mt-total_height,
width, height, depth);
@@ -170,11 +169,11 @@ brw_miptree_layout(struct intel_context *intel, struct
intel_mipmap_tree *mt)
case GL_TEXTURE_2D_ARRAY:
case GL_TEXTURE_1D_ARRAY:
- brw_miptree_layout_texture_array(intel, mt, mt-depth0);
+ brw_miptree_layout_texture_array(intel, mt);
break;
default:
- i945_miptree_layout_2d(mt, 1);
+ i945_miptree_layout_2d(mt);
break;
}
DBG(%s: %dx%dx%d\n, __FUNCTION__,
diff --git a/src/mesa/drivers/dri/intel/intel_fbo.c
b/src/mesa/drivers/dri/intel/intel_fbo.c
index 9398dbd..8c41956 100644
--- a/src/mesa/drivers/dri/intel/intel_fbo.c
+++ b/src/mesa/drivers/dri/intel/intel_fbo.c
@@ -947,8 +947,9 @@ intel_framebuffer_renderbuffer(struct gl_context * ctx,
static bool
intel_update_wrapper(struct gl_context *ctx, struct intel_renderbuffer *irb,
-struct gl_texture_image *texImage)
+struct gl_renderbuffer_attachment *att)
{
+ struct gl_texture_image *texImage = _mesa_get_attachment_teximage(att);
struct intel_texture_image *intel_image = intel_texture_image(texImage);
int width, height, depth;
@@ -993,7 +994,8 @@ intel_update_wrapper(struct gl_context *ctx, struct
intel_renderbuffer *irb,
* This will have the region info needed for hardware rendering.
*/
static struct intel_renderbuffer *
-intel_wrap_texture(struct gl_context * ctx,
