On Thu, Jul 23, 2015 at 6:49 PM, Brian Paul <bri...@vmware.com> wrote: > On 07/23/2015 04:37 PM, Ilia Mirkin wrote: >> >> On Thu, Jul 23, 2015 at 6:34 PM, Brian Paul <bri...@vmware.com> wrote: >>> >>> On 07/23/2015 04:24 PM, Ilia Mirkin wrote: >>>> >>>> >>>> On Wed, Jul 22, 2015 at 1:44 PM, Brian Paul <bri...@vmware.com> wrote: >>>>> >>>>> >>>>> On 07/22/2015 11:29 AM, Ilia Mirkin wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Wed, Jul 22, 2015 at 1:20 PM, Brian Paul <bri...@vmware.com> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 07/22/2015 11:02 AM, Ilia Mirkin wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> This moves the width/height/depth == 0 check to the front and avoids >>>>>>>> doing any other checking when that is the case. >>>>>>>> >>>>>>>> Also moves the dimensions check after the format/type checks so that >>>>>>>> we >>>>>>>> don't bail out with success on a width/height/depth == 0 request >>>>>>>> when >>>>>>>> the format/type don't match. >>>>>>>> >>>>>>>> Bugzilla: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D91425&d=BQIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=Y1TotaJmX1E7sCopvmMJc8PQKFXlHu6QxfcYOydKuII&s=L0lmmuk31G6M4hZEiwnX-IC4CPhW7rsrSCgEYtDgXjw&e= >>>>>>>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> I suspect this isn't really needed in light of my patch to >>>>>>> getteximage-invalid-format-for-packed-type.c >>>>>>> >>>>>>> I believe the test was in error, or at least sloppy, in that it was >>>>>>> calling >>>>>>> glGetTexImage to test format/type validation when there wasn't even a >>>>>>> texture image to return. The OpenGL spec doesn't specify an order >>>>>>> for >>>>>>> error >>>>>>> checking multiple things, so if there's multiple errors, you can't be >>>>>>> sure >>>>>>> which one will be reported first. My patch to the test removes that >>>>>>> ambiguity. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Well, irrespective of the piglit test, the current code is a little >>>>>> suspect -- it does the width/height/depth == 0 checks after it ensures >>>>>> that height == 1 for 1d and depth == 1 for 2d. Any thoughts on that? >>>>> >>>>> >>>>> >>>>> >>>>> Yeah, that makes sense. I guess we don't have any tests that exercise >>>>> getting a 0 x 0 texture image. >>>>> >>>>> Reviewed-by: Brian Paul <bri...@vmware.com> >>>> >>>> >>>> >>>> Ugh, so Dylan points out that this breaks >>>> >>>> bin/arb_direct_state_access-get-textures >>>> >>>> (which I didn't notice, because I guess it got recently enabled on mesa) >>>> >>>> It has this check: >>>> >>>> /* No Storage >>>> * >>>> * The spec doesn't say what should happen in this case. This >>>> is >>>> * addressed by Khronos Bug 13223. >>>> */ >>>> glCreateTextures(GL_TEXTURE_CUBE_MAP, 1, &name); >>>> glGetTextureImage(name, 0, GL_RGBA, GL_UNSIGNED_BYTE, 0, >>>> data); >>>> pass &= piglit_check_gl_error(GL_INVALID_OPERATION); >>>> glDeleteTextures(1, &name); >>>> >>>> But my width == 0 || ... check at the top of dimensions_error_check >>>> short-circuits all that. >>>> >>>> Brian, what situation is the "width == 0 || height == 0 || depth == 0" >>>> supposed to account for? >>> >>> >>> >>> After error checking, it just lets up skip all the >>> ctx->Driver.GetTexSubImage() calls and associated stuff (mapping PBOs, >>> etc). >> >> >> But some of the error checking makes sure that width/height/depth >> aren't 0. And in case of a missing image, width/height/depth are all >> set to 0. That was my confusion. > > > OK, it looks like we can remove the height==1 and depth==1 checks. NVIDIA's > driver does not generate an error if height=0 or depth=0 in the 1D/2D cases. > > Can you take care of that?
Yeah, I broke it, so I get to fix it :) Should have something out in a few hours. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
