Merge lp:~glcompbench-dev/glcompbench/bug-984058 into lp:glcompbench

Proposed by Jesse Barker on 2012-04-18
Status: Merged
Merged at revision: 81
Proposed branch: lp:~glcompbench-dev/glcompbench/bug-984058
Merge into: lp:glcompbench
Diff against target: 152 lines (+26/-15)
4 files modified
src/composite-canvas-egl.cc (+1/-0)
src/composite-canvas-glx.cc (+1/-0)
src/composite-canvas.cc (+19/-10)
src/composite-window.cc (+5/-5)
To merge this branch: bzr merge lp:~glcompbench-dev/glcompbench/bug-984058
Reviewer Review Type Date Requested Status
Alexandros Frantzis 2012-04-18 Pending
Review via email: mp+102594@code.launchpad.net

Description of the change

Canvas: Given the asynchronous nature of event dispatch, we must check for whether we might be handling events for a window that has already been destroyed. So, once we get the next event from the queue, we need to extract the window ID from the event struct, then check to see if there's a pending DestroyNotify event for our window. If so, we remove it from our managed window list, and, if our managed window list was command-line supplied, remove the ID from the list parsed by the option handling code so other canvases don't trip over it. If not, it's business as usual.

To post a comment you must log in.
Alexandros Frantzis (afrantzis) wrote :

I don't see a need for CompositeCanvas::get_window_from_xevent(), we can just use event.xany.window for all events. Other than that, it looks good.

84. By Jesse Barker on 2012-04-19

