On Wed, Feb 22, 2012 at 1:54 PM, Brian Paul <<a href="mailto:brian.e.paul@gmail.com">brian.e.paul@gmail.com</a>> wrote: <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 Wed, Feb 22, 2012 at 3:45 PM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> wrote: > Driver throws assertion failure or segfaults with large textures. > > Reproduces the errors reported in: > <a href="https://bugs.freedesktop.org/show_bug.cgi?id=44970" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=44970</a> > <a href="https://bugs.freedesktop.org/show_bug.cgi?id=46303" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=46303</a> > > Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> > --- > Please ignore previous patch sent on this thread. > V2: This version adds other texture targets and texture data types > Changed the name of test case > > tests/all.tests | 1 + > tests/bugs/CMakeLists.gl.txt | 1 + > tests/bugs/large-textures.c | 224 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 226 insertions(+), 0 deletions(-) > create mode 100644 tests/bugs/large-textures.c > > diff --git a/tests/all.tests b/tests/all.tests > index e4d56b8..d006e9a 100644 > --- a/tests/all.tests > +++ b/tests/all.tests > @@ -569,6 +569,7 @@ add_plain_test(bugs, 'fdo24066') > add_plain_test(bugs, 'fdo25614-genmipmap') > add_plain_test(bugs, 'fdo28551') > add_plain_test(bugs, 'fdo31934') > +add_plain_test(bugs, 'large-textures') > add_plain_test(bugs, 'point-sprite') > add_plain_test(bugs, 'r300-readcache') > add_plain_test(bugs, 'tex1d-2dborder') > diff --git a/tests/bugs/CMakeLists.gl.txt b/tests/bugs/CMakeLists.gl.txt > index 5c1864e..8cdd4d0 100644 > --- a/tests/bugs/CMakeLists.gl.txt > +++ b/tests/bugs/CMakeLists.gl.txt > @@ -32,6 +32,7 @@ add_executable (fdo24066 fdo24066.c) > add_executable (fdo25614-genmipmap fdo25614-genmipmap.c) > add_executable (fdo28551 fdo28551.c) > add_executable (fdo31934 fdo31934.c) > +add_executable (large-textures large-textures.c) > add_executable (tri-tex-crash tri-tex-crash.c) > add_executable (vbo-buffer-unmap vbo-buffer-unmap.c) > > diff --git a/tests/bugs/large-textures.c b/tests/bugs/large-textures.c > new file mode 100644 > index 0000000..8d5751f > --- /dev/null > +++ b/tests/bugs/large-textures.c > @@ -0,0 +1,224 @@ > +/* Copyright © 2012 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + */ > + > +/** > + * \file large-textures.c > + * Verify that large textures are handled correctly in mesa driver > + * > + * This test works by calling glTexImage2D/3D and glTexSubImage2D/3D functions > + * with different texture targets. Each texture target is tested with texture > + * sizes up to the maximum supported texture size. All the calls to > + * glTexImage2D() and glTexSubImage2D() should ensure no segmentation faults/ > + * assertion failures in mesa driver. > + * > + * This test case reproduces the errors reported in: > + * <a href="https://bugs.freedesktop.org/show_bug.cgi?id=44970" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=44970</a> > + * <a href="https://bugs.freedesktop.org/show_bug.cgi?id=46303" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=46303</a> > + * > + * \Author Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> > + */ > + > +#include "piglit-util.h" > +#define MAX_2D_TEXTURE 8192 > +#define COLOR_COMPONENTS 4 > + > +int piglit_width = 100, piglit_height = 100; > +int piglit_window_mode = GLUT_RGBA | GLUT_DOUBLE; > + > +char *target_string[] = { > + "GL_TEXTURE_1D", > + "GL_TEXTURE_2D", > + "GL_TEXTURE_CUBE_MAP", > + "GL_TEXTURE_3D", > + }; > + > +char *format_string[] = { > + "GL_RGBA8", > + "GL_RGBA16", > + "GL_RGBA32F", > + }; </div></div>You shouldn't need lists of strings like that since we have piglit_get_gl_enum_name().</blockquote><div>I will use this function to get the strings. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> > +GLenum target[] = { > + GL_TEXTURE_1D, > + GL_TEXTURE_2D, > + GL_TEXTURE_CUBE_MAP, > + GL_TEXTURE_3D, > + }; > + > +GLenum internalformat[] = { > + //GL_RGBA8, > + //GL_RGBA16, </div>Are those formats commented out for a reason? </blockquote><div>Sorry, I forgot to remove the comments .</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + GL_RGBA32F, If you're going to test a floating point format, you should check if GL_ARB_texture_float is supported. <div><div class="h5"> > + }; > + > + > +GLenum maxTargetEnum(GLenum target) > +{ > + switch(target) { > + case GL_TEXTURE_1D: > + case GL_TEXTURE_2D: > + return GL_MAX_TEXTURE_SIZE; > + case GL_TEXTURE_3D: > + return GL_MAX_3D_TEXTURE_SIZE; > + case GL_TEXTURE_CUBE_MAP_ARB: > + return GL_MAX_CUBE_MAP_TEXTURE_SIZE_ARB; > + case GL_TEXTURE_RECTANGLE: > + return GL_MAX_RECTANGLE_TEXTURE_SIZE; > + case GL_RENDERBUFFER_EXT: > + return GL_MAX_RENDERBUFFER_SIZE_EXT; > + default: > + printf ("Invalid texture target used"); > + return 0; > + } > +} > + > +void piglit_init(int argc, char **argv) > +{ > + GLuint tex; > + GLint maxSide; > + int i, j, k, side; > + const uint64_t nPixels = (uint64_t)(MAX_2D_TEXTURE + 3) * > + (uint64_t)(MAX_2D_TEXTURE + 3); > + /* Allocate the data array for maximum texture size used in this test > + * and initialize to zero > + */ > + GLfloat *pixels = (GLfloat *) calloc(nPixels * COLOR_COMPONENTS, > + sizeof(float)); </div></div>I think you should allocate 'pixels' according to the max texture size you get below. Suppose GL returns a max size of 16384 (which is greater than MAX_2D_TEXTURE). I think you might out of bounds below.</blockquote><div>I will fix this in my next patch. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> > + if ( pixels == NULL) { > + printf("\nUnable to allocate texture data array"); > + piglit_report_result(PIGLIT_SKIP); > + return; > + } > + > + glMatrixMode(GL_PROJECTION); > + glPushMatrix(); > + glLoadIdentity(); > + glOrtho(0, piglit_width, 0, piglit_height, -1, 1); > + glMatrixMode(GL_MODELVIEW); > + glPushMatrix(); > + glLoadIdentity(); > + glClearColor(0.2, 0.2, 0.2, 1.0); > + glClear(GL_COLOR_BUFFER_BIT); > + > + for ( i = 0; i < ARRAY_SIZE(target); i++) { > + /* Query the maximum supported texture size */ > + glGetIntegerv(maxTargetEnum(target[i]), &maxSide); > + printf("\n%s, Maximum allowable texture size = %d\n", > + target_string[i], maxSide); </div>You should probably try using a proxy texture to find the max supported texture size for each internal format. Querying GL_MAX_TEXTURE_SIZE doesn't always give the whole story. For example, a texel may be 1 (A8) or 16 bytes (RGBA float). There may enough be memory for a 16Kx16K texture of the 1-byte format but not the 16-byte format. Maybe you should do two tests: one that just uses the MAX_TEXTURE_SIZE value as-is and another that uses a proxy texture to find the max size for a particular format. In both cases, we don't want crashes or failed assertions.</blockquote><div>I agree. I will add the code for testing with size returned by proxy textures as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im"> > + glGenTextures(1, &tex); > + glBindTexture(target[i], tex); > + glTexParameteri(target[i], GL_TEXTURE_MIN_FILTER, GL_NEAREST); > + glTexParameteri(target[i], GL_TEXTURE_MAG_FILTER, GL_NEAREST); > + glTexParameteri(target[i], GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); > + glTexParameteri(target[i], GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); </div>Is there a reason to set the texture wrap state?</blockquote><div>This is redundant. can be removed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > + for (j = 0; j < ARRAY_SIZE(internalformat); j++) { > + for ( side = maxSide/2; side < maxSide + 2; side++) { > + switch (target[i]) { > + case GL_TEXTURE_1D: > + glTexImage1D(target[i], 0, > + internalformat[j], > + side, 0, > + GL_RGBA, GL_FLOAT, NULL); > + glTexSubImage1D(target[i], 0, 0, > + side/2, GL_RGBA, > + GL_FLOAT, pixels); > + break; > + > + case GL_TEXTURE_2D: > + glTexImage2D(target[i], 0, > + internalformat[j], > + side, side, 0, > + GL_RGBA, GL_FLOAT, NULL); > + glTexSubImage2D(target[i], 0, 0, 0, > + side/2, side/2, > + GL_RGBA, GL_FLOAT, pixels); > + break; > + > + case GL_TEXTURE_3D: > + > + glTexParameteri(target[i], > + GL_TEXTURE_WRAP_R, > + GL_CLAMP_TO_EDGE); > + glTexImage3D(target[i], 0, > + internalformat[j], > + side, side, > + side, 0, GL_RGBA, > + GL_FLOAT, NULL); > + > + glTexSubImage3D(target[i], 0, 0, 0, 0, > + side/2, side/2, side/2, > + GL_RGBA, GL_FLOAT, pixels); > + break; > + > + case GL_TEXTURE_CUBE_MAP_ARB: > + for (k = 0; k < 6; k++) { > + glTexImage2D( > + GL_TEXTURE_CUBE_MAP_POSITIVE_X + k, > + 0, internalformat[j], > + side, side, 0, > + GL_RGBA, GL_FLOAT, NULL); > + } > + for (k = 0; k < 6; k++) { > + glTexSubImage2D( > + GL_TEXTURE_CUBE_MAP_POSITIVE_X + k, > + 0, 0, 0, > + side/2, side/2, > + GL_RGBA, GL_FLOAT, pixels); > + if (glGetError() != GL_NO_ERROR) > + { > + printf("GL Error1: while" > + " testing %s, texture" > + " size = %d, internal" > + " format = %s\n", > + target_string[i], side, > + format_string[j]); > + piglit_reset_gl_error(); > + } > + } > + break; > + } > + if (glGetError() != GL_NO_ERROR) > + { > + printf("GL Error2: while testing %s, texture" > + " size = %d, internal format = %s\n", > + target_string[i], side, > + format_string[j]); </div></div>It would be good to query for an error both after glTexImage() and glTexSubImage() to know more precisely which function might be failing.</blockquote><div>I agree.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"> > + piglit_reset_gl_error(); > + /* skip larger texture sizes for this texture > + * target > + */ > + //break; > + } > + } > + } > + glDeleteTextures(1, &tex); > + } > + /* If execution reaches this point, return PIGLIT_PASS */ > + piglit_report_result(PIGLIT_PASS); > +} > + > +enum piglit_result > +piglit_display(void) > +{ > + return PIGLIT_FAIL; > +} > -- > 1.7.7.6 > </div>> _______________________________________________ > Piglit mailing list > <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div>