On Wed, Jul 12, 2017 at 1:34 AM, Rob Herring <r...@kernel.org> wrote: > On Tue, Jul 11, 2017 at 9:34 AM, Tomasz Figa <tf...@chromium.org> wrote: >> On Tue, Jul 11, 2017 at 11:16 PM, Rob Herring <r...@kernel.org> wrote: >>> On Tue, Jul 11, 2017 at 8:27 AM, Emil Velikov <emil.l.veli...@gmail.com> >>> wrote: >>>> From: Emil Velikov <emil.veli...@collabora.com> >>>> >>>> As said in the EGL_KHR_platform_android extensions >>>> >>>> For each EGLConfig that belongs to the Android platform, the >>>> EGL_NATIVE_VISUAL_ID attribute is an Android window format, such as >>>> WINDOW_FORMAT_RGBA_8888. >>>> >>>> Although it should be applicable overall. >>>> >>>> Even though we use HAL_PIXEL_FORMAT here, those are numerically >>>> identical to the WINDOW_FORMAT_ and AHARDWAREBUFFER_FORMAT_ ones. >>>> >>>> Barring the said format of course. That one is only listed in HAL. >>>> >>>> Keep in mind that even if we try to use the said format, you'll get >>>> caught by droid_create_surface(). The function compares the format of >>>> the underlying window, against the NATIVE_VISUAL_ID of the config. >>>> >>>> Unfortunatelly it only prints a warning, rather than error out, likely >>>> leading to visual corruption. >>>> >>>> While SDL will even call ANativeWindow_setBuffersGeometry() with the >>>> wrong format, and conviniently ignore the [expected] failure. >>>> >>>> Cc: mesa-sta...@lists.freedesktop.org >>>> Cc: Chad Versace <chadvers...@google.com> >>>> Cc: Tomasz Figa <tf...@chromium.org> >>>> Signed-off-by: Emil Velikov <emil.veli...@collabora.com> >>>> --- >>>> I'm about 99.99% sure the above is correct, but I haven't tested it. >>> >>> Isn't this going to break if there's no driver support for RGBA/RGBX >>> which is the case for stable (and master for gallium drvs). >> >> First of all, Android hardcodes HAL_PIXEL_FORMAT_RGBA_8888 in a number >> of places, which means that those users use a patched Android. However >> I'm not sure if we can just break them like this. I'll leave it to you >> guys, though. > > Yes, patched to work around mesa's lack of RGBA/X support. Not sure > why they went this route. Maybe RGBA/X support in mesa was attempted > before. > >> Other than that, CTS seems to require only RGBA_8888 and RGB_565, so >> this change is not going to affect compliance with unpatched Android. > > Okay, good to know.
I just found some interesting evidence that proves we should go forward with this patch: https://android.googlesource.com/platform/frameworks/native/+/master/opengl/libs/EGL/eglApi.cpp#455 Basically this is the EGL wrapper that is used for everything on Android. Whenever eglCreateWindowSurface() is called, it goes through the linked function first, which explicitly limits all the supported formats to RGBA_8888, RGBX_8888 and RGB_565, by forcing one of those matching EGL config's component sizes on the given native window (the native_window_set_buffers_format() call). I don't know how this worked for me before, but essentially it makes it impossible to pass dEQP-EGL tests with RGBA8888 (as per EGL config component widths only) window surface, because it's impossible to have eglCreateWindowSurface() accept BGRA_8888 and in result the config doesn't match the surface, which gives swapped channels in the buffers. Since the support for Gallium landed already, can we move forward with this patch? (If it's worth anything:) Acked-by: Tomasz Figa <tf...@chromium.org> Best regards, Tomasz _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