Canvas: Slightly less intrusive version of the logic used to extract the window
ID from the event structure.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/composite-canvas-egl.cc'
2--- src/composite-canvas-egl.cc 2011-12-12 13:16:47 +0000
3+++ src/composite-canvas-egl.cc 2012-04-19 14:20:25 +0000
4@@ -301,6 +301,7 @@
5
6 std::stringstream ss;
7
8+ ss << " EGL " << eglQueryString(egl_display_, EGL_VERSION) << std::endl;
9 ss << " OpenGL Information" << std::endl;
10 ss << " GL_VENDOR: " << glGetString(GL_VENDOR) << std::endl;
11 ss << " GL_RENDERER: " << glGetString(GL_RENDERER) << std::endl;
12
13=== modified file 'src/composite-canvas-glx.cc'
14--- src/composite-canvas-glx.cc 2011-12-09 16:09:11 +0000
15+++ src/composite-canvas-glx.cc 2012-04-19 14:20:25 +0000
16@@ -254,6 +254,7 @@
17
18 std::stringstream ss;
19
20+ ss << " GLX " << glXGetClientString(xdpy_, GLX_VERSION) << std::endl;
21 ss << " OpenGL Information" << std::endl;
22 ss << " GL_VENDOR: " << glGetString(GL_VENDOR) << std::endl;
23 ss << " GL_RENDERER: " << glGetString(GL_RENDERER) << std::endl;
24
25=== modified file 'src/composite-canvas.cc'
26--- src/composite-canvas.cc 2012-03-09 03:58:54 +0000
27+++ src/composite-canvas.cc 2012-04-19 14:20:25 +0000
28@@ -368,7 +368,6 @@
29 {
30 XEvent event;
31 bool needs_redraw(false);
32- Window window(0);
33 Profiler::ScopedSamples xevent_scoped(CompositeCanvas::profiler_xevent_pair());
34
35 if (Options::idle_redraw && !XPending(xdpy_))
36@@ -376,16 +375,32 @@
37
38 XNextEvent(xdpy_, &event);
39
40- if (event.type == damage_event_ + XDamageNotify) {
41+ // We have to allow for the possibility that the window has actually
42+ // been destroyed, but we saw the current event before the destroy event.
43+ // So, check for a queued-up DestroyNotify event for our window before
44+ // trying to update (bug 984058).
45+ Window window = event.xany.window;
46+ XEvent destroyEvent;
47+ if (True == XCheckTypedWindowEvent(xdpy_, window, DestroyNotify, &destroyEvent)) {
48+ Log::debug("--- Destroy Win: 0x%x\n", window);
49+ remove_window(window);
50+ // If we're using a user provided list of windows, prune this one
51+ // out to make sure we don't create any new objects with a bad
52+ // window list.
53+ if (!Options::window_ids.empty())
54+ {
55+ Options::window_ids.remove(window);
56+ }
57+ needs_redraw = true;
58+ }
59+ else if (event.type == damage_event_ + XDamageNotify) {
60 // This is a little ugly, but then so is violating strict aliasing
61 // to make the hideous cast to reference this as an XDamageNotifyEvent.
62- window = event.xany.window;
63 Log::debug("### Damage Win: 0x%x\n", window);
64 handle_damage(window);
65 needs_redraw = true;
66 }
67 else if (event.type == ConfigureNotify) {
68- window = event.xconfigure.window;
69 Log::debug("Configure Win: 0x%x %d x %d\n",
70 window,
71 width_, height_);
72@@ -398,24 +413,20 @@
73 update_window(window);
74 }
75 else if (event.type == Expose) {
76- window = event.xexpose.window;
77 Log::debug("Expose Win: 0x%x\n", window);
78 needs_redraw = true;
79 }
80 else if (event.type == UnmapNotify) {
81- window = event.xunmap.window;
82 Log::debug("--- Unmap Win: 0x%x\n", window);
83 update_window(window);
84 needs_redraw = true;
85 }
86 else if (event.type == MapNotify) {
87- window = event.xmap.window;
88 Log::debug("+++ Map Win: 0x%x\n", window);
89 update_window(window);
90 needs_redraw = true;
91 }
92 else if (event.type == ReparentNotify) {
93- window = event.xreparent.window;
94 Window parent = event.xreparent.parent;
95 Log::debug("+++ Reparent Win: 0x%x Parent: 0x%x\n", parent);
96 if (parent == root_)
97@@ -425,13 +436,11 @@
98 needs_redraw = true;
99 }
100 else if (event.type == CreateNotify) {
101- window = event.xcreatewindow.window;
102 Log::debug("+++ Add Win: 0x%x Parent: 0x%x\n",
103 window, event.xcreatewindow.parent);
104 add_window(window);
105 }
106 else if (event.type == DestroyNotify) {
107- window = event.xdestroywindow.window;
108 Log::debug("--- Destroy Win: 0x%x\n", window);
109 remove_window(window);
110 needs_redraw = true;
111
112=== modified file 'src/composite-window.cc'
113--- src/composite-window.cc 2011-08-03 14:34:10 +0000
114+++ src/composite-window.cc 2012-04-19 14:20:25 +0000
115@@ -34,7 +34,7 @@
116 if (pix_)
117 XFreePixmap(xdpy_, pix_);
118
119- if (damage_)
120+ if (damage_ && attr_.map_state == IsUnmapped)
121 XDamageDestroy(xdpy_, damage_);
122 }
123
124@@ -72,12 +72,12 @@
125 width_, height_, attr_.map_state);
126
127 /* Update pixmap */
128- if (!pix_ && attr_.map_state != 0) {
129+ if (!pix_ && attr_.map_state != IsUnmapped) {
130 pix_ = XCompositeNameWindowPixmap(xdpy_, win_);
131 Log::debug(" => Pixmap <= 0x%x\n", (unsigned) pix_);
132 update_tfp = true;
133 }
134- else if (pix_ && attr_.map_state == 0) {
135+ else if (pix_ && attr_.map_state == IsUnmapped) {
136 Log::debug(" => Destroying Pixmap 0x%x\n", (unsigned) pix_);
137 pix_to_free = pix_;
138 pix_ = None;
139@@ -85,11 +85,11 @@
140
141 Log::debug(" => Damage State <= 0x%x\n", (unsigned) damage_);
142 /* Update damage object */
143- if (!damage_ && attr_.map_state != 0) {
144+ if (!damage_ && attr_.map_state != IsUnmapped) {
145 damage_ = XDamageCreate(xdpy_, win_, XDamageReportNonEmpty);
146 Log::debug(" => Damage <= 0x%x\n", (unsigned) damage_);
147 }
148- else if (damage_ && attr_.map_state == 0) {
149+ else if (damage_ && attr_.map_state == IsUnmapped) {
150 Log::debug(" => Destroying damage 0x%x\n", (unsigned) damage_);
151 XDamageDestroy(xdpy_, damage_);
152 damage_ = None;

Subscribers

People subscribed via source and target branches

to all changes: