Re: [Mesa-dev] [PATCH 5/5] st/mesa: don't expose ARB_color_buffer_float without driver support in GL core

2013-03-29 Thread Marek Olšák 
On Fri, Mar 29, 2013 at 4:21 PM, Brian Paul  wrote:

> On 03/28/2013 03:24 PM, Marek Olšák wrote:
>
>> ---
>>   src/mesa/state_tracker/st_**extensions.c |   11 +++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/src/mesa/state_tracker/st_**extensions.c
>> b/src/mesa/state_tracker/st_**extensions.c
>> index 11db9d3..2d8b9ef 100644
>> --- a/src/mesa/state_tracker/st_**extensions.c
>> +++ b/src/mesa/state_tracker/st_**extensions.c
>> @@ -629,6 +629,7 @@ void st_init_extensions(struct st_context *st)
>> ctx->Const.**PrimitiveRestartInSoftware = GL_TRUE;
>>  }
>>
>> +   /* ARB_color_buffer_float. */
>>  if (screen->get_param(screen, PIPE_CAP_VERTEX_COLOR_**UNCLAMPED)) {
>> ctx->Extensions.ARB_color_**buffer_float = GL_TRUE;
>>
>> @@ -639,6 +640,16 @@ void st_init_extensions(struct st_context *st)
>> if (!screen->get_param(screen, PIPE_CAP_FRAGMENT_COLOR_**CLAMPED))
>> {
>>st->clamp_frag_color_in_shader = 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;
>> +  }
>>  }
>>
>
> I've read that comment and code several times but I still can't quite
> parse it.
>
> If we disable ARB_color_buffer_float, we'll never get version 3.0 (see
> compute_version()) so there's no core profile.
>
> I'm probably missing something, but could you elaborate on or clarify the
> comments somehow?
>

Yes. The last hunk of the 4th patch is:

diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
index 3d4af59..ecca446 100644
--- a/src/mesa/main/version.c
+++ b/src/mesa/main/version.c
@@ -233,7 +233,8 @@ compute_version(struct gl_context *ctx)
const GLboolean ver_3_0 = (ver_2_1 &&
   ctx->Const.GLSLVersion >= 130 &&
   ctx->Const.MaxSamples >= 4 &&
-  ctx->Extensions.ARB_color_buffer_float &&
+  (ctx->API == API_OPENGL_CORE ||
+   ctx->Extensions.ARB_color_buffer_float) &&
   ctx->Extensions.ARB_depth_buffer_float &&
   ctx->Extensions.ARB_half_float_pixel &&
   ctx->Extensions.ARB_half_float_vertex &&

So basically, if the profile is "core", ARB_color_buffer_float is not
required to get GL 3.0 and higher.

The subset of ARB_color_buffer_float which remained part of the core
profile is only the read color clamping, which is always supported by our
common ReadPixels code. The 4th patch makes necessary changes for
GL_CLAMP_READ_COLOR to be accepted without ARB_color_buffer_float in the
core profile.

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

Re: [Mesa-dev] [PATCH 5/5] st/mesa: don't expose ARB_color_buffer_float without driver support in GL core

2013-03-29 Thread Brian Paul 

On 03/28/2013 03:24 PM, Marek Olšák wrote:

---
  src/mesa/state_tracker/st_extensions.c |   11 +++
  1 file changed, 11 insertions(+)

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 11db9d3..2d8b9ef 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -629,6 +629,7 @@ void st_init_extensions(struct st_context *st)
ctx->Const.PrimitiveRestartInSoftware = GL_TRUE;
 }

+   /* ARB_color_buffer_float. */
 if (screen->get_param(screen, PIPE_CAP_VERTEX_COLOR_UNCLAMPED)) {
ctx->Extensions.ARB_color_buffer_float = GL_TRUE;

@@ -639,6 +640,16 @@ void st_init_extensions(struct st_context *st)
if (!screen->get_param(screen, PIPE_CAP_FRAGMENT_COLOR_CLAMPED)) {
   st->clamp_frag_color_in_shader = 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;
+  }
 }


I've read that comment and code several times but I still can't quite 
parse it.


If we disable ARB_color_buffer_float, we'll never get version 3.0 (see 
compute_version()) so there's no core profile.


I'm probably missing something, but could you elaborate on or clarify 
the comments somehow?



Other than the formatting nits, the series looks good.

Reviewed-by: Brian Paul 

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

[Mesa-dev] [PATCH 5/5] st/mesa: don't expose ARB_color_buffer_float without driver support in GL core

2013-03-28 Thread Marek Olšák 
---
 src/mesa/state_tracker/st_extensions.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 11db9d3..2d8b9ef 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -629,6 +629,7 @@ void st_init_extensions(struct st_context *st)
   ctx->Const.PrimitiveRestartInSoftware = GL_TRUE;
}
 
+   /* ARB_color_buffer_float. */
if (screen->get_param(screen, PIPE_CAP_VERTEX_COLOR_UNCLAMPED)) {
   ctx->Extensions.ARB_color_buffer_float = GL_TRUE;
 
@@ -639,6 +640,16 @@ void st_init_extensions(struct st_context *st)
   if (!screen->get_param(screen, PIPE_CAP_FRAGMENT_COLOR_CLAMPED)) {
  st->clamp_frag_color_in_shader = 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;
+  }
}
 
if (screen->fence_finish) {
-- 
1.7.10.4

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