Bug 67516 - glTexStorage*() functions don't work properly with proxy textures
Summary: glTexStorage*() functions don't work properly with proxy textures
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: mesa-dev
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-30 04:13 UTC by Mikko Juola
Modified: 2013-07-31 13:28 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
texstorage_fail.c, shows the problems as they appear to user code (1.06 KB, text/plain)
2013-07-30 04:13 UTC, Mikko Juola
Details
patch 1/3 (1.85 KB, text/plain)
2013-07-30 04:14 UTC, Mikko Juola
Details
patch 2/3 (1.28 KB, text/plain)
2013-07-30 04:14 UTC, Mikko Juola
Details
patch 3/3 (1.16 KB, text/plain)
2013-07-30 04:15 UTC, Mikko Juola
Details
patch 1/1 (1.14 KB, patch)
2013-07-30 17:56 UTC, Mikko Juola
Details | Splinter Review

Description Mikko Juola 2013-07-30 04:13:35 UTC
Created attachment 83259 [details]
texstorage_fail.c, shows the problems as they appear to user code

There appears to be problems with proxy textures and the texture storage functions. I identified three separate issues:

- The calculation (_mesa_get_tex_max_num_levels() in teximage.c) that checks how many mipmap levels a texture could have has not been written to check proxy textures as well.

- The error checking in the storage functions checks if a non-zero texture has been bound. Proxy textures don't care about that but the check makes glTexStorage*() fail when they should not. As far as I know, you cannot bind a proxy texture to anything so this makes glTexStorage*() always fail for a proxy texture.

- The use of glTexStorage*() on a proxy texture makes the texture immutable. This is not normally detectable by the users (because it is a proxy texture) but the immutability flag makes further glTexStorage*() calls fail. This last issue cannot be seen until the two previous issues are fixed.


I would not be surprised if there were more issues with proxy textures lurking here, given that it looks like these functions were not tested with proxy textures.

I'm attaching three patches that I used to make things work for me. I also attached a short testing program that shows how user code would see these problems.
Comment 1 Mikko Juola 2013-07-30 04:14:16 UTC
Created attachment 83260 [details]
patch 1/3
Comment 2 Mikko Juola 2013-07-30 04:14:33 UTC
Created attachment 83261 [details]
patch 2/3
Comment 3 Mikko Juola 2013-07-30 04:15:08 UTC
Created attachment 83262 [details]
patch 3/3
Comment 4 Brian Paul 2013-07-30 14:17:38 UTC
The patches look good to me.
Reviewed-by: Brian Paul <brianp@vmware.com>
Do you need me to commit them for you?
Comment 5 Mikko Juola 2013-07-30 14:30:33 UTC
Yeah, I think so. This is the first time I've ever submitted any patches to mesa.
Comment 6 Mikko Juola 2013-07-30 17:56:59 UTC
Created attachment 83319 [details] [review]
patch 1/1

I discovered another proxy texture related bug with multisampling textures. glGetTexLevelParameteriv() would fail for GL_PROXY_TEXTURE_2D_MULTISAMPLE and GL_PROXY_TEXTURE_2D_MULTISAMPLE_ARRAY because one of the error checking functions seems to be blissfully unaware these targets exist. I attached another patch to fix this one.

This is technically a different bug to the texture storage proxy texture bugs but since we are on the topic of proxy textures, I attached this one here.
Comment 7 Brian Paul 2013-07-31 13:28:19 UTC
(In reply to comment #6)
> Created attachment 83319 [details] [review] [review]
> patch 1/1
> 
> I discovered another proxy texture related bug with multisampling textures.
> glGetTexLevelParameteriv() would fail for GL_PROXY_TEXTURE_2D_MULTISAMPLE
> and GL_PROXY_TEXTURE_2D_MULTISAMPLE_ARRAY because one of the error checking
> functions seems to be blissfully unaware these targets exist. I attached
> another patch to fix this one.
> 
> This is technically a different bug to the texture storage proxy texture
> bugs but since we are on the topic of proxy textures, I attached this one
> here.

Thanks.  That patch looks good too.  I've pushed these to master.
Closing this bug now.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.