On 27/01/18 01:59, srol...@vmware.com wrote:
> From: Roland Scheidegger <srol...@vmware.com>
>
> Testing for gles there is just confusing - this is about target being
> supported, if it was valid at all was already determined earlier
> (in _legal_parameters). It didn't make sense at all in any case, since
> it would only have said false there for gles for 2d but not 2d arrays etc.

Some historic background: The issue is that in general all the gles
checking were should not be added in the first time.
ARB_internalformat_query2 spec has a lot of "in the case of gles do XXX"
but they were leftovers when the spec was intended for both OpenGL and
OpenGL ES. When we realized this "leftover" thing, the patches were
already on master (this was raised on the thread where we tried to
activate the extension for opengl es).

At that point, we decided to keep the gles checks, just in case in the
future the extension in included on gles too. But now Im starting to
think that it would be better to just clean them.

Having said so, about your patch:

> ---
>  src/mesa/main/formatquery.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
> index 61f798c88f..e0062a64d2 100644
> --- a/src/mesa/main/formatquery.c
> +++ b/src/mesa/main/formatquery.c
> @@ -392,14 +392,12 @@ _is_target_supported(struct gl_context *ctx, GLenum 
> target)
>      *     implementation the "unsupported" answer should be given.
>      *     This is not an error."
>      *
> -    * For OpenGL ES, queries can only be used with GL_RENDERBUFFER or MS.
> +    * Note that legality of targets has already been verified.
>      */
>     switch(target){
>     case GL_TEXTURE_1D:
>     case GL_TEXTURE_2D:
>     case GL_TEXTURE_3D:
> -      if (!_mesa_is_desktop_gl(ctx))
> -         return false;

This seems ok.

>        break;
>  
>     case GL_TEXTURE_1D_ARRAY:
> @@ -702,6 +700,12 @@ _mesa_query_internal_format_default(struct gl_context 
> *ctx, GLenum target,
>     case GL_FRAMEBUFFER_RENDERABLE_LAYERED:
>     case GL_FRAMEBUFFER_BLEND:
>     case GL_FILTER:
> +      /*
> +       * XXX seems a tad optimistic just saying yes to everything here.
> +       * Even for combinations which make no sense...
> +       * And things like TESS_CONTROL_TEXTURE should definitely default to
> +       * NONE if the driver doesn't even support tessellation...
> +       */

This seems as something that would belong to a different patch. Even not
sure if it makes sense to include on the code. Seems more like an entry
on a TODO file for possible improvements.

>        params[0] = GL_FULL_SUPPORT;
>        break;
>     case GL_NUM_TILING_TYPES_EXT:


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to