Merge lp:~glmark2-dev/glmark2/canvas-reset into lp:glmark2/2011.11

Proposed by Jesse Barker
Status: Merged
Merged at revision: 189
Proposed branch: lp:~glmark2-dev/glmark2/canvas-reset
Merge into: lp:glmark2/2011.11
Diff against target: 399 lines (+132/-24)
12 files modified
doc/glmark2.1.in (+4/-0)
src/canvas-x11-egl.cpp (+46/-10)
src/canvas-x11-egl.h (+2/-0)
src/canvas-x11-glx.cpp (+9/-0)
src/canvas-x11-glx.h (+1/-0)
src/canvas-x11.cpp (+17/-9)
src/canvas-x11.h (+10/-0)
src/canvas.h (+10/-0)
src/main.cpp (+6/-0)
src/options.cpp (+6/-0)
src/options.h (+1/-0)
src/scene-desktop.cpp (+20/-5)
To merge this branch: bzr merge lp:~glmark2-dev/glmark2/canvas-reset
Reviewer Review Type Date Requested Status
Alexandros Frantzis Pending
Review via email: mp+88963@code.launchpad.net

Description of the change

Canvas: This adds logic to the canvas and its derivatives to allow a "reset" of the canvas between scenes. In practice what this does is to create a fresh context for each scene. This puts each scene on a level playing field and insulates the scenes from any resource leakage and any resultant performance degradation. Also included is an option to reuse one context for all scenes (the way things have worked up to now) for the sake of being able to test for resource leakage, etc.

SceneDesktop: A latent bug in the init/release symmetry in the RenderObject and its derivatives was detected as a direct result of the Canvas reset changes. This is also fixed in this branch (these changes should be taken regardless).

To post a comment you must log in.
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Thansk for the fix!

Unfortunately, I am still getting artifacts when running shadow,shadow or blur,blur. Strange...

lp:~glmark2-dev/glmark2/canvas-reset updated
183. By Alexandros Frantzis

SceneDesktop: Allow RenderObjects to be safely reused after a GL context change.

