On 15 February 2018 at 09:51, Eric Engestrom <eric.engest...@imgtec.com> wrote:
> On Wednesday, 2018-02-14 09:06:41 -0500, Ilia Mirkin wrote:
>> On Feb 14, 2018 7:38 AM, "Eric Engestrom" <eric.engest...@imgtec.com> wrote:
>>
>> From: Brendan King <brendan.k...@imgtec.com>
>>
>> Don't assume the screen supports OpenGL when creating a new context,
>> use an API that the screen supports.
>>
>> Signed-off-by: Brendan King <brendan.k...@imgtec.com>
>> Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com>
>> [Eric: rebased on current git HEAD]
>> Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
>> ---
>> src/mesa/drivers/dri/common/dri_util.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/common/dri_util.c
>> b/src/mesa/drivers/dri/common/dri_util.c
>> index e6a7d2391a78c45d45a1..3f32b34132e75228e0e0 100644
>> --- a/src/mesa/drivers/dri/common/dri_util.c
>> +++ b/src/mesa/drivers/dri/common/dri_util.c
>> @@ -49,6 +49,7 @@
>> #include "main/debug_output.h"
>> #include "main/errors.h"
>> #include "main/macros.h"
>> +#include "util/bitscan.h"
>>
>> const char __dri2ConfigOptions[] =
>> DRI_CONF_BEGIN
>> @@ -325,7 +326,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
>> mesa_api = API_OPENGLES;
>> break;
>> case __DRI_API_GLES2:
>> + ctx_config.major_version = 2;
>> + mesa_api = API_OPENGLES2;
>> + break;
>> case __DRI_API_GLES3:
>> + ctx_config.major_version = 3;
>>
>>
>> Are you sure about this change? Doesn't seem related, and I'm about 20%
>> sure there was some reason for the current thing.
>
> I take the point that these are two separate bugs, and I'll split the
> change and re-send, but I don't see the "reason for the current thing".
> I'm betting on the 80% :P
>
> Without this `major_version` being set, validate_context_version()
> further down could accept a gles3 context when only gles2 is supported,
> because `major_version` would be 1 for both and never `>2` or `>3`.
>
> This is normally be hidden by the fact an attribute list with
> __DRI_CTX_ATTRIB_MAJOR_VERSION would be passed, but
> driCreateNewContextForAPI() doesn't pass an attribute list, which was in
> turn hidden by driCreateNewContext() always requesting OPENGL, for which
> 1.0 is valid, if you support OpenGL (which a lot of dri drivers do, but
> we don't anymore).
>
Not quite sure about the version thingy, bth. IIRC the spec says that
you can get higher version, as long as it's compatible.
Thus using 2 for GLES2 doesn't sound right. I think this is simply
masking an existing bug coming from the second hunk below.
If we do need something like this, it should be handled right next to
were we handle the desktop GL case.
>>
>> mesa_api = API_OPENGLES2;
>> break;
>> case __DRI_API_OPENGL_CORE:
>> @@ -514,7 +519,14 @@ static __DRIcontext *
>> driCreateNewContext(__DRIscreen *screen, const __DRIconfig *config,
>> __DRIcontext *shared, void *data)
>> {
>> - return driCreateNewContextForAPI(screen, __DRI_API_OPENGL,
>> + int apifs;
>> +
>> + apifs = ffs(screen->api_mask);
>> +
>> + if (!apifs)
>> + return NULL;
>> +
>> + return driCreateNewContextForAPI(screen, apifs - 1,
>> config, shared, data);
This doesn't seem right:
- the __DRI_API_GL* to API_OPENGL* mapping is not linear
- this ends up picking an fairly "arbitrary" API
Instead, I would address the callers to feed the correct API.
From a quick look you're getting hit by either the gbm or the loader one?
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
