Bug 59876 - glGetTexLevelParameteriv broken for indirect rendering
glGetTexLevelParameteriv broken for indirect rendering
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: GLX
9.0
Other All
: medium normal
Assigned To: mesa-dev
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-26 01:06 UTC by Glynn Clements
Modified: 2013-02-19 14:30 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
proposed patch (979 bytes, patch)
2013-02-04 20:20 UTC, Stefan Brüns
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Glynn Clements 2013-01-26 01:06:15 UTC
With indirect rendering, glGetTexLevelParameteriv() is returning garbage, at least for GL_TEXTURE_WIDTH and GL_TEXTURE_HEIGHT.

I don't know whether this is in libGL, XCB or the X server. I've tried it with several X servers (including Xorg, Xvnc, Xvfb, Cygwin's XWin.exe and Xming), but they're all based on the same underlying code base so that doesn't mean much.

I managed to track it down as far as the USE_XCB branch of __indirect_glGetTexLevelParameteriv() in src/glx/indirect.c. The reply contains the following:

> print *reply
$7 = {
  response_type = 1 '\001', 
  pad0 = 0 '\000', 
  sequence = 65, 
  length = 0, 
  pad1 = "\000\000\000", 
  n = 1, 
  datum = 256, 
  pad2 = "\230y'a\000\000\000\000\000\000\000"
}

As xcb_glx_get_tex_level_parameteriv_data_length(reply) (i.e. reply->n) is non-zero, it expects to find the value at xcb_glx_get_tex_level_parameteriv_data() (i.e. following the structure), but the correct value (256) is actually in reply->datum (which would have been used if reply->n was zero).
Comment 1 Stefan Brüns 2013-02-04 20:20:24 UTC
Created attachment 74192 [details] [review]
proposed patch

The libxcb XXX_data_length(reply) function does *not* return the length of the data part, but the number of elements (reply->n). Therefor the condition should be on XXX_data_length(reply) == 1.
Comment 2 Matt Turner 2013-02-04 21:30:16 UTC
Looks like this might be a bug in code Julien wrote?(In reply to comment #1)
> Created attachment 74192 [details] [review] [review]
> proposed patch
> 
> The libxcb XXX_data_length(reply) function does *not* return the length of
> the data part, but the number of elements (reply->n). Therefor the condition
> should be on XXX_data_length(reply) == 1.

Nice. Please send a git-formatted patch to mesa-dev@lists.freedesktop.org and CC Ian Romanick <idr@freedesktop.org>.
Comment 3 Stefan Brüns 2013-02-05 16:32:26 UTC
(In reply to comment #2)
> Nice. Please send a git-formatted patch to mesa-dev@lists.freedesktop.org
> and CC Ian Romanick <idr@freedesktop.org>.

Done.

BTW: The repository paths on http://www.mesa3d.org/repository.html are slightly wrong, its missing a trailing ".git", i.e.
git clone git://anongit.freedesktop.org/git/mesa/mesa.git
Comment 4 Brian Paul 2013-02-05 17:45:10 UTC
(In reply to comment #3)
> 
> BTW: The repository paths on http://www.mesa3d.org/repository.html are
> slightly wrong, its missing a trailing ".git", i.e.
> git clone git://anongit.freedesktop.org/git/mesa/mesa.git

Hmm, I can clone the repo just fine without the ".git" suffix.
Comment 5 Stefan Brüns 2013-02-18 11:20:45 UTC
Can *anyonye* act on this, please?

The bug is obvious and the fix is trivial ...
Comment 6 Brian Paul 2013-02-19 14:30:14 UTC
Patch committed as 5876a5dbc0a6ec9ae7f44b5e483d38ae0d24a259