Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-18 Thread Carl Worth
Kenneth Graunke kenn...@whitecape.org writes:
 This partially fixes a performance regression since commit 33599433c7.
 More work is required to fully fix it in all cases.  This at least helps
 Warsow.

Thanks, Ken.

Reviewed-by: Carl Worth cwo...@cworth.org

-Carl


pgpEBf6hWs_2H.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-16 Thread Kenneth Graunke

On 03/15/2013 04:54 PM, Eric Anholt wrote:

Kenneth Graunke kenn...@whitecape.org writes:


Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
RED, RG, and RGB textures in order to force alpha to 1.0 in case we
actually stored the texture as RGBA.

This had a unforseen performance implication: the shader precompile
assumes that the texture swizzle mode will be XYZW for non-shadow
sampler types.  By setting it to XYZ1, this means every shader used with
a RED, RG, or RGB texture has to be recompiled.  This is a very common
case.

Unfortunately, there's no way to improve the precompile, since RGBA
textures still need XYZW, and there's no way to know by looking at
the shader source what texture formats might be used.

However, we only need to smash alpha to 1.0 if the texture's memory
format actually has alpha bits.  If not, the sampler already returns 1.0
for us without any special swizzling.  XRGB, for example, is a very
common case where this occurs.

This partially fixes a performance regression since commit 33599433c7.
More work is required to fully fix it in all cases.  This at least helps
Warsow.


Now that we have MESA_FORMAT_XBGR16161616_FLOAT and company, we could
potentially make this conditional just die by using those formats.


Absolutely - I believe the real fix is to use XRGB formats when 
sampling, and ARGB formats when rendering.  I'm pretty sure we have 
formats for all of those now.  We should do that.


In the meantime, this patch improves things and is simple and easily 
cherry-pickable...


--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-16 Thread Eric Anholt
Kenneth Graunke kenn...@whitecape.org writes:

 On 03/15/2013 04:54 PM, Eric Anholt wrote:
 Kenneth Graunke kenn...@whitecape.org writes:

 Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
 RED, RG, and RGB textures in order to force alpha to 1.0 in case we
 actually stored the texture as RGBA.

 This had a unforseen performance implication: the shader precompile
 assumes that the texture swizzle mode will be XYZW for non-shadow
 sampler types.  By setting it to XYZ1, this means every shader used with
 a RED, RG, or RGB texture has to be recompiled.  This is a very common
 case.

 Unfortunately, there's no way to improve the precompile, since RGBA
 textures still need XYZW, and there's no way to know by looking at
 the shader source what texture formats might be used.

 However, we only need to smash alpha to 1.0 if the texture's memory
 format actually has alpha bits.  If not, the sampler already returns 1.0
 for us without any special swizzling.  XRGB, for example, is a very
 common case where this occurs.

 This partially fixes a performance regression since commit 33599433c7.
 More work is required to fully fix it in all cases.  This at least helps
 Warsow.

 Now that we have MESA_FORMAT_XBGR16161616_FLOAT and company, we could
 potentially make this conditional just die by using those formats.

 Absolutely - I believe the real fix is to use XRGB formats when 
 sampling, and ARGB formats when rendering.  I'm pretty sure we have 
 formats for all of those now.  We should do that.

 In the meantime, this patch improves things and is simple and easily 
 cherry-pickable...

Fair enough.  Are there some big performance changes?


pgpgynby9VcZq.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-16 Thread Kenneth Graunke

On 03/16/2013 10:09 AM, Eric Anholt wrote:

Kenneth Graunke kenn...@whitecape.org writes:


On 03/15/2013 04:54 PM, Eric Anholt wrote:

Kenneth Graunke kenn...@whitecape.org writes:


Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
RED, RG, and RGB textures in order to force alpha to 1.0 in case we
actually stored the texture as RGBA.

This had a unforseen performance implication: the shader precompile
assumes that the texture swizzle mode will be XYZW for non-shadow
sampler types.  By setting it to XYZ1, this means every shader used with
a RED, RG, or RGB texture has to be recompiled.  This is a very common
case.

Unfortunately, there's no way to improve the precompile, since RGBA
textures still need XYZW, and there's no way to know by looking at
the shader source what texture formats might be used.

However, we only need to smash alpha to 1.0 if the texture's memory
format actually has alpha bits.  If not, the sampler already returns 1.0
for us without any special swizzling.  XRGB, for example, is a very
common case where this occurs.

This partially fixes a performance regression since commit 33599433c7.
More work is required to fully fix it in all cases.  This at least helps
Warsow.


Now that we have MESA_FORMAT_XBGR16161616_FLOAT and company, we could
potentially make this conditional just die by using those formats.


Absolutely - I believe the real fix is to use XRGB formats when
sampling, and ARGB formats when rendering.  I'm pretty sure we have
formats for all of those now.  We should do that.

In the meantime, this patch improves things and is simple and easily
cherry-pickable...


Fair enough.  Are there some big performance changes?


No, not large in terms of FPS.  Generally, when we get this wrong, we 
recompile each shader once and then it's right from then on.  It does 
reduce stuttering quite a bit, though, since it means we don't have to 
recompile on first draw.  I suppose it might affect very short demos.


--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-15 Thread Kenneth Graunke
Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
RED, RG, and RGB textures in order to force alpha to 1.0 in case we
actually stored the texture as RGBA.

This had a unforseen performance implication: the shader precompile
assumes that the texture swizzle mode will be XYZW for non-shadow
sampler types.  By setting it to XYZ1, this means every shader used with
a RED, RG, or RGB texture has to be recompiled.  This is a very common
case.

Unfortunately, there's no way to improve the precompile, since RGBA
textures still need XYZW, and there's no way to know by looking at
the shader source what texture formats might be used.

However, we only need to smash alpha to 1.0 if the texture's memory
format actually has alpha bits.  If not, the sampler already returns 1.0
for us without any special swizzling.  XRGB, for example, is a very
common case where this occurs.

This partially fixes a performance regression since commit 33599433c7.
More work is required to fully fix it in all cases.  This at least helps
Warsow.

NOTE: This is a candidate for the 9.1 branch.

Cc: Carl Worth cwo...@cworth.org
Cc: Eric Anholt e...@anholt.net
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 0cb4b2d..771655d 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -773,7 +773,8 @@ brw_get_texture_swizzle(const struct gl_context *ctx,
case GL_RED:
case GL_RG:
case GL_RGB:
-  swizzles[3] = SWIZZLE_ONE;
+  if (_mesa_get_format_bits(img-TexFormat, GL_ALPHA_BITS)  0)
+ swizzles[3] = SWIZZLE_ONE;
   break;
}
 
-- 
1.8.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-15 Thread Ian Romanick

On 03/15/2013 03:14 PM, Kenneth Graunke wrote:

Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
RED, RG, and RGB textures in order to force alpha to 1.0 in case we
actually stored the texture as RGBA.

This had a unforseen performance implication: the shader precompile
assumes that the texture swizzle mode will be XYZW for non-shadow
sampler types.  By setting it to XYZ1, this means every shader used with
a RED, RG, or RGB texture has to be recompiled.  This is a very common
case.

Unfortunately, there's no way to improve the precompile, since RGBA
textures still need XYZW, and there's no way to know by looking at
the shader source what texture formats might be used.


I suspect that in a lot of cases the other components of the texture 
aren't used by the shader.  Could we use that information somehow?  We 
ought to be able to track which components of each sampler variable are 
used.



However, we only need to smash alpha to 1.0 if the texture's memory
format actually has alpha bits.  If not, the sampler already returns 1.0
for us without any special swizzling.  XRGB, for example, is a very
common case where this occurs.

This partially fixes a performance regression since commit 33599433c7.
More work is required to fully fix it in all cases.  This at least helps
Warsow.

NOTE: This is a candidate for the 9.1 branch.

Cc: Carl Worth cwo...@cworth.org
Cc: Eric Anholt e...@anholt.net
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 0cb4b2d..771655d 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -773,7 +773,8 @@ brw_get_texture_swizzle(const struct gl_context *ctx,
 case GL_RED:
 case GL_RG:
 case GL_RGB:
-  swizzles[3] = SWIZZLE_ONE;
+  if (_mesa_get_format_bits(img-TexFormat, GL_ALPHA_BITS)  0)
+ swizzles[3] = SWIZZLE_ONE;
break;
 }




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Don't use texture swizzling to force alpha to 1.0 if unnecessary.

2013-03-15 Thread Eric Anholt
Kenneth Graunke kenn...@whitecape.org writes:

 Commit 33599433c7 began setting the texture swizzle mode to XYZ1 for
 RED, RG, and RGB textures in order to force alpha to 1.0 in case we
 actually stored the texture as RGBA.

 This had a unforseen performance implication: the shader precompile
 assumes that the texture swizzle mode will be XYZW for non-shadow
 sampler types.  By setting it to XYZ1, this means every shader used with
 a RED, RG, or RGB texture has to be recompiled.  This is a very common
 case.

 Unfortunately, there's no way to improve the precompile, since RGBA
 textures still need XYZW, and there's no way to know by looking at
 the shader source what texture formats might be used.

 However, we only need to smash alpha to 1.0 if the texture's memory
 format actually has alpha bits.  If not, the sampler already returns 1.0
 for us without any special swizzling.  XRGB, for example, is a very
 common case where this occurs.

 This partially fixes a performance regression since commit 33599433c7.
 More work is required to fully fix it in all cases.  This at least helps
 Warsow.

Now that we have MESA_FORMAT_XBGR16161616_FLOAT and company, we could
potentially make this conditional just die by using those formats.


pgpzWJJvQCiwz.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev