Merge lp:~widelands-dev/widelands/opengl_debug into lp:widelands

Proposed by SirVer
Status: Merged
Merged at revision: 7742
Proposed branch: lp:~widelands-dev/widelands/opengl_debug
Merge into: lp:widelands
Diff against target: 634 lines (+270/-107)
13 files modified
cmake/WlFunctions.cmake (+8/-3)
src/graphic/CMakeLists.txt (+2/-0)
src/graphic/gl/initialize.cc (+151/-0)
src/graphic/gl/initialize.h (+44/-0)
src/graphic/gl/system_headers.h (+3/-4)
src/graphic/gl/utils.cc (+18/-33)
src/graphic/gl/utils.h (+3/-7)
src/graphic/graphic.cc (+10/-50)
src/graphic/graphic.h (+5/-2)
src/graphic/text/test/render_richtext.cc (+1/-1)
src/graphic/texture.cc (+20/-3)
src/logic/map_info.cc (+1/-1)
src/wlapplication.cc (+4/-3)
To merge this branch: bzr merge lp:~widelands-dev/widelands/opengl_debug
Reviewer Review Type Date Requested Status
Tino Needs Fixing
GunChleoc Approve
Review via email: mp+283738@code.launchpad.net

Commit message

- Fixes a bug: We never created zero sized textures, but sometimes we tried to render on them. This is uncritical, theoretically, but I fixed it anyways.
- Added a new undocumented command line argument --debug_gl_trace which will log every OpenGL call that is made, together with arguments, return values and glError status. This requires that Widelands is build using -DOPTION_USE_GLBINDING:BOOL=ON. It is a NoOp for GLEW. This will help debugging non-reproducible OpenGL errors that are reported. Tested with glbinding 1.1.0.
- More logging when OpenGL is initialized.
- Pulls out OpenGL initialization into a separate unit.

Description of the change

Small bug fixes and better debug potential for OpenGL - but only if glbinding is used.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/opengl_debug mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_opengl_debug

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Not compiled or tested yet, but code LGTM - just 2 nits.

I think the option and build requirement should be documented somewhere in the code, because we won't find it in this commit message down the road. I think copying the commit message to src/graphic/gl/initialize.cc should be enough for now.

Revision history for this message
kaputtnik (franku) wrote :

This is what i get with this branch:

Set home directory: /home/kaputtnik/.widelands
Widelands executable directory: /home/kaputtnik/widelands-repo/opengl_debug/
Adding directory: /home/kaputtnik/widelands-repo/opengl_debug/.
selected language: (system language)
using locale de_DE.UTF-8
Graphics: Try to set Videomode 800x600
Graphics: OpenGL: Version "3.0 Mesa 11.1.1"
Graphics: SDL_GL_RED_SIZE is 8
Graphics: SDL_GL_GREEN_SIZE is 8
Graphics: SDL_GL_BLUE_SIZE is 8
Graphics: SDL_GL_ALPHA_SIZE is 0
Graphics: SDL_GL_BUFFER_SIZE is 24
Graphics: SDL_GL_DOUBLEBUFFER is 1
Graphics: SDL_GL_DEPTH_SIZE is 24
Graphics: SDL_GL_STENCIL_SIZE is 8
Graphics: SDL_GL_ACCUM_RED_SIZE is 0
Graphics: SDL_GL_ACCUM_GREEN_SIZE is 0
Graphics: SDL_GL_ACCUM_BLUE_SIZE is 0
Graphics: SDL_GL_ACCUM_ALPHA_SIZE is 0
Graphics: SDL_GL_STEREO is 0
Graphics: SDL_GL_MULTISAMPLEBUFFERS is 0
Graphics: SDL_GL_MULTISAMPLESAMPLES is 0
Graphics: SDL_GL_ACCELERATED_VISUAL is 1
Graphics: SDL_GL_CONTEXT_MAJOR_VERSION is 2
Graphics: SDL_GL_CONTEXT_MINOR_VERSION is 1
Graphics: SDL_GL_CONTEXT_FLAGS is 0
Graphics: SDL_GL_CONTEXT_PROFILE_MASK is 2
Graphics: SDL_GL_SHARE_WITH_CURRENT_CONTEXT is 0
Graphics: SDL_GL_FRAMEBUFFER_SRGB_CAPABLE is 0
Graphics: OpenGL: Double buffering enabled
Graphics: OpenGL: Max texture size: 8192
Graphics: OpenGL: ShadingLanguage: "1.30"
**** GRAPHICS REPORT ****
 VIDEO DRIVER x11
 pixel fmt 370546692
 size 800 600
**** END GRAPHICS REPORT ****

Revision history for this message
SirVer (sirver) wrote :

Adresed all comments and copied an explanative text into initialize.cc

Revision history for this message
GunChleoc (gunchleoc) wrote :

Compiler warning:

