On 11/04/2008, Stefan Dösinger stefan@codeweavers.com wrote:
This patch doesn't make sense at all. Wasn't ATIFS supposed to be an implementation of fixed function fragment processing for ATI cards? In that case it doesn't make sense to only use it on cards that don't support ARB or GLSL. That will only make newer ATI cards have worse fixed function fragment processing (assuming ATIFS works as advertised). Of course what it comes down to again is that the shader backend isn't the right place to implement this.
We need that in a shader backend if we are ever going to implement pixel shaders using atifs. We also have to make nvts/nvrc a shader backend if we ever implement pixel shaders using that extension.
I'm not convinced we should lump that together in the shader backend just because it uses the same GL extension, but I guess we've been through that discussion. Fact is that we don't currently implement pixel shaders using atifs, and nobody knows how long it may take before we will, if ever.
As for the worse fragment processing on newer ATI cards: We do not want to use ATIFS on those cards anyway, because ATIFS has a hardcoded limitation of 6 simultanous textures, whereas radeon 9500+ cards support 8 textures with regular OpenGL fixed function processing. Also fglrx supports GL_ATI_envmap_bumpmap on those cards, so we have bump mapping without ATIFS as well(Still missing stuff like specular color input and rare blending parameters though). For newer ATI cards we need an ARBFP / GLSL pipeline replacement, no matter what we do with ATIFS.
I think in practice the specular color input is probably more important than the two extra textures. It's pretty rare for programs to hit the nvrc limit of 4, and if they do they don't usually break as bad as when they try to use the missing specular input.
First of all: We really want that patch to go in, Wine is currently badly broken on fglrx.
Yes it's broken, but I'm not convinced this is the right way to fix it. (I'd be fine with committing this just to fix the breakage, but I do think this is simply more evidence that the atifs stuff is in the wrong place.) Ultimately I'm just giving my opinion of course, it's up to you and Alexandre to decide what to do with that.