<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Mar 12, 2016 at 3:08 PM, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span><p dir="ltr"><br>
On Mar 11, 2016 12:33 PM, "Alejandro Piñeiro" <<a href="mailto:apinheiro@igalia.com" target="_blank">apinheiro@igalia.com</a>> wrote:<br>
><br>
> On 11/03/16 20:15, Anuj Phogat wrote:<br>
> > yoffset is also applicable to 1d array textures.<br>
> ><br>
> > Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>><br>
> > ---<br>
> > I don't know if it fixes any test, but it looked incorrect to me.<br>
><br>
> No one fixed doing a piglit all.py run (also no regression). Didn't test<br>
> with a deqp run.</p>
</span><p dir="ltr">There are very few tests for glGetTexImage.  Not hitting one doesn't mean much.</p><span>
<p dir="ltr">> In any case, I also agree that the change seems to make sense.<br>
><br>
> ><br>
> >  src/mesa/main/texgetimage.c | 2 +-<br>
> >  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> ><br>
> > diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c<br>
> > index 06bc8f1..dc21551 100644<br>
> > --- a/src/mesa/main/texgetimage.c<br>
> > +++ b/src/mesa/main/texgetimage.c<br>
> > @@ -1046,7 +1046,7 @@ dimensions_error_check(struct gl_context *ctx,<br>
> >                          "%s(xoffset = %d)", caller, xoffset);<br>
> >              return true;<br>
> >           }<br>
> > -         if (target != GL_TEXTURE_1D && target != GL_TEXTURE_1D_ARRAY) {<br>
> > +         if (target != GL_TEXTURE_1D) {<br>
> >              if (yoffset % bh != 0) {<br>
> >                 _mesa_error(ctx, GL_INVALID_VALUE,<br>
> >                             "%s(yoffset = %d)", caller, yoffset);</p>
</span><p dir="ltr">I don't think this is correct.  The check is for compressed textures to ensure that the texture coordinates are a multiple of the block size of the texture.  I'm not sure what the rules are for 1-D array compressed textures (if they even exist) bit I'm pretty sure the compression doesn't cross slices.  If anything, we probably want to take the check below that looks at height and pull it into the if too.<br></p></blockquote><div>In a quick look at GL spec I couldn't find any special rules for 1-D array compressed textures. But, looking at the description of glGetCompressedTextureSubImage() in OpenGL 4.5 specification:</div><div><br></div><div>"An INVALID_VALUE error is generated if xoffset, yoffset or zoffset is not a
multiple of the compressed block width, height or depth respectively."<br></div><div><br></div><div>In above statement there are no restrictions on the texture target. So, it looks like we can completely drop the if statement checking for 1D targets.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><p dir="ltr">
</p>
<br>_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
<br></blockquote></div><br></div></div>