[PATCH weston 1/5] gl-renderer: save OpenGL version in renderer context

[Arnaud Vrac]

On Mon, Dec 4, 2017 at 5:31 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Hi Arnaud,
>
> On 29 November 2017 at 14:25, Arnaud Vrac <rawoul at gmail.com> wrote:
>> From: Arnaud Vrac <avrac at freebox.fr>
>>
> Here I'd mention why we care about the version. Esp. since the helper
> itself looks quitie strange.

Hi Emil,

Will do.

>
>> Signed-off-by: Arnaud Vrac <rawoul at gmail.com>
>> ---
>>  libweston/gl-renderer.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
>> index 94d81ef4..0a7db13f 100644
>> --- a/libweston/gl-renderer.c
>> +++ b/libweston/gl-renderer.c
>> @@ -199,6 +199,9 @@ struct gl_renderer {
>>
>>         EGLSurface dummy_surface;
>>
>> +       int gl_major;
>> +       int gl_minor;
> Minor seems unused, document/drop/other?

The minor version might be useful if we ever depend on a GLES 3.1
feature. I don't think it hurts to keep it, I'll document the fields.

>> +
>>         struct wl_array vertices;
>>         struct wl_array vtxcnt;
>>
>> @@ -3572,6 +3575,27 @@ fan_debug_repaint_binding(struct weston_keyboard *keyboard,
>>         weston_compositor_damage_all(compositor);
>>  }
>>
>> +static bool
>> +get_gl_version(int *major, int *minor)
>> +{
>> +       const char *version;
>> +
>> +       version = (const char *) glGetString(GL_VERSION);
>> +       if (version) {
>> +               if (sscanf(version, "%d.%d", major, minor) == 2)
>> +                       return true;
>> +
>> +               if (sscanf(version, "OpenGL ES %d.%d", major, minor) == 2)
>> +                       return true;
>> +       }
>> +
>> +       weston_log("failed to get GL version, default to GLES 2.0\n");
>> +       *major = 2;
>> +       *minor = 0;
>> +
>> +       return false;
> Function returns bool, yet nobody checks that. Normally it's
> considered bad idea to write into user provided memory on error.
> I don't think that parsing a desktop GL string or continuing is a good idea.
>
> In either of those cases we're pretty much stuffed, so might as well
> error out ASAP.

I agree, I'll add a check in the caller and move the warning and
fallback to 2.0 there in case of error.

Thanks,
-Arnaud