[ 11%] Building CXX object src/graphic/CMakeFiles/graphic_gl_utils.dir/gl/initialize.cc.o
/home/bratzbert/sources/widelands/opengl_debug/src/graphic/gl/initialize.cc:32:1: warning: unused parameter ‘trace’ [-Wunused-parameter]
 initialize(const Trace& trace, SDL_Window* sdl_window, GLint* max_texture_size) {
 ^

Revision history for this message
SirVer (sirver) wrote :

fixed the unused variable.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Here's my output - it complains about "debug_gl_trace" not being Boolean, both before and after the commit regarding trace. I simply called ./compile.sh for the compilation, is the switch in CMake set?

[] [/home/bratzbert/sources/widelands/opengl_debug/src/profile/profile.cc:167] debug_gl_trace: '' is not a boolean value
Graphics: Try to set Videomode 800x600
Graphics: OpenGL: Version "2.1 Mesa 10.1.3"
Graphics: SDL_GL_RED_SIZE is 8
Graphics: SDL_GL_GREEN_SIZE is 8
Graphics: SDL_GL_BLUE_SIZE is 8
Graphics: SDL_GL_ALPHA_SIZE is 8
Graphics: SDL_GL_BUFFER_SIZE is 32
Graphics: SDL_GL_DOUBLEBUFFER is 1
Graphics: SDL_GL_DEPTH_SIZE is 24
Graphics: SDL_GL_STENCIL_SIZE is 8
Graphics: SDL_GL_ACCUM_RED_SIZE is 0
Graphics: SDL_GL_ACCUM_GREEN_SIZE is 0
Graphics: SDL_GL_ACCUM_BLUE_SIZE is 0
Graphics: SDL_GL_ACCUM_ALPHA_SIZE is 0
Graphics: SDL_GL_STEREO is 0
Graphics: SDL_GL_MULTISAMPLEBUFFERS is 0
Graphics: SDL_GL_MULTISAMPLESAMPLES is 0
Graphics: SDL_GL_ACCELERATED_VISUAL is 1
Graphics: SDL_GL_CONTEXT_MAJOR_VERSION is 2
Graphics: SDL_GL_CONTEXT_MINOR_VERSION is 1
Graphics: SDL_GL_CONTEXT_FLAGS is 0
Graphics: SDL_GL_CONTEXT_PROFILE_MASK is 2
Graphics: SDL_GL_SHARE_WITH_CURRENT_CONTEXT is 0
Graphics: SDL_GL_FRAMEBUFFER_SRGB_CAPABLE is 0
Graphics: OpenGL: Double buffering enabled
Graphics: OpenGL: Max texture size: 8192
Graphics: OpenGL: ShadingLanguage: "1.20"
**** GRAPHICS REPORT ****
 VIDEO DRIVER x11
 pixel fmt 370546692
 size 800 600
**** END GRAPHICS REPORT ****

Revision history for this message
SirVer (sirver) wrote :

> [] [/home/bratzbert/sources/widelands/opengl_debug/src/profile/profile.cc:167] debug_gl_trace: '' is not a boolean value

Seems like you have it already set in your ~/.widelands/config? strange.

> is the switch in CMake set?

No, we default to using GLEW for compilation since it is mature and packaged and available. You need to set the flag yourself and install glbinding[1] to test this.

cmake -G Ninja -DOPTION_USE_GLBINDING:BOOL=ON -DCMAKE_BUILD_TYPE:STRING="Debug" ..

should do the trick.

[1] https://github.com/cginternals/glbinding

Revision history for this message
GunChleoc (gunchleoc) wrote :

My fault, I passed --debug_gl_trace instead of --debug_gl_trace=yes

My system can't handle ninja, so I tried now:

mkdir build
cd build
cmake -DOPTION_USE_GLBINDING:BOOL=ON -DCMAKE_BUILD_TYPE:STRING="Debug" ..

Which gives me:

CMake Error at CMakeLists.txt:60 (find_package):
  By not providing "Findglbinding.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "glbinding", but CMake did not find one.

  Could not find a package configuration file provided by "glbinding" with
  any of the following names:

    glbindingConfig.cmake
    glbinding-config.cmake

  Add the installation prefix of "glbinding" to CMAKE_PREFIX_PATH or set
  "glbinding_DIR" to a directory containing one of the above files. If
  "glbinding" provides a separate development package or SDK, be sure it has
  been installed.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Duh, I need to install glbinding first... compiling now.

Revision history for this message
GunChleoc (gunchleoc) wrote :

OK, next problem:

bratzbert@museum:~/sources/widelands/opengl_debug$ cd build
bratzbert@museum:~/sources/widelands/opengl_debug/build$ cmake -DOPTION_USE_GLBINDING:BOOL=ON -DCMAKE_BUILD_TYPE:STRING="Debug" .
-- Boost version: 1.54.0
-- Found the following Boost libraries:
-- unit_test_framework
-- regex
-- SDL2_INCLUDE_DIR is /usr/include/SDL2
-- SDL2MAIN_LIBRARY is /usr/lib/x86_64-linux-gnu/libSDL2main.a
-- Found GLBINDING: /usr/local/lib/libglbinding.so
-- Version of Widelands Build is bzr7737[opengl_debug](Debug)
-- Found Doxygen: /usr/bin/doxygen (found version "1.8.6")
-- Configuring done
-- Generating done
-- Build files have been written to: /home/bratzbert/sources/widelands/opengl_debug/build

bratzbert@museum:~/sources/widelands/opengl_debug/build$ make
Scanning dependencies of target BzrRevision
-- Version of Widelands Build is bzr7737[opengl_debug](Debug)
[ 0%] Built target BzrRevision
Scanning dependencies of target third_party_eris

<snip>

[ 11%] Building CXX object src/graphic/CMakeFiles/graphic_gl_utils.dir/gl/initialize.cc.o
/home/bratzbert/sources/widelands/opengl_debug/src/graphic/gl/initialize.cc: In lambda function:
/home/bratzbert/sources/widelands/opengl_debug/src/graphic/gl/initialize.cc:62:29: error: request for member ‘name’ in ‘call.glbinding::FunctionCall::function’, which is of pointer type ‘const glbinding::AbstractFunction* const’ (maybe you meant to use ‘->’ ?)
    log("%s(", call.function.name());
                             ^
make[2]: *** [src/graphic/CMakeFiles/graphic_gl_utils.dir/gl/initialize.cc.o] Error 1
make[1]: *** [src/graphic/CMakeFiles/graphic_gl_utils.dir/all] Error 2
make: *** [all] Error 2

Revision history for this message
wl-zocker (wl-zocker) wrote :

Is this branch to find the problems I encounter? If so, will there be an appveyor build of this branch that will be built with the needed parameters? (I have no building environment on Windows.)

Revision history for this message
TiborB (tiborb95) wrote :

The problem is gone. The code also LGTM.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 377. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/104463853.
Appveyor build 284. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-284.

Revision history for this message
SirVer (sirver) wrote :

> The problem is gone. The code also LGTM.

Tibor, Which problem are you referring to?

> [ 11%] Building CXX object src/graphic/CMakeFiles/graphic_gl_utils.dir/gl/initialize.cc.o
...

I used a older version of glbinding, apparently they changed the interface. I updated to 1.1.0 and got the same error. Fixed now.

> Is this branch to find the problems I encounter?

There are 3 lurking issues with OpenGL right now: bug 1536377, bug 1535732 and a dynamic drawing problem that was reported on Facebook: http://www.youtube.com/watch?v=v8JG1qjZQ9o. I hope this branch gets us closer to fixing all of them.

> If so, will there be an appveyor build of this branch that will be built with the needed parameters?

That is the intention. Tino managed to build this branch on his local machine already and he was trying to make it work on appveyor too. The debug code will also be in release builds, so a build from appveyor will have it.

We should also switch our PPA away from glew and to glbinding, IMHO. It is a much superior gl loader.

Revision history for this message
TiborB (tiborb95) wrote :

@SirVer,

Please ignore my comment, it belongs to other merge request...

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 382. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/104561461.
Appveyor build 289. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-289.

Revision history for this message
Tino (tino79) wrote :

I did not succeed in compiling the glbindings library for now. It does seem to depend on posix threads and does not detect the correct version on my system with mingw-w64.
But i'll continue to try...

I've created a github branch for this branch with a changed appveyor.yml wich enables glbinding: https://ci.appveyor.com/project/widelands-dev/widelands/build/opengl_binding-290

Let's wait if appveyor does manage to build it at some point without a timeout...

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 382. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/104561461.
Appveyor build 289. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-289.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I can compile and run Widelands now.

review: Approve
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 382. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/104561461.
Appveyor build 289. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-289.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 385. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/104601532.
Appveyor build 293. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-293.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 385. State: canceled. Details: https://travis-ci.org/widelands/widelands/builds/104601532.
Appveyor build 293. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-293.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 386. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/104617324.
Appveyor build 294. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_opengl_debug-294.

Revision history for this message
Tino (tino79) wrote :

Ok, i've changed both appveyor and travis scripts to use glbindings:
- Appveyor does fail because it seems at some point GLEW is still included
- Travis fails because no glbindings debian package is installed

I would really like to get both CI systems to build with GLBindings before merging.

review: Needs Fixing
Revision history for this message
wl-zocker (wl-zocker) wrote :

Please ping me once you have something I can try. I will not read all comments in this request in detail.

Revision history for this message
SirVer (sirver) wrote :

> c I would really like to get both CI systems to build with GLBindings before merging.

As discussed in chat, I reverted the CI changes. I'd like to get the code in as it is approved, we can figure out CI in separate branches after that.

> - Appveyor does fail because it seems at some point GLEW is still included

Fixed in 7744.

> - Travis fails because no glbindings debian package is installed

We need to wire up a PPA I guess. More important than travis is our own PPA though - having the trace feature in the PPA would make debugging OpenGL issues on other Linux computers much easier.

> Please ping me once you have something I can try. I will not read all comments in this request in detail.

I will update the bugs and reach out to once this is all figured out. Still a chunk of work to do though.

@bunnybot merge

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cmake/WlFunctions.cmake'
2--- cmake/WlFunctions.cmake 2015-09-19 12:19:37 +0000
3+++ cmake/WlFunctions.cmake 2016-01-25 20:17:39 +0000
4@@ -85,18 +85,23 @@
5 endif()
6
7 # OpenGL and GLEW are one thing for us. If you use the one, you also use the
8- # other.
9+ # other. We always add definitions, because add_definition() is not
10+ # transitive and therefore some dependent targets do not set them properly.
11+ # And a few -D do not hurt anything.
12+ if(OPTION_USE_GLBINDING)
13+ add_definitions("-DUSE_GLBINDING")
14+ else()
15+ add_definitions(${GLEW_EXTRA_DEFINITIONS})
16+ endif()
17 if(ARG_USES_OPENGL)
18 if(OPTION_USE_GLBINDING)
19 wl_include_system_directories(${NAME} ${GLBINDING_INCLUDES})
20 target_link_libraries(${NAME} ${GLBINDING_LIBRARIES})
21 target_link_libraries(${NAME} ${OPENGL_gl_LIBRARY})
22- add_definitions("-DUSE_GLBINDING")
23 else()
24 wl_include_system_directories(${NAME} ${GLEW_INCLUDE_DIR})
25 target_link_libraries(${NAME} ${GLEW_LIBRARY})
26 target_link_libraries(${NAME} ${OPENGL_gl_LIBRARY})
27- add_definitions(${GLEW_EXTRA_DEFINITIONS})
28 endif()
29 endif()
30
31
32=== modified file 'src/graphic/CMakeLists.txt'
33--- src/graphic/CMakeLists.txt 2016-01-19 07:33:34 +0000
34+++ src/graphic/CMakeLists.txt 2016-01-25 20:17:39 +0000
35@@ -87,6 +87,8 @@
36 SRCS
37 gl/blit_data.h
38 gl/coordinate_conversion.h
39+ gl/initialize.cc
40+ gl/initialize.h
41 gl/system_headers.h
42 gl/utils.cc
43 gl/utils.h
44
45=== added file 'src/graphic/gl/initialize.cc'
46--- src/graphic/gl/initialize.cc 1970-01-01 00:00:00 +0000
47+++ src/graphic/gl/initialize.cc 2016-01-25 20:17:39 +0000
48@@ -0,0 +1,151 @@
49+/*
50+ * Copyright (C) 2006-2016 by the Widelands Development Team
51+ *
52+ * This program is free software; you can redistribute it and/or
53+ * modify it under the terms of the GNU General Public License
54+ * as published by the Free Software Foundation; either version 2
55+ * of the License, or (at your option) any later version.
56+ *
57+ * This program is distributed in the hope that it will be useful,
58+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
59+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
60+ * GNU General Public License for more details.
61+ *
62+ * You should have received a copy of the GNU General Public License
63+ * along with this program; if not, write to the Free Software
64+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
65+ *
66+ */
67+
68+#include "graphic/gl/initialize.h"
69+
70+#include <csignal>
71+
72+#include <SDL.h>
73+
74+#include "base/macros.h"
75+#include "graphic/gl/utils.h"
76+
77+namespace Gl {
78+
79+SDL_GLContext initialize(
80+#ifdef USE_GLBINDING
81+ const Trace& trace,
82+#else
83+ const Trace&,
84+#endif
85+ SDL_Window* sdl_window,
86+ GLint* max_texture_size) {
87+ // Request an OpenGL 2 context with double buffering.
88+ SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1);
89+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);
90+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
91+ SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY);
92+ SDL_GL_SetAttribute(SDL_GL_DEPTH_SIZE, 16);
93+ SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
94+
95+ SDL_GLContext gl_context = SDL_GL_CreateContext(sdl_window);
96+ SDL_GL_MakeCurrent(sdl_window, gl_context);
97+
98+#ifdef USE_GLBINDING
99+ glbinding::Binding::initialize();
100+
101+ // The undocumented command line argument --debug_gl_trace will set
102+ // Trace::kYes. This will log every OpenGL call that is made, together with
103+ // arguments, return values and glError status. This requires that Widelands
104+ // is built using -DOPTION_USE_GLBINDING:BOOL=ON. It is a NoOp for GLEW.
105+ if (trace == Trace::kYes) {
106+ setCallbackMaskExcept(
107+ glbinding::CallbackMask::After | glbinding::CallbackMask::ParametersAndReturnValue,
108+ {"glGetError"});
109+ glbinding::setAfterCallback([](const glbinding::FunctionCall& call) {
110+ log("%s(", call.function->name());
111+ for (size_t i = 0; i < call.parameters.size(); ++i) {
112+ log("%s", call.parameters[i]->asString().c_str());
113+ if (i < call.parameters.size() - 1)
114+ log(", ");
115+ }
116+ log(")");
117+ if (call.returnValue) {
118+ log(" -> %s", call.returnValue->asString().c_str());
119+ }
120+ const auto error = glGetError();
121+ log(" [%s]\n", gl_error_to_string(error));
122+ // The next few lines will terminate Widelands if there was any OpenGL
123+ // error. This is useful for super aggressive debugging, but probably
124+ // not for regular builds. Comment it in if you need to understand
125+ // OpenGL problems.
126+ // if (error != GL_NO_ERROR) {
127+ // std::raise(SIGINT);
128+ // }
129+ });
130+ }
131+#else
132+ // See graphic/gl/system_headers.h for an explanation of the next line.
133+ glewExperimental = GL_TRUE;
134+ GLenum err = glewInit();
135+ if (err != GLEW_OK) {
136+ log("glewInit returns %i\nYour OpenGL installation must be __very__ broken. %s\n", err,
137+ glewGetErrorString(err));
138+ throw wexception("glewInit returns %i: Broken OpenGL installation.", err);
139+ }
140+#endif
141+
142+ log(
143+ "Graphics: OpenGL: Version \"%s\"\n", reinterpret_cast<const char*>(glGetString(GL_VERSION)));
144+
145+#define LOG_SDL_GL_ATTRIBUTE(x) \
146+ { \
147+ int value; \
148+ SDL_GL_GetAttribute(x, &value); \
149+ log("Graphics: %s is %d\n", #x, value); \
150+ }
151+
152+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_RED_SIZE)
153+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_GREEN_SIZE)
154+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_BLUE_SIZE);
155+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ALPHA_SIZE);
156+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_BUFFER_SIZE);
157+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_DOUBLEBUFFER);
158+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_DEPTH_SIZE);
159+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_STENCIL_SIZE);
160+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_RED_SIZE);
161+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_GREEN_SIZE);
162+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_BLUE_SIZE);
163+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_ALPHA_SIZE);
164+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_STEREO);
165+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_MULTISAMPLEBUFFERS);
166+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_MULTISAMPLESAMPLES);
167+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCELERATED_VISUAL);
168+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_MAJOR_VERSION);
169+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_MINOR_VERSION);
170+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_FLAGS);
171+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_PROFILE_MASK);
172+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_SHARE_WITH_CURRENT_CONTEXT);
173+ LOG_SDL_GL_ATTRIBUTE(SDL_GL_FRAMEBUFFER_SRGB_CAPABLE);
174+#undef LOG_SDL_GL_ATTRIBUTE
175+
176+ GLboolean glBool;
177+ glGetBooleanv(GL_DOUBLEBUFFER, &glBool);
178+ log("Graphics: OpenGL: Double buffering %s\n", (glBool == GL_TRUE) ? "enabled" : "disabled");
179+
180+ glGetIntegerv(GL_MAX_TEXTURE_SIZE, max_texture_size);
181+ log("Graphics: OpenGL: Max texture size: %u\n", *max_texture_size);
182+
183+ log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n",
184+ reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION)));
185+
186+ glDrawBuffer(GL_BACK);
187+
188+ glDisable(GL_DEPTH_TEST);
189+ glDepthFunc(GL_LEQUAL);
190+
191+ glEnable(GL_BLEND);
192+ glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
193+
194+ glClear(GL_COLOR_BUFFER_BIT);
195+
196+ return gl_context;
197+}
198+
199+} // namespace Gl
200
201=== added file 'src/graphic/gl/initialize.h'
202--- src/graphic/gl/initialize.h 1970-01-01 00:00:00 +0000
203+++ src/graphic/gl/initialize.h 2016-01-25 20:17:39 +0000
204@@ -0,0 +1,44 @@
205+/*
206+ * Copyright (C) 2006-2016 by the Widelands Development Team
207+ *
208+ * This program is free software; you can redistribute it and/or
209+ * modify it under the terms of the GNU General Public License
210+ * as published by the Free Software Foundation; either version 2
211+ * of the License, or (at your option) any later version.
212+ *
213+ * This program is distributed in the hope that it will be useful,
214+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
215+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
216+ * GNU General Public License for more details.
217+ *
218+ * You should have received a copy of the GNU General Public License
219+ * along with this program; if not, write to the Free Software
220+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
221+ *
222+ */
223+
224+#ifndef WL_GRAPHIC_GL_INITIALIZE_H
225+#define WL_GRAPHIC_GL_INITIALIZE_H
226+
227+#include <SDL.h>
228+
229+#include "graphic/gl/system_headers.h"
230+
231+namespace Gl {
232+
233+// Initializes OpenGL. Creates a context for 'window' using SDL and loads the
234+// GL library. Fills in 'max_texture_size' and returns the created SDL_Context
235+// which must be closed by the caller.
236+// If we are built against glbinding, 'trace' will set up tracing for
237+// OpenGL and output every single opengl call ever made, together with it's
238+// arguments, return values and the result from glGetError.
239+enum class Trace {
240+ kYes,
241+ kNo,
242+};
243+SDL_GLContext
244+initialize(const Trace& trace, SDL_Window* window, GLint* max_texture_size);
245+
246+} // namespace Gl
247+
248+#endif // end of include guard: WL_GRAPHIC_GL_INITIALIZE_H
249
250=== modified file 'src/graphic/gl/system_headers.h'
251--- src/graphic/gl/system_headers.h 2016-01-23 15:44:29 +0000
252+++ src/graphic/gl/system_headers.h 2016-01-25 20:17:39 +0000
253@@ -32,10 +32,7 @@
254 // http://stackoverflow.com/questions/13558073/program-crash-on-glgenvertexarrays-call.
255 //
256 // TODO(sirver): glbinding seems to be a sane solution to the GL
257-// loading problem. (https://github.com/hpicgs/glbinding).
258-
259-// GLEW must be first. Do not include any other GL headers, it
260-// should define all functions.
261+// loading problem. Switch to it everywhere. (https://github.com/hpicgs/glbinding).
262
263 #ifdef USE_GLBINDING
264 # include <glbinding/gl/gl.h>
265@@ -45,6 +42,8 @@
266 using namespace gl;
267 CLANG_DIAG_ON("-Wheader-hygiene")
268 #else
269+// GLEW must be first. Do not include any other GL headers, it
270+// should define all functions.
271 # include <GL/glew.h>
272 #endif
273
274
275=== modified file 'src/graphic/gl/utils.cc'
276--- src/graphic/gl/utils.cc 2016-01-23 15:44:29 +0000
277+++ src/graphic/gl/utils.cc 2016-01-25 20:17:39 +0000
278@@ -18,6 +18,7 @@
279
280 #include "graphic/gl/utils.h"
281
282+#include <cassert>
283 #include <memory>
284 #include <string>
285
286@@ -43,41 +44,24 @@
287
288 } // namespace
289
290-GLenum _handle_glerror(const char * file, unsigned int line)
291-{
292- GLenum err = glGetError();
293- if (err == GL_NO_ERROR)
294- return err;
295-
296- log("%s:%d: OpenGL ERROR: ", file, line);
297-
298- switch (err)
299- {
300- case GL_INVALID_VALUE:
301- log("invalid value\n");
302- break;
303- case GL_INVALID_ENUM:
304- log("invalid enum\n");
305- break;
306- case GL_INVALID_OPERATION:
307- log("invalid operation\n");
308- break;
309- case GL_STACK_OVERFLOW:
310- log("stack overflow\n");
311- break;
312- case GL_STACK_UNDERFLOW:
313- log("stack undeflow\n");
314- break;
315- case GL_OUT_OF_MEMORY:
316- log("out of memory\n");
317- break;
318- case GL_TABLE_TOO_LARGE:
319- log("table too large\n");
320- break;
321+const char* gl_error_to_string(const GLenum err) {
322+ CLANG_DIAG_OFF("-Wswitch-enum");
323+#define LOG(a) case a: return #a
324+ switch (err) {
325+ LOG(GL_INVALID_ENUM);
326+ LOG(GL_INVALID_OPERATION);
327+ LOG(GL_INVALID_VALUE);
328+ LOG(GL_NO_ERROR);
329+ LOG(GL_OUT_OF_MEMORY);
330+ LOG(GL_STACK_OVERFLOW);
331+ LOG(GL_STACK_UNDERFLOW);
332+ LOG(GL_TABLE_TOO_LARGE);
333 default:
334- log("unknown\n");
335+ break;
336 }
337- return err;
338+#undef LOG
339+ CLANG_DIAG_ON("-Wswitch-enum");
340+ return "unknown";
341 }
342
343 // Thin wrapper around a Shader object to ensure proper cleanup.
344@@ -214,6 +198,7 @@
345 if (framebuffer != 0) {
346 unbind_texture_if_bound(texture);
347 glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
348+ assert(glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE);
349 }
350 current_framebuffer_ = framebuffer;
351 current_framebuffer_texture_ = texture;
352
353=== modified file 'src/graphic/gl/utils.h'
354--- src/graphic/gl/utils.h 2016-01-09 20:33:51 +0000
355+++ src/graphic/gl/utils.h 2016-01-25 20:17:39 +0000
356@@ -20,6 +20,7 @@
357 #define WL_GRAPHIC_GL_UTILS_H
358
359 #include <memory>
360+#include <string>
361 #include <unordered_map>
362 #include <unordered_set>
363 #include <vector>
364@@ -35,7 +36,8 @@
365
366 class Shader;
367
368-GLenum _handle_glerror(const char * file, unsigned int line);
369+// Returns the name of the 'error'.
370+const char* gl_error_to_string(GLenum error);
371
372 // Thin wrapper around a OpenGL program object to ensure proper cleanup. Throws
373 // on all errors.
374@@ -147,10 +149,4 @@
375
376 } // namespace Gl
377
378-/**
379- * handle_glerror() is intended to make debugging of OpenGL easier. It logs the
380- * error code returned by glGetError and returns the error code.
381- */
382-#define handle_glerror() Gl::_handle_glerror(__FILE__, __LINE__)
383-
384 #endif // end of include guard: WL_GRAPHIC_GL_UTILS_H
385
386=== modified file 'src/graphic/graphic.cc'
387--- src/graphic/graphic.cc 2016-01-23 15:44:29 +0000
388+++ src/graphic/graphic.cc 2016-01-25 20:17:39 +0000
389@@ -31,6 +31,7 @@
390 #include "graphic/font.h"
391 #include "graphic/font_handler.h"
392 #include "graphic/font_handler1.h"
393+#include "graphic/gl/initialize.h"
394 #include "graphic/gl/system_headers.h"
395 #include "graphic/image.h"
396 #include "graphic/image_io.h"
397@@ -69,70 +70,30 @@
398 /**
399 * Initialize the SDL video mode.
400 */
401-void Graphic::initialize(int window_mode_w, int window_mode_h, bool init_fullscreen) {
402+void Graphic::initialize(const TraceGl& trace_gl,
403+ int window_mode_w,
404+ int window_mode_h,
405+ bool init_fullscreen) {
406 window_mode_width_ = window_mode_w;
407 window_mode_height_ = window_mode_h;
408 requires_update_ = true;
409
410- // Request an OpenGL 2 context with double buffering.
411- SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1);
412- SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);
413- SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1);
414- SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY);
415- SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
416-
417-
418 log("Graphics: Try to set Videomode %ux%u\n", window_mode_width_, window_mode_height_);
419 sdl_window_ =
420 SDL_CreateWindow("Widelands Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
421 window_mode_width_, window_mode_height_, SDL_WINDOW_OPENGL);
422
423+ GLint max_texture_size;
424+ gl_context_ = Gl::initialize(
425+ trace_gl == TraceGl::kYes ? Gl::Trace::kYes : Gl::Trace::kNo, sdl_window_,
426+ &max_texture_size);
427+
428 resolution_changed();
429 set_fullscreen(init_fullscreen);
430
431 SDL_SetWindowTitle(sdl_window_, ("Widelands " + build_id() + '(' + build_type() + ')').c_str());
432 set_icon(sdl_window_);
433
434- gl_context_ = SDL_GL_CreateContext(sdl_window_);
435- SDL_GL_MakeCurrent(sdl_window_, gl_context_);
436-
437-#ifdef USE_GLBINDING
438- glbinding::Binding::initialize();
439-#else
440- // See graphic/gl/system_headers.h for an explanation of the next line.
441- glewExperimental = GL_TRUE;
442- GLenum err = glewInit();
443- if (err != GLEW_OK) {
444- log("glewInit returns %i\nYour OpenGL installation must be __very__ broken. %s\n", err,
445- glewGetErrorString(err));
446- throw wexception("glewInit returns %i: Broken OpenGL installation.", err);
447- }
448-#endif
449-
450- log(
451- "Graphics: OpenGL: Version \"%s\"\n", reinterpret_cast<const char*>(glGetString(GL_VERSION)));
452-
453- GLboolean glBool;
454- glGetBooleanv(GL_DOUBLEBUFFER, &glBool);
455- log("Graphics: OpenGL: Double buffering %s\n", (glBool == GL_TRUE) ? "enabled" : "disabled");
456-
457- GLint max_texture_size;
458- glGetIntegerv(GL_MAX_TEXTURE_SIZE, &max_texture_size);
459- log("Graphics: OpenGL: Max texture size: %u\n", max_texture_size);
460-
461- log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n",
462- reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION)));
463-
464- glDrawBuffer(GL_BACK);
465-
466- glDisable(GL_DEPTH_TEST);
467- glDepthFunc(GL_LEQUAL);
468-
469- glEnable(GL_BLEND);
470- glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
471-
472- glClear(GL_COLOR_BUFFER_BIT);
473-
474 SDL_GL_SwapWindow(sdl_window_);
475
476 /* Information about the video capabilities. */
477@@ -148,7 +109,6 @@
478 assert(SDL_BYTESPERPIXEL(disp_mode.format) == 4);
479 }
480
481-
482 std::map<std::string, std::unique_ptr<Texture>> textures_in_atlas;
483 auto texture_atlases = build_texture_atlas(max_texture_size, &textures_in_atlas);
484 image_cache_->fill_with_texture_atlases(
485
486=== modified file 'src/graphic/graphic.h'
487--- src/graphic/graphic.h 2016-01-19 07:28:40 +0000
488+++ src/graphic/graphic.h 2016-01-25 20:17:39 +0000
489@@ -55,8 +55,11 @@
490 ~Graphic();
491
492 // Initializes with the given resolution if fullscreen is false, otherwise a
493- // window that fills the screen.
494- void initialize(int window_mode_w, int window_mode_height, bool fullscreen);
495+ // window that fills the screen. The 'trace_gl' parameter gets passed on to
496+ // 'Gl::initialize'.
497+ enum class TraceGl {kNo, kYes};
498+ void
499+ initialize(const TraceGl& trace_gl, int window_mode_w, int window_mode_height, bool fullscreen);
500
501 // Gets and sets the resolution.
502 void change_resolution(int w, int h);
503
504=== modified file 'src/graphic/text/test/render_richtext.cc'
505--- src/graphic/text/test/render_richtext.cc 2016-01-09 15:27:05 +0000
506+++ src/graphic/text/test/render_richtext.cc 2016-01-25 20:17:39 +0000
507@@ -99,7 +99,7 @@
508 g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR));
509
510 g_gr = new Graphic();
511- g_gr->initialize(1, 1, false);
512+ g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false);
513 }
514
515 } // namespace
516
517=== modified file 'src/graphic/texture.cc'
518--- src/graphic/texture.cc 2016-01-13 07:27:55 +0000
519+++ src/graphic/texture.cc 2016-01-25 20:17:39 +0000
520@@ -103,9 +103,10 @@
521 {
522 init(w, h);
523
524- if (width() <= 0 || height() <= 0) {
525+ if (m_blit_data.texture_id == 0) {
526 return;
527 }
528+
529 glTexImage2D
530 (GL_TEXTURE_2D, 0, static_cast<GLint>(GL_RGBA), width(), height(), 0, GL_RGBA,
531 GL_UNSIGNED_BYTE, nullptr);
532@@ -187,7 +188,7 @@
533 w, h,
534 Rect(0, 0, w, h),
535 };
536- if (width() <= 0 || height() <= 0) {
537+ if (w * h == 0) {
538 return;
539 }
540
541@@ -203,7 +204,7 @@
542 }
543
544 void Texture::lock() {
545- if (width() <= 0 || height() <= 0) {
546+ if (m_blit_data.texture_id == 0) {
547 return;
548 }
549
550@@ -263,6 +264,7 @@
551
552
553 void Texture::setup_gl() {
554+ assert(m_blit_data.texture_id != 0);
555 Gl::State::instance().bind_framebuffer(
556 GlFramebuffer::instance().id(), m_blit_data.texture_id);
557 glViewport(0, 0, width(), height());
558@@ -272,6 +274,9 @@
559 const BlitData& texture,
560 float opacity,
561 BlendMode blend_mode) {
562+ if (m_blit_data.texture_id == 0) {
563+ return;
564+ }
565 setup_gl();
566 BlitProgram::instance().draw(dst_rect, 0.f, texture, BlitData{0, 0, 0, Rect()},
567 RGBAColor(0, 0, 0, 255 * opacity), blend_mode);
568@@ -282,6 +287,9 @@
569 const BlitData& mask,
570 const RGBColor& blend) {
571
572+ if (m_blit_data.texture_id == 0) {
573+ return;
574+ }
575 setup_gl();
576 BlitProgram::instance().draw(dst_rect, 0.f, texture, mask, blend, BlendMode::UseAlpha);
577 }
578@@ -289,18 +297,27 @@
579 void Texture::do_blit_monochrome(const FloatRect& dst_rect,
580 const BlitData& texture,
581 const RGBAColor& blend) {
582+ if (m_blit_data.texture_id == 0) {
583+ return;
584+ }
585 setup_gl();
586 BlitProgram::instance().draw_monochrome(dst_rect, 0.f, texture, blend);
587 }
588
589 void
590 Texture::do_draw_line(const FloatPoint& start, const FloatPoint& end, const RGBColor& color, int line_width) {
591+ if (m_blit_data.texture_id == 0) {
592+ return;
593+ }
594 setup_gl();
595 DrawLineProgram::instance().draw(start, end, 0.f, color, line_width);
596 }
597
598 void
599 Texture::do_fill_rect(const FloatRect& dst_rect, const RGBAColor& color, BlendMode blend_mode) {
600+ if (m_blit_data.texture_id == 0) {
601+ return;
602+ }
603 setup_gl();
604 FillRectProgram::instance().draw(dst_rect, 0.f, color, blend_mode);
605 }
606
607=== modified file 'src/logic/map_info.cc'
608--- src/logic/map_info.cc 2016-01-09 15:27:05 +0000
609+++ src/logic/map_info.cc 2016-01-25 20:17:39 +0000
610@@ -49,7 +49,7 @@
611 g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR));
612
613 g_gr = new Graphic();
614- g_gr->initialize(1, 1, false);
615+ g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false);
616 }
617
618 } // namespace
619
620=== modified file 'src/wlapplication.cc'
621--- src/wlapplication.cc 2016-01-24 20:11:53 +0000
622+++ src/wlapplication.cc 2016-01-25 20:17:39 +0000
623@@ -288,9 +288,10 @@
624 UI::create_fonthandler(&g_gr->images()); // This will create the fontset, so loading it first.
625 UI::g_fh = new UI::FontHandler();
626
627- g_gr->initialize(config.get_int("xres", DEFAULT_RESOLUTION_W),
628- config.get_int("yres", DEFAULT_RESOLUTION_H),
629- config.get_bool("fullscreen", false));
630+ g_gr->initialize(
631+ config.get_bool("debug_gl_trace", false) ? Graphic::TraceGl::kYes : Graphic::TraceGl::kNo,
632+ config.get_int("xres", DEFAULT_RESOLUTION_W), config.get_int("yres", DEFAULT_RESOLUTION_H),
633+ config.get_bool("fullscreen", false));
634 g_sound_handler.init(); // TODO(unknown): memory leak!
635
636

Subscribers

People subscribed via source and target branches

to status/vote changes: