[Mesa-dev] [PATCH-RFC] i965: do not advertise MESA_FORMAT_Z_UNORM16 support

[Kenneth Graunke]

On 02/18/2014 09:48 PM, Chia-I Wu wrote:
> Since 73bc6061f5c3b6a3bb7a8114bb2e1ab77d23cfdb, Z16 support is not advertised
> for OpenGL ES contexts due to the terrible performance.  It is still enabled
> for desktop GL because it was believed GL 3.0+ requires Z16.
> 
> It turns out only GL 3.0 requires Z16, and that is corrected in later GL
> versions.  In light of that, and per Ian's suggestion, stop advertising Z16
> support by default, and add a drirc option, gl30_sized_format_rules, so that
> users can override.

I actually don't think that GL 3.0 requires Z16, either.

In glspec30.20080923.pdf, page 180, it says:
"[...] memory allocation per texture component is assigned by the GL to
match the allocations listed in tables 3.16-3.18 as closely as possible.
[...]

Required Texture Formats
[...]
In addition, implementations are required to support the following sized
internal formats.  Requesting one of these internal formats for any
texture type will allocate exactly the internal component sizes and
types shown for that format in tables 3.16-3.17:"

Notably, however, GL_DEPTH_COMPONENT16 does /not/ appear in table 3.16
or table 3.17.  It appears in table 3.18, where the "exact" rule doesn't
apply, and thus we fall back to the "closely as possible" rule.

The confusing part is that the ordering of the tables in the PDF is:

Table 3.16 (pages 182-184)
Table 3.18 (bottom of page 184 to top of 185)
Table 3.17 (page 185)

I'm guessing that people saw table 3.16, then saw the one after with
DEPTH_COMPONENT* formats, and assumed it was 3.17.  But it's not.

I think we should just drop Z16 support entirely, and I think we should
remove the requirement from the Piglit test.

> This regresses required-sized-texture-formats on GL 3.0.
> 
> Signed-off-by: Chia-I Wu <olv at lunarg.com>
> Cc: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c         | 3 +++
>  src/mesa/drivers/dri/i965/brw_context.h         | 1 +
>  src/mesa/drivers/dri/i965/brw_surface_formats.c | 7 ++++---
>  src/mesa/drivers/dri/i965/intel_screen.c        | 4 ++++
>  4 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index ffbdb94..8ecf80b 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -553,6 +553,9 @@ brw_process_driconf_options(struct brw_context *brw)
>     brw->disable_derivative_optimization =
>        driQueryOptionb(&brw->optionCache, "disable_derivative_optimization");
>  
> +   brw->enable_z16 =
> +      driQueryOptionb(&brw->optionCache, "gl30_sized_format_rules");
> +
>     brw->precompile = driQueryOptionb(&brw->optionCache, "shader_precompile");
>  
>     ctx->Const.ForceGLSLExtensionsWarn =
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index 98e90e2..fd10884 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1093,6 +1093,7 @@ struct brw_context
>     bool disable_throttling;
>     bool precompile;
>     bool disable_derivative_optimization;
> +   bool enable_z16;
>  
>     driOptionCache optionCache;
>     /** @} */
> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> index 6a7e00a..1d5f044 100644
> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
> @@ -623,10 +623,11 @@ brw_init_surface_formats(struct brw_context *brw)
>      * increased depth stalls from a cacheline-based heuristic for detecting
>      * depth stalls.
>      *
> -    * However, desktop GL 3.0+ require that you get exactly 16 bits when
> -    * asking for DEPTH_COMPONENT16, so we have to respect that.
> +    * However, desktop GL 3.0, and no other version, requires that you get
> +    * exactly 16 bits when asking for DEPTH_COMPONENT16, so we have an drirc
> +    * option to decide whether to respect that or not.
>      */
> -   if (_mesa_is_desktop_gl(ctx))
> +   if (brw->enable_z16)
>        ctx->TextureFormatSupported[MESA_FORMAT_Z_UNORM16] = true;
>  
>     /* On hardware that lacks support for ETC1, we map ETC1 to RGBX
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index ba22971..087fc3c 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -64,6 +64,10 @@ DRI_CONF_BEGIN
>        DRI_CONF_OPT_BEGIN_B(disable_derivative_optimization, "false")
>  	 DRI_CONF_DESC(en, "Derivatives with finer granularity by default")
>        DRI_CONF_OPT_END
> +
> +      DRI_CONF_OPT_BEGIN_B(gl30_sized_format_rules, "false")
> +	 DRI_CONF_DESC(en, "Honor GL 3.0 specific rules for sized formats")
> +      DRI_CONF_OPT_END
>     DRI_CONF_SECTION_END
>  
>     DRI_CONF_SECTION_QUALITY
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140219/96b29389/attachment.pgp>