Discussion:
[PATCH 0/6] i965: Add RGBX, RGBA configs, even on gen9
Add Reply
Chad Versace
2017-06-06 20:36:54 UTC
Reply
Permalink
Raw Message
More patches to break your formats... again ;)

The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.

If you want the meat, read patches 2 and 6.

Chad Versace (6):
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs

src/mesa/Android.gen.mk | 12 ++
src/mesa/Makefile.am | 7 +
src/mesa/Makefile.sources | 2 +
src/mesa/drivers/dri/i965/brw_blorp.c | 10 +-
src/mesa/drivers/dri/i965/brw_context.h | 5 +-
src/mesa/drivers/dri/i965/brw_meta_util.c | 2 +-
src/mesa/drivers/dri/i965/brw_surface_formats.c | 94 +++++++-----
src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 +-
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 10 +-
src/mesa/drivers/dri/i965/intel_fbo.c | 34 ++++-
src/mesa/drivers/dri/i965/intel_fbo.h | 6 +-
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 +-
src/mesa/drivers/dri/i965/intel_screen.c | 39 ++++-
src/mesa/drivers/dri/i965/intel_screen.h | 5 +
src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
src/mesa/drivers/dri/i965/intel_tex_image.c | 19 ++-
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 ++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++
19 files changed, 392 insertions(+), 73 deletions(-)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py
--
2.13.0
Chad Versace
2017-06-06 20:36:56 UTC
Reply
Permalink
Raw Message
This enables support for importing RGBX8888 EGLImage textures on
Skylake.

