Bug 356992 - Intel Mesa drivers not supported EGL_SWAP_BEHAVIOR_PRESERVED_BIT
Summary: Intel Mesa drivers not supported EGL_SWAP_BEHAVIOR_PRESERVED_BIT
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: egl (show other bugs)
Version: 5.5.1
Platform: Gentoo Packages Linux
: NOR major
Target Milestone: ---
Assignee: KWin default assignee
URL: https://git.reviewboard.kde.org/r/126...
Keywords:
: 356882 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-12-21 10:53 UTC by Yury Zhuravlev
Modified: 2016-01-29 10:24 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.5.4
thomas.luebking: ReviewRequest+


Attachments
Patch for fix swap behavior problem for intel (2.09 KB, patch)
2015-12-21 19:38 UTC, Yury Zhuravlev
Details
Fix init compositing for intel egl (1.34 KB, patch)
2016-01-07 12:06 UTC, Yury Zhuravlev
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Zhuravlev 2015-12-21 10:53:10 UTC
Subj. You can't use egl backend. 
I removed EGL_SWAP_BEHAVIOR_PRESERVED_BIT for EGL_SURFACE_TYPE and kwin like working.
About mesa: http://lists.freedesktop.org/archives/mesa-dev/2015-November/100869.html

Reproducible: Always
Comment 1 Thomas Lübking 2015-12-21 12:08:10 UTC
Does this work for you?

