[Mesa-dev] [PATCH] glx: Initialize OpenGL version to 1.0

[Rico Schüller]

On 03.09.2013 01:54, Ian Romanick wrote:
> Please send patches only using git-send-email.  Send patches as
> attachments prevents people from being able to provide in-line review
> comments.
Ok, sorry for the trouble.

>
> On 09/01/2013 12:30 PM, Rico Schüller wrote:
>>
>> Some driver/card combinations (r200/RV280, i915/915G) don't support
>> OpenGL 2.1. These create in some corner cases an indirect context
>
> This was a typo on my part.  The Linux ABI requires OpenGL 1.2, so every
> driver will support that.  I meant to type
>
>     uint32_t major_ver = 1;
>     uint32_t minor_ver = 2;
>
> but instead typed
>
>     uint32_t minor_ver = 1;
>     uint32_t major_ver = 2;
>
> Copy and paste did the rest. :(

So yes, we agree here, the version number needs to be fixed. The 
simplest one is to "just change the number". I'm fine with it. I have no 
strong opinion about it. Though I think it should be consistent across 
all initialization occurrences (in dri_common.c/dri2_glx.c/drisw_glx.c).

>
> All of your other changes are, I think, unnecessary code motion.

Yes, that's correct. It just removed the duplicated code and initialized 
all values in one location. As it seems to be unnecessary to initialize 
some variables in e.g. src/glx/dri2_glx.c:dri2_create_context_attribs 
only for the case when num_attribs is 0 in 
src/glx/dri_common.c:dri2_convert_glx_attribs. Hell, in all other cases 
they are overwritten (in dri_common.c) later anyway. So why bother to 
initialize it in dri2_glx.c/drisw_glx.c at all? It probably should be 
put into separate patch (if at all)...

>
> Does making that one change in dri2_glx.c and drisw_glx.c fix the problem?

So imho, the only change really needed to fix this, is setting the 
version to the correct value (minor_ver = 2(or 0), major_ver = 1). The 
other changes are just cosmetic. I tested this with the environment 
variable e.g. MESA_GL_VERSION_OVERRIDE=1.3 (and the attachment 
http://bugs.winehq.org/attachment.cgi?id=45801 from wine bug 34238). The 
tests with real hardware weren't run, yet (as I don't have such hardware).

Cheers
Rico

>
>> instead of a direct context when calling glXCreateContextAttribsARB().
>> This happens because of a bad default value. To avoid this, choose a
>> more sane default OpenGL 1.0 as mentioned in the ARB_create_context spec:
>>      The default values for GLX_CONTEXT_MAJOR_VERSION_ARB and
>>      GLX_CONTEXT_MINOR_VERSION_ARB are 1 and 0 respectively. In this
>>      case, implementations will typically return the most recent version
>>      of OpenGL they support which is backwards compatible with OpenGL 1.0
>>      (e.g. 3.0, 3.1 + GL_ARB_compatibility, or 3.2 compatibility profile)
>>
>> This fixes: http://bugs.winehq.org/show_bug.cgi?id=34238
>>
>> Signed-off-by: Rico Schüller <kgbricola at web.de>
>> ---
>>   src/glx/dri2_glx.c   | 10 +++++-----
>>   src/glx/dri_common.c | 14 +++++++-------
>>   src/glx/drisw_glx.c  | 10 +++++-----
>>   3 files changed, 17 insertions(+), 17 deletions(-)
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>