Differential D10750 Diff 60655 platformsupport/scenes/opengl/abstract_egl_backend.cpp

Changeset View

Standalone View

View Options

platformsupport/scenes/opengl/abstract_egl_backend.cpp

Show All 12 Lines
13but WITHOUT ANY WARRANTY; without even the implied warranty of 13​but WITHOUT ANY WARRANTY; without even the implied warranty of
14MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 14​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15GNU General Public License for more details. 15​GNU General Public License for more details.
16 16​
17You should have received a copy of the GNU General Public License 17​You should have received a copy of the GNU General Public License
18along with this program. If not, see <http://www.gnu.org/licenses/>. 18​along with this program. If not, see <http://www.gnu.org/licenses/>.
19*********************************************************************/ 19​*********************************************************************/
20#include "abstract_egl_backend.h" 20​#include "abstract_egl_backend.h"
21​#include "linux_dmabuf.h"
21#include "texture.h" 22​#include "texture.h"
22#include "composite.h" 23​#include "composite.h"
23#include "egl_context_attribute_builder.h" 24​#include "egl_context_attribute_builder.h"
24#include "options.h" 25​#include "options.h"
25#include "platform.h" 26​#include "platform.h"
26#include "scene.h" 27​#include "scene.h"
27#include "wayland_server.h" 28​#include "wayland_server.h"
28#include <KWayland/Server/buffer_interface.h> 29​#include <KWayland/Server/buffer_interface.h>
▲ Show 20 Lines • Show All 79 Lines • ▼ Show 20 Line(s)98{
108 if (error != EGL_SUCCESS) { 109​ if (error != EGL_SUCCESS) {
109 qCWarning(KWIN_OPENGL) << "Error during eglInitialize " << error; 110​ qCWarning(KWIN_OPENGL) << "Error during eglInitialize " << error;
110 return false; 111​ return false;
111 } 112​ }
112 qCDebug(KWIN_OPENGL) << "Egl Initialize succeeded"; 113​ qCDebug(KWIN_OPENGL) << "Egl Initialize succeeded";
113 114​
114 if (eglBindAPI(isOpenGLES() ? EGL_OPENGL_ES_API : EGL_OPENGL_API) == EGL_FALSE) { 115​ if (eglBindAPI(isOpenGLES() ? EGL_OPENGL_ES_API : EGL_OPENGL_API) == EGL_FALSE) {
115 qCCritical(KWIN_OPENGL) << "bind OpenGL API failed"; 116​ qCCritical(KWIN_OPENGL) << "bind OpenGL API failed";
116 return false; 117​ return false;
anthonyfieroniUnsubmitted
Not Done

Call delete dmabuf will have same effect.

anthonyfieroni: Call delete dmabuf will have same effect.
fredrikUnsubmitted
Not Done

No, it will not. It will leave a dangling pointer in the wl_resource, which will result in a double-free when the client deletes the buffer. Or a segfault the next time it tries to attach the buffer to a surface.

I will expand on this in a separate comment.

fredrik: No, it will not. It will leave a dangling pointer in the wl_resource, which will result in a…
117 } 118​ }
118 qCDebug(KWIN_OPENGL) << "EGL version: " << major << "." << minor; 119​ qCDebug(KWIN_OPENGL) << "EGL version: " << major << "." << minor;
119 const QByteArray eglExtensions = eglQueryString(m_display, EGL_EXTENSIONS); 120​ const QByteArray eglExtensions = eglQueryString(m_display, EGL_EXTENSIONS);
120 setExtensions(eglExtensions.split(' ')); 121​ setExtensions(eglExtensions.split(' '));
121 return true; 122​ return true;
122} 123​}
123 124​
124typedef void (*eglFuncPtr)(); 125​typedef void (*eglFuncPtr)();
Show All 39 Lines164 if (waylandServer()->display()->eglDisplay() != eglDisplay()) {
164 if (!eglBindWaylandDisplayWL(eglDisplay(), *(WaylandServer::self()->display()))) { 165​ if (!eglBindWaylandDisplayWL(eglDisplay(), *(WaylandServer::self()->display()))) {
165 eglUnbindWaylandDisplayWL = nullptr; 166​ eglUnbindWaylandDisplayWL = nullptr;
166 eglQueryWaylandBufferWL = nullptr; 167​ eglQueryWaylandBufferWL = nullptr;
167 } else { 168​ } else {
168 waylandServer()->display()->setEglDisplay(eglDisplay()); 169​ waylandServer()->display()->setEglDisplay(eglDisplay());
169 } 170​ }
170 } 171​ }
171 } 172​ }
173​
174​ LinuxDmabuf::factory(this);
172} 175​}
173 176​
174void AbstractEglBackend::initClientExtensions() 177​void AbstractEglBackend::initClientExtensions()
175{ 178​{
176 // Get the list of client extensions 179​ // Get the list of client extensions
177 const char* clientExtensionsCString = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS); 180​ const char* clientExtensionsCString = eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
178 const QByteArray clientExtensionsString = QByteArray::fromRawData(clientExtensionsCString, qstrlen(clientExtensionsCString)); 181​ const QByteArray clientExtensionsString = QByteArray::fromRawData(clientExtensionsCString, qstrlen(clientExtensionsCString));
179 if (clientExtensionsString.isEmpty()) { 182​ if (clientExtensionsString.isEmpty()) {
▲ Show 20 Lines • Show All 133 Lines • ▼ Show 20 Line(s)
313} 316​}
314 317​
315void AbstractEglBackend::setSurface(const EGLSurface &surface) 318​void AbstractEglBackend::setSurface(const EGLSurface &surface)
316{ 319​{
317 m_surface = surface; 320​ m_surface = surface;
318 kwinApp()->platform()->setSceneEglSurface(surface); 321​ kwinApp()->platform()->setSceneEglSurface(surface);
319} 322​}
320 323​
321AbstractEglTexture::AbstractEglTexture(SceneOpenGLTexture *texture, AbstractEglBackend *backend) 324​AbstractEglTexture::AbstractEglTexture(SceneOpenGLTexture *texture, AbstractEglBackend *backend)
romanggAuthorUnsubmitted
Done

Name should be more descriptive in relation to functionality, also it's not a signal, so "aboutTo" imo not recommended.

Suggestion: removeDmabufBuffer

romangg: Name should be more descriptive in relation to functionality, also it's not a signal, so…
322 : SceneOpenGLTexturePrivate() 325​ : SceneOpenGLTexturePrivate()
323 , q(texture) 326​ , q(texture)
324 , m_backend(backend) 327​ , m_backend(backend)
325 , m_image(EGL_NO_IMAGE_KHR) 328​ , m_image(EGL_NO_IMAGE_KHR)
326{ 329​{
327 m_target = GL_TEXTURE_2D; 330​ m_target = GL_TEXTURE_2D;
328} 331​}
329 332​
Show All 17 Lines349 if (updateFromFBO(pixmap->fbo())) {
347 return true; 350​ return true;
348 } 351​ }
349 return false; 352​ return false;
350 } 353​ }
351 // try Wayland loading 354​ // try Wayland loading
352 if (auto s = pixmap->surface()) { 355​ if (auto s = pixmap->surface()) {
353 s->resetTrackedDamage(); 356​ s->resetTrackedDamage();
354 } 357​ }
355 if (buffer->shmBuffer()) { 358​ if (buffer->linuxDmabufBuffer()) {
359​ return loadDmabufTexture(buffer);
360​ } else if (buffer->shmBuffer()) {
356 return loadShmTexture(buffer); 361​ return loadShmTexture(buffer);
357 } else { ​
358 return loadEglTexture(buffer); ​
359 } 362​ }
363​ return loadEglTexture(buffer);
360} 364​}
361 365​
362void AbstractEglTexture::updateTexture(WindowPixmap *pixmap) 366​void AbstractEglTexture::updateTexture(WindowPixmap *pixmap)
363{ 367​{
364 const auto &buffer = pixmap->buffer(); 368​ const auto &buffer = pixmap->buffer();
365 if (buffer.isNull()) { 369​ if (buffer.isNull()) {
366 const auto &fbo = pixmap->fbo(); 370​ const auto &fbo = pixmap->fbo();
367 if (!fbo.isNull()) { 371​ if (!fbo.isNull()) {
368 if (m_texture != fbo->texture()) { 372​ if (m_texture != fbo->texture()) {
369 updateFromFBO(fbo); 373​ updateFromFBO(fbo);
zzagUnsubmitted
Not Done

What's holding us from doing that?

zzag: What's holding us from doing that?
fredrikUnsubmitted
Not Done

We would need to create a separate EGL image and a separate texture for each plane.
The scene would need to bind each of those textures to separate texture binding points, and we would need to generate and use a shader that samples texels from each plane and performs YUV to RGB conversion.

fredrik: We would need to create a separate EGL image and a separate texture for each plane. The scene…
romanggAuthorUnsubmitted
Not Done

Thanks for the explanation @fredrik. It wasn't clear to me how multi-planes buffers are handled (also that it's the same notion as for hw-planes is annoying!).

Until we support multi-plane buffers shouldn't we then filter out all multi-plane formats in the supportedFormats call such that the client does not even try to use these instead of just failing with null here?

romangg: Thanks for the explanation @fredrik. It wasn't clear to me how multi-planes buffers are handled…
370 } 374​ }
371 return; 375​ return;
372 } 376​ }
373 return; 377​ return;
374 } 378​ }
375 auto s = pixmap->surface(); 379​ auto s = pixmap->surface();
380​ if (DmabufBuffer *dmabuf = static_cast<DmabufBuffer *>(buffer->linuxDmabufBuffer())) {
381​ q->bind();
382​ glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, (GLeglImageOES) dmabuf->image());
383​ q->unbind();
384​ if (m_image != EGL_NO_IMAGE_KHR) {
385​ eglDestroyImageKHR(m_backend->eglDisplay(), m_image);
zzagUnsubmitted
Not Done

Perhaps we need to add modifier only if EGL_EXT_image_dma_buf_import_modifiers is present.

zzag: Perhaps we need to add modifier only if EGL_EXT_image_dma_buf_import_modifiers is present.
386​ }
387​ m_image = EGL_NO_IMAGE_KHR; // The wl_buffer has ownership of the image
388​ // The origin in a dmabuf-buffer is at the upper-left corner, so the meaning
389​ // of Y-inverted is the inverse of OpenGL.
390​ const bool yInverted = !(dmabuf->flags() & KWayland::Server::LinuxDmabufUnstableV1Interface::YInverted);
391​ if (m_size != dmabuf->size() || yInverted != q->isYInverted()) {
392​ m_size = dmabuf->size();
393​ q->setYInverted(yInverted);
394​ }
395​ if (s) {
396​ s->resetTrackedDamage();
397​ }
398​ return;
399​ }
376 if (!buffer->shmBuffer()) { 400​ if (!buffer->shmBuffer()) {
377 q->bind(); 401​ q->bind();
378 EGLImageKHR image = attach(buffer); 402​ EGLImageKHR image = attach(buffer);
379 q->unbind(); 403​ q->unbind();
380 if (image != EGL_NO_IMAGE_KHR) { 404​ if (image != EGL_NO_IMAGE_KHR) {
405​ if (m_image != EGL_NO_IMAGE_KHR) {
381 eglDestroyImageKHR(m_backend->eglDisplay(), m_image); 406​ eglDestroyImageKHR(m_backend->eglDisplay(), m_image);
407​ }
romanggAuthorUnsubmitted
Done

rm

romangg: rm
romanggAuthorUnsubmitted
Done

Nope, better not. Overlooked the m_.

romangg: Nope, better not. Overlooked the `m_`.
382 m_image = image; 408​ m_image = image;
383 } 409​ }
384 if (s) { 410​ if (s) {
385 s->resetTrackedDamage(); 411​ s->resetTrackedDamage();
386 } 412​ }
387 return; 413​ return;
388 } 414​ }
389 // shm fallback 415​ // shm fallback
▲ Show 20 Lines • Show All 109 Lines • ▼ Show 20 Line(s)524 if (EGL_NO_IMAGE_KHR == m_image) {
499 qCDebug(KWIN_OPENGL) << "failed to create egl image"; 525​ qCDebug(KWIN_OPENGL) << "failed to create egl image";
500 q->discard(); 526​ q->discard();
501 return false; 527​ return false;
502 } 528​ }
503 529​
504 return true; 530​ return true;
505} 531​}
506 532​
533​bool AbstractEglTexture::loadDmabufTexture(const QPointer< KWayland::Server::BufferInterface > &buffer)
534​{
535​ DmabufBuffer *dmabuf = static_cast<DmabufBuffer *>(buffer->linuxDmabufBuffer());
536​ if (!dmabuf || dmabuf->image() == EGL_NO_IMAGE_KHR) {
537​ qCritical(KWIN_OPENGL) << "Invalid dmabuf-based wl_buffer";
538​ q->discard();
539​ return false;
540​ }
541​
542​ assert(m_image == EGL_NO_IMAGE_KHR);
543​
544​ glGenTextures(1, &m_texture);
545​ q->setWrapMode(GL_CLAMP_TO_EDGE);
546​ q->setFilter(GL_NEAREST);
547​ q->bind();
548​ glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, (GLeglImageOES) dmabuf->image());
549​ q->unbind();
550​
551​ m_size = dmabuf->size();
552​ q->setYInverted(!(dmabuf->flags() & KWayland::Server::LinuxDmabufUnstableV1Interface::YInverted));
553​
554​ return true;
555​}
556​
507EGLImageKHR AbstractEglTexture::attach(const QPointer< KWayland::Server::BufferInterface > &buffer) 557​EGLImageKHR AbstractEglTexture::attach(const QPointer< KWayland::Server::BufferInterface > &buffer)
508{ 558​{
509 EGLint format, yInverted; 559​ EGLint format, yInverted;
510 eglQueryWaylandBufferWL(m_backend->eglDisplay(), buffer->resource(), EGL_TEXTURE_FORMAT, &format); 560​ eglQueryWaylandBufferWL(m_backend->eglDisplay(), buffer->resource(), EGL_TEXTURE_FORMAT, &format);
511 if (format != EGL_TEXTURE_RGB && format != EGL_TEXTURE_RGBA) { 561​ if (format != EGL_TEXTURE_RGB && format != EGL_TEXTURE_RGBA) {
512 qCDebug(KWIN_OPENGL) << "Unsupported texture format: " << format; 562​ qCDebug(KWIN_OPENGL) << "Unsupported texture format: " << format;
513 return EGL_NO_IMAGE_KHR; 563​ return EGL_NO_IMAGE_KHR;
514 } 564​ }
Show All 27 Lines586{
542 q->setWrapMode(GL_CLAMP_TO_EDGE); 592​ q->setWrapMode(GL_CLAMP_TO_EDGE);
543 q->setFilter(GL_LINEAR); 593​ q->setFilter(GL_LINEAR);
544 q->setYInverted(false); 594​ q->setYInverted(false);
545 updateMatrix(); 595​ updateMatrix();
546 return true; 596​ return true;
547} 597​}
548 598​
549} 599​}
550 600​
anthonyfieroniUnsubmitted
Not Done

This function is useless.

anthonyfieroni: This function is useless.