Merge lp:~compiz-team/compiz/compiz.fix_927168 into lp:compiz/0.9.8

Proposed by Sam Spilsbury
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 3380
Merged at revision: 3376
Proposed branch: lp:~compiz-team/compiz/compiz.fix_927168
Merge into: lp:compiz/0.9.8
Diff against target: 736 lines (+489/-40)
14 files modified
plugins/copytex/src/copytex.cpp (+5/-4)
plugins/copytex/src/copytex.h (+5/-4)
plugins/decor/src/decor.cpp (+3/-1)
plugins/opengl/CMakeLists.txt (+4/-0)
plugins/opengl/include/opengl/opengl.h (+1/-1)
plugins/opengl/include/opengl/pixmapsource.h (+40/-0)
plugins/opengl/include/opengl/texture.h (+10/-5)
plugins/opengl/src/glxtfpbind/CMakeLists.txt (+31/-0)
plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h (+53/-0)
plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp (+53/-0)
plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt (+24/-0)
plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp (+157/-0)
plugins/opengl/src/privatetexture.h (+16/-12)
plugins/opengl/src/texture.cpp (+87/-13)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_927168
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
jenkins (community) continuous-integration Approve
Review via email: mp+124709@code.launchpad.net

Commit message

Check that pixmaps which aren't managed by us actually exist before binding.

It was possible for there to be a race condition for a pixmap to become
invalid on the server side if the client which did have control over thier
lifecycle freed them before we were done with them or disconnected. Drivers
have normally handled this condition by reading the contents of the undefined
memory directly without an error, however drivers such as LLVMpipe will
effectively dereference an invalid pointer and crash when you do this.

LP: #927168:
compiz crashed with SIGSEGV in
memmove() from
drisw_update_tex_buffer() from
dri_set_tex_buffer2() from
drisw_bind_tex_image() from
__glXBindTexImageEXT() from
TfpTexture::enable()

Most of the time we don't need to employ this check. Its only when we know
that we don't control the lifecycle of a pixmap that race conditions
such as these can occur.

Description of the change

== Change:

    Check that pixmaps which aren't managed by us actually exist before binding.

    It was possible for there to be a race condition for a pixmap to become
    invalid on the server side if the client which did have control over thier
    lifecycle freed them before we were done with them or disconnected. Drivers
    have normally handled this condition by reading the contents of the undefined
    memory directly without an error, however drivers such as LLVMpipe will
    effectively dereference an invalid pointer and crash when you do this.

    Eg:

    compiz crashed with SIGSEGV in memmove() from drisw_update_tex_buffer() from dri_set_tex_buffer2() from drisw_bind_tex_image() from __glXBindTexImageEXT() from TfpTexture::enable()

    Most of the time we don't need to employ this check. Its only when we know
    that we don't control the lifecycle of a pixmap that race conditions
    such as these can occurr.

== Test

    Five unit tests included. Testing that this fix can be effective can be done as follows:

    1. Start compiz with llvmpipe (eg LIBGL_ALWAYS_SOFTWARE=1)
    2. Set a breakpoint on TfpTexture::bindTexImage
    3. Hover over the decoration buttons so as to cause a damage event (and a require rebind)
    4. Once the breakpoint is hit, run killall -9 gtk-window-decorator
    5. Continue the program. It shouldn't crash.

To post a comment you must log in.
3380. By Sam Spilsbury

Actually provide the hint

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I can't find any obvious regressions and valgrind is happy.

