From a16b98a46896f83effbc3dfb515a9473e967ccc2 Mon Sep 17 00:00:00 2001 From: rdb Date: Thu, 3 Jul 2014 17:10:41 +0000 Subject: [PATCH] Disable error checking by default for a substantial performance increase --- .../src/glstuff/glGraphicsStateGuardian_src.I | 6 +- .../glstuff/glGraphicsStateGuardian_src.cxx | 74 ++++++++++++++----- .../src/glstuff/glGraphicsStateGuardian_src.h | 5 +- panda/src/glstuff/glShaderContext_src.cxx | 4 +- panda/src/glstuff/glmisc_src.cxx | 16 ++-- panda/src/glstuff/glmisc_src.h | 4 +- 6 files changed, 74 insertions(+), 35 deletions(-) diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.I b/panda/src/glstuff/glGraphicsStateGuardian_src.I index bebd422988..9008a3cd23 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.I +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.I @@ -45,7 +45,7 @@ report_errors(int line, const char *source_file) { INLINE void CLP(GraphicsStateGuardian):: report_my_errors(int line, const char *source_file) { #ifndef NDEBUG - if (_track_errors) { + if (_check_errors) { GLenum error_code = glGetError(); if (error_code != GL_NO_ERROR) { if (!report_errors_loop(line, source_file, error_code, _error_count)) { @@ -83,7 +83,7 @@ clear_errors(int line, const char *source_file) { // Access: Public // Description: This works like report_my_errors(), except that it // always runs, even in the NDEBUG case (but not when -// _track_errors is false), and it never calls +// _check_errors is false), and it never calls // panic_deactivate(). It is designed to be called when // it is important to clear the error stack (for // instance, because we want to be able to reliably @@ -91,7 +91,7 @@ clear_errors(int line, const char *source_file) { //////////////////////////////////////////////////////////////////// INLINE void CLP(GraphicsStateGuardian):: clear_my_errors(int line, const char *source_file) { - if (_track_errors) { + if (_check_errors) { GLenum error_code = glGetError(); if (error_code != GL_NO_ERROR) { int error_count = 0; diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 6dda6a2495..3f7a660ace 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -306,9 +306,9 @@ CLP(GraphicsStateGuardian)(GraphicsEngine *engine, GraphicsPipe *pipe) : // the window tells us otherwise. _is_hardware = true; - // calling glGetError() forces a sync, this turns it off if you want to. - _track_errors = !gl_force_no_error; - _allow_flush = !gl_force_no_flush; + // calling glGetError() forces a sync, this turns it on if you want to. + _check_errors = gl_check_errors; + _force_flush = gl_force_flush; #ifdef DO_PSTATS if (gl_finish) { @@ -436,8 +436,8 @@ reset() { report_extensions(); // Initialize OpenGL debugging output first, if enabled and supported. + _supports_debug = false; if (gl_debug) { - bool supports_debug = false; PFNGLDEBUGMESSAGECALLBACKPROC _glDebugMessageCallback; PFNGLDEBUGMESSAGECONTROLPROC _glDebugMessageControl; @@ -454,17 +454,17 @@ reset() { get_extension_func("glDebugMessageControl"); #endif glEnable(GL_DEBUG_OUTPUT); // Not supported in ARB version - supports_debug = true; + _supports_debug = true; } else if (has_extension("GL_ARB_debug_output")) { _glDebugMessageCallback = (PFNGLDEBUGMESSAGECALLBACKPROC) get_extension_func("glDebugMessageCallbackARB"); _glDebugMessageControl = (PFNGLDEBUGMESSAGECONTROLPROC) get_extension_func("glDebugMessageControlARB"); - supports_debug = true; + _supports_debug = true; } - if (supports_debug) { + if (_supports_debug) { // Set the categories we want to listen to. _glDebugMessageControl(GL_DONT_CARE, GL_DONT_CARE, GL_DEBUG_SEVERITY_HIGH, 0, NULL, GLCAT.is_error()); @@ -487,6 +487,11 @@ reset() { } } else { GLCAT.debug() << "gl-debug NOT enabled.\n"; + + // However, still check if it is supported. + _supports_debug = is_at_least_gl_version(4, 3) + || has_extension("GL_KHR_debug") + || has_extension("GL_ARB_debug_output"); } _supported_geom_rendering = @@ -2503,14 +2508,46 @@ end_frame(Thread *current_thread) { // necessary if this is a single-buffered visual, so that the frame // will be finished drawing before we return to the application. // It's not clear what effect this has on our total frame time. - if (_allow_flush) { + if (_force_flush || _current_properties->is_single_buffered()) { gl_flush(); } maybe_gl_finish(); - report_my_gl_errors(); -} +#ifndef NDEBUG + if (_check_errors || (_supports_debug && gl_debug)) { + report_my_gl_errors(); + } else { + // If _check_errors is false, we still want to check for errors + // once during this frame, so that we know if anything went wrong. + GLenum error_code = glGetError(); + if (error_code != GL_NO_ERROR) { + int error_count = 0; + bool deactivate = !report_errors_loop(__LINE__, __FILE__, error_code, error_count); + if (error_count == 1) { + GLCAT.error() + << "An OpenGL error (" << get_error_string(error_code) + << ") has occurred."; + } else { + GLCAT.error() + << error_count << " OpenGL errors have occurred."; + } + + if (_supports_debug) { + GLCAT.error(false) << " Set gl-debug #t " + << "in your PRC file to display more information.\n"; + } else { + GLCAT.error(false) << " Set gl-check-errors #t " + << "in your PRC file to display more information.\n"; + } + + if (deactivate) { + panic_deactivate(); + } + } + } +#endif +} //////////////////////////////////////////////////////////////////// // Function: GLGraphicsStateGuardian::begin_draw_primitives @@ -3079,14 +3116,12 @@ disable_standard_vertex_arrays() glDisableClientState(GL_NORMAL_ARRAY); glDisableClientState(GL_COLOR_ARRAY); GLPf(Color4)(1.0f, 1.0f, 1.0f, 1.0f); - report_my_gl_errors(); for (int stage_index=0; stage_index < _last_max_stage_index; stage_index++) { _glClientActiveTexture(GL_TEXTURE0 + stage_index); glDisableClientState(GL_TEXTURE_COORD_ARRAY); } _last_max_stage_index = 0; - report_my_gl_errors(); #ifndef OPENGLES if (_supports_vertex_blend) { @@ -3764,7 +3799,6 @@ update_texture(TextureContext *tc, bool force) { //////////////////////////////////////////////////////////////////// void CLP(GraphicsStateGuardian):: release_texture(TextureContext *tc) { - report_my_gl_errors(); CLP(TextureContext) *gtc = DCAST(CLP(TextureContext), tc); glDeleteTextures(1, >c->_index); @@ -4380,6 +4414,8 @@ end_occlusion_query() { //////////////////////////////////////////////////////////////////// void CLP(GraphicsStateGuardian):: dispatch_compute(int num_groups_x, int num_groups_y, int num_groups_z) { + maybe_gl_finish(); + PStatTimer timer(_compute_dispatch_pcollector); nassertv(_supports_compute_shaders); nassertv(_current_shader_context != NULL); @@ -5709,9 +5745,7 @@ draw_immediate_composite_primitives(const GeomPrimitivePipelineReader *reader, G void CLP(GraphicsStateGuardian):: gl_flush() const { PStatTimer timer(_flush_pcollector); - if (_allow_flush) { - glFlush(); - } + glFlush(); } //////////////////////////////////////////////////////////////////// @@ -5721,7 +5755,7 @@ gl_flush() const { //////////////////////////////////////////////////////////////////// GLenum CLP(GraphicsStateGuardian):: gl_get_error() const { - if (_track_errors) { + if (_check_errors) { return glGetError(); } else { return GL_NO_ERROR; @@ -9666,9 +9700,9 @@ upload_texture_image(CLP(TextureContext) *gtc, // object. if (GLCAT.is_debug()) { GLCAT.debug() - << "loading new texture object, " << width << " x " << height - << " x " << depth << ", z = " << z << ", mipmaps " << num_ram_mipmap_levels - << ", uses_mipmaps = " << uses_mipmaps << "\n"; + << "loading new texture object for " << tex->get_name() << ", " << width + << " x " << height << " x " << depth << ", z = " << z << ", mipmaps " + << num_ram_mipmap_levels << ", uses_mipmaps = " << uses_mipmaps << "\n"; } if (num_ram_mipmap_levels == 0) { diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.h b/panda/src/glstuff/glGraphicsStateGuardian_src.h index 3403c446dd..90bca73524 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.h +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.h @@ -728,8 +728,9 @@ public: //RenderState::SlotMask _inv_state_mask; - bool _track_errors; - bool _allow_flush; + bool _check_errors; + bool _force_flush; + bool _supports_debug; #ifndef NDEBUG bool _show_texture_usage; diff --git a/panda/src/glstuff/glShaderContext_src.cxx b/panda/src/glstuff/glShaderContext_src.cxx index bcf4bca016..f6115f2129 100755 --- a/panda/src/glstuff/glShaderContext_src.cxx +++ b/panda/src/glstuff/glShaderContext_src.cxx @@ -1099,6 +1099,8 @@ update_shader_texture_bindings(ShaderContext *prev) { return; } + _glgsg->_glMemoryBarrier(GL_ALL_BARRIER_BITS); + #ifndef OPENGLES // First bind all the 'image units'; a bit of an esoteric OpenGL feature right now. int num_image_units = min(_glsl_img_inputs.size(), (size_t)_glgsg->_max_image_units); @@ -1316,7 +1318,6 @@ glsl_compile_entry_point(Shader::ShaderType type) { return 0; } - _glgsg->report_my_gl_errors(); return handle; } @@ -1356,7 +1357,6 @@ glsl_compile_shader() { glGetIntegerv(GL_MAX_GEOMETRY_OUTPUT_VERTICES, &max_vertices); _glgsg->_glProgramParameteri(_glsl_program, GL_GEOMETRY_VERTICES_OUT_ARB, max_vertices); #endif - _glgsg->report_my_gl_errors(); } if (!_shader->get_text(Shader::ST_tess_control).empty()) { diff --git a/panda/src/glstuff/glmisc_src.cxx b/panda/src/glstuff/glmisc_src.cxx index 5ec109cf75..83a15025a0 100644 --- a/panda/src/glstuff/glmisc_src.cxx +++ b/panda/src/glstuff/glmisc_src.cxx @@ -173,13 +173,17 @@ ConfigVariableBool gl_matrix_palette ("gl-matrix-palette", false, PRC_DESC("Temporary hack variable protecting untested code. See glGraphicsStateGuardian_src.cxx.")); -ConfigVariableBool gl_force_no_error - ("gl-force-no-error", false, - PRC_DESC("Avoid reporting OpenGL errors, for a small performance benefit.")); +ConfigVariableBool gl_check_errors + ("gl-check-errors", false, + PRC_DESC("Regularly call glGetError() to check for OpenGL errors. " + "This will slow down rendering significantly. If your " + "video driver supports it, you should use gl-debug instead.")); -ConfigVariableBool gl_force_no_flush - ("gl-force-no-flush", false, - PRC_DESC("Avoid calling glFlush(), for a potential performance benefit. This may be a little dangerous.")); +ConfigVariableBool gl_force_flush + ("gl-force-flush", false, + PRC_DESC("Call this to force a call to glFlush() after rendering a " + "frame, even when using a double-buffered framebuffer. " + "This can incur a significant performance penalty.")); ConfigVariableBool gl_separate_specular_color ("gl-separate-specular-color", true, diff --git a/panda/src/glstuff/glmisc_src.h b/panda/src/glstuff/glmisc_src.h index 2daf5fab13..378d585511 100644 --- a/panda/src/glstuff/glmisc_src.h +++ b/panda/src/glstuff/glmisc_src.h @@ -62,8 +62,8 @@ extern ConfigVariableBool gl_debug_buffers; extern ConfigVariableBool gl_finish; extern ConfigVariableBool gl_force_depth_stencil; extern ConfigVariableBool gl_matrix_palette; -extern ConfigVariableBool gl_force_no_error; -extern ConfigVariableBool gl_force_no_flush; +extern ConfigVariableBool gl_check_errors; +extern ConfigVariableBool gl_force_flush; extern ConfigVariableBool gl_separate_specular_color; extern ConfigVariableBool gl_cube_map_seamless; extern ConfigVariableBool gl_dump_compiled_shaders;