On 11/02/2013 08:41 AM, Chad Versace wrote:
> This clamps GL_MAX_SAMPLES to the given value. If negative, then no
> clamping occurs.
>
> CC: Paul Berry <stereotype...@gmail.com>
> CC: Eric Anholt <e...@anholt.net>
> Signed-off-by: Chad Versace <chad.vers...@linux.intel.com>
> ---
>
>
> This patch lives on my branch driconf-clamp-max_samples.
>
>
> src/mesa/drivers/dri/i965/brw_context.c | 41
> ++++++++++++++++++++++++--------
> src/mesa/drivers/dri/i965/intel_screen.c | 8 ++++++-
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 38147e9..32faf72 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -273,6 +273,10 @@ brw_initialize_context_constants(struct brw_context *brw)
> {
> struct gl_context *ctx = &brw->ctx;
>
> + int max_samples;
> + const int *legal_max_samples;
> + int clamp_max_samples;
> +
I'd prefer to see these declared at their use (okay in C99), but it's up to you.
I also think "supported_msaa_modes" might be nicer than "legal_max_samples."
Again, up to you though.
> ctx->Const.QueryCounterBits.Timestamp = 36;
>
> ctx->Const.StripTextureBorder = true;
> @@ -333,19 +337,36 @@ brw_initialize_context_constants(struct brw_context
> *brw)
>
> ctx->Const.AlwaysUseGetTransformFeedbackVertexCount = true;
>
> - if (brw->gen == 6) {
> - ctx->Const.MaxSamples = 4;
> - ctx->Const.MaxColorTextureSamples = 4;
> - ctx->Const.MaxDepthTextureSamples = 4;
> - ctx->Const.MaxIntegerSamples = 4;
> - } else if (brw->gen >= 7) {
> - ctx->Const.MaxSamples = 8;
> - ctx->Const.MaxColorTextureSamples = 8;
> - ctx->Const.MaxDepthTextureSamples = 8;
> - ctx->Const.MaxIntegerSamples = 8;
> + if (brw->gen >= 7)
> ctx->Const.MaxProgramTextureGatherComponents = 4;
> +
Would you mind adding this:
if (brw->gen >= 8) {
legal_max_samples = (int[]){16, 8, 4, 2, 0};
} else if (brw->gen >= 7) {
> + if (brw->gen >= 7) {
> + legal_max_samples = (int[]){8, 4, 0};
> + } else if (brw->gen == 6) {
> + legal_max_samples = (int[]){4, 0};
> + } else {
> + legal_max_samples = (int[]){0};
> + }
Also, hey, you used one of my favorite C99 features! Nice :)
> +
> + clamp_max_samples = driQueryOptioni(&brw->optionCache,
> "clamp_max_samples");
> + if (clamp_max_samples < 0) {
> + clamp_max_samples = INT_MAX;
> + }
> +
> + /* Set max_samples = min(max(legal_max_samples), clamp_max_samples). */
It doesn't actually - if I specify clamp_max_samples=3, I won't get max_samples
== 3 :)
> + max_samples = 0;
> + for (int i = 0; legal_max_samples[i] != 0; ++i) {
> + if (legal_max_samples[i] <= clamp_max_samples) {
> + max_samples = legal_max_samples[i];
> + break;
> + }
> }
This might be clearer as:
int max_samples = legal_max_samples[0];
int clamp_max_samples = driQueryOptioni(&brw->optionCache,
"clamp_max_samples");
if (clamp_max_samples > 0) {
/* Select the largest supported MSAA mode at or below clamp_max_samples.
*/
for (int i = 0; legal_max_samples[i] != 0; ++i) {
if (legal_max_samples[i] <= clamp_max_samples) {
max_samples = legal_max_samples[i];
break;
}
}
}
Since the true maximum is always the first element, you can just set it to that,
and override it to a smaller value when clamped.
That said, there's another larger problem with this patch:
$ clamp_max_samples=2 glxinfo |& grep 'OpenGL version'
OpenGL version string: 2.1 Mesa 10.0.0-devel (git-1733459)
I would prefer to see it not affect the version advertised. This probably means
overriding these values later.
>
> + ctx->Const.MaxSamples = max_samples;
> + ctx->Const.MaxColorTextureSamples = max_samples;
> + ctx->Const.MaxDepthTextureSamples = max_samples;
> + ctx->Const.MaxIntegerSamples = max_samples;
> +
> ctx->Const.MinLineWidth = 1.0;
> ctx->Const.MinLineWidthAA = 1.0;
> ctx->Const.MaxLineWidth = 5.0;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index eafafa2..ce8124b 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -63,11 +63,17 @@ 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_SECTION_END
> +
> DRI_CONF_SECTION_QUALITY
> DRI_CONF_FORCE_S3TC_ENABLE("false")
> +
> + DRI_CONF_OPT_BEGIN(clamp_max_samples, int, -1)
> + DRI_CONF_DESC(en, "Clamp the value of GL_MAX_SAMPLES to the "
> + "given integer. If negative, then do not clamp.")
> + DRI_CONF_OPT_END
> DRI_CONF_SECTION_END
> +
> DRI_CONF_SECTION_DEBUG
> DRI_CONF_NO_RAST("false")
> DRI_CONF_ALWAYS_FLUSH_BATCH("false")
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