When setting the size of a RenderObject, "clear" the texture contents if
they were invalidated, even if the texture size didn't change.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'doc/glmark2.1.in'
--- doc/glmark2.1.in 2012-01-05 09:46:41 +0000
+++ doc/glmark2.1.in 2012-01-18 16:44:23 +0000
@@ -25,6 +25,10 @@
25Don't update the screen by swapping the front and25Don't update the screen by swapping the front and
26back buffer, use glFinish() instead26back buffer, use glFinish() instead
27.TP27.TP
28\fB\-\-reuse\-context\fR
29Use a single context for all scenes
30(by default, each scene gets its own context)
31.TP
28\fB\-s\fR, \fB\-\-size WxH\fR32\fB\-s\fR, \fB\-\-size WxH\fR
29Size of the output window (default: 800x600)33Size of the output window (default: 800x600)
30.TP34.TP
3135
=== modified file 'src/canvas-x11-egl.cpp'
--- src/canvas-x11-egl.cpp 2011-11-02 14:56:58 +0000
+++ src/canvas-x11-egl.cpp 2012-01-18 16:44:23 +0000
@@ -67,6 +67,9 @@
67 if (!ensure_egl_surface())67 if (!ensure_egl_surface())
68 return false;68 return false;
6969
70 if (!ensure_egl_context())
71 return false;
72
70 if (egl_context_ == eglGetCurrentContext())73 if (egl_context_ == eglGetCurrentContext())
71 return true;74 return true;
7275
@@ -175,8 +178,35 @@
175}178}
176179
177bool180bool
178CanvasX11EGL::ensure_egl_surface()181CanvasX11EGL::reset_context()
179{182{
183 if (!ensure_egl_display())
184 return false;
185
186 if (!egl_context_)
187 return true;
188
189 if (eglDestroyContext(egl_display_, egl_context_) == EGL_FALSE) {
190 Log::debug("eglDestroyContext() failed with error: 0x%x\n",
191 eglGetError());
192 }
193
194 egl_context_ = 0;
195 return true;
196}
197
198bool
199CanvasX11EGL::ensure_egl_context()
200{
201 if (egl_context_)
202 return true;
203
204 if (!ensure_egl_display())
205 return false;
206
207 if (!ensure_egl_config())
208 return false;
209
180 static const EGLint ctx_attribs[] = {210 static const EGLint ctx_attribs[] = {
181#ifdef USE_GLESv2211#ifdef USE_GLESv2
182 EGL_CONTEXT_CLIENT_VERSION, 2,212 EGL_CONTEXT_CLIENT_VERSION, 2,
@@ -184,6 +214,20 @@
184 EGL_NONE214 EGL_NONE
185 };215 };
186216
217 egl_context_ = eglCreateContext(egl_display_, egl_config_,
218 EGL_NO_CONTEXT, ctx_attribs);
219 if (!egl_context_) {
220 Log::error("eglCreateContext() failed with error: 0x%x\n",
221 eglGetError());
222 return false;
223 }
224
225 return true;
226}
227
228bool
229CanvasX11EGL::ensure_egl_surface()
230{
187 if (egl_surface_)231 if (egl_surface_)
188 return true;232 return true;
189233
@@ -196,14 +240,6 @@
196 eglBindAPI(EGL_OPENGL_API);240 eglBindAPI(EGL_OPENGL_API);
197#endif241#endif
198242
199 egl_context_ = eglCreateContext(egl_display_, egl_config_,
200 EGL_NO_CONTEXT, ctx_attribs);
201 if (!egl_context_) {
202 Log::error("eglCreateContext() failed with error: %d\n",
203 eglGetError());
204 return false;
205 }
206
207 egl_surface_ = eglCreateWindowSurface(egl_display_, egl_config_,243 egl_surface_ = eglCreateWindowSurface(egl_display_, egl_config_,
208 (EGLNativeWindowType) xwin_,244 (EGLNativeWindowType) xwin_,
209 NULL);245 NULL);
210246
=== modified file 'src/canvas-x11-egl.h'
--- src/canvas-x11-egl.h 2011-11-01 16:49:42 +0000
+++ src/canvas-x11-egl.h 2012-01-18 16:44:23 +0000
@@ -41,11 +41,13 @@
41protected:41protected:
42 XVisualInfo *get_xvisualinfo();42 XVisualInfo *get_xvisualinfo();
43 bool make_current();43 bool make_current();
44 bool reset_context();
44 void swap_buffers() { eglSwapBuffers(egl_display_, egl_surface_); }45 void swap_buffers() { eglSwapBuffers(egl_display_, egl_surface_); }
4546
46private:47private:
47 bool ensure_egl_display();48 bool ensure_egl_display();
48 bool ensure_egl_config();49 bool ensure_egl_config();
50 bool ensure_egl_context();
49 bool ensure_egl_surface();51 bool ensure_egl_surface();
50 void init_gl_extensions();52 void init_gl_extensions();
5153
5254
=== modified file 'src/canvas-x11-glx.cpp'
--- src/canvas-x11-glx.cpp 2011-11-11 11:07:15 +0000
+++ src/canvas-x11-glx.cpp 2012-01-18 16:44:23 +0000
@@ -201,6 +201,15 @@
201}201}
202202
203bool203bool
204CanvasX11GLX::reset_context()
205{
206 glXDestroyContext(xdpy_, glx_context_);
207 glx_context_ = 0;
208
209 return true;
210}
211
212bool
204CanvasX11GLX::ensure_glx_context()213CanvasX11GLX::ensure_glx_context()
205{214{
206 if (glx_context_)215 if (glx_context_)
207216
=== modified file 'src/canvas-x11-glx.h'
--- src/canvas-x11-glx.h 2011-11-01 16:49:42 +0000
+++ src/canvas-x11-glx.h 2012-01-18 16:44:23 +0000
@@ -41,6 +41,7 @@
41protected:41protected:
42 XVisualInfo *get_xvisualinfo();42 XVisualInfo *get_xvisualinfo();
43 bool make_current();43 bool make_current();
44 bool reset_context();
44 void swap_buffers() { glXSwapBuffers(xdpy_, xwin_); }45 void swap_buffers() { glXSwapBuffers(xdpy_, xwin_); }
4546
46private:47private:
4748
=== modified file 'src/canvas-x11.cpp'
--- src/canvas-x11.cpp 2011-11-24 15:52:33 +0000
+++ src/canvas-x11.cpp 2012-01-18 16:44:23 +0000
@@ -32,17 +32,10 @@
32/******************32/******************
33 * Public methods *33 * Public methods *
34 ******************/34 ******************/
35
36bool35bool
37CanvasX11::init()36CanvasX11::reset()
38{37{
39 xdpy_ = XOpenDisplay(NULL);38 if (!reset_context())
40 if (!xdpy_)
41 return false;
42
43 resize_no_viewport(width_, height_);
44
45 if (!xwin_)
46 return false;39 return false;
4740
48 if (!make_current())41 if (!make_current())
@@ -67,6 +60,21 @@
67 return true;60 return true;
68}61}
6962
63bool
64CanvasX11::init()
65{
66 xdpy_ = XOpenDisplay(NULL);
67 if (!xdpy_)
68 return false;
69
70 resize_no_viewport(width_, height_);
71
72 if (!xwin_)
73 return false;
74
75 return reset();
76}
77
70void78void
71CanvasX11::visible(bool visible)79CanvasX11::visible(bool visible)
72{80{
7381
=== modified file 'src/canvas-x11.h'
--- src/canvas-x11.h 2011-11-24 15:52:33 +0000
+++ src/canvas-x11.h 2012-01-18 16:44:23 +0000
@@ -35,6 +35,7 @@
35 ~CanvasX11() {}35 ~CanvasX11() {}
3636
37 virtual bool init();37 virtual bool init();
38 virtual bool reset();
38 virtual void visible(bool visible);39 virtual void visible(bool visible);
39 virtual void clear();40 virtual void clear();
40 virtual void update();41 virtual void update();
@@ -69,6 +70,15 @@
69 virtual bool make_current() = 0;70 virtual bool make_current() = 0;
7071
71 /**72 /**
73 * Resets the underlying GL context for rendering.
74 *
75 * This method should be implemented in derived classes.
76 *
77 * @return whether the operation succeeded
78 */
79 virtual bool reset_context() = 0;
80
81 /**
72 * Swaps the GL buffers (assuming double buffering is used).82 * Swaps the GL buffers (assuming double buffering is used).
73 *83 *
74 * This method should be implemented in derived classes.84 * This method should be implemented in derived classes.
7585
=== modified file 'src/canvas.h'
--- src/canvas.h 2011-11-10 10:33:26 +0000
+++ src/canvas.h 2012-01-18 16:44:23 +0000
@@ -96,6 +96,16 @@
96 virtual bool init() { return false; }96 virtual bool init() { return false; }
9797
98 /**98 /**
99 * Resets the canvas, destroying and recreating resources to give each new
100 * test scenario a fresh context for rendering.
101 *
102 * This method should be implemented in derived classes.
103 *
104 * @return whether reset succeeded
105 */
106 virtual bool reset() { return false; }
107
108 /**
99 * Changes the visibility of the canvas.109 * Changes the visibility of the canvas.
100 *110 *
101 * The canvas is initially not visible.111 * The canvas is initially not visible.
102112
=== modified file 'src/main.cpp'
--- src/main.cpp 2012-01-17 15:44:04 +0000
+++ src/main.cpp 2012-01-18 16:44:23 +0000
@@ -174,6 +174,9 @@
174 bench_iter != benchmarks.end();174 bench_iter != benchmarks.end();
175 bench_iter++)175 bench_iter++)
176 {176 {
177 if (!Options::reuse_context)
178 canvas.reset();
179
177 bool keep_running = true;180 bool keep_running = true;
178 Benchmark *bench = *bench_iter;181 Benchmark *bench = *bench_iter;
179 uint64_t fps_timestamp = Util::get_timestamp_us();182 uint64_t fps_timestamp = Util::get_timestamp_us();
@@ -232,6 +235,9 @@
232 bench_iter != benchmarks.end();235 bench_iter != benchmarks.end();
233 bench_iter++)236 bench_iter++)
234 {237 {
238 if (!Options::reuse_context)
239 canvas.reset();
240
235 Benchmark *bench = *bench_iter;241 Benchmark *bench = *bench_iter;
236 Scene &scene = bench->setup_scene();242 Scene &scene = bench->setup_scene();
237243
238244
=== modified file 'src/options.cpp'
--- src/options.cpp 2011-12-13 14:27:23 +0000
+++ src/options.cpp 2012-01-18 16:44:23 +0000
@@ -41,12 +41,14 @@
41bool Options::show_debug = false;41bool Options::show_debug = false;
42bool Options::show_fps = false;42bool Options::show_fps = false;
43bool Options::show_help = false;43bool Options::show_help = false;
44bool Options::reuse_context = false;
4445
45static struct option long_options[] = {46static struct option long_options[] = {
46 {"benchmark", 1, 0, 0},47 {"benchmark", 1, 0, 0},
47 {"benchmark-file", 1, 0, 0},48 {"benchmark-file", 1, 0, 0},
48 {"validate", 0, 0, 0},49 {"validate", 0, 0, 0},
49 {"no-swap-buffers", 0, 0, 0},50 {"no-swap-buffers", 0, 0, 0},
51 {"reuse-context", 0, 0, 0},
50 {"size", 1, 0, 0},52 {"size", 1, 0, 0},
51 {"list-scenes", 0, 0, 0},53 {"list-scenes", 0, 0, 0},
52 {"show-all-options", 0, 0, 0},54 {"show-all-options", 0, 0, 0},
@@ -95,6 +97,8 @@
95 " running the benchmarks\n"97 " running the benchmarks\n"
96 " --no-swap-buffers Don't update the canvas by swapping the front and\n"98 " --no-swap-buffers Don't update the canvas by swapping the front and\n"
97 " back buffer, use glFinish() instead\n"99 " back buffer, use glFinish() instead\n"
100 " --reuse-context Use a single context for all scenes\n"
101 " (by default, each scene gets its own context)\n"
98 " -s, --size WxH Size of the output window (default: 800x600)\n"102 " -s, --size WxH Size of the output window (default: 800x600)\n"
99 " -l, --list-scenes Display information about the available scenes\n"103 " -l, --list-scenes Display information about the available scenes\n"
100 " and their options\n"104 " and their options\n"
@@ -132,6 +136,8 @@
132 Options::validate = true;136 Options::validate = true;
133 else if (!strcmp(optname, "no-swap-buffers"))137 else if (!strcmp(optname, "no-swap-buffers"))
134 Options::swap_buffers = false;138 Options::swap_buffers = false;
139 else if (!strcmp(optname, "reuse-context"))
140 Options::reuse_context = true;
135 else if (c == 's' || !strcmp(optname, "size"))141 else if (c == 's' || !strcmp(optname, "size"))
136 parse_size(optarg, Options::size);142 parse_size(optarg, Options::size);
137 else if (c == 'l' || !strcmp(optname, "list-scenes"))143 else if (c == 'l' || !strcmp(optname, "list-scenes"))
138144
=== modified file 'src/options.h'
--- src/options.h 2011-12-13 14:27:23 +0000
+++ src/options.h 2012-01-18 16:44:23 +0000
@@ -41,6 +41,7 @@
41 static bool show_debug;41 static bool show_debug;
42 static bool show_fps;42 static bool show_fps;
43 static bool show_help;43 static bool show_help;
44 static bool reuse_context;
44};45};
4546
46#endif /* OPTIONS_H_ */47#endif /* OPTIONS_H_ */
4748
=== modified file 'src/scene-desktop.cpp'
--- src/scene-desktop.cpp 2011-12-08 11:09:09 +0000
+++ src/scene-desktop.cpp 2012-01-18 16:44:23 +0000
@@ -106,9 +106,9 @@
106class RenderObject106class RenderObject
107{107{
108public:108public:
109 RenderObject() : texture_(0), fbo_(0), rotation_rad_(0) { }109 RenderObject() :
110110 texture_(0), fbo_(0), rotation_rad_(0),
111 virtual ~RenderObject() { release(); }111 texture_contents_invalid_(true) { }
112112
113 virtual void init()113 virtual void init()
114 {114 {
@@ -138,6 +138,7 @@
138 frg_source.str());138 frg_source.str());
139 }139 }
140140
141 texture_contents_invalid_ = true;
141 RenderObject::use_count++;142 RenderObject::use_count++;
142 }143 }
143144
@@ -182,7 +183,12 @@
182 glBindTexture(GL_TEXTURE_2D, texture_);183 glBindTexture(GL_TEXTURE_2D, texture_);
183 glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, size_.x(), size_.y(), 0,184 glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, size_.x(), size_.y(), 0,
184 GL_RGBA, GL_UNSIGNED_BYTE, 0);185 GL_RGBA, GL_UNSIGNED_BYTE, 0);
186 texture_contents_invalid_ = true;
187 }
188
189 if (texture_contents_invalid_) {
185 clear();190 clear();
191 texture_contents_invalid_ = false;
186 }192 }
187 }193 }
188194
@@ -332,6 +338,7 @@
332 }338 }
333339
334 float rotation_rad_;340 float rotation_rad_;
341 bool texture_contents_invalid_;
335 static int use_count;342 static int use_count;
336343
337};344};
@@ -348,6 +355,7 @@
348{355{
349public:356public:
350 virtual void init() {}357 virtual void init() {}
358 virtual void release() {}
351};359};
352360
353/**361/**
@@ -625,7 +633,6 @@
625 shadow_v_.release();633 shadow_v_.release();
626 shadow_corner_.release();634 shadow_corner_.release();
627 if (draw_contents_)635 if (draw_contents_)
628 if (draw_contents_)
629 window_contents_.release();636 window_contents_.release();
630 }637 }
631638
@@ -849,7 +856,15 @@
849void856void
850SceneDesktop::teardown()857SceneDesktop::teardown()
851{858{
852 Util::dispose_pointer_vector(priv_->windows);859 for (std::vector<RenderObject*>::iterator winIt = priv_->windows.begin();
860 winIt != priv_->windows.end();
861 winIt++)
862 {
863 RenderObject* curObj = *winIt;
864 curObj->release();
865 delete curObj;
866 }
867 priv_->windows.clear();
853 priv_->screen.make_current();868 priv_->screen.make_current();
854869
855 glEnable(GL_DEPTH_TEST);870 glEnable(GL_DEPTH_TEST);

Subscribers

People subscribed via source and target branches

to all changes: