[Mesa-dev] [PATCH 0/6] glGetTexImage fixes

[Erik Faye-Lund]

On Fri, 2018-11-23 at 15:50 +0100, Juan A. Suarez Romero wrote:
> On Thu, 2018-11-22 at 17:48 +0100, Erik Faye-Lund wrote:
> > I was recently investigating a gl-error that appears some times
> > while
> > using virgl, where I got a pretty nonsensical GL-error:
> > 
> > Mesa: User error: GL_INVALID_VALUE in glGetnTexImageARB(depth = 0)
> > 
> > Now, the reason why this is nonsensical, is that glGetnTexImage
> > doesn't 
> > have a 'depth'-argument, this value is generated internally in
> > mesa.
> > 
> > What happens is that virgl ends up asking for the teximage for a
> > non-existen mipmap-level. Whoops, that shouldn't be done, but
> > while 
> > we're in this area, let's clean this up.
> > 
> > My first fix was to revert 42891dbaa12 ("gettextsubimage: verify
> > zoffset
> > and depth are correct"). But that would render that commit moot,
> > but it
> > actually does fix something. So I'd rather not; it's a good change,
> > but
> > this logic should only apply to glGetTextureSubImage.
> > 
> > So I decided to give glGetTextureSubImage its own error checking.
> > 
> > While working on this, I also realized that the error introduced in
> > b37b35a5d26 ("getteximage: assume texture image is empty for non
> > defined
> > levels") was flawed; it shouldn't apply in the case where *all* of
> > width, xoffset and the texture's width are zero. And since we end
> > up
> > checking explicitly for these conditions later, this should simply
> > be a
> > no-op as per the spec. And we already have some support-code for
> > this.
> > 
> > No regressions found in piglit.
> 
> The series looks good to me. 
> 
> 
> Reviewed-by: Juan A. Suarez <jasuarez at igalia.com>
> 

Thanks a lot :)

I've fixed up the subject-typo locally, and I'll let this sit over the
weekend until I push it out (unless someone finds a problem in the mean
time, of course).