Chrome OS needs support for RGBX8888 EGLImage textures because because
the Android framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys
surfaces, which the Chrome OS compositor consumes as dma_bufs. On
hardware for which RGBX is unsupported or disabled, normally core Mesa
provides the RGBX->RGBA fallback during glTexStorage. But the DRIimage
code bypasses core Mesa, so we must do the fallback in i965.
---
src/mesa/drivers/dri/i965/intel_tex_image.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
index 649b3907d1..92c6c15c72 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -5,6 +5,7 @@
#include "main/bufferobj.h"
#include "main/context.h"
#include "main/formats.h"
+#include "main/format_fallback.h"
#include "main/glformats.h"
#include "main/image.h"
#include "main/pbo.h"
@@ -254,8 +255,22 @@ create_mt_for_dri_image(struct brw_context *brw,
struct gl_context *ctx = &brw->ctx;
struct intel_mipmap_tree *mt;
uint32_t draw_x, draw_y;
+ mesa_format format = image->format;
+
+ if (!ctx->TextureFormatSupported[format]) {
+ /* The texture storage paths in core Mesa detect if the driver does not
+ * support the user-requested format, and then searches for a
+ * fallback format. The DRIimage code bypasses core Mesa, though. So we
+ * do the fallbacks here for important formats.
+ *
+ * We must support DRM_FOURCC_XBGR8888 textures because the Android
+ * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces, which
+ * the Chrome OS compositor consumes as dma_buf EGLImages.
+ */
+ format = _mesa_format_fallback_rgbx_to_rgba(format);
+ }

- if (!ctx->TextureFormatSupported[image->format])
+ if (!ctx->TextureFormatSupported[format])
return NULL;

/* Disable creation of the texture's aux buffers because the driver exposes
@@ -263,7 +278,7 @@ create_mt_for_dri_image(struct brw_context *brw,
* buffer's content to the main buffer nor for invalidating the aux buffer's
* content.
*/
- mt = intel_miptree_create_for_bo(brw, image->bo, image->format,
+ mt = intel_miptree_create_for_bo(brw, image->bo, format,
0, image->width, image->height, 1,
image->pitch,
MIPTREE_LAYOUT_DISABLE_AUX);
--
2.13.0
Daniel Stone
2017-06-06 21:12:38 UTC
Reply
Permalink
Raw Message
Hi Chad,
Post by Chad Versace
@@ -254,8 +255,22 @@ create_mt_for_dri_image(struct brw_context *brw,
struct gl_context *ctx = &brw->ctx;
struct intel_mipmap_tree *mt;
uint32_t draw_x, draw_y;
+ mesa_format format = image->format;
+
+ if (!ctx->TextureFormatSupported[format]) {
+ /* The texture storage paths in core Mesa detect if the driver does not
+ * support the user-requested format, and then searches for a
+ * fallback format. The DRIimage code bypasses core Mesa, though. So we
+ * do the fallbacks here for important formats.
+ *
+ * We must support DRM_FOURCC_XBGR8888 textures because the Android
+ * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces, which
+ * the Chrome OS compositor consumes as dma_buf EGLImages.
+ */
+ format = _mesa_format_fallback_rgbx_to_rgba(format);
+ }
- if (!ctx->TextureFormatSupported[image->format])
+ if (!ctx->TextureFormatSupported[format])
return NULL;
/* Disable creation of the texture's aux buffers because the driver exposes
@@ -263,7 +278,7 @@ create_mt_for_dri_image(struct brw_context *brw,
* buffer's content to the main buffer nor for invalidating the aux buffer's
* content.
*/
- mt = intel_miptree_create_for_bo(brw, image->bo, image->format,
+ mt = intel_miptree_create_for_bo(brw, image->bo, format,
0, image->width, image->height, 1,
image->pitch,
MIPTREE_LAYOUT_DISABLE_AUX);
I wonder if it wouldn't be better to do this in
intel_create_image_from_name. That way it would be more obvious
up-front what's happening, and also it'd be easy to add the analogue
to intel_create_image_from_fds for the explicit dmabuf (as opposed to
ANativeBuffer; your description of 'from dmabufs' threw me because the
dmabuf fd import path would fail immediately on FourCC lookup) import
path.

As an aside, it's safe to enable in Wayland (IMO anyway), because we
only use the DRM format there; there's no concept of a 'surface
format' or visuals inside the Wayland client EGL platform, just direct
sampling from whichever buffer was last attached. EGL_NATIVE_VISUAL_ID
is empty, because we don't have anything to expose to the client.
Probably famous last words tho.

Cheers,
Daniel
Chad Versace
2017-06-07 23:45:09 UTC
Reply
Permalink
Raw Message
Post by Daniel Stone
Hi Chad,
Post by Chad Versace
@@ -254,8 +255,22 @@ create_mt_for_dri_image(struct brw_context *brw,
struct gl_context *ctx = &brw->ctx;
struct intel_mipmap_tree *mt;
uint32_t draw_x, draw_y;
+ mesa_format format = image->format;
+
+ if (!ctx->TextureFormatSupported[format]) {
+ /* The texture storage paths in core Mesa detect if the driver does not
+ * support the user-requested format, and then searches for a
+ * fallback format. The DRIimage code bypasses core Mesa, though. So we
+ * do the fallbacks here for important formats.
+ *
+ * We must support DRM_FOURCC_XBGR8888 textures because the Android
+ * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces, which
+ * the Chrome OS compositor consumes as dma_buf EGLImages.
+ */
+ format = _mesa_format_fallback_rgbx_to_rgba(format);
+ }
- if (!ctx->TextureFormatSupported[image->format])
+ if (!ctx->TextureFormatSupported[format])
return NULL;
I dislike what I wrote above. There's a much better way to do the
fallback, a way that handles more types of fallback than rgbx->rgba and
that's the same as the fallback used by glTexStorage2D(). The better way
is to re-use the core Mesa code that the comment refers to, like this:

mesa_format format = ctx->Driver.ChooseTextureFormat(ctx, GL_TEXTURE_2D,
internalFormat, GL_NONE, GL_NONE);

As precedent, that's exactly what intel_renderbuffer_format() does.
Post by Daniel Stone
Post by Chad Versace
/* Disable creation of the texture's aux buffers because the driver exposes
@@ -263,7 +278,7 @@ create_mt_for_dri_image(struct brw_context *brw,
* buffer's content to the main buffer nor for invalidating the aux buffer's
* content.
*/
- mt = intel_miptree_create_for_bo(brw, image->bo, image->format,
+ mt = intel_miptree_create_for_bo(brw, image->bo, format,
0, image->width, image->height, 1,
image->pitch,
MIPTREE_LAYOUT_DISABLE_AUX);
I wonder if it wouldn't be better to do this in
intel_create_image_from_name. That way it would be more obvious
up-front what's happening,
I agree that the intent would become more obvious if the format fallback
were done at time of import instead of gl*Storage. But I see two
arguments against it:

1. First, the weaker argument.

The chosen fallback format,
and even the choice to do a fallback at all, is a property of the
image's usage and not a property of the image itself. A single
image can have multiple uses during its lifetime, and the driver
may need a different fallback or no fallback for each. I'm
defining "image usage" here in terms of
glEGLImageTargetTexture2DOES, glEGLImageTargetRenderbufferStorageOES, and
GL_TEXURE_EXTERNAL_OES vs GL_TEXTURE_2D.

Which reminds me... I should have submitted an analgous patch for
glEGLImageTargetRenderbufferStorageOES().

Since the driver may support a given format for texturing but not
rendering, or for rendering but not texturing, we would need to do at
least two format fallbacks during image import, and cache the fallback
results in the image struct. This approach is possible, but...
onto the next bullet.

2. A more practical argument.

If possible, it's better to do the fallback for
glEGLImageTextureTarget2DOES() in the same way as for
glTexStorage2D(), as I explained above. But that requires access
to a GL context; eglCreateImage may be called without
a context. [EGL_EXT_image_dma_buf_import explicitly requires that
eglCreateImage(context=EGL_CONTEXT_NONE)]; and therefore
intel_create_image_name() and friends also have no context.
Post by Daniel Stone
and also it'd be easy to add the analogue
to intel_create_image_from_fds for the explicit dmabuf (as opposed to
ANativeBuffer; your description of 'from dmabufs' threw me because the
dmabuf fd import path would fail immediately on FourCC lookup) import
path.
I did mean dma_buf, not AHardwareBuffer or ANativeBuffer, in this patch.

The patch series is secretly crazy. It's implicitly partitioned into
3 sets of patches: patches that touch code that will run only on
Android; that will run only on Chrome OS; and that will run on both.

- The patch "i965/dri: Support R8G8B8A8 and R8G8B8X8 configs" falls
into the "runs only on Android category". It deals with creating an
EGLSurface from a AHardwareBuffer with HAL_PIXEL_FORMAT_RGBX_8888.

- This patch falls into the "runs only on Chrome OS" category. It
helps the Chrome OS compositor import the above-mentioned client's
R8G8B8X8 EGLSurface through eglCreateImage(EGL_LINUX_DMA_BUF) and
glEGLImageTextureTarget2DOES. Outside the Android container, there's
no AHardwareBuffer; it's all dma_bufs here.

Anyway, why would it fail during fourcc lookup? The table
intel_screen.c:intel_image_formats[] contains an entry for
__DRI_IMAGE_FOURCC_XBGR8888. And egl_dri2.c:dri2_check_dma_buf_format()
knows about DRM_FORMAT_XBGR8888 too.
Post by Daniel Stone
As an aside, it's safe to enable in Wayland (IMO anyway), because we
only use the DRM format there; there's no concept of a 'surface
format' or visuals inside the Wayland client EGL platform, just direct
sampling from whichever buffer was last attached. EGL_NATIVE_VISUAL_ID
is empty, because we don't have anything to expose to the client.
Probably famous last words tho.
Good to know. That's one less thing my patches may break.
Daniel Stone
2017-06-08 08:13:32 UTC
Reply
Permalink
Raw Message
Hi Chad,
Post by Chad Versace
Post by Daniel Stone
I wonder if it wouldn't be better to do this in
intel_create_image_from_name. That way it would be more obvious
up-front what's happening,
I agree that the intent would become more obvious if the format fallback
were done at time of import instead of gl*Storage. But I see two
Thanks a lot for the detailed explanation! Makes sense.
Post by Chad Versace
Anyway, why would it fail during fourcc lookup? The table
intel_screen.c:intel_image_formats[] contains an entry for
__DRI_IMAGE_FOURCC_XBGR8888. And egl_dri2.c:dri2_check_dma_buf_format()
knows about DRM_FORMAT_XBGR8888 too.
Apparently I can't read, or type, so missed it in the table.

Cheers,
Daniel
Jason Ekstrand
2017-06-21 00:05:50 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
Post by Daniel Stone
Hi Chad,
Post by Chad Versace
@@ -254,8 +255,22 @@ create_mt_for_dri_image(struct brw_context *brw,
struct gl_context *ctx = &brw->ctx;
struct intel_mipmap_tree *mt;
uint32_t draw_x, draw_y;
+ mesa_format format = image->format;
+
+ if (!ctx->TextureFormatSupported[format]) {
+ /* The texture storage paths in core Mesa detect if the driver
does not
Post by Daniel Stone
Post by Chad Versace
+ * support the user-requested format, and then searches for a
+ * fallback format. The DRIimage code bypasses core Mesa,
though. So we
Post by Daniel Stone
Post by Chad Versace
+ * do the fallbacks here for important formats.
+ *
+ * We must support DRM_FOURCC_XBGR8888 textures because the
Android
Post by Daniel Stone
Post by Chad Versace
+ * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys
surfaces, which
Post by Daniel Stone
Post by Chad Versace
+ * the Chrome OS compositor consumes as dma_buf EGLImages.
+ */
+ format = _mesa_format_fallback_rgbx_to_rgba(format);
+ }
- if (!ctx->TextureFormatSupported[image->format])
+ if (!ctx->TextureFormatSupported[format])
return NULL;
I dislike what I wrote above. There's a much better way to do the
fallback, a way that handles more types of fallback than rgbx->rgba and
that's the same as the fallback used by glTexStorage2D(). The better way
mesa_format format = ctx->Driver.ChooseTextureFormat(ctx,
GL_TEXTURE_2D,
internalFormat, GL_NONE, GL_NONE);
As precedent, that's exactly what intel_renderbuffer_format() does.
Does this mean we're dropping patch 1? If not, I sent out a new version
which I find much easier to comprehend.
Post by Chad Versace
Post by Daniel Stone
Post by Chad Versace
/* Disable creation of the texture's aux buffers because the
driver exposes
Post by Daniel Stone
Post by Chad Versace
@@ -263,7 +278,7 @@ create_mt_for_dri_image(struct brw_context *brw,
* buffer's content to the main buffer nor for invalidating the
aux buffer's
Post by Daniel Stone
Post by Chad Versace
* content.
*/
- mt = intel_miptree_create_for_bo(brw, image->bo, image->format,
+ mt = intel_miptree_create_for_bo(brw, image->bo, format,
0, image->width, image->height, 1,
image->pitch,
MIPTREE_LAYOUT_DISABLE_AUX);
I wonder if it wouldn't be better to do this in
intel_create_image_from_name. That way it would be more obvious
up-front what's happening,
I agree that the intent would become more obvious if the format fallback
were done at time of import instead of gl*Storage. But I see two
1. First, the weaker argument.
The chosen fallback format,
and even the choice to do a fallback at all, is a property of the
image's usage and not a property of the image itself. A single
image can have multiple uses during its lifetime, and the driver
may need a different fallback or no fallback for each. I'm
defining "image usage" here in terms of
glEGLImageTargetTexture2DOES, glEGLImageTargetRenderbufferStorageOES, and
GL_TEXURE_EXTERNAL_OES vs GL_TEXTURE_2D.
Which reminds me... I should have submitted an analgous patch for
glEGLImageTargetRenderbufferStorageOES().
Since the driver may support a given format for texturing but not
rendering, or for rendering but not texturing, we would need to do at
least two format fallbacks during image import, and cache the fallback
results in the image struct. This approach is possible, but...
onto the next bullet.
I don't think that argument is all that weak
Post by Chad Versace
2. A more practical argument.
If possible, it's better to do the fallback for
glEGLImageTextureTarget2DOES() in the same way as for
glTexStorage2D(), as I explained above. But that requires access
to a GL context; eglCreateImage may be called without
a context. [EGL_EXT_image_dma_buf_import explicitly requires that
eglCreateImage(context=EGL_CONTEXT_NONE)]; and therefore
intel_create_image_name() and friends also have no context.
But this one really sets it. Let's have the image have the "correct"
format and fix it in miptree_create_for_dri_image. Speaking of such, I
just rewrote that function and I'm not sure how this fits in.
Post by Chad Versace
Post by Daniel Stone
and also it'd be easy to add the analogue
to intel_create_image_from_fds for the explicit dmabuf (as opposed to
ANativeBuffer; your description of 'from dmabufs' threw me because the
dmabuf fd import path would fail immediately on FourCC lookup) import
path.
I did mean dma_buf, not AHardwareBuffer or ANativeBuffer, in this patch.
The patch series is secretly crazy. It's implicitly partitioned into
3 sets of patches: patches that touch code that will run only on
Android; that will run only on Chrome OS; and that will run on both.
- The patch "i965/dri: Support R8G8B8A8 and R8G8B8X8 configs" falls
into the "runs only on Android category". It deals with creating an
EGLSurface from a AHardwareBuffer with HAL_PIXEL_FORMAT_RGBX_8888.
- This patch falls into the "runs only on Chrome OS" category. It
helps the Chrome OS compositor import the above-mentioned client's
R8G8B8X8 EGLSurface through eglCreateImage(EGL_LINUX_DMA_BUF) and
glEGLImageTextureTarget2DOES. Outside the Android container, there's
no AHardwareBuffer; it's all dma_bufs here.
Anyway, why would it fail during fourcc lookup? The table
intel_screen.c:intel_image_formats[] contains an entry for
__DRI_IMAGE_FOURCC_XBGR8888. And egl_dri2.c:dri2_check_dma_buf_format()
knows about DRM_FORMAT_XBGR8888 too.
Post by Daniel Stone
As an aside, it's safe to enable in Wayland (IMO anyway), because we
only use the DRM format there; there's no concept of a 'surface
format' or visuals inside the Wayland client EGL platform, just direct
sampling from whichever buffer was last attached. EGL_NATIVE_VISUAL_ID
is empty, because we don't have anything to expose to the client.
Probably famous last words tho.
Good to know. That's one less thing my patches may break.
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Chad Versace
2017-06-21 20:26:39 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
Post by Daniel Stone
Hi Chad,
Post by Chad Versace
@@ -254,8 +255,22 @@ create_mt_for_dri_image(struct brw_context *brw,
     struct gl_context *ctx = &brw->ctx;
     struct intel_mipmap_tree *mt;
     uint32_t draw_x, draw_y;
+   mesa_format format = image->format;
+
+   if (!ctx->TextureFormatSupported[format]) {
+      /* The texture storage paths in core Mesa detect if the driver
does not
Post by Daniel Stone
Post by Chad Versace
+       * support the user-requested format, and then searches for a
+       * fallback format. The DRIimage code bypasses core Mesa,
though. So we
Post by Daniel Stone
Post by Chad Versace
+       * do the fallbacks here for important formats.
+       *
+       * We must support DRM_FOURCC_XBGR8888 textures because the
Android
Post by Daniel Stone
Post by Chad Versace
+       * framework produces HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces,
which
Post by Daniel Stone
Post by Chad Versace
+       * the Chrome OS compositor consumes as dma_buf EGLImages.
+       */
+      format = _mesa_format_fallback_rgbx_to_rgba(format);
+   }
-   if (!ctx->TextureFormatSupported[image->format])
+   if (!ctx->TextureFormatSupported[format])
        return NULL;
I dislike what I wrote above. There's a much better way to do the
fallback, a way that handles more types of fallback than rgbx->rgba and
that's the same as the fallback used by glTexStorage2D(). The better way
    mesa_format format = ctx->Driver.ChooseTextureFormat(ctx,
GL_TEXTURE_2D,
                                                         internalFormat,
GL_NONE, GL_NONE);
As precedent, that's exactly what intel_renderbuffer_format() does.
Does this mean we're dropping patch 1?  If not, I sent out a new version which
I find much easier to comprehend.
We still need patch 1 for the intelCreateBuffer paths, which have no
current context, and therefore no ctx->Driver.ChooseTextureFormat.
Post by Chad Versace
Post by Daniel Stone
Post by Chad Versace
     /* Disable creation of the texture's aux buffers because the driver
exposes
Post by Daniel Stone
Post by Chad Versace
@@ -263,7 +278,7 @@ create_mt_for_dri_image(struct brw_context *brw,
      * buffer's content to the main buffer nor for invalidating the aux
buffer's
Post by Daniel Stone
Post by Chad Versace
      * content.
      */
-   mt = intel_miptree_create_for_bo(brw, image->bo, image->format,
+   mt = intel_miptree_create_for_bo(brw, image->bo, format,
                                      0, image->width, image->height, 1,
                                      image->pitch,
                                      MIPTREE_LAYOUT_DISABLE_AUX);
I wonder if it wouldn't be better to do this in
intel_create_image_from_name. That way it would be more obvious
up-front what's happening,
I agree that the intent would become more obvious if the format fallback
were done at time of import instead of gl*Storage. But I see two
    1. First, the weaker argument.
       The chosen fallback format,
       and even the choice to do a fallback at all, is a property of the
       image's usage and not a property of the image itself. A single
       image can have multiple uses during its lifetime, and the driver
       may need a different fallback or no fallback for each. I'm
       defining "image usage" here in terms of
       glEGLImageTargetTexture2DOES, glEGLImageTargetRenderbufferSt
orageOES, and
       GL_TEXURE_EXTERNAL_OES vs GL_TEXTURE_2D.
       Which reminds me... I should have submitted an analgous patch for
       glEGLImageTargetRenderbufferStorageOES().
       Since the driver may support a given format for texturing but not
       rendering, or for rendering but not texturing, we would need to do
at
       least two format fallbacks during image import, and cache the
fallback
       results in the image struct. This approach is possible, but...
       onto the next bullet.
I don't think that argument is all that weak
 
    2. A more practical argument.
       If possible, it's better to do the fallback for
       glEGLImageTextureTarget2DOES() in the same way as for
       glTexStorage2D(), as I explained above. But that requires access
       to a GL context; eglCreateImage may be called without
       a context. [EGL_EXT_image_dma_buf_import explicitly requires that
       eglCreateImage(context=EGL_CONTEXT_NONE)]; and therefore
       intel_create_image_name() and friends also have no context.
But this one really sets it.  Let's have the image have the "correct" format
that function and I'm not sure how this fits in.
Good to hear we agree.

Chad Versace
2017-06-06 20:36:57 UTC
Reply
Permalink
Raw Message
I'm swimming in a vortex of formats. Mesa formats, isl formats, DRI
formats, GL formats, etc.

It's easy to misinterpret the following brw_context members unless
you've recently read their definition. In upcoming patches, I change
them from embedded arrays to simple pointers; after that, even their
definition doesn't help, because the MESA_FORMAT_COUNT hint will no
longer be present.

Rename them to prevent further confusion. While we're renaming, choose
shorter names too.

-format_supported_as_render_target
+mesa_format_supports_render

-render_target_format
+mesa_to_isl_render_format
---
src/mesa/drivers/dri/i965/brw_blorp.c | 10 +++++-----
src/mesa/drivers/dri/i965/brw_context.h | 4 ++--
src/mesa/drivers/dri/i965/brw_meta_util.c | 2 +-
src/mesa/drivers/dri/i965/brw_surface_formats.c | 20 ++++++++++----------
src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 +-
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 10 +++++-----
src/mesa/drivers/dri/i965/intel_fbo.c | 2 +-
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++--
src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
9 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index 28be620429..34f6bc4c84 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -243,8 +243,8 @@ brw_blorp_to_isl_format(struct brw_context *brw, mesa_format format,
return ISL_FORMAT_R16_UNORM;
default: {
if (is_render_target) {
- assert(brw->format_supported_as_render_target[format]);
- return brw->render_target_format[format];
+ assert(brw->mesa_format_supports_render[format]);
+ return brw->mesa_to_isl_render_format[format];
} else {
return brw_isl_format_for_mesa_format(format);
}
@@ -607,7 +607,7 @@ brw_blorp_copytexsubimage(struct brw_context *brw,
_mesa_get_format_base_format(dst_mt->format) == GL_DEPTH_STENCIL)
return false;

- if (!brw->format_supported_as_render_target[dst_image->TexFormat])
+ if (!brw->mesa_format_supports_render[dst_image->TexFormat])
return false;

/* Source clipping shouldn't be necessary, since copytexsubimage (in
@@ -858,7 +858,7 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb,
struct blorp_batch batch;
blorp_batch_init(&brw->blorp, &batch, brw, 0);
blorp_fast_clear(&batch, &surf,
- brw->render_target_format[format],
+ brw->mesa_to_isl_render_format[format],
level, logical_layer, num_layers,
x0, y0, x1, y1);
blorp_batch_finish(&batch);
@@ -884,7 +884,7 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb,
struct blorp_batch batch;
blorp_batch_init(&brw->blorp, &batch, brw, 0);
blorp_clear(&batch, &surf,
- brw->render_target_format[format],
+ brw->mesa_to_isl_render_format[format],
ISL_SWIZZLE_IDENTITY,
level, irb_logical_mt_layer(irb), num_layers,
x0, y0, x1, y1,
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index c15abe1d48..17a76f0808 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1161,8 +1161,8 @@ struct brw_context
const struct brw_tracked_state render_atoms[76];
const struct brw_tracked_state compute_atoms[11];

- enum isl_format render_target_format[MESA_FORMAT_COUNT];
- bool format_supported_as_render_target[MESA_FORMAT_COUNT];
+ enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT];
+ bool mesa_format_supports_render[MESA_FORMAT_COUNT];

/* PrimitiveRestart */
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c b/src/mesa/drivers/dri/i965/brw_meta_util.c
index cbc2dedde8..0342a527d7 100644
--- a/src/mesa/drivers/dri/i965/brw_meta_util.c
+++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
@@ -289,7 +289,7 @@ brw_is_color_fast_clear_compatible(struct brw_context *brw,
*/
if (brw->gen >= 9 &&
brw_isl_format_for_mesa_format(mt->format) !=
- brw->render_target_format[mt->format])
+ brw->mesa_to_isl_render_format[mt->format])
return false;

/* Gen9 doesn't support fast clear on single-sampled SRGB buffers. When
diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c
index f878317e92..c33cafa836 100644
--- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
+++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
@@ -301,21 +301,21 @@ brw_init_surface_formats(struct brw_context *brw)
*/
if (isl_format_supports_rendering(devinfo, render) &&
(isl_format_supports_alpha_blending(devinfo, render) || is_integer)) {
- brw->render_target_format[format] = render;
- brw->format_supported_as_render_target[format] = true;
+ brw->mesa_to_isl_render_format[format] = render;
+ brw->mesa_format_supports_render[format] = true;
}
}

/* We will check this table for FBO completeness, but the surface format
* table above only covered color rendering.
*/
- brw->format_supported_as_render_target[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
- brw->format_supported_as_render_target[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
- brw->format_supported_as_render_target[MESA_FORMAT_S_UINT8] = true;
- brw->format_supported_as_render_target[MESA_FORMAT_Z_FLOAT32] = true;
- brw->format_supported_as_render_target[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_S_UINT8] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_Z_FLOAT32] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
if (brw->gen >= 8)
- brw->format_supported_as_render_target[MESA_FORMAT_Z_UNORM16] = true;
+ brw->mesa_format_supports_render[MESA_FORMAT_Z_UNORM16] = true;

/* We remap depth formats to a supported texturing format in
* translate_tex_format().
@@ -367,7 +367,7 @@ brw_init_surface_formats(struct brw_context *brw)

for (int i = 0; i < ARRAY_SIZE(rgbx_formats); i++) {
ctx->TextureFormatSupported[rgbx_formats[i]] = false;
- brw->format_supported_as_render_target[rgbx_formats[i]] = false;
+ brw->mesa_format_supports_render[rgbx_formats[i]] = false;
}
}

@@ -424,7 +424,7 @@ brw_render_target_supported(struct brw_context *brw,
return false;
}

- return brw->format_supported_as_render_target[format];
+ return brw->mesa_format_supports_render[format];
}

enum isl_format
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index 1f0a1e9a38..79891dd847 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -585,7 +585,7 @@ brw_miptree_choose_tiling(struct brw_context *brw,
* alignment of 4 as often as we can, this shouldn't happen very often.
*/
if (brw->gen == 7 && mt->valign == 2 &&
- brw->format_supported_as_render_target[mt->format]) {
+ brw->mesa_format_supports_render[mt->format]) {
return I915_TILING_X;
}

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 3d54c141d0..83b503e1b7 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -205,7 +205,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
assert(brw_render_target_supported(brw, rb));

mesa_format rb_format = _mesa_get_render_format(ctx, intel_rb_format(irb));
- if (unlikely(!brw->format_supported_as_render_target[rb_format])) {
+ if (unlikely(!brw->mesa_format_supports_render[rb_format])) {
_mesa_problem(ctx, "%s: renderbuffer format %s unsupported\n",
__func__, _mesa_get_format_name(rb_format));
}
@@ -216,7 +216,7 @@ brw_update_renderbuffer_surface(struct brw_context *brw,
MAX2(irb->mt->num_samples, 1) : 1;

struct isl_view view = {
- .format = brw->render_target_format[rb_format],
+ .format = brw->mesa_to_isl_render_format[rb_format],
.base_level = irb->mt_level - irb->mt->first_level,
.levels = 1,
.base_array_layer = irb->mt_layer / layer_multiplier,
@@ -1013,8 +1013,8 @@ gen4_update_renderbuffer_surface(struct brw_context *brw,

surf = brw_state_batch(brw, 6 * 4, 32, &offset);

- format = brw->render_target_format[rb_format];
- if (unlikely(!brw->format_supported_as_render_target[rb_format])) {
+ format = brw->mesa_to_isl_render_format[rb_format];
+ if (unlikely(!brw->mesa_format_supports_render[rb_format])) {
_mesa_problem(ctx, "%s: renderbuffer format %s unsupported\n",
__func__, _mesa_get_format_name(rb_format));
}
@@ -1173,7 +1173,7 @@ update_renderbuffer_read_surfaces(struct brw_context *brw)
uint32_t *surf_offset = &brw->wm.base.surf_offset[surf_index];

if (irb) {
- const enum isl_format format = brw->render_target_format[
+ const enum isl_format format = brw->mesa_to_isl_render_format[
_mesa_get_render_format(ctx, intel_rb_format(irb))];
assert(isl_format_supports_sampling(&brw->screen->devinfo,
format));
diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index a24ddd0d88..88e2fc7bf1 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -348,7 +348,7 @@ intel_image_target_renderbuffer_storage(struct gl_context *ctx,
}

/* __DRIimage is opaque to the core so it has to be checked here */
- if (!brw->format_supported_as_render_target[image->format]) {
+ if (!brw->mesa_format_supports_render[image->format]) {
_mesa_error(ctx, GL_INVALID_OPERATION,
"glEGLImageTargetRenderbufferStorage(unsupported image format)");
return;
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index b77b7fdadd..1890fb2089 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -204,7 +204,7 @@ intel_miptree_supports_non_msrt_fast_clear(struct brw_context *brw,
/* There's no point in using an MCS buffer if the surface isn't in a
* renderable format.
*/
- if (!brw->format_supported_as_render_target[mt->format])
+ if (!brw->mesa_format_supports_render[mt->format])
return false;

if (brw->gen >= 9) {
@@ -3310,7 +3310,7 @@ intel_miptree_get_isl_surf(struct brw_context *brw,
break;
default:
surf->usage = ISL_SURF_USAGE_TEXTURE_BIT;
- if (brw->format_supported_as_render_target[mt->format])
+ if (brw->mesa_format_supports_render[mt->format])
surf->usage = ISL_SURF_USAGE_RENDER_TARGET_BIT;
break;
}
diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c
index 6da666c884..1751f109f7 100644
--- a/src/mesa/drivers/dri/i965/intel_tex.c
+++ b/src/mesa/drivers/dri/i965/intel_tex.c
@@ -325,7 +325,7 @@ intel_set_texture_storage_for_buffer_object(struct gl_context *ctx,
return false;
}

- if (!brw->format_supported_as_render_target[image->TexFormat]) {
+ if (!brw->mesa_format_supports_render[image->TexFormat]) {
perf_debug("Non-renderable PBO format; fallback to CPU mapping\n");
return false;
}
--
2.13.0
Chad Versace
2017-06-06 20:36:55 UTC
Reply
Permalink
Raw Message
The new function takes a mesa_format and, if the format is an alpha
format with a non-alpha variant, returns the non-alpha format.
Otherwise, it returns the original format.

Example:
input -> output

// Fallback exists
MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16

// No fallback
MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32

i965 will use this for EGLImages and DRIimages.
---
src/mesa/Android.gen.mk | 12 +++
src/mesa/Makefile.am | 7 ++
src/mesa/Makefile.sources | 2 +
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 +++++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++++++++++++++++++
6 files changed, 233 insertions(+)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py

diff --git a/src/mesa/Android.gen.mk b/src/mesa/Android.gen.mk
index 42d4ba1969..8dcfb460e9 100644
--- a/src/mesa/Android.gen.mk
+++ b/src/mesa/Android.gen.mk
@@ -34,6 +34,7 @@ sources := \
main/enums.c \
main/api_exec.c \
main/dispatch.h \
+ main/format_fallback.c \
main/format_pack.c \
main/format_unpack.c \
main/format_info.h \
@@ -135,6 +136,17 @@ $(intermediates)/main/get_hash.h: $(glapi)/gl_and_es_API.xml \
$(LOCAL_PATH)/main/get_hash_params.py $(GET_HASH_GEN)
$(call es-gen)

+FORMAT_FALLBACK := $(LOCAL_PATH)/main/format_fallback.py
+format_fallback_deps := \
+ $(LOCAL_PATH)/main/formats.csv \
+ $(LOCAL_PATH)/main/format_parser.py \
+ $(FORMAT_FALLBACK)
+
+$(intermediates)/main/format_fallback.c: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(FORMAT_FALLBACK)
+$(intermediates)/main/format_fallback.c: PRIVATE_XML :=
+$(intermediates)/main/format_fallback.c: $(format_fallback_deps)
+ $(call es-gen, $<)
+
FORMAT_INFO := $(LOCAL_PATH)/main/format_info.py
format_info_deps := \
$(LOCAL_PATH)/main/formats.csv \
diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
index 53f311d2a9..2c39374aef 100644
--- a/src/mesa/Makefile.am
+++ b/src/mesa/Makefile.am
@@ -37,6 +37,7 @@ include Makefile.sources

EXTRA_DIST = \
drivers/SConscript \
+ main/format_fallback.py \
main/format_info.py \
main/format_pack.py \
main/format_parser.py \
@@ -54,6 +55,7 @@ EXTRA_DIST = \

BUILT_SOURCES = \
main/get_hash.h \
+ main/format_fallback.c \
main/format_info.h \
main/format_pack.c \
main/format_unpack.c \
@@ -70,6 +72,11 @@ main/get_hash.h: ../mapi/glapi/gen/gl_and_es_API.xml main/get_hash_params.py \
$(PYTHON_GEN) $(srcdir)/main/get_hash_generator.py \
-f $(srcdir)/../mapi/glapi/gen/gl_and_es_API.xml > $@

+main/format_fallback.c: main/format_fallback.py \
+ main/format_parser.py \
+ main/formats.csv
+ $(PYTHON_GEN) $(srcdir)/main/format_fallback.py $(srcdir)/main/formats.csv > $@
+
main/format_info.h: main/formats.csv \
main/format_parser.py main/format_info.py
$(PYTHON_GEN) $(srcdir)/main/format_info.py $(srcdir)/main/formats.csv > $@
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 8a65fbe663..642100b956 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -94,6 +94,8 @@ MAIN_FILES = \
main/ffvertex_prog.h \
main/fog.c \
main/fog.h \
+ main/format_fallback.h \
+ main/format_fallback.c \
main/format_info.h \
main/format_pack.h \
main/format_pack.c \
diff --git a/src/mesa/main/.gitignore b/src/mesa/main/.gitignore
index 836d8f104a..8cc33cfee6 100644
--- a/src/mesa/main/.gitignore
+++ b/src/mesa/main/.gitignore
@@ -4,6 +4,7 @@ enums.c
remap_helper.h
get_hash.h
get_hash.h.tmp
+format_fallback.c
format_info.h
format_info.c
format_pack.c
diff --git a/src/mesa/main/format_fallback.h b/src/mesa/main/format_fallback.h
new file mode 100644
index 0000000000..5ca8269b2f
--- /dev/null
+++ b/src/mesa/main/format_fallback.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2017 Google
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef FORMAT_FALLBACK_H
+#define FORMAT_FALLBACK_H
+
+#include "formats.h"
+
+mesa_format
+_mesa_format_fallback_rgbx_to_rgba(mesa_format format);
+
+#endif /* FORMAT_FALLBACK_H */
diff --git a/src/mesa/main/format_fallback.py b/src/mesa/main/format_fallback.py
new file mode 100644
index 0000000000..253cd0e3e7
--- /dev/null
+++ b/src/mesa/main/format_fallback.py
@@ -0,0 +1,180 @@
+# Copyright 2017 Google
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sub license, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice (including the
+# next paragraph) shall be included in all copies or substantial portions
+# of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+# IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+# ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+# stdlib
+import argparse
+from sys import stdout
+from textwrap import dedent
+
+# local
+import format_parser
+
+def format_is_rgbx(fmt):
+ return fmt.has_channel('a')
+
+def parse_args():
+ p = argparse.ArgumentParser()
+ p.add_argument("csv")
+ return p.parse_args()
+
+def write_preamble(out):
+ out.write(dedent('''\
+ /*
+ * Copyright 2017 Google
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+ /*
+ * This file is GENERATED. Do not edit it manually or commit it into version
+ * control.
+ */
+
+ #include "format_fallback.h"
+
+ '''))
+
+def format_get_alpha_channel_string(fmt):
+ """Return alpha channel's substring in the format name. If the format has
+ no alpha channel, then return None.
+
+ The name of alpha formats fall into the following cases:
+
+ - The alpha channel's size follows the "A". For example,
+ MESA_FORMAT_R8G8B8A8_UNORM. In this case, the channel substring
+ is "A8".
+
+ - The alpha channel's size implicitly appears as the datatype size. For
+ example, MESA_FORMAT_RGBA_FLOAT32. In this case, the channel
+ substring is just "A".
+ """
+
+ alpha_channel = fmt.get_channel('a')
+ if not alpha_channel:
+ return None
+
+ assert alpha_channel.size > 0
+
+ alpha_channel_name = "A" + str(alpha_channel.size)
+ if alpha_channel_name in fmt.name:
+ return alpha_channel_name
+
+ return "A"
+
+def format_convert_alpha_to_x(fmt, all_formats):
+ """If the format is an alpha format for which a non-alpha variant
+ exists with the same bit layout, then return the non-alpha format.
+ Otherwise return None.
+ """
+
+ a_str = format_get_alpha_channel_string(fmt)
+ if not a_str:
+ # Format has no alpha channel
+ return None
+
+ x_str = a_str.replace("A", "X")
+
+ # Succesively replace each occurence of a_str in the format name with
+ # x_str until we find a valid format name.
+ i = -1
+ while True:
+ i += 1
+ i = fmt.name.find(a_str, i)
+ if i == -1:
+ break
+
+ x_fmt_name = fmt.name[:i] + fmt.name[i:].replace(a_str, x_str)
+ # Assert that the string replacement actually occured.
+ assert x_fmt_name != fmt.name
+
+ x_fmt = all_formats.get(x_fmt_name, None)
+ if x_fmt is None:
+ continue
+
+ return x_fmt
+
+ return None
+
+def write_func_mesa_format_fallback_rgbx_to_rgba(out, formats):
+ out.write(dedent('''\
+ /**
+ * If the format has an alpha channel, and there exists a non-alpha
+ * variant of the format with an identical bit layout, then return
+ * the non-alpha format. Otherwise return the original format.
+ *
+ * Examples:
+ * Fallback exists:
+ * MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
+ * MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16
+ *
+ * No fallback:
+ * MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
+ * MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32
+ */
+ mesa_format
+ _mesa_format_fallback_rgbx_to_rgba(mesa_format format)
+ {
+ switch (format) {
+ '''))
+
+ for alpha_format in formats.values():
+ x_format = format_convert_alpha_to_x(alpha_format, formats)
+ if x_format is None:
+ continue
+
+ out.write(" case {}: return {};\n".format(
+ x_format.name, alpha_format.name))
+
+ out.write(dedent('''\
+ default: return format;
+ }
+ }
+ '''))
+
+def main():
+ pargs = parse_args()
+
+ formats = {}
+ for fmt in format_parser.parse(pargs.csv):
+ formats[fmt.name] = fmt
+
+ write_preamble(stdout)
+ write_func_mesa_format_fallback_rgbx_to_rgba(stdout, formats)
+
+if __name__ == "__main__":
+ main()
--
2.13.0
Dylan Baker
2017-06-06 20:51:07 UTC
Reply
Permalink
Raw Message
Quoting Chad Versace (2017-06-06 13:36:55)
Post by Chad Versace
The new function takes a mesa_format and, if the format is an alpha
format with a non-alpha variant, returns the non-alpha format.
Otherwise, it returns the original format.
input -> output
// Fallback exists
MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16
// No fallback
MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32
i965 will use this for EGLImages and DRIimages.
---
src/mesa/Android.gen.mk | 12 +++
src/mesa/Makefile.am | 7 ++
src/mesa/Makefile.sources | 2 +
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 +++++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++++++++++++++++++
6 files changed, 233 insertions(+)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py
diff --git a/src/mesa/Android.gen.mk b/src/mesa/Android.gen.mk
index 42d4ba1969..8dcfb460e9 100644
--- a/src/mesa/Android.gen.mk
+++ b/src/mesa/Android.gen.mk
@@ -34,6 +34,7 @@ sources := \
main/enums.c \
main/api_exec.c \
main/dispatch.h \
+ main/format_fallback.c \
main/format_pack.c \
main/format_unpack.c \
main/format_info.h \
@@ -135,6 +136,17 @@ $(intermediates)/main/get_hash.h: $(glapi)/gl_and_es_API.xml \
$(LOCAL_PATH)/main/get_hash_params.py $(GET_HASH_GEN)
$(call es-gen)
+FORMAT_FALLBACK := $(LOCAL_PATH)/main/format_fallback.py
+format_fallback_deps := \
+ $(LOCAL_PATH)/main/formats.csv \
+ $(LOCAL_PATH)/main/format_parser.py \
+ $(FORMAT_FALLBACK)
+
+$(intermediates)/main/format_fallback.c: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(FORMAT_FALLBACK)
+$(intermediates)/main/format_fallback.c: PRIVATE_XML :=
+$(intermediates)/main/format_fallback.c: $(format_fallback_deps)
+ $(call es-gen, $<)
+
FORMAT_INFO := $(LOCAL_PATH)/main/format_info.py
format_info_deps := \
$(LOCAL_PATH)/main/formats.csv \
diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am
index 53f311d2a9..2c39374aef 100644
--- a/src/mesa/Makefile.am
+++ b/src/mesa/Makefile.am
@@ -37,6 +37,7 @@ include Makefile.sources
EXTRA_DIST = \
drivers/SConscript \
+ main/format_fallback.py \
main/format_info.py \
main/format_pack.py \
main/format_parser.py \
@@ -54,6 +55,7 @@ EXTRA_DIST = \
BUILT_SOURCES = \
main/get_hash.h \
+ main/format_fallback.c \
main/format_info.h \
main/format_pack.c \
main/format_unpack.c \
@@ -70,6 +72,11 @@ main/get_hash.h: ../mapi/glapi/gen/gl_and_es_API.xml main/get_hash_params.py \
$(PYTHON_GEN) $(srcdir)/main/get_hash_generator.py \
+main/format_fallback.c: main/format_fallback.py \
+ main/format_parser.py \
+ main/formats.csv
+
main/format_info.h: main/formats.csv \
main/format_parser.py main/format_info.py
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 8a65fbe663..642100b956 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -94,6 +94,8 @@ MAIN_FILES = \
main/ffvertex_prog.h \
main/fog.c \
main/fog.h \
+ main/format_fallback.h \
+ main/format_fallback.c \
main/format_info.h \
main/format_pack.h \
main/format_pack.c \
diff --git a/src/mesa/main/.gitignore b/src/mesa/main/.gitignore
index 836d8f104a..8cc33cfee6 100644
--- a/src/mesa/main/.gitignore
+++ b/src/mesa/main/.gitignore
@@ -4,6 +4,7 @@ enums.c
remap_helper.h
get_hash.h
get_hash.h.tmp
+format_fallback.c
format_info.h
format_info.c
format_pack.c
diff --git a/src/mesa/main/format_fallback.h b/src/mesa/main/format_fallback.h
new file mode 100644
index 0000000000..5ca8269b2f
--- /dev/null
+++ b/src/mesa/main/format_fallback.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2017 Google
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef FORMAT_FALLBACK_H
+#define FORMAT_FALLBACK_H
+
+#include "formats.h"
+
+mesa_format
+_mesa_format_fallback_rgbx_to_rgba(mesa_format format);
+
+#endif /* FORMAT_FALLBACK_H */
diff --git a/src/mesa/main/format_fallback.py b/src/mesa/main/format_fallback.py
new file mode 100644
index 0000000000..253cd0e3e7
--- /dev/null
+++ b/src/mesa/main/format_fallback.py
@@ -0,0 +1,180 @@
+# Copyright 2017 Google
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sub license, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+#
+# The above copyright notice and this permission notice (including the
+# next paragraph) shall be included in all copies or substantial portions
+# of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+# IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+# ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+# stdlib
+import argparse
+from sys import stdout
+from textwrap import dedent
+
+# local
+import format_parser
+
+ return fmt.has_channel('a')
+
+ p = argparse.ArgumentParser()
+ p.add_argument("csv")
+ return p.parse_args()
+
+ out.write(dedent('''\
+ /*
+ * Copyright 2017 Google
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ *
+ * The above copyright notice and this permission notice shall be included
+ * in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+ /*
+ * This file is GENERATED. Do not edit it manually or commit it into version
+ * control.
+ */
+
+ #include "format_fallback.h"
+
+ '''))
+
+ """Return alpha channel's substring in the format name. If the format has
+ no alpha channel, then return None.
+
+
+ - The alpha channel's size follows the "A". For example,
+ MESA_FORMAT_R8G8B8A8_UNORM. In this case, the channel substring
+ is "A8".
+
+ - The alpha channel's size implicitly appears as the datatype size. For
+ example, MESA_FORMAT_RGBA_FLOAT32. In this case, the channel
+ substring is just "A".
+ """
+
+ alpha_channel = fmt.get_channel('a')
+ return None
+
+ assert alpha_channel.size > 0
+
+ alpha_channel_name = "A" + str(alpha_channel.size)
+ return alpha_channel_name
+
+ return "A"
+
+ """If the format is an alpha format for which a non-alpha variant
+ exists with the same bit layout, then return the non-alpha format.
+ Otherwise return None.
+ """
+
+ a_str = format_get_alpha_channel_string(fmt)
+ # Format has no alpha channel
+ return None
+
+ x_str = a_str.replace("A", "X")
+
+ # Succesively replace each occurence of a_str in the format name with
+ # x_str until we find a valid format name.
+ i = -1
+ i += 1
+ i = fmt.name.find(a_str, i)
+ break
+
+ x_fmt_name = fmt.name[:i] + fmt.name[i:].replace(a_str, x_str)
+ # Assert that the string replacement actually occured.
+ assert x_fmt_name != fmt.name
+
+ x_fmt = all_formats.get(x_fmt_name, None)
+ continue
+
+ return x_fmt
+
+ return None
+
+ out.write(dedent('''\
+ /**
+ * If the format has an alpha channel, and there exists a non-alpha
+ * variant of the format with an identical bit layout, then return
+ * the non-alpha format. Otherwise return the original format.
+ *
+ * MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
+ * MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16
+ *
+ * MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
+ * MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32
+ */
+ mesa_format
+ _mesa_format_fallback_rgbx_to_rgba(mesa_format format)
+ {
+ switch (format) {
+ '''))
+
+ x_format = format_convert_alpha_to_x(alpha_format, formats)
+ continue
+
+ out.write(" case {}: return {};\n".format(
+ x_format.name, alpha_format.name))
+
+ out.write(dedent('''\
+ default: return format;
+ }
+ }
+ '''))
+
+ pargs = parse_args()
+
+ formats = {}
+ formats[fmt.name] = fmt
You could simplify this as:
formats = {f.name: f for f in format_parser.parse(pargs.csv)}
Post by Chad Versace
+
+ write_preamble(stdout)
+ write_func_mesa_format_fallback_rgbx_to_rgba(stdout, formats)
We really shouldn't write to stdout like this, it can cause all kinds of
breakages if there's ever a UTF-8 character (say ©) and the terminal doesn't
have a unicode locale it'll fail, if you just open the file you want (say one
passed as an argument) then it doesn't matter what the console supports. We do
this all over the place so it's not a blocker for me, but I still think it's a
bad idea to write to stdout.

If you decide not to change this you at the very least need to call
stdout.flush() after write_func_mesa_format_fallback_Rgbx_to_rgba.

Dylan
Post by Chad Versace
+
+ main()
--
2.13.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Chad Versace
2017-06-06 22:11:18 UTC
Reply
Permalink
Raw Message
Post by Dylan Baker
Quoting Chad Versace (2017-06-06 13:36:55)
Post by Chad Versace
The new function takes a mesa_format and, if the format is an alpha
format with a non-alpha variant, returns the non-alpha format.
Otherwise, it returns the original format.
input -> output
// Fallback exists
MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16
// No fallback
MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32
i965 will use this for EGLImages and DRIimages.
---
src/mesa/Android.gen.mk | 12 +++
src/mesa/Makefile.am | 7 ++
src/mesa/Makefile.sources | 2 +
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 +++++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++++++++++++++++++
6 files changed, 233 insertions(+)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py
[snip]
Post by Dylan Baker
Post by Chad Versace
+ pargs = parse_args()
+
+ formats = {}
+ formats[fmt.name] = fmt
formats = {f.name: f for f in format_parser.parse(pargs.csv)}
Thanks. I'll do that.
Post by Dylan Baker
Post by Chad Versace
+
+ write_preamble(stdout)
+ write_func_mesa_format_fallback_rgbx_to_rgba(stdout, formats)
We really shouldn't write to stdout like this, it can cause all kinds of
breakages if there's ever a UTF-8 character (say ©) and the terminal doesn't
have a unicode locale it'll fail,
Ugh. I wasn't aware that Python's stdout was broken. Is Python's
sys.stdout opened in "text" mode, and is that the cause of the
brokenness?

Does it still fail if stdout is redirected to a file? Because that's the
only case that matters here.
Post by Dylan Baker
if you just open the file you want (say one
passed as an argument) then it doesn't matter what the console supports. We do
this all over the place so it's not a blocker for me, but I still think it's a
bad idea to write to stdout.
If you decide not to change this you at the very least need to call
stdout.flush() after write_func_mesa_format_fallback_Rgbx_to_rgba.
Dylan Baker
2017-06-06 22:31:03 UTC
Reply
Permalink
Raw Message
Quoting Chad Versace (2017-06-06 15:11:18)
Post by Chad Versace
Post by Dylan Baker
Quoting Chad Versace (2017-06-06 13:36:55)
Post by Chad Versace
The new function takes a mesa_format and, if the format is an alpha
format with a non-alpha variant, returns the non-alpha format.
Otherwise, it returns the original format.
input -> output
// Fallback exists
MESA_FORMAT_R8G8B8X8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_RGBX_UNORM16 -> MESA_FORMAT_RGBA_UNORM16
// No fallback
MESA_FORMAT_R8G8B8A8_UNORM -> MESA_FORMAT_R8G8B8A8_UNORM
MESA_FORMAT_Z_FLOAT32 -> MESA_FORMAT_Z_FLOAT32
i965 will use this for EGLImages and DRIimages.
---
src/mesa/Android.gen.mk | 12 +++
src/mesa/Makefile.am | 7 ++
src/mesa/Makefile.sources | 2 +
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 +++++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++++++++++++++++++
6 files changed, 233 insertions(+)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py
[snip]
Post by Dylan Baker
Post by Chad Versace
+ pargs = parse_args()
+
+ formats = {}
+ formats[fmt.name] = fmt
formats = {f.name: f for f in format_parser.parse(pargs.csv)}
Thanks. I'll do that.
Post by Dylan Baker
Post by Chad Versace
+
+ write_preamble(stdout)
+ write_func_mesa_format_fallback_rgbx_to_rgba(stdout, formats)
We really shouldn't write to stdout like this, it can cause all kinds of
breakages if there's ever a UTF-8 character (say ©) and the terminal doesn't
have a unicode locale it'll fail,
Ugh. I wasn't aware that Python's stdout was broken. Is Python's
sys.stdout opened in "text" mode, and is that the cause of the
brokenness?
Does it still fail if stdout is redirected to a file? Because that's the
only case that matters here.
It's not python, it's the shell (I think). In this case it won't be a problem
since you don't have any non-ascii characters, but we've run into cases where
someone (like me) adds "Copyright © 3001 Mystery Science Theatre" and then
breaks some (but not many) systems. I know this because I added such a copyright
and broke someone's system, and eventually we narrowed it down to the fact that
this person didn't have a UTF-8 locale but I did, we ended up just removing the
© character from the output to fix it.
Post by Chad Versace
Post by Dylan Baker
if you just open the file you want (say one
passed as an argument) then it doesn't matter what the console supports. We do
this all over the place so it's not a blocker for me, but I still think it's a
bad idea to write to stdout.
If you decide not to change this you at the very least need to call
stdout.flush() after write_func_mesa_format_fallback_Rgbx_to_rgba.
Chad Versace
2017-06-07 22:25:02 UTC
Reply
Permalink
Raw Message
Post by Dylan Baker
Quoting Chad Versace (2017-06-06 15:11:18)
Post by Chad Versace
Post by Dylan Baker
Quoting Chad Versace (2017-06-06 13:36:55)
Post by Chad Versace
+ write_preamble(stdout)
+ write_func_mesa_format_fallback_rgbx_to_rgba(stdout, formats)
We really shouldn't write to stdout like this, it can cause all kinds of
breakages if there's ever a UTF-8 character (say ©) and the terminal doesn't
have a unicode locale it'll fail,
Ugh. I wasn't aware that Python's stdout was broken. Is Python's
sys.stdout opened in "text" mode, and is that the cause of the
brokenness?
Does it still fail if stdout is redirected to a file? Because that's the
only case that matters here.
It's not python, it's the shell (I think). In this case it won't be a problem
since you don't have any non-ascii characters, but we've run into cases where
someone (like me) adds "Copyright © 3001 Mystery Science Theatre" and then
breaks some (but not many) systems. I know this because I added such a copyright
and broke someone's system, and eventually we narrowed it down to the fact that
this person didn't have a UTF-8 locale but I did, we ended up just removing the
© character from the output to fix it.
Ok, then it's not really a problem for build-system scripts.
Chad Versace
2017-06-06 20:36:58 UTC
Reply
Permalink
Raw Message
The param is currently unused. It will later be used it to support
R8G8B8X8 EGLConfigs on Skylake.
---
src/mesa/drivers/dri/i965/intel_fbo.c | 8 +++++---
src/mesa/drivers/dri/i965/intel_fbo.h | 6 ++++--
src/mesa/drivers/dri/i965/intel_screen.c | 14 ++++++++------
3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 88e2fc7bf1..16d1325736 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -438,7 +438,8 @@ intel_nop_alloc_storage(struct gl_context * ctx, struct gl_renderbuffer *rb,
* \param num_samples must be quantized.
*/
struct intel_renderbuffer *
-intel_create_winsys_renderbuffer(mesa_format format, unsigned num_samples)
+intel_create_winsys_renderbuffer(struct intel_screen *screen,
+ mesa_format format, unsigned num_samples)
{
struct intel_renderbuffer *irb = CALLOC_STRUCT(intel_renderbuffer);
if (!irb)
@@ -470,11 +471,12 @@ intel_create_winsys_renderbuffer(mesa_format format, unsigned num_samples)
* \param num_samples must be quantized.
*/
struct intel_renderbuffer *
-intel_create_private_renderbuffer(mesa_format format, unsigned num_samples)
+intel_create_private_renderbuffer(struct intel_screen *screen,
+ mesa_format format, unsigned num_samples)
{
struct intel_renderbuffer *irb;

- irb = intel_create_winsys_renderbuffer(format, num_samples);
+ irb = intel_create_winsys_renderbuffer(screen, format, num_samples);
irb->Base.Base.AllocStorage = intel_alloc_private_renderbuffer_storage;

return irb;
diff --git a/src/mesa/drivers/dri/i965/intel_fbo.h b/src/mesa/drivers/dri/i965/intel_fbo.h
index 2d2ef1ebc6..752e4f36a8 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.h
+++ b/src/mesa/drivers/dri/i965/intel_fbo.h
@@ -167,10 +167,12 @@ intel_rb_format(const struct intel_renderbuffer *rb)
}

extern struct intel_renderbuffer *
-intel_create_winsys_renderbuffer(mesa_format format, unsigned num_samples);
+intel_create_winsys_renderbuffer(struct intel_screen *screen,
+ mesa_format format, unsigned num_samples);

struct intel_renderbuffer *
-intel_create_private_renderbuffer(mesa_format format, unsigned num_samples);
+intel_create_private_renderbuffer(struct intel_screen *screen,
+ mesa_format format, unsigned num_samples);

struct gl_renderbuffer*
intel_create_wrapped_renderbuffer(struct gl_context * ctx,
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 22f6d9af03..7733d91fea 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1200,11 +1200,11 @@ intelCreateBuffer(__DRIscreen *dri_screen,
}

/* setup the hardware-based renderbuffers */
- rb = intel_create_winsys_renderbuffer(rgbFormat, num_samples);
+ rb = intel_create_winsys_renderbuffer(screen, rgbFormat, num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_FRONT_LEFT, &rb->Base.Base);

if (mesaVis->doubleBufferMode) {
- rb = intel_create_winsys_renderbuffer(rgbFormat, num_samples);
+ rb = intel_create_winsys_renderbuffer(screen, rgbFormat, num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_BACK_LEFT, &rb->Base.Base);
}

@@ -1217,10 +1217,11 @@ intelCreateBuffer(__DRIscreen *dri_screen,
assert(mesaVis->stencilBits == 8);

if (screen->devinfo.has_hiz_and_separate_stencil) {
- rb = intel_create_private_renderbuffer(MESA_FORMAT_Z24_UNORM_X8_UINT,
+ rb = intel_create_private_renderbuffer(screen,
+ MESA_FORMAT_Z24_UNORM_X8_UINT,
num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_DEPTH, &rb->Base.Base);
- rb = intel_create_private_renderbuffer(MESA_FORMAT_S_UINT8,
+ rb = intel_create_private_renderbuffer(screen, MESA_FORMAT_S_UINT8,
num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_STENCIL, &rb->Base.Base);
} else {
@@ -1228,7 +1229,8 @@ intelCreateBuffer(__DRIscreen *dri_screen,
* Use combined depth/stencil. Note that the renderbuffer is
* attached to two attachment points.
*/
- rb = intel_create_private_renderbuffer(MESA_FORMAT_Z24_UNORM_S8_UINT,
+ rb = intel_create_private_renderbuffer(screen,
+ MESA_FORMAT_Z24_UNORM_S8_UINT,
num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_DEPTH, &rb->Base.Base);
_mesa_attach_and_reference_rb(fb, BUFFER_STENCIL, &rb->Base.Base);
@@ -1236,7 +1238,7 @@ intelCreateBuffer(__DRIscreen *dri_screen,
}
else if (mesaVis->depthBits == 16) {
assert(mesaVis->stencilBits == 0);
- rb = intel_create_private_renderbuffer(MESA_FORMAT_Z_UNORM16,
+ rb = intel_create_private_renderbuffer(screen, MESA_FORMAT_Z_UNORM16,
num_samples);
_mesa_attach_and_own_rb(fb, BUFFER_DEPTH, &rb->Base.Base);
}
--
2.13.0
Chad Versace
2017-06-06 20:37:00 UTC
Reply
Permalink
Raw Message
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888.

Even though all RGBX formats are disabled on gen9 by
brw_surface_formats.c, the new configs work correctly on Broxton thanks
to _mesa_format_fallback_rgbx_to_rgba().

On GLX, this creates no new configs, and therefore breaks no existing
apps. See in-patch comments for explanation. I tested with glxinfo and
glxgears on Skylake.

On Wayland, this also creates no new configs, and therfore breaks no
existing apps. (I tested with mesa-demos' eglinfo and es2gears_wayland
on Skylake). The reason differs from GLX, though. In
dri2_wl_add_configs_for_visual(), the format table contains only
B8G8R8X8, B8G8R8A8, and B5G6B5; and dri2_add_config() correctly matches
EGLConfig to format by inspecting channel masks.

On Android, in Chrome OS, I tested this on a Broxton device. I confirmed
that the Google Play Store's EGLSurface used HAL_PIXEL_FORMAT_RGBA_8888,
and that an Asteroid game's EGLSurface used HAL_PIXEL_FORMAT_RGBX_8888.
Both apps worked well. (Disclaimer: I didn't test this patch on Android
with Mesa master. I backported this patch series to an older Android
branch).
---
src/mesa/drivers/dri/i965/intel_fbo.c | 24 ++++++++++++++++++++++--
src/mesa/drivers/dri/i965/intel_screen.c | 23 ++++++++++++++++++++++-
2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 16d1325736..e56d30a2c0 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -34,6 +34,7 @@
#include "main/teximage.h"
#include "main/image.h"
#include "main/condrender.h"
+#include "main/format_fallback.h"
#include "util/hash_table.h"
#include "util/set.h"

@@ -450,10 +451,29 @@ intel_create_winsys_renderbuffer(struct intel_screen *screen,

_mesa_init_renderbuffer(rb, 0);
rb->ClassID = INTEL_RB_CLASS;
+ rb->NumSamples = num_samples;
+
+ /* The base format and internal format must be derived from the user-visible
+ * format (that is, the gl_config's format), even if we internally use
+ * choose a different format for the renderbuffer. Otherwise, rendering may
+ * use incorrect channel write masks.
+ */
rb->_BaseFormat = _mesa_get_format_base_format(format);
- rb->Format = format;
rb->InternalFormat = rb->_BaseFormat;
- rb->NumSamples = num_samples;
+
+ rb->Format = format;
+ if (!screen->mesa_format_supports_render[rb->Format]) {
+ /* The glRenderbufferStorage paths in core Mesa detect if the driver
+ * does not support the user-requested format, and then searches for
+ * a falback format. The DRI code bypasses core Mesa, though. So we do
+ * the fallbacks here.
+ *
+ * We must support MESA_FORMAT_R8G8B8X8 on Android because the Android
+ * framework requires HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces.
+ */
+ rb->Format = _mesa_format_fallback_rgbx_to_rgba(rb->Format);
+ assert(screen->mesa_format_supports_render[rb->Format]);
+ }

/* intel-specific methods */
rb->Delete = intel_delete_renderbuffer;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 563065b91f..a9d132f868 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1547,7 +1547,28 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
static const mesa_format formats[] = {
MESA_FORMAT_B5G6R5_UNORM,
MESA_FORMAT_B8G8R8A8_UNORM,
- MESA_FORMAT_B8G8R8X8_UNORM
+ MESA_FORMAT_B8G8R8X8_UNORM,
+
+ /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+ * Likewise for RGBX and BGRX. Otherwise, the GLX client and the GLX
+ * server may disagree on which format the GLXFBConfig represents,
+ * resulting in swapped color channels.
+ *
+ * The problem, as of 2017-05-30:
+ * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+ * order and chooses the first __DRIconfig with the expected channel
+ * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+ * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+ *
+ * EGL does not suffer from this problem. It correctly compares the
+ * channel masks when matching EGLConfig to __DRIconfig.
+ */
+
+ /* Required by Android, for HAL_PIXEL_FORMAT_RGBA_8888. */
+ MESA_FORMAT_R8G8B8A8_UNORM,
+
+ /* Required by Android, for HAL_PIXEL_FORMAT_RGBX_8888. */
+ MESA_FORMAT_R8G8B8X8_UNORM,
};

/* GLX_SWAP_COPY_OML is not supported due to page flipping. */
--
2.13.0
Rob Herring
2017-06-08 12:40:20 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888.
Even though all RGBX formats are disabled on gen9 by
brw_surface_formats.c, the new configs work correctly on Broxton thanks
to _mesa_format_fallback_rgbx_to_rgba().
On GLX, this creates no new configs, and therefore breaks no existing
apps. See in-patch comments for explanation. I tested with glxinfo and
glxgears on Skylake.
On Wayland, this also creates no new configs, and therfore breaks no
existing apps. (I tested with mesa-demos' eglinfo and es2gears_wayland
on Skylake). The reason differs from GLX, though. In
dri2_wl_add_configs_for_visual(), the format table contains only
B8G8R8X8, B8G8R8A8, and B5G6B5; and dri2_add_config() correctly matches
EGLConfig to format by inspecting channel masks.
On Android, in Chrome OS, I tested this on a Broxton device. I confirmed
that the Google Play Store's EGLSurface used HAL_PIXEL_FORMAT_RGBA_8888,
and that an Asteroid game's EGLSurface used HAL_PIXEL_FORMAT_RGBX_8888.
Both apps worked well. (Disclaimer: I didn't test this patch on Android
with Mesa master. I backported this patch series to an older Android
branch).
---
src/mesa/drivers/dri/i965/intel_fbo.c | 24 ++++++++++++++++++++++--
src/mesa/drivers/dri/i965/intel_screen.c | 23 ++++++++++++++++++++++-
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 16d1325736..e56d30a2c0 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -34,6 +34,7 @@
#include "main/teximage.h"
#include "main/image.h"
#include "main/condrender.h"
+#include "main/format_fallback.h"
#include "util/hash_table.h"
#include "util/set.h"
@@ -450,10 +451,29 @@ intel_create_winsys_renderbuffer(struct intel_screen *screen,
_mesa_init_renderbuffer(rb, 0);
rb->ClassID = INTEL_RB_CLASS;
+ rb->NumSamples = num_samples;
+
+ /* The base format and internal format must be derived from the user-visible
+ * format (that is, the gl_config's format), even if we internally use
+ * choose a different format for the renderbuffer. Otherwise, rendering may
+ * use incorrect channel write masks.
+ */
rb->_BaseFormat = _mesa_get_format_base_format(format);
- rb->Format = format;
rb->InternalFormat = rb->_BaseFormat;
- rb->NumSamples = num_samples;
+
+ rb->Format = format;
+ if (!screen->mesa_format_supports_render[rb->Format]) {
+ /* The glRenderbufferStorage paths in core Mesa detect if the driver
+ * does not support the user-requested format, and then searches for
+ * a falback format. The DRI code bypasses core Mesa, though. So we do
+ * the fallbacks here.
+ *
+ * We must support MESA_FORMAT_R8G8B8X8 on Android because the Android
+ * framework requires HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces.
+ */
+ rb->Format = _mesa_format_fallback_rgbx_to_rgba(rb->Format);
+ assert(screen->mesa_format_supports_render[rb->Format]);
+ }
/* intel-specific methods */
rb->Delete = intel_delete_renderbuffer;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 563065b91f..a9d132f868 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1547,7 +1547,28 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
static const mesa_format formats[] = {
MESA_FORMAT_B5G6R5_UNORM,
MESA_FORMAT_B8G8R8A8_UNORM,
- MESA_FORMAT_B8G8R8X8_UNORM
+ MESA_FORMAT_B8G8R8X8_UNORM,
+
+ /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+ * Likewise for RGBX and BGRX. Otherwise, the GLX client and the GLX
+ * server may disagree on which format the GLXFBConfig represents,
+ * resulting in swapped color channels.
+ *
+ * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+ * order and chooses the first __DRIconfig with the expected channel
+ * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+ * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+ *
+ * EGL does not suffer from this problem. It correctly compares the
+ * channel masks when matching EGLConfig to __DRIconfig.
+ */
When I originally added this for gallium (commit ccdcf91104a5f), I
happened to add these formats to the beginning rather than end of the
list. Is simply changing the order really enough to avoid the issue
with GLX?

Rob
Chad Versace
2017-06-08 18:09:17 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
Post by Chad Versace
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888.
Even though all RGBX formats are disabled on gen9 by
brw_surface_formats.c, the new configs work correctly on Broxton thanks
to _mesa_format_fallback_rgbx_to_rgba().
On GLX, this creates no new configs, and therefore breaks no existing
apps. See in-patch comments for explanation. I tested with glxinfo and
glxgears on Skylake.
On Wayland, this also creates no new configs, and therfore breaks no
existing apps. (I tested with mesa-demos' eglinfo and es2gears_wayland
on Skylake). The reason differs from GLX, though. In
dri2_wl_add_configs_for_visual(), the format table contains only
B8G8R8X8, B8G8R8A8, and B5G6B5; and dri2_add_config() correctly matches
EGLConfig to format by inspecting channel masks.
On Android, in Chrome OS, I tested this on a Broxton device. I confirmed
that the Google Play Store's EGLSurface used HAL_PIXEL_FORMAT_RGBA_8888,
and that an Asteroid game's EGLSurface used HAL_PIXEL_FORMAT_RGBX_8888.
Both apps worked well. (Disclaimer: I didn't test this patch on Android
with Mesa master. I backported this patch series to an older Android
branch).
---
src/mesa/drivers/dri/i965/intel_fbo.c | 24 ++++++++++++++++++++++--
src/mesa/drivers/dri/i965/intel_screen.c | 23 ++++++++++++++++++++++-
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 16d1325736..e56d30a2c0 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -34,6 +34,7 @@
#include "main/teximage.h"
#include "main/image.h"
#include "main/condrender.h"
+#include "main/format_fallback.h"
#include "util/hash_table.h"
#include "util/set.h"
@@ -450,10 +451,29 @@ intel_create_winsys_renderbuffer(struct intel_screen *screen,
_mesa_init_renderbuffer(rb, 0);
rb->ClassID = INTEL_RB_CLASS;
+ rb->NumSamples = num_samples;
+
+ /* The base format and internal format must be derived from the user-visible
+ * format (that is, the gl_config's format), even if we internally use
+ * choose a different format for the renderbuffer. Otherwise, rendering may
+ * use incorrect channel write masks.
+ */
rb->_BaseFormat = _mesa_get_format_base_format(format);
- rb->Format = format;
rb->InternalFormat = rb->_BaseFormat;
- rb->NumSamples = num_samples;
+
+ rb->Format = format;
+ if (!screen->mesa_format_supports_render[rb->Format]) {
+ /* The glRenderbufferStorage paths in core Mesa detect if the driver
+ * does not support the user-requested format, and then searches for
+ * a falback format. The DRI code bypasses core Mesa, though. So we do
+ * the fallbacks here.
+ *
+ * We must support MESA_FORMAT_R8G8B8X8 on Android because the Android
+ * framework requires HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces.
+ */
+ rb->Format = _mesa_format_fallback_rgbx_to_rgba(rb->Format);
+ assert(screen->mesa_format_supports_render[rb->Format]);
+ }
/* intel-specific methods */
rb->Delete = intel_delete_renderbuffer;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 563065b91f..a9d132f868 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1547,7 +1547,28 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
static const mesa_format formats[] = {
MESA_FORMAT_B5G6R5_UNORM,
MESA_FORMAT_B8G8R8A8_UNORM,
- MESA_FORMAT_B8G8R8X8_UNORM
+ MESA_FORMAT_B8G8R8X8_UNORM,
+
+ /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+ * Likewise for RGBX and BGRX. Otherwise, the GLX client and the GLX
+ * server may disagree on which format the GLXFBConfig represents,
+ * resulting in swapped color channels.
+ *
+ * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+ * order and chooses the first __DRIconfig with the expected channel
+ * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+ * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+ *
+ * EGL does not suffer from this problem. It correctly compares the
+ * channel masks when matching EGLConfig to __DRIconfig.
+ */
When I originally added this for gallium (commit ccdcf91104a5f), I
happened to add these formats to the beginning rather than end of the
list. Is simply changing the order really enough to avoid the issue
with GLX?
According to my experiments with i965, definitely yes.

When I place the new formats at the beginning of the array, the glxgears
colors are swapped. When I place the new formats at the end of the
array, the glxgears color are correct.

To understand why, I stepped through, with gdb, the mesa:src/glx code
that associates X visuals to DRI configs. I don't claim to fully
understand how that code works, but I felt confident enough to write the
above comment and claim that GLX ignores the new formats, as long as
they occur at the end.
Emil Velikov
2017-06-09 12:54:59 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888.
Even though all RGBX formats are disabled on gen9 by
brw_surface_formats.c, the new configs work correctly on Broxton thanks
to _mesa_format_fallback_rgbx_to_rgba().
On GLX, this creates no new configs, and therefore breaks no existing
apps. See in-patch comments for explanation. I tested with glxinfo and
glxgears on Skylake.
On Wayland, this also creates no new configs, and therfore breaks no
existing apps. (I tested with mesa-demos' eglinfo and es2gears_wayland
on Skylake). The reason differs from GLX, though. In
dri2_wl_add_configs_for_visual(), the format table contains only
B8G8R8X8, B8G8R8A8, and B5G6B5; and dri2_add_config() correctly matches
EGLConfig to format by inspecting channel masks.
On Android, in Chrome OS, I tested this on a Broxton device. I confirmed
that the Google Play Store's EGLSurface used HAL_PIXEL_FORMAT_RGBA_8888,
and that an Asteroid game's EGLSurface used HAL_PIXEL_FORMAT_RGBX_8888.
Both apps worked well. (Disclaimer: I didn't test this patch on Android
with Mesa master. I backported this patch series to an older Android
branch).
---
src/mesa/drivers/dri/i965/intel_fbo.c | 24 ++++++++++++++++++++++--
src/mesa/drivers/dri/i965/intel_screen.c | 23 ++++++++++++++++++++++-
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 16d1325736..e56d30a2c0 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -34,6 +34,7 @@
#include "main/teximage.h"
#include "main/image.h"
#include "main/condrender.h"
+#include "main/format_fallback.h"
#include "util/hash_table.h"
#include "util/set.h"
@@ -450,10 +451,29 @@ intel_create_winsys_renderbuffer(struct intel_screen *screen,
_mesa_init_renderbuffer(rb, 0);
rb->ClassID = INTEL_RB_CLASS;
+ rb->NumSamples = num_samples;
+
+ /* The base format and internal format must be derived from the user-visible
+ * format (that is, the gl_config's format), even if we internally use
+ * choose a different format for the renderbuffer. Otherwise, rendering may
+ * use incorrect channel write masks.
+ */
rb->_BaseFormat = _mesa_get_format_base_format(format);
- rb->Format = format;
rb->InternalFormat = rb->_BaseFormat;
- rb->NumSamples = num_samples;
+
+ rb->Format = format;
+ if (!screen->mesa_format_supports_render[rb->Format]) {
+ /* The glRenderbufferStorage paths in core Mesa detect if the driver
+ * does not support the user-requested format, and then searches for
+ * a falback format. The DRI code bypasses core Mesa, though. So we do
+ * the fallbacks here.
+ *
+ * We must support MESA_FORMAT_R8G8B8X8 on Android because the Android
+ * framework requires HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces.
+ */
+ rb->Format = _mesa_format_fallback_rgbx_to_rgba(rb->Format);
+ assert(screen->mesa_format_supports_render[rb->Format]);
+ }
/* intel-specific methods */
rb->Delete = intel_delete_renderbuffer;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 563065b91f..a9d132f868 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1547,7 +1547,28 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
static const mesa_format formats[] = {
MESA_FORMAT_B5G6R5_UNORM,
MESA_FORMAT_B8G8R8A8_UNORM,
- MESA_FORMAT_B8G8R8X8_UNORM
+ MESA_FORMAT_B8G8R8X8_UNORM,
+
+ /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+ * Likewise for RGBX and BGRX. Otherwise, the GLX client and the GLX
+ * server may disagree on which format the GLXFBConfig represents,
+ * resulting in swapped color channels.
+ *
+ * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+ * order and chooses the first __DRIconfig with the expected channel
+ * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+ * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+ *
+ * EGL does not suffer from this problem. It correctly compares the
+ * channel masks when matching EGLConfig to __DRIconfig.
+ */
+
Huge thank you for the comprehensive comment Chad. It will raise quite
a few eyebrows should someone consider touching any of it :-)
Guess as we add support for RGBX/RGBA on !Android EGL, the comment may
need a tweak. But that's can follow in due time.

Thanks again!
Emil
P.S. /me wonders why src/glx does not bother with any masks. Not even
during comparison...
Chad Versace
2017-06-06 20:36:59 UTC
Reply
Permalink
Raw Message
This allows us to query the driver's supported formats in i965's DRI code,
where often there is available a DRIscreen but no GL context.

To reduce diff noise, this patch does not completely remove
brw_context's format arrays. It just redeclares them as pointers which
point to the arrays in intel_screen.

Specifically, move these two arrays from brw_context to intel_screen:
mesa_to_isl_render_format[]
mesa_format_supports_render[]

And add a new array to intel_screen,
mesa_format_supportex_texture[]
which brw_init_surface_formats() copies to ctx->TextureFormatSupported.
---
src/mesa/drivers/dri/i965/brw_context.h | 5 +-
src/mesa/drivers/dri/i965/brw_surface_formats.c | 92 +++++++++++++++----------
src/mesa/drivers/dri/i965/intel_screen.c | 2 +
src/mesa/drivers/dri/i965/intel_screen.h | 5 ++
4 files changed, 64 insertions(+), 40 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 17a76f0808..476981bfad 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1161,8 +1161,8 @@ struct brw_context
const struct brw_tracked_state render_atoms[76];
const struct brw_tracked_state compute_atoms[11];

- enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT];
- bool mesa_format_supports_render[MESA_FORMAT_COUNT];
+ const enum isl_format *mesa_to_isl_render_format;
+ const bool *mesa_format_supports_render;

/* PrimitiveRestart */
struct {
@@ -1419,6 +1419,7 @@ void brw_upload_image_surfaces(struct brw_context *brw,
struct brw_stage_prog_data *prog_data);

/* brw_surface_formats.c */
+void intel_screen_init_surface_formats(struct intel_screen *screen);
void brw_init_surface_formats(struct brw_context *brw);
bool brw_render_target_supported(struct brw_context *brw,
struct gl_renderbuffer *rb);
diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c b/src/mesa/drivers/dri/i965/brw_surface_formats.c
index c33cafa836..a2bc1ded6d 100644
--- a/src/mesa/drivers/dri/i965/brw_surface_formats.c
+++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c
@@ -203,17 +203,16 @@ brw_isl_format_for_mesa_format(mesa_format mesa_format)
}

void
-brw_init_surface_formats(struct brw_context *brw)
+intel_screen_init_surface_formats(struct intel_screen *screen)
{
- const struct gen_device_info *devinfo = &brw->screen->devinfo;
- struct gl_context *ctx = &brw->ctx;
- int gen;
+ const struct gen_device_info *devinfo = &screen->devinfo;
mesa_format format;

- memset(&ctx->TextureFormatSupported, 0, sizeof(ctx->TextureFormatSupported));
+ memset(&screen->mesa_format_supports_texture, 0,
+ sizeof(screen->mesa_format_supports_texture));

- gen = brw->gen * 10;
- if (brw->is_g4x || brw->is_haswell)
+ int gen = devinfo->gen * 10;
+ if (devinfo->is_g4x || devinfo->is_haswell)
gen += 5;

for (format = MESA_FORMAT_NONE + 1; format < MESA_FORMAT_COUNT; format++) {
@@ -237,7 +236,7 @@ brw_init_surface_formats(struct brw_context *brw)

if (isl_format_supports_sampling(devinfo, texture) &&
(isl_format_supports_filtering(devinfo, texture) || is_integer))
- ctx->TextureFormatSupported[format] = true;
+ screen->mesa_format_supports_texture[format] = true;

/* Re-map some render target formats to make them supported when they
* wouldn't be using their format for texturing.
@@ -301,30 +300,30 @@ brw_init_surface_formats(struct brw_context *brw)
*/
if (isl_format_supports_rendering(devinfo, render) &&
(isl_format_supports_alpha_blending(devinfo, render) || is_integer)) {
- brw->mesa_to_isl_render_format[format] = render;
- brw->mesa_format_supports_render[format] = true;
+ screen->mesa_to_isl_render_format[format] = render;
+ screen->mesa_format_supports_render[format] = true;
}
}

/* We will check this table for FBO completeness, but the surface format
* table above only covered color rendering.
*/
- brw->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
- brw->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
- brw->mesa_format_supports_render[MESA_FORMAT_S_UINT8] = true;
- brw->mesa_format_supports_render[MESA_FORMAT_Z_FLOAT32] = true;
- brw->mesa_format_supports_render[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
- if (brw->gen >= 8)
- brw->mesa_format_supports_render[MESA_FORMAT_Z_UNORM16] = true;
+ screen->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
+ screen->mesa_format_supports_render[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
+ screen->mesa_format_supports_render[MESA_FORMAT_S_UINT8] = true;
+ screen->mesa_format_supports_render[MESA_FORMAT_Z_FLOAT32] = true;
+ screen->mesa_format_supports_render[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
+ if (gen >= 80)
+ screen->mesa_format_supports_render[MESA_FORMAT_Z_UNORM16] = true;

/* We remap depth formats to a supported texturing format in
* translate_tex_format().
*/
- ctx->TextureFormatSupported[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_Z_FLOAT32] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_S_UINT8] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_Z24_UNORM_S8_UINT] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_Z24_UNORM_X8_UINT] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_Z_FLOAT32] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_Z32_FLOAT_S8X24_UINT] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_S_UINT8] = true;

/* Benchmarking shows that Z16 is slower than Z24, so there's no reason to
* use it unless you're under memory (not memory bandwidth) pressure.
@@ -340,8 +339,8 @@ brw_init_surface_formats(struct brw_context *brw)
* With the PMA stall workaround in place, Z16 is faster than Z24, as it
* should be.
*/
- if (brw->gen >= 8)
- ctx->TextureFormatSupported[MESA_FORMAT_Z_UNORM16] = true;
+ if (gen >= 80)
+ screen->mesa_format_supports_texture[MESA_FORMAT_Z_UNORM16] = true;

/* The RGBX formats are not renderable. Normally these get mapped
* internally to RGBA formats when rendering. However on Gen9+ when this
@@ -356,7 +355,7 @@ brw_init_surface_formats(struct brw_context *brw)
* doesn't implement this swizzle override. We don't need to do this for
* BGRX because that actually is supported natively on Gen8+.
*/
- if (brw->gen >= 9) {
+ if (gen >= 90) {
static const mesa_format rgbx_formats[] = {
MESA_FORMAT_R8G8B8X8_UNORM,
MESA_FORMAT_R8G8B8X8_SRGB,
@@ -366,30 +365,47 @@ brw_init_surface_formats(struct brw_context *brw)
};

for (int i = 0; i < ARRAY_SIZE(rgbx_formats); i++) {
- ctx->TextureFormatSupported[rgbx_formats[i]] = false;
- brw->mesa_format_supports_render[rgbx_formats[i]] = false;
+ screen->mesa_format_supports_texture[rgbx_formats[i]] = false;
+ screen->mesa_format_supports_render[rgbx_formats[i]] = false;
}
}

/* On hardware that lacks support for ETC1, we map ETC1 to RGBX
* during glCompressedTexImage2D(). See intel_mipmap_tree::wraps_etc1.
*/
- ctx->TextureFormatSupported[MESA_FORMAT_ETC1_RGB8] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC1_RGB8] = true;

/* On hardware that lacks support for ETC2, we map ETC2 to a suitable
* MESA_FORMAT during glCompressedTexImage2D().
* See intel_mipmap_tree::wraps_etc2.
*/
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_RGB8] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_SRGB8] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_RGBA8_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_SRGB8_ALPHA8_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_R11_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_RG11_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_SIGNED_R11_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_SIGNED_RG11_EAC] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_RGB8_PUNCHTHROUGH_ALPHA1] = true;
- ctx->TextureFormatSupported[MESA_FORMAT_ETC2_SRGB8_PUNCHTHROUGH_ALPHA1] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_RGB8] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_SRGB8] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_RGBA8_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_SRGB8_ALPHA8_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_R11_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_RG11_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_SIGNED_R11_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_SIGNED_RG11_EAC] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_RGB8_PUNCHTHROUGH_ALPHA1] = true;
+ screen->mesa_format_supports_texture[MESA_FORMAT_ETC2_SRGB8_PUNCHTHROUGH_ALPHA1] = true;
+}
+
+void
+brw_init_surface_formats(struct brw_context *brw)
+{
+ struct intel_screen *screen = brw->screen;
+ struct gl_context *ctx = &brw->ctx;
+
+ brw->mesa_format_supports_render = screen->mesa_format_supports_render;
+ brw->mesa_to_isl_render_format = screen->mesa_to_isl_render_format;
+
+ STATIC_ASSERT(ARRAY_SIZE(ctx->TextureFormatSupported) ==
+ ARRAY_SIZE(screen->mesa_format_supports_texture));
+
+ for (unsigned i = 0; i < ARRAY_SIZE(ctx->TextureFormatSupported); ++i) {
+ ctx->TextureFormatSupported[i] = screen->mesa_format_supports_texture[i];
+ }
}

bool
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 7733d91fea..563065b91f 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -2116,6 +2116,8 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
screen->has_exec_fence =
intel_get_boolean(screen, I915_PARAM_HAS_EXEC_FENCE);

+ intel_screen_init_surface_formats(screen);
+
return (const __DRIconfig**) intel_screen_make_configs(dri_screen);
}

diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
index f9c1db6df1..f78b3e8f74 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.h
+++ b/src/mesa/drivers/dri/i965/intel_screen.h
@@ -31,6 +31,7 @@

#include <GL/internal/dri_interface.h>

+#include "isl/isl.h"
#include "dri_util.h"
#include "brw_bufmgr.h"
#include "common/gen_device_info.h"
@@ -107,6 +108,10 @@ struct intel_screen
* Number of EUs reported by the I915_PARAM_EU_TOTAL parameter
*/
int eu_total;
+
+ bool mesa_format_supports_texture[MESA_FORMAT_COUNT];
+ bool mesa_format_supports_render[MESA_FORMAT_COUNT];
+ enum isl_format mesa_to_isl_render_format[MESA_FORMAT_COUNT];
};

extern void intelDestroyContext(__DRIcontext * driContextPriv);
--
2.13.0
Tomasz Figa
2017-06-08 03:05:03 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!

Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071

Best regards,
Tomasz
Tapani Pälli
2017-06-08 06:36:27 UTC
Reply
Permalink
Raw Message
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
I just tested this set on Android and I'm getting wrong colors with
this, red and blue swapped.

// Tapani
Tapani Pälli
2017-06-08 07:08:58 UTC
Reply
Permalink
Raw Message
Post by Tapani Pälli
On Wed, Jun 7, 2017 at 5:36 AM, Chad Versace
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
I just tested this set on Android and I'm getting wrong colors with
this, red and blue swapped.
I can 'fix' this by reordering the configs in intel_screen_make_configs
so that the new configs (RGBA, RGBX) required by Android are before the
old ones (BGRA, BGRX).

// Tapani
Tomasz Figa
2017-06-08 07:16:54 UTC
Reply
Permalink
Raw Message
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.

However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.

Best regards,
Tomasz
Chad Versace
2017-06-08 18:27:35 UTC
Reply
Permalink
Raw Message
Post by Tomasz Figa
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
apps in that bug report and reply back with the results.
Post by Tomasz Figa
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
Uh oh.

I did something dumb. When testing this on ARC++, I chose
a BLACK-AND-WHITE game! I'll retest with a color game.

https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
Post by Tomasz Figa
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.
Yes.
Post by Tomasz Figa
However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.
Android certainly inspects the channel masks, using dri2_add_config().
So I'm also curious why Tapani sees swapped channels. The
platform_android.c is patched with format hacks, and I assume the same
is true for android-ia. Maybe the hacks are intefering somehow. I'll
investigate.
Tapani Pälli
2017-06-09 07:37:21 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
Post by Tomasz Figa
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
apps in that bug report and reply back with the results.
Post by Tomasz Figa
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
Uh oh.
I did something dumb. When testing this on ARC++, I chose
a BLACK-AND-WHITE game! I'll retest with a color game.
It was pretty hard to keep coffee in mouth when reading this comment.
Post by Chad Versace
https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
Post by Tomasz Figa
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.
Yes.
Post by Tomasz Figa
However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.
Android certainly inspects the channel masks, using dri2_add_config().
So I'm also curious why Tapani sees swapped channels. The
platform_android.c is patched with format hacks, and I assume the same
is true for android-ia. Maybe the hacks are intefering somehow. I'll
investigate.
We shouldn't have related 'hacks', but here are some changes that might
be interesting or somehow related:

add EGL_ALPHA_SIZE attrib:

https://github.com/android-ia/frameworks-native/commit/8237c9f8eb36d4f9ead40c8cb4a68ae9518d3c9f

sorting display configs:

https://github.com/android-ia/frameworks-native/pull/2/commits/bb29af0777e747effacd239565f52aad96c45558

visuals being added:

https://github.com/android-ia/external-mesa/commit/ceac31ff7e53ec5034bc379d37ce365c000e9e4a


// Tapani
Chad Versace
2017-06-13 18:55:14 UTC
Reply
Permalink
Raw Message
Post by Tapani Pälli
Post by Chad Versace
Post by Tomasz Figa
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
apps in that bug report and reply back with the results.
Post by Tomasz Figa
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
Uh oh.
I did something dumb. When testing this on ARC++, I chose
a BLACK-AND-WHITE game! I'll retest with a color game.
It was pretty hard to keep coffee in mouth when reading this comment.
Post by Chad Versace
https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
Post by Tomasz Figa
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.
Yes.
Post by Tomasz Figa
However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.
Android certainly inspects the channel masks, using dri2_add_config().
So I'm also curious why Tapani sees swapped channels. The
platform_android.c is patched with format hacks, and I assume the same
is true for android-ia. Maybe the hacks are intefering somehow. I'll
investigate.
We shouldn't have related 'hacks', but here are some changes that might be
https://github.com/android-ia/frameworks-native/commit/8237c9f8eb36d4f9ead40c8cb4a68ae9518d3c9f
https://github.com/android-ia/frameworks-native/pull/2/commits/bb29af0777e747effacd239565f52aad96c45558
https://github.com/android-ia/external-mesa/commit/ceac31ff7e53ec5034bc379d37ce365c000e9e4a
I confirmed that this series causes no regressions with Gnome and KDE
apps. For KDE, I used Amarok as the test app. To prove to myself
I *really* was testing this series, I tested the series with the new
formats placed at the end of the array, and then I tested again with the new
formats placed at the start of the array. As expected, Amarok's colors
were correct in the first test, and were wrong in the second test (blue
and red were swapped.

I still haven't succeeded in fully testing these patches against
Android, for a combination of technical and non-technical reasons. I'm
travelling this week, so I will probably be able to resume testing
Android apps on Thursday.
Rob Herring
2017-06-15 13:08:43 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
Post by Tapani Pälli
Post by Chad Versace
Post by Tomasz Figa
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
apps in that bug report and reply back with the results.
Post by Tomasz Figa
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
Uh oh.
I did something dumb. When testing this on ARC++, I chose
a BLACK-AND-WHITE game! I'll retest with a color game.
It was pretty hard to keep coffee in mouth when reading this comment.
Post by Chad Versace
https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
Post by Tomasz Figa
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.
Yes.
Post by Tomasz Figa
However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.
Android certainly inspects the channel masks, using dri2_add_config().
So I'm also curious why Tapani sees swapped channels. The
platform_android.c is patched with format hacks, and I assume the same
is true for android-ia. Maybe the hacks are intefering somehow. I'll
investigate.
We shouldn't have related 'hacks', but here are some changes that might be
https://github.com/android-ia/frameworks-native/commit/8237c9f8eb36d4f9ead40c8cb4a68ae9518d3c9f
https://github.com/android-ia/frameworks-native/pull/2/commits/bb29af0777e747effacd239565f52aad96c45558
https://github.com/android-ia/external-mesa/commit/ceac31ff7e53ec5034bc379d37ce365c000e9e4a
I confirmed that this series causes no regressions with Gnome and KDE
apps. For KDE, I used Amarok as the test app. To prove to myself
I *really* was testing this series, I tested the series with the new
formats placed at the end of the array, and then I tested again with the new
formats placed at the start of the array. As expected, Amarok's colors
were correct in the first test, and were wrong in the second test (blue
and red were swapped.
I still haven't succeeded in fully testing these patches against
Android, for a combination of technical and non-technical reasons. I'm
travelling this week, so I will probably be able to resume testing
Android apps on Thursday.
While not i965, I tested the similar code and change in gallium. With
the RGBA/RGBX formats at the end of the list I get:

EGL-MAIN: Native format mismatch: 0x1 != 0x5

That's coming from SurfaceFlinger.

Rob
Chad Versace
2017-06-16 18:34:23 UTC
Reply
Permalink
Raw Message
Post by Rob Herring
Post by Chad Versace
Post by Tapani Pälli
Post by Chad Versace
Post by Tomasz Figa
Post by Tomasz Figa
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Thanks a lot Chad!
Just to make sure, did you have a chance to test it with X11 apps,
that were reported to have incorrect colors last time we tried
enabling these formats? I.e.
https://bugs.freedesktop.org/show_bug.cgi?id=95071
tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
apps in that bug report and reply back with the results.
Post by Tomasz Figa
I just tested this set on Android and I'm getting wrong colors with this,
red and blue swapped.
Uh oh.
I did something dumb. When testing this on ARC++, I chose
a BLACK-AND-WHITE game! I'll retest with a color game.
It was pretty hard to keep coffee in mouth when reading this comment.
Post by Chad Versace
https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
Post by Tomasz Figa
I can 'fix' this by reordering the configs in intel_screen_make_configs so
that the new configs (RGBA, RGBX) required by Android are before the old
ones (BGRA, BGRX).
I think that would have exactly the opposite effect on the X11 apps I
mentioned before. In my testing, they were sensitive to the order of
configs, due to component bit masks not being considered in selection,
only bit depths.
Yes.
Post by Tomasz Figa
However, for Android, I thought EGL already used bit masks, so
possibly there is still an unrelated bug somewhere.
Android certainly inspects the channel masks, using dri2_add_config().
So I'm also curious why Tapani sees swapped channels. The
platform_android.c is patched with format hacks, and I assume the same
is true for android-ia. Maybe the hacks are intefering somehow. I'll
investigate.
We shouldn't have related 'hacks', but here are some changes that might be
https://github.com/android-ia/frameworks-native/commit/8237c9f8eb36d4f9ead40c8cb4a68ae9518d3c9f
https://github.com/android-ia/frameworks-native/pull/2/commits/bb29af0777e747effacd239565f52aad96c45558
https://github.com/android-ia/external-mesa/commit/ceac31ff7e53ec5034bc379d37ce365c000e9e4a
I confirmed that this series causes no regressions with Gnome and KDE
apps. For KDE, I used Amarok as the test app. To prove to myself
I *really* was testing this series, I tested the series with the new
formats placed at the end of the array, and then I tested again with the new
formats placed at the start of the array. As expected, Amarok's colors
were correct in the first test, and were wrong in the second test (blue
and red were swapped.
I still haven't succeeded in fully testing these patches against
Android, for a combination of technical and non-technical reasons. I'm
travelling this week, so I will probably be able to resume testing
Android apps on Thursday.
While not i965, I tested the similar code and change in gallium. With
EGL-MAIN: Native format mismatch: 0x1 != 0x5
That's coming from SurfaceFlinger.
Yes, I'm also seeing that error in logcat. I'm investigating now.
Chad Versace
2017-06-17 02:41:54 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
Post by Rob Herring
While not i965, I tested the similar code and change in gallium. With
EGL-MAIN: Native format mismatch: 0x1 != 0x5
That's coming from SurfaceFlinger.
Yes, I'm also seeing that error in logcat. I'm investigating now.
I solved the problem, at least on my Android machine. I wrote patches.

[Mesa-dev] [PATCH 0/5] egl/android: Change order of EGLConfig generation
https://lists.freedesktop.org/archives/mesa-dev/2017-June/159792.html

After combining this rgbx patch series with the above patch series,
everything looks good to me on my Android setup. I tested with the
Google Play Store and a Unity 3D benchmark.
Kenneth Graunke
2017-06-20 22:46:56 UTC
Reply
Permalink
Raw Message
Post by Chad Versace
More patches to break your formats... again ;)
The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
Chrome OS from updating its Android drivers, because earlier this year
Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
This patch series safely (hopefully?) fixes that problem.
If you want the meat, read patches 2 and 6.
mesa: Add _mesa_format_fallback_rgba_to_rgbx()
i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
i965: Rename some vague format members of brw_context
i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
i965: Move brw_context format arrays to intel_screen
i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
src/mesa/Android.gen.mk | 12 ++
src/mesa/Makefile.am | 7 +
src/mesa/Makefile.sources | 2 +
src/mesa/drivers/dri/i965/brw_blorp.c | 10 +-
src/mesa/drivers/dri/i965/brw_context.h | 5 +-
src/mesa/drivers/dri/i965/brw_meta_util.c | 2 +-
src/mesa/drivers/dri/i965/brw_surface_formats.c | 94 +++++++-----
src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 +-
src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 10 +-
src/mesa/drivers/dri/i965/intel_fbo.c | 34 ++++-
src/mesa/drivers/dri/i965/intel_fbo.h | 6 +-
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 +-
src/mesa/drivers/dri/i965/intel_screen.c | 39 ++++-
src/mesa/drivers/dri/i965/intel_screen.h | 5 +
src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
src/mesa/drivers/dri/i965/intel_tex_image.c | 19 ++-
src/mesa/main/.gitignore | 1 +
src/mesa/main/format_fallback.h | 31 ++++
src/mesa/main/format_fallback.py | 180 +++++++++++++++++++++++
19 files changed, 392 insertions(+), 73 deletions(-)
create mode 100644 src/mesa/main/format_fallback.h
create mode 100644 src/mesa/main/format_fallback.py
Patches 3-5 are:
Reviewed-by: Kenneth Graunke <***@whitecape.org>
Loading...