On Thu, Feb 9, 2012 at 1:07 PM, Brian Paul <<a href="mailto:brianp@vmware.com">brianp@vmware.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 02/09/2012 01:57 PM, Brian Paul wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 02/09/2012 12:15 PM, Anuj Phogat wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Thu, Feb 9, 2012 at 11:02 AM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a> <mailto:<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>>> wrote: On 02/09/2012 08:14 AM, Brian Paul wrote: Anuj, The patch you committed (15986d2), the first version you posted that tests: if (width < 2 * border || width > maxSize) return GL_FALSE; causes my copy of conform (mustpass.c / proxy texture test) to fail with swrast/softpipe/llvmpipe. It passes w/ NVIDIA's driver. I thought you were going to commit the patch that does: if (width < 2 * border || width > 2 * border + maxSize) return GL_FALSE; That allows conform to pass here. Allow me to remove the egg from my face. I dug a bit deeper into the test case that was previously failing and conform / mustpass. What I said before (<a href="http://lists.freedesktop.org/__archives/mesa-dev/2012-__February/018562.html" target="_blank">http://lists.freedesktop.org/__archives/mesa-dev/2012-__February/018562.html</a> <<a href="http://lists.freedesktop.org/archives/mesa-dev/2012-February/018562.html" target="_blank">http://lists.freedesktop.org/archives/mesa-dev/2012-February/018562.html</a>>) was wrong. You and Jose were correct. Sorry. </blockquote> No biggie. <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Intel oglconform test case (max_values negative.textureSize.textureCube) also passes with the patch i earlier posted as per Brian's comments: <a href="http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html" target="_blank">http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html</a> <%20<a href="http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html" target="_blank">http://lists.freedesktop.org/archives/mesa-dev/2012-January/018410.html</a>> I'll make the required changes as per this patch. </blockquote> Thanks, the test passes again. But I have a follow-up comment for the patch in question... </blockquote> </div></div> Actually, this isn't directly related to your patch- the issue is with texture borders and array textures. For a 1D texture array, the border only applies to the width. For a 2D texture array the border applies to the width and height but not the depth. We're not handling this correctly everywhere. For example, in _mesa_init_teximage_fields() we compute the 'Height2' field like this: if (height == 1) { /* 1-D texture */ img->Height2 = 1; img->HeightLog2 = 0; } else { img->Height2 = height - 2 * border; /* == 1 << img->HeightLog2; */ img->HeightLog2 = _mesa_logbase2(img->Height2); } The else clause does the wrong thing if the texture is a 1D array texture with a border. Similarly for Depth2 and 2D array textures. Granted, borders are seldom used, but it would be good to go over all the teximage code and fix these mistakes. </blockquote><div> yes, i agree. I'll send out a separate patch to fix such cases in teximage code. </div> </div>