[Mesa-dev] [PATCH 2/2] mesa: don't advertise ARB_texture_buffer_object in legacy contexts
Ian Romanick
idr at freedesktop.org
Mon Dec 10 15:43:01 PST 2012
On 12/10/2012 02:21 PM, Brian Paul wrote:
> On 12/10/2012 12:32 PM, Marek Olšák wrote:
>> On Mon, Dec 10, 2012 at 5:25 PM, Brian Paul<brianp at vmware.com> wrote:
>>> On 12/08/2012 03:02 PM, Marek Olšák wrote:
>>>>
>>>> ---
>>>> src/mesa/drivers/dri/intel/intel_extensions.c | 5 +----
>>>> src/mesa/main/bufferobj.c | 4 ++--
>>>> src/mesa/main/extensions.c | 2 +-
>>>> src/mesa/main/get.c | 9 ++++++++-
>>>> src/mesa/main/teximage.c | 15 +++++++--------
>>>> src/mesa/main/texobj.c | 6 +++---
>>>> src/mesa/main/texparam.c | 2 +-
>>>> 7 files changed, 23 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/intel/intel_extensions.c
>>>> b/src/mesa/drivers/dri/intel/intel_extensions.c
>>>> index df886a5..7b2b595 100755
>>>> --- a/src/mesa/drivers/dri/intel/intel_extensions.c
>>>> +++ b/src/mesa/drivers/dri/intel/intel_extensions.c
>>>> @@ -102,10 +102,7 @@ intelInitExtensions(struct gl_context *ctx)
>>>> ctx->Extensions.ARB_blend_func_extended =
>>>> !driQueryOptionb(&intel->optionCache, "disable_blend_func_extended");
>>>> ctx->Extensions.ARB_draw_buffers_blend = true;
>>>> ctx->Extensions.ARB_uniform_buffer_object = true;
>>>> -
>>>> - if (ctx->API == API_OPENGL_CORE) {
>>>> - ctx->Extensions.ARB_texture_buffer_object = true;
>>>> - }
>>>> + ctx->Extensions.ARB_texture_buffer_object = true;
>>>> }
>>>>
>>>> if (intel->gen>= 5)
>>>> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>>>> index 6733644..4a84430 100644
>>>> --- a/src/mesa/main/bufferobj.c
>>>> +++ b/src/mesa/main/bufferobj.c
>>>> @@ -93,8 +93,8 @@ get_buffer_target(struct gl_context *ctx, GLenum
>>>> target)
>>>> }
>>>> break;
>>>> case GL_TEXTURE_BUFFER:
>>>> - if (_mesa_is_desktop_gl(ctx)
>>>> -&& ctx->Extensions.ARB_texture_buffer_object) {
>>>>
>>>> + if (ctx->API == API_OPENGL_CORE&&
>>>> + ctx->Extensions.ARB_texture_buffer_object) {
>>>> return&ctx->Texture.BufferObject;
>>>>
>>>> }
>>>> break;
>>>> diff --git a/src/mesa/main/extensions.c b/src/mesa/main/extensions.c
>>>> index 11cbea2..e86e436 100644
>>>> --- a/src/mesa/main/extensions.c
>>>> +++ b/src/mesa/main/extensions.c
>>>> @@ -126,7 +126,7 @@ static const struct extension extension_table[] = {
>>>> { "GL_ARB_shadow", o(ARB_shadow),
>>>> GLL, 2001 },
>>>> { "GL_ARB_sync", o(ARB_sync),
>>>> GL, 2003 },
>>>> { "GL_ARB_texture_border_clamp",
>>>> o(ARB_texture_border_clamp), GLL, 2000 },
>>>> - { "GL_ARB_texture_buffer_object",
>>>> o(ARB_texture_buffer_object), GL, 2008 },
>>>> + { "GL_ARB_texture_buffer_object",
>>>> o(ARB_texture_buffer_object), GLC, 2008 },
>>>> { "GL_ARB_texture_compression", o(dummy_true),
>>>> GLL, 2000 },
>>>> { "GL_ARB_texture_compression_rgtc",
>>>> o(ARB_texture_compression_rgtc), GL, 2004 },
>>>> { "GL_ARB_texture_cube_map",
>>>> o(ARB_texture_cube_map), GLL, 1999 },
>>>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
>>>> index 97dccd0..f3dbda2 100644
>>>> --- a/src/mesa/main/get.c
>>>> +++ b/src/mesa/main/get.c
>>>> @@ -129,6 +129,7 @@ enum value_extra {
>>>> EXTRA_VERSION_31,
>>>> EXTRA_VERSION_32,
>>>> EXTRA_API_GL,
>>>> + EXTRA_API_GL_CORE,
>>>> EXTRA_API_ES2,
>>>> EXTRA_NEW_BUFFERS,
>>>> EXTRA_NEW_FRAG_CLAMP,
>>>> @@ -283,6 +284,7 @@ static const int extra_GLSL_130[] = {
>>>> };
>>>>
>>>> static const int extra_texture_buffer_object[] = {
>>>> + EXTRA_API_GL_CORE,
>>>> EXTRA_VERSION_31,
>>>> EXT(ARB_texture_buffer_object),
>>>> EXTRA_END
>>>> @@ -329,7 +331,6 @@ EXTRA_EXT2(ARB_vertex_program,
>>>> ARB_fragment_program);
>>>> EXTRA_EXT(ARB_geometry_shader4);
>>>> EXTRA_EXT(ARB_color_buffer_float);
>>>> EXTRA_EXT(EXT_framebuffer_sRGB);
>>>> -EXTRA_EXT(ARB_texture_buffer_object);
>>>> EXTRA_EXT(OES_EGL_image_external);
>>>> EXTRA_EXT(ARB_blend_func_extended);
>>>> EXTRA_EXT(ARB_uniform_buffer_object);
>>>> @@ -879,6 +880,12 @@ check_extra(struct gl_context *ctx, const char
>>>> *func,
>>>> const struct value_desc *d
>>>> enabled++;
>>>> }
>>>> break;
>>>> + case EXTRA_API_GL_CORE:
>>>> + if (ctx->API == API_OPENGL_CORE) {
>>>> + total++;
>>>> + enabled++;
>>>> + }
>>>> + break;
>>>> case EXTRA_NEW_BUFFERS:
>>>> if (ctx->NewState& _NEW_BUFFERS)
>>>>
>>>> _mesa_update_state(ctx);
>>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>>> index 83b7e14..47c4ed2 100644
>>>> --- a/src/mesa/main/teximage.c
>>>> +++ b/src/mesa/main/teximage.c
>>>> @@ -791,9 +791,9 @@ _mesa_select_tex_object(struct gl_context *ctx,
>>>> case GL_PROXY_TEXTURE_2D_ARRAY_EXT:
>>>> return arrayTex ?
>>>> ctx->Texture.ProxyTex[TEXTURE_2D_ARRAY_INDEX]
>>>> : NULL;
>>>> case GL_TEXTURE_BUFFER:
>>>> - return _mesa_is_desktop_gl(ctx)
>>>> -&& ctx->Extensions.ARB_texture_buffer_object
>>>>
>>>> - ? texUnit->CurrentTex[TEXTURE_BUFFER_INDEX] : NULL;
>>>> + return ctx->API == API_OPENGL_CORE&&
>>>> + ctx->Extensions.ARB_texture_buffer_object ?
>>>> + texUnit->CurrentTex[TEXTURE_BUFFER_INDEX] : NULL;
>>>> case GL_TEXTURE_EXTERNAL_OES:
>>>> return ctx->Extensions.OES_EGL_image_external
>>>> ? texUnit->CurrentTex[TEXTURE_EXTERNAL_INDEX] : NULL;
>>>> @@ -994,9 +994,8 @@ _mesa_max_texture_levels(struct gl_context *ctx,
>>>> GLenum target)
>>>> return ctx->Extensions.ARB_texture_cube_map_array
>>>> ? ctx->Const.MaxCubeTextureLevels : 0;
>>>> case GL_TEXTURE_BUFFER:
>>>> - return _mesa_is_desktop_gl(ctx)
>>>> -&& ctx->Extensions.ARB_texture_buffer_object
>>>>
>>>> - ? 1 : 0;
>>>> + return ctx->API == API_OPENGL_CORE&&
>>>> + ctx->Extensions.ARB_texture_buffer_object ? 1 : 0;
>>>> case GL_TEXTURE_EXTERNAL_OES:
>>>> /* fall-through */
>>>> default:
>>>> @@ -3895,8 +3894,8 @@ _mesa_TexBuffer(GLenum target, GLenum
>>>> internalFormat, GLuint buffer)
>>>> GET_CURRENT_CONTEXT(ctx);
>>>> ASSERT_OUTSIDE_BEGIN_END_AND_FLUSH(ctx);
>>>>
>>>> - if (!(ctx->Extensions.ARB_texture_buffer_object
>>>> -&& _mesa_is_desktop_gl(ctx))) {
>>>>
>>>> + if (!(ctx->API == API_OPENGL_CORE&&
>>>> + ctx->Extensions.ARB_texture_buffer_object)) {
>>>> _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBuffer");
>>>> return;
>>>> }
>>>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
>>>> index d650c75..cb7ac19 100644
>>>> --- a/src/mesa/main/texobj.c
>>>> +++ b/src/mesa/main/texobj.c
>>>> @@ -1148,9 +1148,9 @@ target_enum_to_index(struct gl_context *ctx,
>>>> GLenum
>>>> target)
>>>> || _mesa_is_gles3(ctx)
>>>> ? TEXTURE_2D_ARRAY_INDEX : -1;
>>>> case GL_TEXTURE_BUFFER_ARB:
>>>> - return _mesa_is_desktop_gl(ctx)
>>>> -&& ctx->Extensions.ARB_texture_buffer_object
>>>>
>>>> - ? TEXTURE_BUFFER_INDEX : -1;
>>>> + return ctx->API == API_OPENGL_CORE&&
>>>> + ctx->Extensions.ARB_texture_buffer_object ?
>>>> + TEXTURE_BUFFER_INDEX : -1;
>>>> case GL_TEXTURE_EXTERNAL_OES:
>>>> return _mesa_is_gles(ctx)&&
>>>> ctx->Extensions.OES_EGL_image_external
>>>>
>>>> ? TEXTURE_EXTERNAL_INDEX : -1;
>>>> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
>>>> index c2d161f..ca5a21f 100644
>>>> --- a/src/mesa/main/texparam.c
>>>> +++ b/src/mesa/main/texparam.c
>>>> @@ -976,7 +976,7 @@ legal_get_tex_level_parameter_target(struct
>>>> gl_context
>>>> *ctx, GLenum target)
>>>> * From the OpenGL 3.1 spec:
>>>> * "target may also be TEXTURE_BUFFER, indicating the texture
>>>> buffer."
>>>> */
>>>> - return _mesa_is_desktop_gl(ctx)&& ctx->Version>= 31;
>>>> + return ctx->API == API_OPENGL_CORE&& ctx->Version>= 31;
>>>> default:
>>>> return GL_FALSE;
>>>> }
>>>
>>>
>>>
>>> I guess I don't quite get this. Why not just make sure
>>> ctx->Extensions.ARB_texture_buffer_object is off when ctx->API is not
>>> API_OPENGL_CORE?
>>>
>>> AFAICT, ARB_texture_buffer_object was only getting turned on by the
>>> intel
>>> driver when we were creating a core context anyway.
>>>
>>> There are other examples of this sort of thing where we're checking "if
>>> extension is enabled AND API=something" where it seems to me that we
>>> should
>>> only enable the extension depending on the API.
>>
>> The idea is that drivers only expose the features they implement and
>> core Mesa determines which of them should be exposed to the user.
>> That's how it works between GL and GLES at least. I thought the core
>> profile should have been treated similarly, but what do I know.
>> Extensions have always been enabled based on the API - the APIs for
>> each extension are listed in extensions.c.
>
> Ah right. I think I brought this up before-- in extensions.c the
> extension_table[] encodes which extensions are supported by which API.
> We could loop over the extensions and turn off the ones which aren't
> supported by a given API during context setup.
>
> I think someone said there was some problem with that but I'd have to go
> dig through email to refresh my memory.
The problem is that Mesa uses some functionality internally regardless
of the API.
>> I think Intel added the API check into their driver because there were
>> hw-specific limitations not allowing them to expose TBOs for the
>> legacy context, and they didn't want to explicitly disallow some other
>> driver to implement GL3.1 with ARB_compatibility.
>>
>> Anyway, it's possible I won't push this series at all. It depends on
>> how hard it would be to implement GL3.1 with ARB_compatibility.
>
> OK.
I'm very strongly opposed to ARB_compatibility or compatibility profiles
being exposed in Mesa at all. It's a disaster, and I don't believe that
anyone intends to create an adequate batter of tests cases to verify
that its correct. Any code that isn't tested is broken. Broken code
doesn't help anyone. This 20 year old crap needs to just die.
NVIDIA supports it because they want to support CAD apps with huge code
bases that nobody understands (or wants to touch). We have no such
requirement, so we have no reason to burden our codebase with support
for these legacy features. We should work to make our codebase less
complex, not more complex.
More information about the mesa-dev
mailing list