SirVer has proposed merging lp:~widelands-dev/widelands/opengl_debug into lp:widelands.
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. - More logging when OpenGL is initialized. - Pulls out OpenGL initialization into a separate unit. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/opengl_debug/+merge/283738 Small bug fixes and better debug potential for OpenGL - but only if glbinding is used. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/opengl_debug into lp:widelands.
=== modified file 'src/graphic/CMakeLists.txt' --- src/graphic/CMakeLists.txt 2016-01-19 07:33:34 +0000 +++ src/graphic/CMakeLists.txt 2016-01-24 12:46:35 +0000 @@ -87,6 +87,8 @@ SRCS gl/blit_data.h gl/coordinate_conversion.h + gl/initialize.cc + gl/initialize.h gl/system_headers.h gl/utils.cc gl/utils.h === added file 'src/graphic/gl/initialize.cc' --- src/graphic/gl/initialize.cc 1970-01-01 00:00:00 +0000 +++ src/graphic/gl/initialize.cc 2016-01-24 12:46:35 +0000 @@ -0,0 +1,141 @@ +/* + * Copyright (C) 2006-2016 by the Widelands Development Team + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +#include "graphic/gl/initialize.h" + +#include <csignal> + +#include <SDL.h> + +#include "base/macros.h" +#include "graphic/gl/utils.h" + +namespace Gl { + +SDL_GLContext +initialize(const Trace& trace, SDL_Window* sdl_window, GLint* max_texture_size) { + // Request an OpenGL 2 context with double buffering. + SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1); + SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2); + SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1); + SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY); + SDL_GL_SetAttribute(SDL_GL_DEPTH_SIZE, 16); + SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1); + + SDL_GLContext gl_context = SDL_GL_CreateContext(sdl_window); + SDL_GL_MakeCurrent(sdl_window, gl_context); + +#ifdef USE_GLBINDING + glbinding::Binding::initialize(); + + if (trace == Trace::kYes) { + setCallbackMaskExcept( + glbinding::CallbackMask::After | glbinding::CallbackMask::ParametersAndReturnValue, + {"glGetError"}); + glbinding::setAfterCallback([](const glbinding::FunctionCall& call) { + log("%s(", call.function.name()); + for (size_t i = 0; i < call.parameters.size(); ++i) { + log("%s", call.parameters[i]->asString().c_str()); + if (i < call.parameters.size() - 1) + log(", "); + } + log(")"); + if (call.returnValue) { + log(" -> %s", call.returnValue->asString().c_str()); + } + const auto error = glGetError(); + log(" [%s]\n", gl_error_to_string(error)); + // The next few lines will terminate Widelands if there was any OpenGL + // error. This is useful for super aggressive debugging, but probably + // not for regular builds. Comment it in if you need to understand + // OpenGL problems. + // if (error != GL_NO_ERROR) { + // std::raise(SIGINT); + // } + }); + } +#else + // See graphic/gl/system_headers.h for an explanation of the next line. + glewExperimental = GL_TRUE; + GLenum err = glewInit(); + if (err != GLEW_OK) { + log("glewInit returns %i\nYour OpenGL installation must be __very__ broken. %s\n", err, + glewGetErrorString(err)); + throw wexception("glewInit returns %i: Broken OpenGL installation.", err); + } +#endif + + log( + "Graphics: OpenGL: Version \"%s\"\n", reinterpret_cast<const char*>(glGetString(GL_VERSION))); + +#define LOG_SDL_GL_ATTRIBUTE(x) \ + { \ + int value; \ + SDL_GL_GetAttribute(x, &value); \ + log("Graphics: %s is %d\n", #x, value); \ + } + + LOG_SDL_GL_ATTRIBUTE(SDL_GL_RED_SIZE) + LOG_SDL_GL_ATTRIBUTE(SDL_GL_GREEN_SIZE) + LOG_SDL_GL_ATTRIBUTE(SDL_GL_BLUE_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ALPHA_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_BUFFER_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_DOUBLEBUFFER); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_DEPTH_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_STENCIL_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_RED_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_GREEN_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_BLUE_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCUM_ALPHA_SIZE); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_STEREO); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_MULTISAMPLEBUFFERS); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_MULTISAMPLESAMPLES); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_ACCELERATED_VISUAL); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_MAJOR_VERSION); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_MINOR_VERSION); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_FLAGS); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_CONTEXT_PROFILE_MASK); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_SHARE_WITH_CURRENT_CONTEXT); + LOG_SDL_GL_ATTRIBUTE(SDL_GL_FRAMEBUFFER_SRGB_CAPABLE); +#undef LOG_SDL_GL_ATTRIBUTE + + GLboolean glBool; + glGetBooleanv(GL_DOUBLEBUFFER, &glBool); + log("Graphics: OpenGL: Double buffering %s\n", (glBool == GL_TRUE) ? "enabled" : "disabled"); + + glGetIntegerv(GL_MAX_TEXTURE_SIZE, max_texture_size); + log("Graphics: OpenGL: Max texture size: %u\n", *max_texture_size); + + log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n", + reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION))); + + glDrawBuffer(GL_BACK); + + glDisable(GL_DEPTH_TEST); + glDepthFunc(GL_LEQUAL); + + glEnable(GL_BLEND); + glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); + + glClear(GL_COLOR_BUFFER_BIT); + + return gl_context; +} + +} // namespace Gl === added file 'src/graphic/gl/initialize.h' --- src/graphic/gl/initialize.h 1970-01-01 00:00:00 +0000 +++ src/graphic/gl/initialize.h 2016-01-24 12:46:35 +0000 @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2006-2016 by the Widelands Development Team + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + * + */ + +#ifndef WL_GRAPHIC_GL_INITIALIZE_H +#define WL_GRAPHIC_GL_INITIALIZE_H + +#include <SDL.h> + +#include "graphic/gl/system_headers.h" + +namespace Gl { + +// Initializes OpenGL. Creates a context for 'window' using SDL and loads the +// GL library. Fills in 'max_texture_size' and returns the created SDL_Context +// which must be closed by the caller. +// If we are build against glbinding, 'trace' will set up tracing for +// OpenGL and output every single opengl call ever made, together with it's +// arguments, return values and the result from glGetError. +enum class Trace { + kYes, + kNo, +}; +SDL_GLContext +initialize(const Trace& trace, SDL_Window* window, GLint* max_texture_size); + +} // namespace Gl + +#endif // end of include guard: WL_GRAPHIC_GL_INITIALIZE_H === modified file 'src/graphic/gl/utils.cc' --- src/graphic/gl/utils.cc 2016-01-23 15:44:29 +0000 +++ src/graphic/gl/utils.cc 2016-01-24 12:46:35 +0000 @@ -18,6 +18,7 @@ #include "graphic/gl/utils.h" +#include <cassert> #include <memory> #include <string> @@ -43,41 +44,24 @@ } // namespace -GLenum _handle_glerror(const char * file, unsigned int line) -{ - GLenum err = glGetError(); - if (err == GL_NO_ERROR) - return err; - - log("%s:%d: OpenGL ERROR: ", file, line); - - switch (err) - { - case GL_INVALID_VALUE: - log("invalid value\n"); - break; - case GL_INVALID_ENUM: - log("invalid enum\n"); - break; - case GL_INVALID_OPERATION: - log("invalid operation\n"); - break; - case GL_STACK_OVERFLOW: - log("stack overflow\n"); - break; - case GL_STACK_UNDERFLOW: - log("stack undeflow\n"); - break; - case GL_OUT_OF_MEMORY: - log("out of memory\n"); - break; - case GL_TABLE_TOO_LARGE: - log("table too large\n"); - break; +const char* gl_error_to_string(const GLenum err) { + CLANG_DIAG_OFF("-Wswitch-enum"); +#define LOG(a) case a: return #a + switch (err) { + LOG(GL_INVALID_ENUM); + LOG(GL_INVALID_OPERATION); + LOG(GL_INVALID_VALUE); + LOG(GL_NO_ERROR); + LOG(GL_OUT_OF_MEMORY); + LOG(GL_STACK_OVERFLOW); + LOG(GL_STACK_UNDERFLOW); + LOG(GL_TABLE_TOO_LARGE); default: - log("unknown\n"); + break; } - return err; +#undef LOG + CLANG_DIAG_ON("-Wswitch-enum"); + return "unknown"; } // Thin wrapper around a Shader object to ensure proper cleanup. @@ -214,6 +198,7 @@ if (framebuffer != 0) { unbind_texture_if_bound(texture); glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0); + assert(glCheckFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE); } current_framebuffer_ = framebuffer; current_framebuffer_texture_ = texture; === modified file 'src/graphic/gl/utils.h' --- src/graphic/gl/utils.h 2016-01-09 20:33:51 +0000 +++ src/graphic/gl/utils.h 2016-01-24 12:46:35 +0000 @@ -20,6 +20,7 @@ #define WL_GRAPHIC_GL_UTILS_H #include <memory> +#include <string> #include <unordered_map> #include <unordered_set> #include <vector> @@ -35,7 +36,8 @@ class Shader; -GLenum _handle_glerror(const char * file, unsigned int line); +// Returns the name of the 'error'. +const char* gl_error_to_string(GLenum error); // Thin wrapper around a OpenGL program object to ensure proper cleanup. Throws // on all errors. @@ -147,10 +149,4 @@ } // namespace Gl -/** - * handle_glerror() is intended to make debugging of OpenGL easier. It logs the - * error code returned by glGetError and returns the error code. - */ -#define handle_glerror() Gl::_handle_glerror(__FILE__, __LINE__) - #endif // end of include guard: WL_GRAPHIC_GL_UTILS_H === modified file 'src/graphic/graphic.cc' --- src/graphic/graphic.cc 2016-01-23 15:44:29 +0000 +++ src/graphic/graphic.cc 2016-01-24 12:46:35 +0000 @@ -31,6 +31,7 @@ #include "graphic/font.h" #include "graphic/font_handler.h" #include "graphic/font_handler1.h" +#include "graphic/gl/initialize.h" #include "graphic/gl/system_headers.h" #include "graphic/image.h" #include "graphic/image_io.h" @@ -69,70 +70,30 @@ /** * Initialize the SDL video mode. */ -void Graphic::initialize(int window_mode_w, int window_mode_h, bool init_fullscreen) { +void Graphic::initialize(const TraceGl& trace_gl, + int window_mode_w, + int window_mode_h, + bool init_fullscreen) { window_mode_width_ = window_mode_w; window_mode_height_ = window_mode_h; requires_update_ = true; - // Request an OpenGL 2 context with double buffering. - SDL_GL_SetAttribute(SDL_GL_ACCELERATED_VISUAL, 1); - SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2); - SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 1); - SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_COMPATIBILITY); - SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1); - - log("Graphics: Try to set Videomode %ux%u\n", window_mode_width_, window_mode_height_); sdl_window_ = SDL_CreateWindow("Widelands Window", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, window_mode_width_, window_mode_height_, SDL_WINDOW_OPENGL); + GLint max_texture_size; + gl_context_ = Gl::initialize( + trace_gl == TraceGl::kYes ? Gl::Trace::kYes : Gl::Trace::kNo, sdl_window_, + &max_texture_size); + resolution_changed(); set_fullscreen(init_fullscreen); SDL_SetWindowTitle(sdl_window_, ("Widelands " + build_id() + '(' + build_type() + ')').c_str()); set_icon(sdl_window_); - gl_context_ = SDL_GL_CreateContext(sdl_window_); - SDL_GL_MakeCurrent(sdl_window_, gl_context_); - -#ifdef USE_GLBINDING - glbinding::Binding::initialize(); -#else - // See graphic/gl/system_headers.h for an explanation of the next line. - glewExperimental = GL_TRUE; - GLenum err = glewInit(); - if (err != GLEW_OK) { - log("glewInit returns %i\nYour OpenGL installation must be __very__ broken. %s\n", err, - glewGetErrorString(err)); - throw wexception("glewInit returns %i: Broken OpenGL installation.", err); - } -#endif - - log( - "Graphics: OpenGL: Version \"%s\"\n", reinterpret_cast<const char*>(glGetString(GL_VERSION))); - - GLboolean glBool; - glGetBooleanv(GL_DOUBLEBUFFER, &glBool); - log("Graphics: OpenGL: Double buffering %s\n", (glBool == GL_TRUE) ? "enabled" : "disabled"); - - GLint max_texture_size; - glGetIntegerv(GL_MAX_TEXTURE_SIZE, &max_texture_size); - log("Graphics: OpenGL: Max texture size: %u\n", max_texture_size); - - log("Graphics: OpenGL: ShadingLanguage: \"%s\"\n", - reinterpret_cast<const char*>(glGetString(GL_SHADING_LANGUAGE_VERSION))); - - glDrawBuffer(GL_BACK); - - glDisable(GL_DEPTH_TEST); - glDepthFunc(GL_LEQUAL); - - glEnable(GL_BLEND); - glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); - - glClear(GL_COLOR_BUFFER_BIT); - SDL_GL_SwapWindow(sdl_window_); /* Information about the video capabilities. */ @@ -148,7 +109,6 @@ assert(SDL_BYTESPERPIXEL(disp_mode.format) == 4); } - std::map<std::string, std::unique_ptr<Texture>> textures_in_atlas; auto texture_atlases = build_texture_atlas(max_texture_size, &textures_in_atlas); image_cache_->fill_with_texture_atlases( === modified file 'src/graphic/graphic.h' --- src/graphic/graphic.h 2016-01-19 07:28:40 +0000 +++ src/graphic/graphic.h 2016-01-24 12:46:35 +0000 @@ -55,8 +55,11 @@ ~Graphic(); // Initializes with the given resolution if fullscreen is false, otherwise a - // window that fills the screen. - void initialize(int window_mode_w, int window_mode_height, bool fullscreen); + // window that fills the screen. The 'trace_gl' parameter gets passed on to + // 'Gl::initialize'. + enum class TraceGl {kNo, kYes}; + void + initialize(const TraceGl& trace_gl, int window_mode_w, int window_mode_height, bool fullscreen); // Gets and sets the resolution. void change_resolution(int w, int h); === modified file 'src/graphic/text/test/render_richtext.cc' --- src/graphic/text/test/render_richtext.cc 2016-01-09 15:27:05 +0000 +++ src/graphic/text/test/render_richtext.cc 2016-01-24 12:46:35 +0000 @@ -99,7 +99,7 @@ g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR)); g_gr = new Graphic(); - g_gr->initialize(1, 1, false); + g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false); } } // namespace === modified file 'src/graphic/texture.cc' --- src/graphic/texture.cc 2016-01-13 07:27:55 +0000 +++ src/graphic/texture.cc 2016-01-24 12:46:35 +0000 @@ -103,9 +103,10 @@ { init(w, h); - if (width() <= 0 || height() <= 0) { + if (m_blit_data.texture_id == 0) { return; } + glTexImage2D (GL_TEXTURE_2D, 0, static_cast<GLint>(GL_RGBA), width(), height(), 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr); @@ -187,7 +188,7 @@ w, h, Rect(0, 0, w, h), }; - if (width() <= 0 || height() <= 0) { + if (w * h == 0) { return; } @@ -203,7 +204,7 @@ } void Texture::lock() { - if (width() <= 0 || height() <= 0) { + if (m_blit_data.texture_id == 0) { return; } @@ -263,6 +264,7 @@ void Texture::setup_gl() { + assert(m_blit_data.texture_id != 0); Gl::State::instance().bind_framebuffer( GlFramebuffer::instance().id(), m_blit_data.texture_id); glViewport(0, 0, width(), height()); @@ -272,6 +274,9 @@ const BlitData& texture, float opacity, BlendMode blend_mode) { + if (m_blit_data.texture_id == 0) { + return; + } setup_gl(); BlitProgram::instance().draw(dst_rect, 0.f, texture, BlitData{0, 0, 0, Rect()}, RGBAColor(0, 0, 0, 255 * opacity), blend_mode); @@ -282,6 +287,9 @@ const BlitData& mask, const RGBColor& blend) { + if (m_blit_data.texture_id == 0) { + return; + } setup_gl(); BlitProgram::instance().draw(dst_rect, 0.f, texture, mask, blend, BlendMode::UseAlpha); } @@ -289,18 +297,27 @@ void Texture::do_blit_monochrome(const FloatRect& dst_rect, const BlitData& texture, const RGBAColor& blend) { + if (m_blit_data.texture_id == 0) { + return; + } setup_gl(); BlitProgram::instance().draw_monochrome(dst_rect, 0.f, texture, blend); } void Texture::do_draw_line(const FloatPoint& start, const FloatPoint& end, const RGBColor& color, int line_width) { + if (m_blit_data.texture_id == 0) { + return; + } setup_gl(); DrawLineProgram::instance().draw(start, end, 0.f, color, line_width); } void Texture::do_fill_rect(const FloatRect& dst_rect, const RGBAColor& color, BlendMode blend_mode) { + if (m_blit_data.texture_id == 0) { + return; + } setup_gl(); FillRectProgram::instance().draw(dst_rect, 0.f, color, blend_mode); } === modified file 'src/logic/map_info.cc' --- src/logic/map_info.cc 2016-01-09 15:27:05 +0000 +++ src/logic/map_info.cc 2016-01-24 12:46:35 +0000 @@ -49,7 +49,7 @@ g_fs->add_file_system(&FileSystem::create(INSTALL_DATADIR)); g_gr = new Graphic(); - g_gr->initialize(1, 1, false); + g_gr->initialize(Graphic::TraceGl::kNo, 1, 1, false); } } // namespace === modified file 'src/wlapplication.cc' --- src/wlapplication.cc 2016-01-17 09:55:27 +0000 +++ src/wlapplication.cc 2016-01-24 12:46:35 +0000 @@ -288,9 +288,10 @@ UI::create_fonthandler(&g_gr->images()); // This will create the fontset, so loading it first. UI::g_fh = new UI::FontHandler(); - g_gr->initialize(config.get_int("xres", DEFAULT_RESOLUTION_W), - config.get_int("yres", DEFAULT_RESOLUTION_H), - config.get_bool("fullscreen", false)); + g_gr->initialize( + config.get_bool("debug_gl_trace", false) ? Graphic::TraceGl::kYes : Graphic::TraceGl::kNo, + config.get_int("xres", DEFAULT_RESOLUTION_W), config.get_int("yres", DEFAULT_RESOLUTION_H), + config.get_bool("fullscreen", false)); g_sound_handler.init(); // TODO(unknown): memory leak!
_______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp