On 31 March 2016 at 19:51, Matteo Bruni mbruni@codeweavers.com wrote:
- shader_addline(buffer, "if (alpha_test)\n");
- if (alpha_func != WINED3D_CMP_NEVER)
shader_addline(buffer, " if (!(%s[0].a %s alpha_ref))\n",get_fragment_output(gl_info), comparison_operator[alpha_func - WINED3D_CMP_NEVER]);- shader_addline(buffer, " discard;\n");
Is there really an advantage to having the "alpha_test" uniform instead of just mapping disabled alpha test to WINED3D_CMP_ALWAYS?
@@ -973,6 +981,7 @@ struct ps_compile_args { WORD texcoords_initialized; /* MAX_TEXTURES, 8 */ BOOL pointsprite; BOOL flatshading;
- enum wined3d_cmp_func alpha_func;
};
enum fog_src_type { @@ -2022,6 +2031,7 @@ struct ffp_frag_settings unsigned char pointsprite : 1; unsigned char flatshading : 1; unsigned char padding : 5;
- enum wined3d_cmp_func alpha_func;
"alpha_func" only needs 4 bits (3 if you just mask the 4th bit), so would still fit in the padding. That applies somewhat to struct ps_compile_args as well, although we may care less there.
2016-03-31 22:58 GMT+02:00 Henri Verbeet hverbeet@gmail.com:
On 31 March 2016 at 19:51, Matteo Bruni mbruni@codeweavers.com wrote:
- shader_addline(buffer, "if (alpha_test)\n");
- if (alpha_func != WINED3D_CMP_NEVER)
shader_addline(buffer, " if (!(%s[0].a %s alpha_ref))\n",get_fragment_output(gl_info), comparison_operator[alpha_func - WINED3D_CMP_NEVER]);- shader_addline(buffer, " discard;\n");
Is there really an advantage to having the "alpha_test" uniform instead of just mapping disabled alpha test to WINED3D_CMP_ALWAYS?
The advantage is to avoid creating two shader variants for alpha test enabled vs disabled and flipping between the two when the WINED3D_RS_ALPHATESTENABLE state is toggled. I only have some limited empiric evidence but it seems like changing the alpha test function is very rare in practice while toggling the alpha test enabled / disabled is relatively common.
@@ -973,6 +981,7 @@ struct ps_compile_args { WORD texcoords_initialized; /* MAX_TEXTURES, 8 */ BOOL pointsprite; BOOL flatshading;
- enum wined3d_cmp_func alpha_func;
};
enum fog_src_type { @@ -2022,6 +2031,7 @@ struct ffp_frag_settings unsigned char pointsprite : 1; unsigned char flatshading : 1; unsigned char padding : 5;
- enum wined3d_cmp_func alpha_func;
"alpha_func" only needs 4 bits (3 if you just mask the 4th bit), so would still fit in the padding. That applies somewhat to struct ps_compile_args as well, although we may care less there.
Yes, it can be compressed. I'll have a look.
On 1 April 2016 at 01:48, Matteo Bruni matteo.mystral@gmail.com wrote:
The advantage is to avoid creating two shader variants for alpha test enabled vs disabled and flipping between the two when the WINED3D_RS_ALPHATESTENABLE state is toggled. I only have some limited empiric evidence but it seems like changing the alpha test function is very rare in practice while toggling the alpha test enabled / disabled is relatively common.
Does that happen with a lot of different shaders though? I.e., if the draws with enabled alpha test use different shaders than the ones with disabled alpha test it doesn't really matter. Perhaps more importantly though, are you sure having discard() in the shader doesn't end up disabling HiZ/HiS in some cases?
2016-04-01 12:01 GMT+02:00 Henri Verbeet hverbeet@gmail.com:
On 1 April 2016 at 01:48, Matteo Bruni matteo.mystral@gmail.com wrote:
The advantage is to avoid creating two shader variants for alpha test enabled vs disabled and flipping between the two when the WINED3D_RS_ALPHATESTENABLE state is toggled. I only have some limited empiric evidence but it seems like changing the alpha test function is very rare in practice while toggling the alpha test enabled / disabled is relatively common.
Does that happen with a lot of different shaders though? I.e., if the draws with enabled alpha test use different shaders than the ones with disabled alpha test it doesn't really matter. Perhaps more importantly though, are you sure having discard() in the shader doesn't end up disabling HiZ/HiS in some cases?
Right, discard() might disable HiZ or other depth / stencil optimizations and that seems like a good enough reason on its own for mapping disabled alpha test to WINED3D_CMP_ALWAYS.
WRT alpha test usage patterns, I tested a few games I had around. The Witcher was one of the games I used when I wrote the first version of this patch and, as I recalled, it often toggles ALPHATESTENABLED between individual draws. But actually it also makes a lot of state changes on average (many of them redundant like flipping ALPHATESTENABLE multiple times between 2 draws...) and as a result the selected GLSL program changes very often. Bottom line is, we might avoid a handful of shader switches but very little in the big picture. None of the other games I tried have shown much in avoided shader switching with the current patch either (with the Blizzard launcher really being the only application triggering it in a noticeable amount). I guess dropping the alpha_test uniform it is.
-
Henri Verbeet -
Matteo Bruni