From 3af7395ab8db38375bc888814693f594f2fabb8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Sat, 11 Jun 2011 14:48:16 +0200 Subject: [PATCH] avoid calling QPixmap::paintEngine() in scene rendering by checking the graphicssystem at startup also avoid pixmap and memory leaking on the xrender backend, validate some pointers on deletion in SceneOpnGL and avoid attempts to render ::isNull pixmaps --- libkwineffects/kwinglobals.cpp | 9 ++++++ libkwineffects/kwinglobals.h | 4 +++ libkwineffects/kwinxrenderutils.cpp | 8 +++-- scene_opengl.cpp | 37 ++++++++++++++---------- scene_xrender.cpp | 45 +++++++++++++++++++---------- 5 files changed, 70 insertions(+), 33 deletions(-) diff --git a/libkwineffects/kwinglobals.cpp b/libkwineffects/kwinglobals.cpp index 378db17c8d..d126a7cc9a 100644 --- a/libkwineffects/kwinglobals.cpp +++ b/libkwineffects/kwinglobals.cpp @@ -20,6 +20,10 @@ along with this program. If not, see . #include "kwinglobals.h" +#include +#include +#include + #include #include @@ -67,6 +71,7 @@ int Extensions::render_version = 0; bool Extensions::has_glx = false; bool Extensions::has_sync = false; int Extensions::sync_event_base = 0; +bool Extensions::non_native_pixmaps = false; // for fillExtensionsData() const char* Extensions::data_extensions[ 32 ]; int Extensions::data_nextensions; @@ -158,6 +163,10 @@ void Extensions::init() } } #endif + QPixmap pix(1,1); + QPainter p(&pix); + non_native_pixmaps = p.paintEngine()->type() != QPaintEngine::X11; + p.end(); kDebug(1212) << "Extensions: shape: 0x" << QString::number(shape_version, 16) << " composite: 0x" << QString::number(composite_version, 16) << " render: 0x" << QString::number(render_version, 16) diff --git a/libkwineffects/kwinglobals.h b/libkwineffects/kwinglobals.h index 204cdb0fa8..2a4ea7196d 100644 --- a/libkwineffects/kwinglobals.h +++ b/libkwineffects/kwinglobals.h @@ -171,6 +171,9 @@ public: static bool syncAvailable() { return has_sync; } + static bool nonNativePixmaps() { + return non_native_pixmaps; + } static int syncAlarmNotifyEvent(); static void fillExtensionsData(const char**& extensions, int& nextensions, int*&majors, int*& error_bases); private: @@ -191,6 +194,7 @@ private: static int data_nextensions; static int data_opcodes[ 32 ]; static int data_error_bases[ 32 ]; + static bool non_native_pixmaps; }; } // namespace diff --git a/libkwineffects/kwinxrenderutils.cpp b/libkwineffects/kwinxrenderutils.cpp index 27a977cab2..bc58705b3f 100644 --- a/libkwineffects/kwinxrenderutils.cpp +++ b/libkwineffects/kwinxrenderutils.cpp @@ -19,6 +19,7 @@ along with this program. If not, see . *********************************************************************/ #include "kwinxrenderutils.h" +#include "kwinglobals.h" #ifdef KWIN_HAVE_XRENDER_COMPOSITING @@ -26,7 +27,6 @@ along with this program. If not, see . #include #include #include -#include namespace KWin { @@ -193,13 +193,15 @@ static Picture createPicture(Pixmap pix, int depth) XRenderPicture::XRenderPicture(QPixmap pix) { - if (pix.paintEngine()->type() != QPaintEngine::X11 || pix.handle() == 0) { + if (Extensions::nonNativePixmaps()) { Pixmap xPix = XCreatePixmap(display(), rootWindow(), pix.width(), pix.height(), pix.depth()); - QPixmap tempPix = QPixmap::fromX11Pixmap(xPix); + QPixmap tempPix = QPixmap::fromX11Pixmap(xPix, QPixmap::ExplicitlyShared); tempPix.fill(Qt::transparent); QPainter p(&tempPix); p.drawPixmap(QPoint(0, 0), pix); + p.end(); d = new XRenderPictureData(createPicture(tempPix.handle(), tempPix.depth())); + XFreePixmap(display(), xPix); } else { d = new XRenderPictureData(createPicture(pix.handle(), pix.depth())); } diff --git a/scene_opengl.cpp b/scene_opengl.cpp index f570e0efd1..b314587af7 100644 --- a/scene_opengl.cpp +++ b/scene_opengl.cpp @@ -92,7 +92,6 @@ Sources and other compositing managers: #include #include #include -#include namespace KWin { @@ -341,7 +340,7 @@ bool SceneOpenGL::Texture::load(const QPixmap& pixmap, GLenum target) return false; // Checking whether QPixmap comes with its own X11 Pixmap - if (pixmap.paintEngine() == 0 || pixmap.paintEngine()->type() != QPaintEngine::X11 || pixmap.handle() == 0) { + if (Extensions::nonNativePixmaps()) { return GLTexture::load(pixmap.toImage(), target); } @@ -1347,21 +1346,24 @@ void SceneOpenGL::EffectFrame::render(QRegion region, double opacity, double fra if (!m_effectFrame->selection().isNull()) { if (!m_selectionTexture) { // Lazy creation QPixmap pixmap = m_effectFrame->selectionFrame().framePixmap(); - m_selectionTexture = new Texture(pixmap); + if (!pixmap.isNull()) + m_selectionTexture = new Texture(pixmap); } - if (shader) { - const float a = opacity * frameOpacity; - shader->setUniform(GLShader::ModulationConstant, QVector4D(a, a, a, a)); + if (m_selectionTexture) { + if (shader) { + const float a = opacity * frameOpacity; + shader->setUniform(GLShader::ModulationConstant, QVector4D(a, a, a, a)); + } + #ifndef KWIN_HAVE_OPENGLES + else + glColor4f(1.0, 1.0, 1.0, opacity * frameOpacity); + #endif + glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); + m_selectionTexture->bind(); + m_selectionTexture->render(region, m_effectFrame->selection()); + m_selectionTexture->unbind(); + glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); } -#ifndef KWIN_HAVE_OPENGLES - else - glColor4f(1.0, 1.0, 1.0, opacity * frameOpacity); -#endif - glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); - m_selectionTexture->bind(); - m_selectionTexture->render(region, m_effectFrame->selection()); - m_selectionTexture->unbind(); - glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); } // Render icon @@ -1462,6 +1464,7 @@ void SceneOpenGL::EffectFrame::render(QRegion region, double opacity, double fra void SceneOpenGL::EffectFrame::updateTexture() { delete m_texture; + m_texture = 0L; if (m_effectFrame->style() == EffectFrameStyled) { QPixmap pixmap = m_effectFrame->frame().framePixmap(); m_texture = new Texture(pixmap); @@ -1471,7 +1474,9 @@ void SceneOpenGL::EffectFrame::updateTexture() void SceneOpenGL::EffectFrame::updateTextTexture() { delete m_textTexture; + m_textTexture = 0L; delete m_textPixmap; + m_textPixmap = 0L; if (m_effectFrame->text().isEmpty()) return; @@ -1504,7 +1509,9 @@ void SceneOpenGL::EffectFrame::updateTextTexture() void SceneOpenGL::EffectFrame::updateUnstyledTexture() { delete m_unstyledTexture; + m_unstyledTexture = 0L; delete m_unstyledPixmap; + m_unstyledPixmap = 0L; // Based off circle() from kwinxrenderutils.cpp #define CS 8 m_unstyledPixmap = new QPixmap(2 * CS, 2 * CS); diff --git a/scene_xrender.cpp b/scene_xrender.cpp index d263d16b88..f2ede4e508 100644 --- a/scene_xrender.cpp +++ b/scene_xrender.cpp @@ -53,7 +53,6 @@ along with this program. If not, see . #include #include -#include namespace KWin { @@ -191,9 +190,11 @@ void SceneXrender::selfCheckSetup() img.setPixel(1, 1, QColor(Qt::black).rgb()); img.setPixel(2, 1, QColor(Qt::white).rgb()); QPixmap pix = QPixmap::fromImage(img); + bool mustFreePix = false; if (pix.handle() == 0) { + mustFreePix = true; Pixmap xPix = XCreatePixmap(display(), rootWindow(), pix.width(), pix.height(), DefaultDepth(display(), DefaultScreen(display()))); - pix = QPixmap::fromX11Pixmap(xPix); + pix = QPixmap::fromX11Pixmap(xPix, QPixmap::ExplicitlyShared); QPainter p(&pix); p.drawImage(QPoint(0, 0), img); } @@ -221,6 +222,8 @@ void SceneXrender::selfCheckSetup() XFreePixmap(display(), wpix); XDestroyWindow(display(), window); } + if (mustFreePix) + XFreePixmap(display(), pix.handle()); err.error(true); // just sync and discard } @@ -588,11 +591,13 @@ void SceneXrender::Window::prepareTempPixmap(const QPixmap *left, const QPixmap { const QRect r = static_cast(toplevel)->decorationRect(); + if (temp_pixmap && Extensions::nonNativePixmaps()) + XFreePixmap(display(), temp_pixmap->handle()); // The picture owns the pixmap now if (!temp_pixmap) temp_pixmap = new QPixmap(r.width(), r.height()); else if (temp_pixmap->width() < r.width() || temp_pixmap->height() < r.height()) *temp_pixmap = QPixmap(r.width(), r.height()); - if (temp_pixmap->paintEngine()->type() != QPaintEngine::X11 || temp_pixmap->handle() == 0) { + if (Extensions::nonNativePixmaps()) { Pixmap pix = XCreatePixmap(display(), rootWindow(), temp_pixmap->width(), temp_pixmap->height(), DefaultDepth(display(), DefaultScreen(display()))); *temp_pixmap = QPixmap::fromX11Pixmap(pix); } @@ -952,20 +957,25 @@ void SceneXrender::EffectFrame::render(QRegion region, double opacity, double fr if (!m_picture) { // Lazy creation updatePicture(); } - qreal left, top, right, bottom; - m_effectFrame->frame().getMargins(left, top, right, bottom); // m_geometry is the inner geometry - QRect geom = m_effectFrame->geometry().adjusted(-left, -top, right, bottom); - XRenderComposite(display(), PictOpOver, *m_picture, None, effects->xrenderBufferPicture(), - 0, 0, 0, 0, geom.x(), geom.y(), geom.width(), geom.height()); - + if (m_picture) { + qreal left, top, right, bottom; + m_effectFrame->frame().getMargins(left, top, right, bottom); // m_geometry is the inner geometry + QRect geom = m_effectFrame->geometry().adjusted(-left, -top, right, bottom); + XRenderComposite(display(), PictOpOver, *m_picture, None, effects->xrenderBufferPicture(), + 0, 0, 0, 0, geom.x(), geom.y(), geom.width(), geom.height()); + } } if (!m_effectFrame->selection().isNull()) { if (!m_selectionPicture) { // Lazy creation - m_selectionPicture = new XRenderPicture(m_effectFrame->selectionFrame().framePixmap()); + const QPixmap pix = m_effectFrame->selectionFrame().framePixmap(); + if (!pix.isNull()) // don't try if there's no content + m_selectionPicture = new XRenderPicture(m_effectFrame->selectionFrame().framePixmap()); + } + if (m_selectionPicture) { + const QRect geom = m_effectFrame->selection(); + XRenderComposite(display(), PictOpOver, *m_selectionPicture, None, effects->xrenderBufferPicture(), + 0, 0, 0, 0, geom.x(), geom.y(), geom.width(), geom.height()); } - const QRect geom = m_effectFrame->selection(); - XRenderComposite(display(), PictOpOver, *m_selectionPicture, None, effects->xrenderBufferPicture(), - 0, 0, 0, 0, geom.x(), geom.y(), geom.width(), geom.height()); } XRenderPicture fill = xRenderBlendPicture(opacity); @@ -995,14 +1005,19 @@ void SceneXrender::EffectFrame::render(QRegion region, double opacity, double fr void SceneXrender::EffectFrame::updatePicture() { delete m_picture; - if (m_effectFrame->style() == EffectFrameStyled) - m_picture = new XRenderPicture(m_effectFrame->frame().framePixmap()); + m_picture = 0L; + if (m_effectFrame->style() == EffectFrameStyled) { + const QPixmap pix = m_effectFrame->frame().framePixmap(); + if (!pix.isNull()) + m_picture = new XRenderPicture(pix); + } } void SceneXrender::EffectFrame::updateTextPicture() { // Mostly copied from SceneOpenGL::EffectFrame::updateTextTexture() above delete m_textPicture; + m_textPicture = 0L; if (m_effectFrame->text().isEmpty()) { return;