[Mesa-dev] [PATCH 0/6] i965: Add RGBX, RGBA configs, even on gen9

[Chad Versace]

On Fri 09 Jun 2017, Tapani Pälli wrote:
> 
> 
> On 06/08/2017 09:27 PM, Chad Versace wrote:
> > On Thu 08 Jun 2017, Tomasz Figa wrote:
> > > On Thu, Jun 8, 2017 at 4:08 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
> > > > 
> > > > On 06/08/2017 09:36 AM, Tapani Pälli wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 06/08/2017 06:05 AM, Tomasz Figa wrote:
> > > > > > 
> > > > > > On Wed, Jun 7, 2017 at 5:36 AM, Chad Versace <chadversary at chromium.org>
> > > > > > wrote:
> > > > > > > 
> > > > > > > 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
> > > > > > 
> > > > > > 
> > > > > > 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.
> > 
> > > > > 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.
> 
> > https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
> > 
> > > > 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.
> > 
> > > 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

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.