This bug is a very big deal and causing constant duplicates, so I won't let any style complaints get in the way of approving it.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/copytex/src/copytex.cpp'
--- plugins/copytex/src/copytex.cpp 2012-09-07 22:37:20 +0000
+++ plugins/copytex/src/copytex.cpp 2012-09-17 15:27:20 +0000
@@ -37,10 +37,11 @@
37};37};
3838
39GLTexture::List39GLTexture::List
40CopyPixmap::bindPixmapToTexture (Pixmap pixmap,40CopyPixmap::bindPixmapToTexture (Pixmap pixmap,
41 int width,41 int width,
42 int height,42 int height,
43 int depth)43 int depth,
44 compiz::opengl::PixmapSource source)
44{45{
45 if (depth != 32 && depth != 24)46 if (depth != 32 && depth != 24)
46 return GLTexture::List ();47 return GLTexture::List ();
4748
=== modified file 'plugins/copytex/src/copytex.h'
--- plugins/copytex/src/copytex.h 2012-09-07 22:37:20 +0000
+++ plugins/copytex/src/copytex.h 2012-09-17 15:27:20 +0000
@@ -52,10 +52,11 @@
5252
53 ~CopyPixmap ();53 ~CopyPixmap ();
5454
55 static GLTexture::List bindPixmapToTexture (Pixmap pixmap,55 static GLTexture::List bindPixmapToTexture (Pixmap pixmap,
56 int width,56 int width,
57 int height,57 int height,
58 int depth);58 int depth,
59 compiz::opengl::PixmapSource source);
5960
60 public:61 public:
61 Textures textures;62 Textures textures;
6263
=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp 2012-09-13 10:58:13 +0000
+++ plugins/decor/src/decor.cpp 2012-09-17 15:27:20 +0000
@@ -337,7 +337,9 @@
337 }337 }
338338
339 bindFailed = false;339 bindFailed = false;
340 textures = GLTexture::bindPixmapToTexture (pixmap->getPixmap (), width, height, depth);340 textures = GLTexture::bindPixmapToTexture (pixmap->getPixmap (),
341 width, height, depth,
342 compiz::opengl::ExternallyManaged);
341 if (textures.size () != 1)343 if (textures.size () != 1)
342 {344 {
343 bindFailed = true;345 bindFailed = true;
344346
=== modified file 'plugins/opengl/CMakeLists.txt'
--- plugins/opengl/CMakeLists.txt 2012-09-04 10:19:19 +0000
+++ plugins/opengl/CMakeLists.txt 2012-09-17 15:27:20 +0000
@@ -5,10 +5,14 @@
5set (INTERNAL_LIBRARIES5set (INTERNAL_LIBRARIES
6 compiz_opengl_double_buffer6 compiz_opengl_double_buffer
7 compiz_opengl_fsregion7 compiz_opengl_fsregion
8 compiz_opengl_glx_tfp_bind
8)9)
910
10add_subdirectory (src/doublebuffer)11add_subdirectory (src/doublebuffer)
11add_subdirectory (src/fsregion)12add_subdirectory (src/fsregion)
13add_subdirectory (src/glxtfpbind)
14
15include_directories (src/glxtfpbind/include)
1216
13if (USE_GLES)17if (USE_GLES)
14 compiz_plugin(opengl PLUGINDEPS composite CFLAGSADD "-DUSE_GLES -std=c++0x" LIBRARIES ${OPENGLES2_LIBRARIES} ${INTERNAL_LIBRARIES} dl INCDIRS ${OPENGLES2_INCLUDE_DIR})18 compiz_plugin(opengl PLUGINDEPS composite CFLAGSADD "-DUSE_GLES -std=c++0x" LIBRARIES ${OPENGLES2_LIBRARIES} ${INTERNAL_LIBRARIES} dl INCDIRS ${OPENGLES2_INCLUDE_DIR})
1519
=== modified file 'plugins/opengl/include/opengl/opengl.h'
--- plugins/opengl/include/opengl/opengl.h 2012-09-07 23:29:42 +0000
+++ plugins/opengl/include/opengl/opengl.h 2012-09-17 15:27:20 +0000
@@ -50,7 +50,7 @@
50#include <opengl/programcache.h>50#include <opengl/programcache.h>
51#include <opengl/shadercache.h>51#include <opengl/shadercache.h>
5252
53#define COMPIZ_OPENGL_ABI 553#define COMPIZ_OPENGL_ABI 6
5454
55/*55/*
56 * Some plugins check for #ifdef USE_MODERN_COMPIZ_GL. Support it for now, but56 * Some plugins check for #ifdef USE_MODERN_COMPIZ_GL. Support it for now, but
5757
=== added file 'plugins/opengl/include/opengl/pixmapsource.h'
--- plugins/opengl/include/opengl/pixmapsource.h 1970-01-01 00:00:00 +0000
+++ plugins/opengl/include/opengl/pixmapsource.h 2012-09-17 15:27:20 +0000
@@ -0,0 +1,40 @@
1/*
2 *
3 * Copyright (c) 2012 Canonical Ltd.
4 * Authors: Sam Spilsbury <sam.spilsbury@canonical.com>
5 *
6 * Permission is hereby granted, free of charge, to any person obtaining a
7 * copy of this software and associated documentation files (the "Software"),
8 * to deal in the Software without restriction, including without limitation
9 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
10 * and/or sell copies of the Software, and to permit persons to whom the
11 * Software is furnished to do so, subject to the following conditions:
12 *
13 * The above copyright notice and this permission notice shall be included in
14 * all copies or substantial portions of the Software.
15 *
16 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
21 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
22 * DEALINGS IN THE SOFTWARE.
23 */
24
25#ifndef _COMPIZ_OPENGL_PIXMAP_SOURCE_H
26#define _COMPIZ_OPENGL_PIXMAP_SOURCE_H
27
28namespace compiz
29{
30 namespace opengl
31 {
32 typedef enum _PixmapSource
33 {
34 InternallyManaged = 0,
35 ExternallyManaged = 1
36 } PixmapSource;
37 }
38}
39
40#endif
041
=== modified file 'plugins/opengl/include/opengl/texture.h'
--- plugins/opengl/include/opengl/texture.h 2012-05-17 10:41:21 +0000
+++ plugins/opengl/include/opengl/texture.h 2012-09-17 15:27:20 +0000
@@ -43,6 +43,8 @@
4343
44#include <vector>44#include <vector>
4545
46#include <opengl/pixmapsource.h>
47
4648
47#define POWER_OF_TWO(v) ((v & (v - 1)) == 0)49#define POWER_OF_TWO(v) ((v & (v - 1)) == 0)
4850
@@ -104,7 +106,7 @@
104 void clear ();106 void clear ();
105 };107 };
106108
107 typedef boost::function<List (Pixmap, int, int, int)> BindPixmapProc;109 typedef boost::function<List (Pixmap, int, int, int, compiz::opengl::PixmapSource)> BindPixmapProc;
108 typedef unsigned int BindPixmapHandle;110 typedef unsigned int BindPixmapHandle;
109111
110 public:112 public:
@@ -181,11 +183,14 @@
181 * @param width Specifies the width of the texture183 * @param width Specifies the width of the texture
182 * @param height Specifies the height of the texture184 * @param height Specifies the height of the texture
183 * @param depth Specifies the color depth of the texture185 * @param depth Specifies the color depth of the texture
186 * @param source Whether the pixmap lifecycle is managed externall
184 */187 */
185 static List bindPixmapToTexture (Pixmap pixmap,188 static List bindPixmapToTexture (Pixmap pixmap,
186 int width,189 int width,
187 int height,190 int height,
188 int depth);191 int depth,
192 compiz::opengl::PixmapSource source
193 = compiz::opengl::InternallyManaged);
189194
190 /**195 /**
191 * Returns a GLTexture::List with the contents of of196 * Returns a GLTexture::List with the contents of of
192197
=== added directory 'plugins/opengl/src/glxtfpbind'
=== added file 'plugins/opengl/src/glxtfpbind/CMakeLists.txt'
--- plugins/opengl/src/glxtfpbind/CMakeLists.txt 1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/CMakeLists.txt 2012-09-17 15:27:20 +0000
@@ -0,0 +1,31 @@
1INCLUDE_DIRECTORIES (
2 ${compiz_SOURCE_DIR}/src/servergrab/include
3 ${CMAKE_CURRENT_SOURCE_DIR}/../../include
4 ${CMAKE_CURRENT_SOURCE_DIR}/include
5 ${CMAKE_CURRENT_SOURCE_DIR}/src
6
7 ${Boost_INCLUDE_DIRS}
8)
9
10LINK_DIRECTORIES (${COMPIZ_LIBRARY_DIRS})
11
12SET(
13 SRCS
14 ${CMAKE_CURRENT_SOURCE_DIR}/src/glx-tfp-bind.cpp
15)
16
17ADD_LIBRARY(
18 compiz_opengl_glx_tfp_bind STATIC
19
20 ${SRCS}
21)
22
23if (COMPIZ_BUILD_TESTING)
24ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
25endif (COMPIZ_BUILD_TESTING)
26
27TARGET_LINK_LIBRARIES(
28 compiz_opengl_glx_tfp_bind
29
30 compiz_servergrab
31)
032
=== added directory 'plugins/opengl/src/glxtfpbind/include'
=== added file 'plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h'
--- plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h 1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/include/glx-tfp-bind.h 2012-09-17 15:27:20 +0000
@@ -0,0 +1,53 @@
1/*
2 * Compiz, opengl plugin, GLX_EXT_texture_from_pixmap rebind logic
3 *
4 * Copyright (c) 2012 Canonical Ltd.
5 * Authors: Sam Spilsbury <sam.spilsbury@canonical.com>
6 *
7 * Permission is hereby granted, free of charge, to any person obtaining a
8 * copy of this software and associated documentation files (the "Software"),
9 * to deal in the Software without restriction, including without limitation
10 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
11 * and/or sell copies of the Software, and to permit persons to whom the
12 * Software is furnished to do so, subject to the following conditions:
13 *
14 * The above copyright notice and this permission notice shall be included in
15 * all copies or substantial portions of the Software.
16 *
17 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
18 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
19 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
20 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
21 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
22 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
23 * DEALINGS IN THE SOFTWARE.
24 */
25#ifndef _COMPIZ_OPENGL_GLX_TFP_BIND_H
26#define _COMPIZ_OPENGL_GLX_TFP_BIND_H
27
28#include <opengl/pixmapsource.h>
29#include <boost/function.hpp>
30
31class ServerGrabInterface;
32typedef unsigned long Pixmap;
33typedef unsigned long GLXPixmap;
34
35namespace compiz
36{
37 namespace opengl
38 {
39 typedef boost::function <bool (Pixmap)> PixmapCheckValidityFunc;
40 typedef boost::function <void (GLXPixmap)> BindTexImageEXTFunc;
41 typedef boost::function <void ()> WaitGLXFunc;
42
43 bool bindTexImageGLX (ServerGrabInterface *,
44 Pixmap,
45 GLXPixmap,
46 const PixmapCheckValidityFunc &,
47 const BindTexImageEXTFunc &,
48 const WaitGLXFunc &,
49 PixmapSource);
50
51 } // namespace opengl
52} // namespace compiz
53#endif
054
=== added directory 'plugins/opengl/src/glxtfpbind/src'
=== added file 'plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp'
--- plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp 1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/src/glx-tfp-bind.cpp 2012-09-17 15:27:20 +0000
@@ -0,0 +1,53 @@
1/*
2 * Compiz, opengl plugin, GLX_EXT_texture_from_pixmap rebind logic
3 *
4 * Copyright (c) 2012 Canonical Ltd.
5 * Authors: Sam Spilsbury <sam.spilsbury@canonical.com>
6 *
7 * Permission is hereby granted, free of charge, to any person obtaining a
8 * copy of this software and associated documentation files (the "Software"),
9 * to deal in the Software without restriction, including without limitation
10 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
11 * and/or sell copies of the Software, and to permit persons to whom the
12 * Software is furnished to do so, subject to the following conditions:
13 *
14 * The above copyright notice and this permission notice shall be included in
15 * all copies or substantial portions of the Software.
16 *
17 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
18 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
19 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
20 * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
21 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
22 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
23 * DEALINGS IN THE SOFTWARE.
24 */
25#include <opengl/pixmapsource.h>
26#include <core/servergrab.h>
27#include "glx-tfp-bind.h"
28
29namespace cgl = compiz::opengl;
30
31bool
32cgl::bindTexImageGLX (ServerGrabInterface *serverGrabInterface,
33 Pixmap x11Pixmap,
34 GLXPixmap glxPixmap,
35 const cgl::PixmapCheckValidityFunc &checkPixmapValidity,
36 const cgl::BindTexImageEXTFunc &bindTexImageEXT,
37 const cgl::WaitGLXFunc &waitGLX,
38 cgl::PixmapSource source)
39{
40 ServerLock lock (serverGrabInterface);
41
42 waitGLX ();
43
44 /* External pixmaps can disappear on us, but not
45 * while we have a server grab at least */
46 if (source == cgl::ExternallyManaged)
47 if (!checkPixmapValidity (x11Pixmap))
48 return false;
49
50 bindTexImageEXT (glxPixmap);
51
52 return true;
53}
054
=== added directory 'plugins/opengl/src/glxtfpbind/tests'
=== added file 'plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt'
--- plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt 1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/tests/CMakeLists.txt 2012-09-17 15:27:20 +0000
@@ -0,0 +1,24 @@
1find_library (GMOCK_LIBRARY gmock)
2find_library (GMOCK_MAIN_LIBRARY gmock_main)
3
4if (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
5 message ("Google Mock and Google Test not found - cannot build tests!")
6 set (COMPIZ_BUILD_TESTING OFF)
7endif (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
8
9include_directories (${GTEST_INCLUDE_DIRS})
10
11link_directories (${COMPIZ_LIBRARY_DIRS})
12
13add_executable (compiz_test_opengl_glx_tfp_bind
14 ${CMAKE_CURRENT_SOURCE_DIR}/test-opengl-glx-tfp-bind.cpp)
15
16target_link_libraries (compiz_test_opengl_glx_tfp_bind
17 compiz_opengl_glx_tfp_bind
18 ${GTEST_BOTH_LIBRARIES}
19 ${GMOCK_LIBRARY}
20 ${GMOCK_MAIN_LIBRARY}
21 ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
22 )
23
24compiz_discover_tests (compiz_test_opengl_glx_tfp_bind COVERAGE compiz_opengl_glx_tfp_bind)
025
=== added file 'plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp'
--- plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp 1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/glxtfpbind/tests/test-opengl-glx-tfp-bind.cpp 2012-09-17 15:27:20 +0000
@@ -0,0 +1,157 @@
1#include <boost/bind.hpp>
2
3#include <gtest/gtest.h>
4#include <gmock/gmock.h>
5
6#include <core/servergrab.h>
7#include "glx-tfp-bind.h"
8
9using ::testing::InSequence;
10using ::testing::NiceMock;
11using ::testing::StrictMock;
12using ::testing::Return;
13
14namespace cgl = compiz::opengl;
15
16namespace
17{
18 const Pixmap pixmap = 1;
19 const GLXPixmap glxPixmap = 2;
20
21 void emptyWaitGLX () {}
22 bool emptyCheckPixmap (Pixmap p) { return true; }
23 void emptyBindTexImage (GLXPixmap p) {}
24
25 cgl::WaitGLXFunc waitGLX () { return boost::bind (emptyWaitGLX); }
26 cgl::PixmapCheckValidityFunc pixmapCheckValidity () { return boost::bind (emptyCheckPixmap, _1); }
27 cgl::BindTexImageEXTFunc bindTexImageEXT () { return boost::bind (emptyBindTexImage, _1); }
28}
29
30class MockWaitGLX
31{
32 public:
33
34 MOCK_METHOD0 (waitGLX, void ());
35};
36
37class MockPixmapCheckValidity
38{
39 public:
40
41 MOCK_METHOD1 (checkValidity, bool (Pixmap));
42};
43
44class MockBindTexImageEXT
45{
46 public:
47
48 MOCK_METHOD1 (bindTexImageEXT, void (GLXPixmap));
49};
50
51class MockServerGrab :
52 public ServerGrabInterface
53{
54 public:
55
56 MOCK_METHOD0 (grabServer, void ());
57 MOCK_METHOD0 (syncServer, void ());
58 MOCK_METHOD0 (ungrabServer, void ());
59};
60
61TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestTakesServerGrab)
62{
63 MockServerGrab mockServerGrab;
64 InSequence s;
65
66 EXPECT_CALL (mockServerGrab, grabServer ());
67 EXPECT_CALL (mockServerGrab, syncServer ());
68 EXPECT_CALL (mockServerGrab, ungrabServer ());
69 EXPECT_CALL (mockServerGrab, syncServer ());
70
71 cgl::bindTexImageGLX (&mockServerGrab,
72 pixmap,
73 glxPixmap,
74 pixmapCheckValidity (),
75 bindTexImageEXT (),
76 waitGLX (),
77 cgl::InternallyManaged);
78}
79
80TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCallsWaitGLX)
81{
82 NiceMock <MockServerGrab> mockServerGrab;
83 MockWaitGLX mockWaitGLX;
84
85 cgl::WaitGLXFunc waitGLXFuncMock (boost::bind (&MockWaitGLX::waitGLX, &mockWaitGLX));
86
87 EXPECT_CALL (mockWaitGLX, waitGLX ());
88
89 cgl::bindTexImageGLX (&mockServerGrab,
90 pixmap,
91 glxPixmap,
92 pixmapCheckValidity (),
93 bindTexImageEXT (),
94 waitGLXFuncMock,
95 cgl::InternallyManaged);
96}
97
98TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestNoCallToCheckValidityIfInternalAndImmediateBind)
99{
100 NiceMock <MockServerGrab> mockServerGrab;
101 StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
102 StrictMock <MockBindTexImageEXT> mockBindTexImage;
103
104 cgl::PixmapCheckValidityFunc pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
105 cgl::BindTexImageEXTFunc bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
106
107 EXPECT_CALL (mockBindTexImage, bindTexImageEXT (glxPixmap));
108
109 EXPECT_TRUE (cgl::bindTexImageGLX (&mockServerGrab,
110 pixmap,
111 glxPixmap,
112 pixmapCheckFunc,
113 bindTexImageEXTFunc,
114 waitGLX (),
115 cgl::InternallyManaged));
116}
117
118TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCheckValidityIfExternalNoBindIfInvalid)
119{
120 NiceMock <MockServerGrab> mockServerGrab;
121 StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
122 StrictMock <MockBindTexImageEXT> mockBindTexImage;
123
124 cgl::PixmapCheckValidityFunc pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
125 cgl::BindTexImageEXTFunc bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
126
127 EXPECT_CALL (mockPixmapCheck, checkValidity (pixmap)).WillOnce (Return (false));
128
129 EXPECT_FALSE (cgl::bindTexImageGLX (&mockServerGrab,
130 pixmap,
131 glxPixmap,
132 pixmapCheckFunc,
133 bindTexImageEXTFunc,
134 waitGLX (),
135 cgl::ExternallyManaged));
136}
137
138TEST (CompizOpenGLGLXTextureFromPixmapBindTest, TestCheckValidityIfExternalBindIfValid)
139{
140 NiceMock <MockServerGrab> mockServerGrab;
141 StrictMock <MockPixmapCheckValidity> mockPixmapCheck;
142 StrictMock <MockBindTexImageEXT> mockBindTexImage;
143
144 cgl::PixmapCheckValidityFunc pixmapCheckFunc (boost::bind (&MockPixmapCheckValidity::checkValidity, &mockPixmapCheck, _1));
145 cgl::BindTexImageEXTFunc bindTexImageEXTFunc (boost::bind (&MockBindTexImageEXT::bindTexImageEXT, &mockBindTexImage, _1));
146
147 EXPECT_CALL (mockPixmapCheck, checkValidity (pixmap)).WillOnce (Return (true));
148 EXPECT_CALL (mockBindTexImage, bindTexImageEXT (glxPixmap));
149
150 EXPECT_TRUE (cgl::bindTexImageGLX (&mockServerGrab,
151 pixmap,
152 glxPixmap,
153 pixmapCheckFunc,
154 bindTexImageEXTFunc,
155 waitGLX (),
156 cgl::ExternallyManaged));
157}
0158
=== modified file 'plugins/opengl/src/privatetexture.h'
--- plugins/opengl/src/privatetexture.h 2012-07-04 07:07:37 +0000
+++ plugins/opengl/src/privatetexture.h 2012-09-17 15:27:20 +0000
@@ -79,10 +79,11 @@
7979
80 void enable (Filter filter);80 void enable (Filter filter);
8181
82 static List bindPixmapToTexture (Pixmap pixmap,82 static List bindPixmapToTexture (Pixmap pixmap,
83 int width,83 int width,
84 int height,84 int height,
85 int depth);85 int depth,
86 compiz::opengl::PixmapSource source);
8687
87 public:88 public:
88 bool damaged;89 bool damaged;
@@ -102,16 +103,19 @@
102 bool bindTexImage (const GLXPixmap &);103 bool bindTexImage (const GLXPixmap &);
103 void releaseTexImage ();104 void releaseTexImage ();
104105
105 static List bindPixmapToTexture (Pixmap pixmap,106 static List bindPixmapToTexture (Pixmap pixmap,
106 int width,107 int width,
107 int height,108 int height,
108 int depth);109 int depth,
110 compiz::opengl::PixmapSource source);
109111
110 public:112 public:
111 GLXPixmap pixmap;113 Pixmap x11Pixmap;
112 bool damaged;114 GLXPixmap pixmap;
113 Damage damage;115 bool damaged;
114 bool updateMipMap;116 Damage damage;
117 bool updateMipMap;
118 compiz::opengl::PixmapSource source;
115};119};
116120
117extern std::map<Damage, TfpTexture*> boundPixmapTex;121extern std::map<Damage, TfpTexture*> boundPixmapTex;
118122
=== modified file 'plugins/opengl/src/texture.cpp'
--- plugins/opengl/src/texture.cpp 2012-08-13 07:31:34 +0000
+++ plugins/opengl/src/texture.cpp 2012-09-17 15:27:20 +0000
@@ -31,11 +31,17 @@
31#include <stdlib.h>31#include <stdlib.h>
32#include <string.h>32#include <string.h>
3333
34#include <boost/scoped_ptr.hpp>
35
34#include <core/servergrab.h>36#include <core/servergrab.h>
35#include <opengl/texture.h>37#include <opengl/texture.h>
36#include <privatetexture.h>38#include <privatetexture.h>
37#include "privates.h"39#include "privates.h"
3840
41#include "glx-tfp-bind.h"
42
43namespace cgl = compiz::opengl;
44
39#ifdef USE_GLES45#ifdef USE_GLES
40std::map<Damage, EglTexture*> boundPixmapTex;46std::map<Damage, EglTexture*> boundPixmapTex;
41#else47#else
@@ -421,23 +427,58 @@
421}427}
422428
423GLTexture::List429GLTexture::List
424GLTexture::bindPixmapToTexture (Pixmap pixmap,430GLTexture::bindPixmapToTexture (Pixmap pixmap,
425 int width,431 int width,
426 int height,432 int height,
427 int depth)433 int depth,
434 compiz::opengl::PixmapSource source)
428{435{
429 GLTexture::List rv;436 GLTexture::List rv;
430437
431 foreach (BindPixmapProc &proc, GLScreen::get (screen)->priv->bindPixmap)438 foreach (BindPixmapProc &proc, GLScreen::get (screen)->priv->bindPixmap)
432 {439 {
433 if (!proc.empty ())440 if (!proc.empty ())
434 rv = proc (pixmap, width, height, depth);441 rv = proc (pixmap, width, height, depth, source);
435 if (rv.size ())442 if (rv.size ())
436 return rv;443 return rv;
437 }444 }
438 return GLTexture::List ();445 return GLTexture::List ();
439}446}
440447
448namespace
449{
450 bool checkPixmapValidityGLX (Pixmap pixmap)
451 {
452 Window windowReturn;
453 unsigned int uiReturn;
454 int iReturn;
455
456 if (!XGetGeometry (screen->dpy (),
457 pixmap,
458 &windowReturn,
459 &iReturn,
460 &iReturn,
461 &uiReturn,
462 &uiReturn,
463 &uiReturn,
464 &uiReturn))
465 return false;
466 return true;
467 }
468
469 const cgl::WaitGLXFunc & waitGLXFunc ()
470 {
471 static const cgl::WaitGLXFunc f (boost::bind (glXWaitX));
472 return f;
473 }
474
475 const cgl::PixmapCheckValidityFunc & checkPixmapValidityFunc ()
476 {
477 static const cgl::PixmapCheckValidityFunc f (boost::bind (checkPixmapValidityGLX, _1));
478 return f;
479 }
480}
481
441#ifdef USE_GLES482#ifdef USE_GLES
442EglTexture::EglTexture () :483EglTexture::EglTexture () :
443 damaged (true),484 damaged (true),
@@ -538,6 +579,20 @@
538}579}
539#else580#else
540581
582namespace
583{
584 const cgl::BindTexImageEXTFunc & bindTexImageEXT ()
585 {
586 static int *attrib_list = NULL;
587 static const cgl::BindTexImageEXTFunc f (boost::bind (GL::bindTexImage,
588 screen->dpy (),
589 _1,
590 GLX_FRONT_LEFT_EXT,
591 attrib_list));
592 return f;
593 }
594}
595
541TfpTexture::TfpTexture () :596TfpTexture::TfpTexture () :
542 pixmap (0),597 pixmap (0),
543 damaged (true),598 damaged (true),
@@ -569,10 +624,13 @@
569bool624bool
570TfpTexture::bindTexImage (const GLXPixmap &glxPixmap)625TfpTexture::bindTexImage (const GLXPixmap &glxPixmap)
571{626{
572 ServerLock sg (screen->serverGrabInterface ());627 return compiz::opengl::bindTexImageGLX (screen->serverGrabInterface (),
573 glXWaitX ();628 x11Pixmap,
574 (*GL::bindTexImage) (screen->dpy (), glxPixmap, GLX_FRONT_LEFT_EXT, NULL);629 glxPixmap,
575 return true;630 checkPixmapValidityFunc (),
631 bindTexImageEXT (),
632 waitGLXFunc (),
633 source);
576}634}
577635
578void636void
@@ -582,10 +640,11 @@
582}640}
583641
584GLTexture::List642GLTexture::List
585TfpTexture::bindPixmapToTexture (Pixmap pixmap,643TfpTexture::bindPixmapToTexture (Pixmap pixmap,
586 int width,644 int width,
587 int height,645 int height,
588 int depth)646 int depth,
647 cgl::PixmapSource source)
589{648{
590 if ((int) width > GL::maxTextureSize || (int) height > GL::maxTextureSize ||649 if ((int) width > GL::maxTextureSize || (int) height > GL::maxTextureSize ||
591 !GL::textureFromPixmap)650 !GL::textureFromPixmap)
@@ -650,6 +709,19 @@
650709
651 attribs[i++] = None;710 attribs[i++] = None;
652711
712 /* We need to take a server grab here if the pixmap is
713 * externally managed as we need to be able to query
714 * if it actually exists */
715 boost::scoped_ptr <ServerLock> lock;
716
717 if (source == cgl::ExternallyManaged)
718 {
719 lock.reset (new ServerLock (screen->serverGrabInterface ()));
720
721 if (!checkPixmapValidityGLX (pixmap))
722 return false;
723 }
724
653 glxPixmap = (*GL::createPixmap) (screen->dpy (), config->fbConfig,725 glxPixmap = (*GL::createPixmap) (screen->dpy (), config->fbConfig,
654 pixmap, attribs);726 pixmap, attribs);
655727
@@ -711,6 +783,8 @@
711 tex->setData (texTarget, matrix, mipmap);783 tex->setData (texTarget, matrix, mipmap);
712 tex->setGeometry (0, 0, width, height);784 tex->setGeometry (0, 0, width, height);
713 tex->pixmap = glxPixmap;785 tex->pixmap = glxPixmap;
786 tex->x11Pixmap = pixmap;
787 tex->source = source;
714788
715 rv[0] = tex;789 rv[0] = tex;
716790

Subscribers

People subscribed via source and target branches