[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: TFP and strict binding
From: Philip Falkner <philip.falkner () gmail ! com>
Date: 2007-06-27 19:03:51
Message-ID: 200706271503.51930.philip.falkner () gmail ! com
[Download RAW message or body]
On Tuesday 26 June 2007 08:08:02 Lubos Lunak wrote:
> On Monday 25 of June 2007, Philip Falkner wrote:
> > On Monday 25 June 2007 11:31:48 Rivo Laks wrote:
> > > Ühel kenal päeval (esmaspäev 25 juuni 2007) kirjutas Philip Falkner:
> > > > I think the right answer is to release the binding (when non-strict
> > > > binding), and destroy bound_glxpixmap from discardWindowPixmap().
> > > > Then the next SceneOpenGL::Texture::load() will just pick it up.
> > > > Does that make sense?
> > >
> > > Your patch mostly works but sometimes it crashes when choosing desktop
> > > in DesktopGrid because effectWindow()->sceneWindow() is 0 in
> > > composite.cpp:410
> >
> > Fixed, thanks.
Well, actually not. I fixed the crash, but that only exposed the real
problem.
The sceneWindow() result is NULL after a call to
EffectsHandlerImpl::stackingOrder(). When that function calls effectWindow(
c ) for each client, each client gets its sceneWindow() set to NULL.
DesktopGrid calls stackingOrder() as part of windowAt(), so if you activate
DesktopGrid, and click on another desktop, activate it again, and click back
to the first desktop, in tfp_mode it doesn't work even with my patch because
sceneWindow() isn't reset from NULL until the painting pass, which is after
discardWindowPixmap().
Is there any reason why effectWindow( Toplevel* ) needs to setSceneWindow(
NULL )? Sure, in that function we don't know if there's an associated scene
window or not, but is that reason enough? Commenting it out doesn't seem to
hurt anything...
> Was there a patch supposed to be attached here :) ? But yes, the patch in
> general looks ok to me, maybe only if you could tidy it up a bit - the
> comment for SceneOpenGL::Window::releaseTexture() looks pretty confusing in
> connection with the function name, and I'd prefer if the code in
> composite.cpp didn't do such specific code but instead called some
> pixmapDiscarded() virtual and the OpenGL one would take care of what needs
> to be done for OpenGL.
Sure. Except that in rewriting it that way, I found another problem. :)
In SceneOpenGL::windowClosed/Deleted(), we remove the scene window from the
list, but don't setSceneWindow( NULL ) for the corresponding effect window;
crash. See scene_opengl.cpp and scene_xrender.cpp.
--
Philip Falkner
["tfp-fix-v2.patch" (text/x-diff)]
Index: composite.cpp
===================================================================
--- composite.cpp (revision 680915)
+++ composite.cpp (working copy)
@@ -404,6 +404,8 @@
return;
XFreePixmap( display(), window_pix );
window_pix = None;
+ if( effectWindow() != NULL && effectWindow()->sceneWindow() != NULL )
+ effectWindow()->sceneWindow()->pixmapDiscarded();
}
Pixmap Toplevel::createWindowPixmap()
Index: scene.h
===================================================================
--- scene.h (revision 680915)
+++ scene.h (working copy)
@@ -131,6 +131,8 @@
// perform the actual painting of the window
virtual void performPaint( int mask, QRegion region, WindowPaintData data ) = 0;
virtual void prepareForPainting() {}
+ // do any cleanup needed when the window's composite pixmap is discarded
+ virtual void pixmapDiscarded() {}
int x() const;
int y() const;
int width() const;
Index: scene_opengl.h
===================================================================
--- scene_opengl.h (revision 680915)
+++ scene_opengl.h (working copy)
@@ -98,6 +98,7 @@
virtual bool load( const QImage& image, GLenum target = GL_TEXTURE_2D );
virtual bool load( const QPixmap& pixmap, GLenum target = GL_TEXTURE_2D );
virtual void discard();
+ virtual void release(); // undo the tfp_mode binding
virtual void bind();
virtual void unbind();
@@ -119,6 +120,7 @@
virtual ~Window();
virtual void performPaint( int mask, QRegion region, WindowPaintData data );
virtual void prepareForPainting();
+ virtual void pixmapDiscarded();
bool bindTexture();
void discardTexture();
void discardVertices();
Index: scene_opengl.cpp
===================================================================
--- scene_opengl.cpp (revision 680915)
+++ scene_opengl.cpp (working copy)
@@ -691,6 +691,7 @@
else
{
delete windows.take( c );
+ c->effectWindow()->setSceneWindow( NULL );
}
}
@@ -698,6 +699,7 @@
{
assert( windows.contains( c ));
delete windows.take( c );
+ c->effectWindow()->setSceneWindow( NULL );
}
void SceneOpenGL::windowGeometryShapeChanged( Toplevel* c )
@@ -749,16 +751,19 @@
void SceneOpenGL::Texture::discard()
{
if( mTexture != None )
+ release();
+ GLTexture::discard();
+ }
+
+void SceneOpenGL::Texture::release()
+ {
+ if( tfp_mode && bound_glxpixmap != None )
{
- if( tfp_mode )
- {
- if( !options->glStrictBinding )
- glXReleaseTexImageEXT( display(), bound_glxpixmap, GLX_FRONT_LEFT_EXT );
- glXDestroyGLXPixmap( display(), bound_glxpixmap );
- bound_glxpixmap = None;
- }
+ if( !options->glStrictBinding )
+ glXReleaseTexImageEXT( display(), bound_glxpixmap, GLX_FRONT_LEFT_EXT );
+ glXDestroyGLXPixmap( display(), bound_glxpixmap );
+ bound_glxpixmap = None;
}
- GLTexture::discard();
}
void SceneOpenGL::Texture::findTarget()
@@ -1183,6 +1188,12 @@
currentYResolution = -1;
}
+// when the window's composite pixmap is discarded, undo binding it to the texture
+void SceneOpenGL::Window::pixmapDiscarded()
+ {
+ texture.release();
+ }
+
// paint the window
void SceneOpenGL::Window::performPaint( int mask, QRegion region, WindowPaintData data )
{
Index: scene_xrender.cpp
===================================================================
--- scene_xrender.cpp (revision 680915)
+++ scene_xrender.cpp (working copy)
@@ -287,6 +287,7 @@
else
{
delete windows.take( c );
+ c->effectWindow()->setSceneWindow( NULL );
}
}
@@ -294,6 +295,7 @@
{
assert( windows.contains( c ));
delete windows.take( c );
+ c->effectWindow()->setSceneWindow( NULL );
}
void SceneXrender::windowAdded( Toplevel* c )
Index: effects.cpp
===================================================================
--- effects.cpp (revision 680915)
+++ effects.cpp (working copy)
@@ -1082,7 +1082,7 @@
EffectWindow* effectWindow( Toplevel* w )
{
EffectWindowImpl* ret = w->effectWindow();
- ret->setSceneWindow( NULL ); // just in case
+ //temporary hack? ret->setSceneWindow( NULL ); // just in case
return ret;
}
_______________________________________________
Kwin mailing list
Kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic