On Mon, 2018-12-03 at 17:24 +0000, Emil Velikov wrote:
> Hi Erik,
>
> On Fri, 23 Nov 2018 at 10:54, Erik Faye-Lund
> <erik.faye-l...@collabora.com> wrote:
> > This makes the logic a little bit easier to follow; this is
> > *either*
> > about ES2 compatibility *or* about gles. GL_RGB565 was added
> > already in
> > OpenGL ES 1.0.
> >
> AFAICT GL_RGB565 was introduced in OpenGLES1 with
> GL_OES_framebuffer_object.
>
> Every driver supports that extension, so
> _mesa_has_OES_framebuffer_object is an overkill.
> Esp. since src/mesa/main/fbobject.c doesn't do it - although it that
> one could use _mesa_has_ARB_ES2_compatibility().
>
> fbobject.c could be tweaked another day.
>
Hmmpf. I already pushed the series, but you're right. Seems I confused
GL_RGB565 and GL_RGB + GL_UNSIGNED_SHORT_5_6_5, of which the latter was
there from GLES 1.0.
I supposed you're right, though; this won't misbehave on any current
drivers. But it would be nice to clean it up somehow. Perhaps I should
add a comment as a follow-up, something like this?
---8<---
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 7506c238232..79d2a9739d7 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -2313,6 +2313,8 @@ _mesa_base_tex_format(const struct gl_context
*ctx, GLint internalFormat)
}
if (_mesa_has_ARB_ES2_compatibility(ctx) || _mesa_is_gles(ctx)) {
+ /* GL_RGB565 requires OES_framebuffer_object on GLES 1.x, but
all drivers
+ * support that extension, so let's drop the complication */
switch (internalFormat) {
case GL_RGB565:
return GL_RGB;
---8<---
Otherwise, I also don't think it's *too* bad to do this, just for
clarity:
---8<---
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 7506c238232..ea73068d025 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -2312,7 +2312,9 @@ _mesa_base_tex_format(const struct gl_context
*ctx, GLint internalFormat)
}
}
- if (_mesa_has_ARB_ES2_compatibility(ctx) || _mesa_is_gles(ctx)) {
+ if (_mesa_has_ARB_ES2_compatibility(ctx) ||
+ _mesa_has_OES_framebuffer_object(ctx) ||
+ ctx->API == API_OPENGLES2) {
switch (internalFormat) {
case GL_RGB565:
return GL_RGB;
---8<---
..but yeah, I suppose we need some more guards in fbobject.c as well.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
