glGenerateMipmap now does not force textures to become render targets....

glGenerateMipmap now does not force textures to become render targets.

For textures that are not already render targets, use the CPU to filter them and mark them as dirty.

I tested it by modifying the "mip_map_2d" demo to call glGenerateMipmap instead of generating the mipmaps itself. I also used the debugger to force it to take the render target path and verified that it still worked.

We've been having problems with the display being reset when video memory pressure is heavy with one of our demos. This patch made a noticable improvement on Vista but I don't think it will necessarily help on XP.


Committed: http://code.google.com/p/angleproject/source/detail?r=490

[apatrick1, 2010-11-22 23:24:40]

Hi Daniel,

Another patch for you.

Thanks,

Al

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp
File src/libGLESv2/Texture.cpp (right):

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp#newcod...
src/libGLESv2/Texture.cpp:1668: if
(FAILED(D3DXLoadSurfaceFromSurface(mImageArray[i].surface, NULL, NULL,
mImageArray[i - 1].surface, NULL, NULL, D3DX_FILTER_LINEAR, 0)))
FILTER_LINEAR instead of FILTER_BOX because I still have a plan to add support
for the OES_texture_npot extension.

[dgkoch, 2010-11-23 21:44:54]

This (obviously) adds more of a dependency on D3DX.  Not necessary a problem,
but something to be aware of if we ever want to get rid of it…

Daniel

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp
File src/libGLESv2/Texture.cpp (right):

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp#newcod...
src/libGLESv2/Texture.cpp:1668: if
(FAILED(D3DXLoadSurfaceFromSurface(mImageArray[i].surface, NULL, NULL,
mImageArray[i - 1].surface, NULL, NULL, D3DX_FILTER_LINEAR, 0)))
On 2010/11/22 23:24:41, apatrick1 wrote:
> FILTER_LINEAR instead of FILTER_BOX because I still have a plan to add support
> for the OES_texture_npot extension.

This should be FILTER_BOX (it's even recommended by the ES2 spec!).  Not sure
how OES_texture_npot is relevant here as it doesn't lift the restrictions on
glGenerateMipmap, it just basically relaxes the completeness rules.  And for
some reason I thought someone already added OES_texture_npot support…

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp#newcod...
src/libGLESv2/Texture.cpp:2339: if
(FAILED(D3DXLoadSurfaceFromSurface(mImageArray[f][i].surface, NULL, NULL,
mImageArray[f][i - 1].surface, NULL, NULL, D3DX_FILTER_LINEAR, 0)))
should be BOX_FILTER here as well.

[dgkoch, 2010-11-23 21:45:10]

This (obviously) adds more of a dependency on D3DX.  Not necessary a problem,
but something to be aware of if we ever want to get rid of it…

Daniel

[apatrick1, 2010-11-23 21:58:58]

I put together a patch for the npot extension a while back but I didn't check it
in because it was likely to affect the compressed texture changes you were
making. It has since become somewhat lower priority.

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp
File src/libGLESv2/Texture.cpp (right):

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp#newcod...
src/libGLESv2/Texture.cpp:1668: if
(FAILED(D3DXLoadSurfaceFromSurface(mImageArray[i].surface, NULL, NULL,
mImageArray[i - 1].surface, NULL, NULL, D3DX_FILTER_LINEAR, 0)))
On 2010/11/23 21:44:54, dgkoch wrote:
> On 2010/11/22 23:24:41, apatrick1 wrote:
> > FILTER_LINEAR instead of FILTER_BOX because I still have a plan to add
support
> > for the OES_texture_npot extension.
> 
> This should be FILTER_BOX (it's even recommended by the ES2 spec!).  Not sure
> how OES_texture_npot is relevant here as it doesn't lift the restrictions on
> glGenerateMipmap, it just basically relaxes the completeness rules.  And for
> some reason I thought someone already added OES_texture_npot support…

Done.

http://codereview.appspot.com/3262041/diff/1/src/libGLESv2/Texture.cpp#newcod...
src/libGLESv2/Texture.cpp:2339: if
(FAILED(D3DXLoadSurfaceFromSurface(mImageArray[f][i].surface, NULL, NULL,
mImageArray[f][i - 1].surface, NULL, NULL, D3DX_FILTER_LINEAR, 0)))
On 2010/11/23 21:44:54, dgkoch wrote:
> should be BOX_FILTER here as well.

Done.

[dgkoch, 2010-11-23 22:02:12]

yeah, I found that patch again.

LGTM now.