On Fri, Feb 10, 2012 at 3:00 PM, Brian Paul <span dir="ltr"><<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On 02/10/2012 02:39 PM, Anuj Phogat wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
This patch adds the pixel pack operations in glGetTexImage for<br>
compressed textures with unsigned, normalized values. It also<br>
fixes the failures in intel oglconform pxstore-gettex due to<br>
following sub test cases:<br>
<br>
- Test all mipmaps with byte swapping enabled<br>
- Test all small mipmaps with all allowable alignment values<br>
- Test subimage packing for all mipmap levels<br>
<br>
Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=40864" target="_blank">https://bugs.freedesktop.org/<u></u>show_bug.cgi?id=40864</a><br>
<br>
Note: This is a candidate for stable branches<br>
<br>
Signed-off-by: Anuj Phogat<<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>><br>
---<br>
src/mesa/drivers/common/meta.c | 55 ++++++++++++++++++++++++++++++<u></u>+++++++--<br>
1 files changed, 52 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/src/mesa/drivers/common/<u></u>meta.c b/src/mesa/drivers/common/<u></u>meta.c<br>
index aa5fef8..97c1912 100644<br>
--- a/src/mesa/drivers/common/<u></u>meta.c<br>
+++ b/src/mesa/drivers/common/<u></u>meta.c<br>
@@ -53,6 +53,7 @@<br>
#include "main/pixel.h"<br>
#include "main/pbo.h"<br>
#include "main/polygon.h"<br>
+#include "main/pack.h"<br>
#include "main/readpix.h"<br>
#include "main/scissor.h"<br>
#include "main/shaderapi.h"<br>
@@ -3438,6 +3439,14 @@ _mesa_meta_GetTexImage(struct gl_context *ctx,<br>
GLenum format, GLenum type, GLvoid *pixels,<br>
struct gl_texture_image *texImage)<br>
{<br>
+ GLuint row;<br>
+ GLfloat *srcRow;<br>
+ void *tempImage, *dest;<br>
+ const GLuint width = texImage->Width;<br>
+ const GLuint height = texImage->Height;<br>
+ const GLint tempRowLength = 0;<br>
+ GLbitfield transferOps = 0x0;<br>
+<br>
/* We can only use the decompress-with-blit method here if the texels are<br>
* unsigned, normalized values. We could handle signed and unnormalized<br>
* with floating point renderbuffers...<br>
@@ -3445,14 +3454,54 @@ _mesa_meta_GetTexImage(struct gl_context *ctx,<br>
if (_mesa_is_format_compressed(<u></u>texImage->TexFormat)&&<br>
_mesa_get_format_datatype(<u></u>texImage->TexFormat)<br>
== GL_UNSIGNED_NORMALIZED) {<br>
+<br>
struct gl_texture_object *texObj = texImage->TexObject;<br>
- const GLuint slice = 0; /* only 2D compressed textures for now */<br>
+ /* only 2D compressed textures for now */<br>
+ const GLuint slice = 0;<br>
+ const int dimensions = 2;<br>
+<br>
+ if (format == GL_LUMINANCE ||<br>
+ format == GL_LUMINANCE_ALPHA) {<br>
+ transferOps |= IMAGE_CLAMP_BIT;<br>
+ }<br>
+<br>
+ /* Allocate a temporary 2D RGBA float buffer */<br>
+ tempImage = malloc(width * height * 4 * sizeof(GLfloat));<br>
+ if (tempImage == NULL) {<br>
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexImage");<br>
+ return;<br>
+ }<br>
+<br>
/* Need to unlock the texture here to prevent deadlock... */<br>
_mesa_unlock_texture(ctx, texObj);<br>
- decompress_texture_image(ctx, texImage, slice, format, type, pixels,<br>
- ctx->Pack.RowLength);<br>
+<br>
+ /* Decompress in to temporary buffer with format = GL_RGBA,<br>
+ * type = GL_FLOAT and RowLength = 0 then pack in to user<br>
+ * supplied buffer<br>
+ */<br>
+ decompress_texture_image(ctx, texImage, slice, GL_RGBA, GL_FLOAT,<br>
+ tempImage, tempRowLength);<br>
/* ... and relock it */<br>
_mesa_lock_texture(ctx, texObj);<br>
+<br>
+ srcRow = (GLfloat*) tempImage;<br>
+ for (row = 0; row< height; row++) {<br></div></div>
+ dest = _mesa_image_address(<u></u>dimensions,&ctx->Pack, pixels,<div class="im"><br>
+ width, height, format, type,<br>
+ 0, row, 0);<br>
+ if (_mesa_is_integer_format(<u></u>format)) {<br>
</div></blockquote>
<br>
I don't think the format can be integer-valued at this point if we tested for datatype = GL_UNSIGNED_NORMALIZED above.<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
+ _mesa_pack_rgba_span_int(ctx, width, (GLuint (*)[4]) srcRow,<br>
+ format, type, dest);<br>
+ }<br>
+ else {<br>
+ _mesa_pack_rgba_span_float(<u></u>ctx, width, (GLfloat (*)[4]) srcRow,<br></div>
+ format, type, dest,&ctx->Pack,<div class="im"><br>
+ transferOps);<br>
+ }<br>
+ srcRow += width * 4;<br>
+ }<br>
+<br>
+ free(tempImage);<br>
}<br>
else {<br>
_mesa_get_teximage(ctx, format, type, pixels, texImage);<br>
</div></blockquote>
<br>
<br>
If the main problem here is the pixel store/pack operations, I think that another fix is much simpler:<br>
<br>
First, let's remove the destRowLength parameter to decompress_texture_image(). We're just passing ctx->Pack.RowLength but then we assign that value to ctx->Pack.RowLength at line 3412. That's pointless.<br>
<br>
Then, instead of calling _mesa_meta_begin(ctx, MESA_META_ALL) at line 3277, use (MESA_META_ALL & ~MESA_META_PIXEL_STORE). The current pixel store params will stay in effect and _mesa_ReadPixels() call at line 3413 will apply them for us.<br>
</blockquote><div><br></div><div>Yes, this fix works for me and is much simpler. I'll send out a patch for review.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
BTW, I think we could avoid some FBO resizing/reallocating if we change line 3295 to read:<br>
<br>
if (width > decompress->Width || height > decompress->Height) {</blockquote><div> </div></div>I agree.<div><br></div><div>Thanks</div><div>Anuj</div>