On 10.01.2017 16:21, Ilia Mirkin wrote:
This will just cause shader based workarounds in the affected applications, no? What's the benefit of removing this? Fwiw, Nvidia hw has support for this.
It's only supposed to remove the extension string, nothing else. Can you explain in which scenario you would expect a shader-based workaround?
The benefit of removing this is that it sidesteps a silly spec lawyering issue, which is that:
1. If we advertise an OpenGL core context, we're supposed to _not_ support GL_CLAMP_VERTEX/FRAGMENT_COLOR_ARB in ClampColor.
2. ARB_color_buffer_float on the other hand says that those enums _should_ be supported.
It's not clear how to reconcile these two when both are advertised. The GLCTS expects the enums to be unsupported in core contexts, no matter which extensions are present.
If the NVidia blob advertises ARB_color_buffer_float also in core contexts, it probably just doesn't "fully implement" the extension. We could do the same by changing the check in _mesa_ClampColor instead of this patch
I don't care much either way, but this patch brings us in line with what the Intel driver has been doing since 2013...
Nicolai
On Jan 10, 2017 9:59 AM, "Nicolai Hähnle" <nhaeh...@gmail.com <mailto:nhaeh...@gmail.com>> wrote: From: Nicolai Hähnle <nicolai.haeh...@amd.com <mailto:nicolai.haeh...@amd.com>> Some parts of the extension were explicitly removed for core profiles, and all the remaining functionality has been in core since core profiles exist. So there's no loss of exposed functionality. The corresponding change was applied to i965 in 2013 (commit bd850cb4f2c77e2eb6716c865c40b9976633fc23), so it's not like applications could be surprised by this behavior either. Fixes GL45-CTS.gtf30.GL3Tests.half_float.half_float_textures. --- src/mesa/state_tracker/st_context.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c index 0eae971..a2df704 100644 --- a/src/mesa/state_tracker/st_context.c +++ b/src/mesa/state_tracker/st_context.c @@ -450,32 +450,28 @@ st_create_context_priv( struct gl_context *ctx, struct pipe_context *pipe, /* Enable shader-based fallbacks for ARB_color_buffer_float if needed. */ if (screen->get_param(screen, PIPE_CAP_VERTEX_COLOR_UNCLAMPED)) { if (!screen->get_param(screen, PIPE_CAP_VERTEX_COLOR_CLAMPED)) { st->clamp_vert_color_in_shader = GL_TRUE; } if (!screen->get_param(screen, PIPE_CAP_FRAGMENT_COLOR_CLAMPED)) { st->clamp_frag_color_in_shader = GL_TRUE; } - - /* For drivers which cannot do color clamping, it's better to just - * disable ARB_color_buffer_float in the core profile, because - * the clamping is deprecated there anyway. */ - if (ctx->API == API_OPENGL_CORE && - (st->clamp_frag_color_in_shader || st->clamp_vert_color_in_shader)) { - st->clamp_vert_color_in_shader = GL_FALSE; - st->clamp_frag_color_in_shader = GL_FALSE; - ctx->Extensions.ARB_color_buffer_float = GL_FALSE; - } } + /* Disable ARB_color_buffer_float for core contexts, since some of its + * functionality was explicitly removed. + */ + if (ctx->API == API_OPENGL_CORE) + ctx->Extensions.ARB_color_buffer_float = GL_FALSE; + /* called after _mesa_create_context/_mesa_init_point, fix default user * settable max point size up */ ctx->Point.MaxSize = MAX2(ctx->Const.MaxPointSize, ctx->Const.MaxPointSizeAA); /* For vertex shaders, make sure not to emit saturate when SM 3.0 is not supported */ ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].EmitNoSat = !st->has_shader_model3; if (!ctx->Extensions.ARB_gpu_shader5) { for (i = 0; i < MESA_SHADER_STAGES; i++) -- 2.7.4 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> https://lists.freedesktop.org/mailman/listinfo/mesa-dev <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