diff --git a/eglonxbackend.cpp b/eglonxbackend.cpp
index 480e533..06e5a16 100644
--- a/eglonxbackend.cpp
+++ b/eglonxbackend.cpp
@@ -266,7 +266,7 @@ EGLSurface EglOnXBackend::createSurface(xcb_window_t window)
 
 bool EglOnXBackend::initBufferConfigs()
 {
-    const EGLint config_attribs[] = {
+    EGLint config_attribs[] = {
         EGL_SURFACE_TYPE,         EGL_WINDOW_BIT|EGL_SWAP_BEHAVIOR_PRESERVED_BIT,
         EGL_RED_SIZE,             1,
         EGL_GREEN_SIZE,           1,
@@ -280,8 +280,11 @@ bool EglOnXBackend::initBufferConfigs()
     EGLint count;
     EGLConfig configs[1024];
     if (eglChooseConfig(eglDisplay(), config_attribs, configs, 1024, &count) == EGL_FALSE) {
-        qCCritical(KWIN_CORE) << "choose config failed";
-        return false;
+        config_attribs[1] = EGL_WINDOW_BIT;
+        if (eglChooseConfig(eglDisplay(), config_attribs, configs, 1024, &count) == EGL_FALSE) {
+            qCCritical(KWIN_CORE) << "choose config failed";
+            return false;
+        }
     }
 
     ScopedCPointer<xcb_get_window_attributes_reply_t> attribs(xcb_get_window_attributes_reply(m_connection,
Comment 2 Yury Zhuravlev 2015-12-21 19:29:24 UTC
> Does this work for you?

Nope becouse EGL_BAD_CONFIG return only in eglCreatePlatformWindowSurfaceEXT and eglChooseConfig well done.
Comment 3 Yury Zhuravlev 2015-12-21 19:38:30 UTC
Created attachment 96244 [details]
Patch for fix swap behavior problem for intel
Comment 4 Yury Zhuravlev 2015-12-21 19:41:05 UTC
(In reply to Uriy Zhuravlev from comment #3)
> Created attachment 96244 [details]
> Patch for fix swap behavior problem for intel

All effects work but I don't like EGL_BAD_MATCH after EGL init...
Comment 5 Thomas Lübking 2015-12-22 23:00:53 UTC
Meh. Martin simply wanted to avoid it if buffer_age is supported (what makes sense).
Do you have EGL_EXT_buffer_age in the client extensions?

qDebug() << "buffer age?" << hasClientExtension(QByteArrayLiteral("EGL_EXT_buffer_age"));

in ::initRenderingContext()
Comment 6 Yury Zhuravlev 2015-12-27 11:20:26 UTC
(In reply to Thomas Lübking from comment #5)
> Meh. Martin simply wanted to avoid it if buffer_age is supported (what makes
> sense).
> Do you have EGL_EXT_buffer_age in the client extensions?
> 
> qDebug() << "buffer age?" <<
> hasClientExtension(QByteArrayLiteral("EGL_EXT_buffer_age"));
> 
> in ::initRenderingContext()

buffer age? false

Sorry for the long answer.
Comment 7 Thomas Lübking 2015-12-29 14:53:19 UTC
Errr... *after*  the "initClientExtensions()" call???
Can you in case post the output of "es2_info"?
Comment 8 Yury Zhuravlev 2015-12-29 15:07:02 UTC
Please:

EGL_VERSION: 1.4 (DRI2)
EGL_VENDOR: Mesa Project
EGL_EXTENSIONS:
    EGL_CHROMIUM_sync_control, EGL_EXT_buffer_age, 
    EGL_EXT_create_context_robustness, EGL_EXT_image_dma_buf_import, 
    EGL_KHR_create_context, EGL_KHR_fence_sync, 
    EGL_KHR_get_all_proc_addresses, EGL_KHR_gl_renderbuffer_image, 
    EGL_KHR_gl_texture_2D_image, EGL_KHR_gl_texture_cubemap_image, 
    EGL_KHR_image, EGL_KHR_image_base, EGL_KHR_image_pixmap, 
    EGL_KHR_surfaceless_context, EGL_KHR_wait_sync, 
    EGL_MESA_configless_context, EGL_MESA_drm_image, 
    EGL_MESA_image_dma_buf_export, EGL_NOK_texture_from_pixmap, 
    EGL_WL_bind_wayland_display
EGL_CLIENT_APIS: OpenGL OpenGL_ES OpenGL_ES2 OpenGL_ES3 
GL_VERSION: OpenGL ES 3.0 Mesa 11.2.0-devel (git-b201a6e)
GL_RENDERER: Mesa DRI Intel(R) Haswell Mobile 
GL_EXTENSIONS:
    GL_EXT_blend_minmax, GL_EXT_multi_draw_arrays, 
    GL_EXT_texture_filter_anisotropic, GL_EXT_texture_compression_dxt1, 
    GL_EXT_texture_format_BGRA8888, GL_OES_compressed_ETC1_RGB8_texture, 
    GL_OES_depth24, GL_OES_element_index_uint, GL_OES_fbo_render_mipmap, 
    GL_OES_mapbuffer, GL_OES_rgb8_rgba8, GL_OES_standard_derivatives, 
    GL_OES_stencil8, GL_OES_texture_3D, GL_OES_texture_float, 
    GL_OES_texture_float_linear, GL_OES_texture_half_float, 
    GL_OES_texture_half_float_linear, GL_OES_texture_npot, GL_OES_EGL_image, 
    GL_OES_depth_texture, GL_OES_packed_depth_stencil, 
    GL_EXT_texture_type_2_10_10_10_REV, GL_OES_get_program_binary, 
    GL_APPLE_texture_max_level, GL_EXT_discard_framebuffer, 
    GL_EXT_read_format_bgra, GL_NV_fbo_color_attachments, 
    GL_OES_EGL_image_external, GL_OES_EGL_sync, GL_OES_vertex_array_object, 
    GL_ANGLE_texture_compression_dxt3, GL_ANGLE_texture_compression_dxt5, 
    GL_EXT_texture_rg, GL_EXT_unpack_subimage, GL_NV_draw_buffers, 
    GL_NV_read_buffer, GL_NV_read_depth, GL_NV_read_depth_stencil, 
    GL_NV_read_stencil, GL_EXT_draw_buffers, GL_EXT_map_buffer_range, 
    GL_KHR_debug, GL_OES_depth_texture_cube_map, GL_OES_surfaceless_context, 
    GL_EXT_color_buffer_float, GL_EXT_separate_shader_objects, 
    GL_EXT_shader_integer_mix, GL_INTEL_performance_query, 
    GL_EXT_draw_elements_base_vertex, GL_KHR_context_flush_control, 
    GL_OES_draw_elements_base_vertex, GL_EXT_blend_func_extended
Comment 9 Thomas Lübking 2015-12-29 16:21:17 UTC
(In reply to Uriy Zhuravlev from comment #8)
> Please:

"Here you are" ;-)

The extension seems there, to be clear, this:

diff --git a/eglonxbackend.cpp b/eglonxbackend.cpp
index 480e533..d7e2fa0 100644
--- a/eglonxbackend.cpp
+++ b/eglonxbackend.cpp
@@ -166,6 +166,7 @@ void EglOnXBackend::init()
 bool EglOnXBackend::initRenderingContext()
 {
     initClientExtensions();
+    qDebug() << "buffer age?" << hasClientExtension(QByteArrayLiteral("EGL_EXT_buffer_age"));
     EGLDisplay dpy;
 
     // Use eglGetPlatformDisplayEXT() to get the display pointer


prints "false" on your side?
Comment 10 Yury Zhuravlev 2015-12-29 16:42:52 UTC
(In reply to Thomas Lübking from comment #9)
> prints "false" on your side?

Yes. My past result printed after initClientExtensions. :)
Comment 11 Martin Flöser 2016-01-04 10:50:17 UTC
which mesa version has that new behavior? When we discussed it on the mailing list my mesa didn't have it yet, so I didn't look into the implementation.
Comment 12 Yury Zhuravlev 2016-01-04 12:42:55 UTC
(In reply to Martin Gräßlin from comment #11)
> which mesa version has that new behavior? When we discussed it on the
> mailing list my mesa didn't have it yet, so I didn't look into the
> implementation.

Actual git master.
Comment 13 Thomas Lübking 2016-01-04 16:03:45 UTC
What puzzles me is that EGL_EXT_buffer_age seems to be available, so why would it not be in the list of client extensions?
Comment 14 Yury Zhuravlev 2016-01-04 23:56:27 UTC
(In reply to Thomas Lübking from comment #13)
> What puzzles me is that EGL_EXT_buffer_age seems to be available, so why
> would it not be in the list of client extensions?

Because in:
void AbstractEglBackend::initClientExtensions()

eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS) returned only:
EGL_EXT_client_extensions EGL_EXT_platform_base EGL_EXT_platform_wayland EGL_EXT_platform_x11 EGL_KHR_client_get_all_proc_addresses EGL_MESA_platform_gbm

and yes es2_info relay difference between eglinfo:
EGL API version: 1.4
EGL vendor string: Mesa Project
EGL version string: 1.4 (DRI2)
EGL client APIs: OpenGL OpenGL_ES OpenGL_ES2 OpenGL_ES3 
EGL extensions string:
    EGL_CHROMIUM_sync_control EGL_EXT_buffer_age
    EGL_EXT_create_context_robustness EGL_EXT_image_dma_buf_import
    EGL_KHR_create_context EGL_KHR_fence_sync
    EGL_KHR_get_all_proc_addresses EGL_KHR_gl_renderbuffer_image
    EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_cubemap_image
    EGL_KHR_image EGL_KHR_image_base EGL_KHR_image_pixmap
    EGL_KHR_surfaceless_context EGL_KHR_wait_sync
    EGL_MESA_configless_context EGL_MESA_drm_image
    EGL_MESA_image_dma_buf_export EGL_NOK_texture_from_pixmap
    EGL_WL_bind_wayland_display
EGL client extensions string:
    EGL_EXT_client_extensions EGL_EXT_platform_base
    EGL_EXT_platform_wayland EGL_EXT_platform_x11
    EGL_KHR_client_get_all_proc_addresses EGL_MESA_platform_gbm

"EGL client extensions string"  -- It solves your puzzle.
Comment 15 Yury Zhuravlev 2016-01-06 11:20:19 UTC
Hey. Do you have any thoughts? I think es2 != egl. 

PS Now I have to use a LXQT instead KDE. Plasma5 + kwin instability at Intel. :(
Comment 16 Thomas Lübking 2016-01-06 11:56:01 UTC
We need to pre-fetch egl extensions init to know wheter BA is supported, untested patch:

diff --git a/eglonxbackend.cpp b/eglonxbackend.cpp
index 480e533..abacdaa 100644
--- a/eglonxbackend.cpp
+++ b/eglonxbackend.cpp
@@ -89,6 +89,8 @@ EglOnXBackend::~EglOnXBackend()
 
 void EglOnXBackend::init()
 {
+    initEgl();       // required to toggle
+    initBufferAge(); // EGL_SWAP_BEHAVIOR_PRESERVED_BIT
     if (!initRenderingContext()) {
         setFailed(QStringLiteral("Could not initialize rendering context"));
         return;
@@ -267,7 +269,7 @@ EGLSurface EglOnXBackend::createSurface(xcb_window_t window)
 bool EglOnXBackend::initBufferConfigs()
 {
     const EGLint config_attribs[] = {
-        EGL_SURFACE_TYPE,         EGL_WINDOW_BIT|EGL_SWAP_BEHAVIOR_PRESERVED_BIT,
+        EGL_SURFACE_TYPE,         EGL_WINDOW_BIT|(supportsBufferAge() ? 0 : EGL_SWAP_BEHAVIOR_PRESERVED_BIT),
         EGL_RED_SIZE,             1,
         EGL_GREEN_SIZE,           1,
         EGL_BLUE_SIZE,            1,
Comment 17 Yury Zhuravlev 2016-01-06 12:30:34 UTC
1. initEGL(); insted initEgl(); 
2. Nope... it's not work.
Comment 18 Yury Zhuravlev 2016-01-06 12:57:39 UTC
EGL_WINDOW_BIT|(supportsBufferAge() ? 0 : EGL_SWAP_BEHAVIOR_PRESERVED_BIT)
maybe you mean:
EGL_WINDOW_BIT|(supportsBufferAge() ? EGL_SWAP_BEHAVIOR_PRESERVED_BIT : 0)

?
Comment 19 Thomas Lübking 2016-01-06 17:26:33 UTC
No, we only want to skip that bit if we've buffer_age support anyway.
I've not looked into it, but guess the extension check requires a context, what means we cannot depend this bit on the absence of buffer_age - or we cannot avoid surface recreation - either because it failed due to that bit or because we skipped the bit initially and afterwards figure we need it because we lack buffer_age support. This sucks.

Can you debug the result of initEGL?

diff --git a/eglonxbackend.cpp b/eglonxbackend.cpp
index 480e533..abacdaa 100644
--- a/eglonxbackend.cpp
+++ b/eglonxbackend.cpp
@@ -89,6 +89,8 @@ EglOnXBackend::~EglOnXBackend()
 
 void EglOnXBackend::init()
 {
+    initEgl();       // required to toggle
+    initBufferAge(); // EGL_SWAP_BEHAVIOR_PRESERVED_BIT
     if (!initRenderingContext()) {
         setFailed(QStringLiteral("Could not initialize rendering context"));
         return;
@@ -267,7 +269,7 @@ EGLSurface EglOnXBackend::createSurface(xcb_window_t window)
 bool EglOnXBackend::initBufferConfigs()
 {
     const EGLint config_attribs[] = {
-        EGL_SURFACE_TYPE,         EGL_WINDOW_BIT|EGL_SWAP_BEHAVIOR_PRESERVED_BIT,
+        EGL_SURFACE_TYPE,         EGL_WINDOW_BIT|(supportsBufferAge() ? 0 : EGL_SWAP_BEHAVIOR_PRESERVED_BIT),
         EGL_RED_SIZE,             1,
         EGL_GREEN_SIZE,           1,
         EGL_BLUE_SIZE,            1,
diff --git a/libkwineffects/kwinglutils.cpp b/libkwineffects/kwinglutils.cpp
index 438b785..b44b821 100644
--- a/libkwineffects/kwinglutils.cpp
+++ b/libkwineffects/kwinglutils.cpp
@@ -101,6 +101,7 @@ void initEGL()
     eglVersion = MAKE_GL_VERSION(major, minor, 0);
     const QByteArray string = eglQueryString(dpy, EGL_EXTENSIONS);
     eglExtensions = string.split(' ');
+    qDebug() << eglExtensions;
     eglResolveFunctions();
 }
Comment 20 Yury Zhuravlev 2016-01-06 21:15:02 UTC
I have two outputs, first:
("EGL_EXT_client_extensions", "EGL_EXT_platform_base", "EGL_EXT_platform_wayland", "EGL_EXT_platform_x11", "EGL_KHR_client_get_all_proc_addresses", "EGL_MESA_platform_gbm")
extension "EGL_EXT_buffer_age"    () () ("EGL_EXT_client_extensions", "EGL_EXT_platform_base", "EGL_EXT_platform_wayland", "EGL_EXT_platform_x11", "EGL_KHR_client_get_all_proc_addresses", "EGL_MESA_platform_gbm") 

and second:
("EGL_CHROMIUM_sync_control", "EGL_EXT_buffer_age", "EGL_EXT_create_context_robustness", "EGL_EXT_image_dma_buf_import", "EGL_KHR_create_context", "EGL_KHR_fence_sync", "EGL_KHR_get_all_proc_addresses", "EGL_KHR_gl_renderbuffer_image", "EGL_KHR_gl_texture_2D_image", "EGL_KHR_gl_texture_cubemap_image", "EGL_KHR_image", "EGL_KHR_image_base", "EGL_KHR_image_pixmap", "EGL_KHR_surfaceless_context", "EGL_KHR_wait_sync", "EGL_MESA_configless_context", "EGL_MESA_drm_image", "EGL_MESA_image_dma_buf_export", "EGL_NOK_texture_from_pixmap", "EGL_WL_bind_wayland_display", "")
Comment 21 Thomas Lübking 2016-01-06 23:20:29 UTC
The first one looks like other debug out (on your local system, not caused by the patch) of the client extensions and a match of EGL_EXT_buffer_age against all three extension lists.

The second one looks promising and things should work. As the first one suggest a local patch - what's your exact diff agains origin/master and/or would it work as expected if you strip everything except for the patch in comment #16 ?
Comment 22 Yury Zhuravlev 2016-01-07 12:06:20 UTC
Created attachment 96500 [details]
Fix init compositing for intel egl

Excuse me, I'm a little tired of this debug. So I decided to investigate the problem itself.
Attached is my new patch.
The main difference from your proposal is what I use:
EGLDisplay dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);
insted:
EGLDisplay dpy = eglGetCurrentDisplay();

because from docs:
"eglGetCurrentDisplay returns the current EGL display connection for the current EGL rendering context, as specified by eglMakeCurrent. If no context is current, EGL_NO_DISPLAY is returned."

and eglMakeCurrent call only from initRenderingContext. 

For EGL_NO_DISPLAY glQueryString(dpy, EGL_EXTENSIONS) returns only EGL client extensions. 

This patch solve my problem... While it may be ugly. It may be necessary to add arg to initEGL?
Comment 23 Thomas Lübking 2016-01-07 12:14:22 UTC
if (dpy == EGL_NO_DISPLAY)
   dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);

should do.
Martin?
Comment 24 Yury Zhuravlev 2016-01-11 09:30:46 UTC
> if (dpy == EGL_NO_DISPLAY)
>   dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);

I will try this. Although it seems to me a very good solution.

Martin?
Comment 25 Martin Flöser 2016-01-11 11:32:30 UTC
if it works I'm fine with it.
Comment 26 Thomas Lübking 2016-01-17 21:08:52 UTC
Let's get this off the list
https://git.reviewboard.kde.org/r/126783/
Comment 27 Yury Zhuravlev 2016-01-17 21:16:59 UTC
>Let's get this off the list

Good! Patch works well.
Comment 28 AnAkkk 2016-01-17 21:27:35 UTC
There seem to be a mistake in the patch:

    EGLDisplay dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);
    if (dpy == EGL_NO_DISPLAY)
        dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY);

I guess the first line was not supposed to be changed, as this is now calling the same function again with the same parameters in case it fails.
Comment 29 Thomas Lübking 2016-01-17 21:43:14 UTC
Hehe - many thanks for covering my ass =)
Comment 30 AnAkkk 2016-01-17 21:46:07 UTC
No problem :)

I don't know anything about EGL, but shouldn't the call be eglGetCurrentDisplay() instead of eglGetDisplay() like it is in the updated patched version? Seems like eglGetDisplay always take a parameter.
Comment 31 Yury Zhuravlev 2016-01-17 22:22:14 UTC
(In reply to AnAkkk from comment #30)
> No problem :)
> 
> I don't know anything about EGL, but shouldn't the call be
> eglGetCurrentDisplay() instead of eglGetDisplay() like it is in the updated
> patched version? Seems like eglGetDisplay always take a parameter.

Yep, current version is wrong. :)
Comment 32 Thomas Lübking 2016-01-17 22:23:51 UTC
This kinda starts to become embarrassing... :-\
Comment 33 Yury Zhuravlev 2016-01-17 22:26:42 UTC
(In reply to Thomas Lübking from comment #32)
> This kinda starts to become embarrassing... :-\

It happens to everyone. The current version is going and work perfectly. I hope it will be quick in the upstream.
Comment 34 Thomas Lübking 2016-01-18 21:35:55 UTC
Git commit e641022bf9482a11209577b5654cd43231be0755 by Thomas Lübking.
Committed on 18/01/2016 at 10:02.
Pushed by luebking into branch 'Plasma/5.5'.

skip SWAP_BEHAVIOR_PRESERVED for supportsBufferAge

pointless and unsupported on latter MESA/DRI3
http://lists.freedesktop.org/archives/mesa-dev/2015-November/100869.html

Thanks to Uriy Zhuravlev for reporting and investigation
FIXED-IN: 5.5.4

M  +3    -1    eglonxbackend.cpp
M  +2    -0    libkwineffects/kwinglutils.cpp

http://commits.kde.org/kwin/e641022bf9482a11209577b5654cd43231be0755
Comment 35 Thomas Lübking 2016-01-18 21:36:48 UTC
Git commit c7aefc6b6b62c704929fc74338da2ca0d07b547a by Thomas Lübking.
Committed on 18/01/2016 at 21:32.
Pushed by luebking into branch 'master'.

skip SWAP_BEHAVIOR_PRESERVED for supportsBufferAge

pointless and unsupported on latter MESA/DRI3
http://lists.freedesktop.org/archives/mesa-dev/2015-November/100869.html

Thanks to Uriy Zhuravlev for reporting and investigation
Related: bug 356882
FIXED-IN: 5.5.4
REVIEW: 126783

M  +3    -1    eglonxbackend.cpp
M  +2    -0    libkwineffects/kwinglutils.cpp

http://commits.kde.org/kwin/c7aefc6b6b62c704929fc74338da2ca0d07b547a
Comment 36 Thomas Lübking 2016-01-29 10:24:16 UTC
*** Bug 356882 has been marked as a duplicate of this bug. ***