From 718eab5c2ee286d9cf2a0d9086d46c92c18814ec Mon Sep 17 00:00:00 2001 From: liavt Date: Mon, 24 Jul 2017 10:16:48 -0500 Subject: [PATCH] fixed bugs caused by issue #12 The GetLastError() bug is referenced in glfw/glfw#1053 For now, os::clearError() is called after glfwTerminate() as a workaround to the bug --- .travis.yml | 4 ++-- demos/Rotations.cpp | 7 ++++++- demos/Text.cpp | 1 + include/MACE/Core/Error.h | 4 ++-- include/MACE/MACE.h | 24 ++++++++++++++++++++---- scripts/travis-install.sh | 15 ++------------- src/Core/Error.cpp | 2 +- src/Core/System.cpp | 4 +++- src/Graphics/Context.cpp | 17 +++++++---------- src/Graphics/OGL/OGL33Renderer.cpp | 23 +++++++++++++++-------- src/Graphics/Window.cpp | 24 ++++++++++++------------ src/Utility/Signal.cpp | 12 ++++++++++-- tests/Main.cpp | 6 ++++-- 13 files changed, 85 insertions(+), 58 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6dd3f2c1f..aa807f8bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -66,8 +66,8 @@ before_install: - if [[ "$COMPILER" == "g++-5" ]]; then export CXX=g++-5;export CC=gcc-5; fi - if [[ "$COMPILER" == "g++-6" ]]; then export CXX=g++-6;export CC=gcc-6; fi - $CXX --version - - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew update; brew install glew glfw3 freetype openal-soft; fi - - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then sudo apt-get install libopencv-dev xorg-dev libopenal-dev libfreetype6 libfreetype6-dev libglu1-mesa-dev libglew-dev libglfw2;sudo apt-get update ; git clone https://github.com/glfw/glfw.git ./glfw ; cd ./glfw ; git checkout latest ;cmake . -DBUILD_SHARED_LIBS=ON ; make ; sudo make install ; sudo ldconfig ; cd .. ; fi + +install: sudo bash ./scripts/travis-install.sh before_script: - mkdir build/ diff --git a/demos/Rotations.cpp b/demos/Rotations.cpp index 73d488b32..48feba027 100644 --- a/demos/Rotations.cpp +++ b/demos/Rotations.cpp @@ -147,7 +147,7 @@ int main() { os::ErrorModule errModule = os::ErrorModule(); instance.addModule(errModule); - mc::Initializer i(instance); + instance.init(); while (instance.isRunning()) { instance.update(); @@ -159,9 +159,14 @@ int main() { mc::os::wait(33); } + instance.destroy(); } catch (const std::exception& e) { Error::handleError(e, instance); return -1; + } catch (...) { + std::cerr << "An unknown exception occured"; + return -1; } + return 0; } \ No newline at end of file diff --git a/demos/Text.cpp b/demos/Text.cpp index af56f4ae1..04dccba1f 100644 --- a/demos/Text.cpp +++ b/demos/Text.cpp @@ -9,6 +9,7 @@ The above copyright notice and this permission notice shall be included in all c */ #include + using namespace mc; gfx::Text topLeft, center, topRight, botLeft, botRight; diff --git a/include/MACE/Core/Error.h b/include/MACE/Core/Error.h index c52c973dd..52e07342f 100644 --- a/include/MACE/Core/Error.h +++ b/include/MACE/Core/Error.h @@ -22,12 +22,12 @@ namespace mc { */ class Error: public std::runtime_error { public: - static std::string getErrorDump(const std::exception& e, Instance* i = nullptr); + static std::string getErrorDump(const std::exception& e, const Instance* i = nullptr); /** Stops MACE and prints an exception to console accordingly. This should be used every time a fatal exception is thrown. */ - static void handleError[[noreturn]](const std::exception& e, Instance* i = nullptr); + static void handleError[[noreturn]](const std::exception& e, Instance* i = nullptr);// i is not const because requestStop() will be called /** @copydoc Error::handleError(const std::exception& e, Instance*) diff --git a/include/MACE/MACE.h b/include/MACE/MACE.h index 614600c65..485d1e4a4 100644 --- a/include/MACE/MACE.h +++ b/include/MACE/MACE.h @@ -11,6 +11,10 @@ The above copyright notice and this permission notice shall be included in all c #ifndef MACE__MACE_H #define MACE__MACE_H +#include +#include +#include + /** Namespace for everything in MACE. This includes constants, typedefs, tests, classes, and variables.

@@ -23,10 +27,22 @@ It is usually safe to type `using namespace mc`. However, be weary that this may @note No MACE header file will bring all of `mc` into the global namespace. @todo Add a proper logger */ -namespace mc {} +namespace mc { + unsigned int getMACEVersionMajor() { + return MACE_VERSION_MAJOR; + } -#include -#include -#include + unsigned int getMACEVersionMinor() { + return MACE_VERSION_MINOR; + } + + unsigned int getMACEVersionPatch() { + return MACE_VERSION_PATCH; + } + + const char* getMACEVersionString() { + return MACE_STRINGIFY(MACE_VERSION); + } +} #endif \ No newline at end of file diff --git a/scripts/travis-install.sh b/scripts/travis-install.sh index 7c96397e1..8eaa45226 100644 --- a/scripts/travis-install.sh +++ b/scripts/travis-install.sh @@ -1,20 +1,9 @@ #!/bin/bash if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then - sudo ./scripts/install-brew.sh; + sudo bash ./scripts/install-brew.sh; fi if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then - sudo ./scripts/install-apt.sh; - add-apt-repository -y ppa:oibaf/graphics-drivers; - sudo apt-get install xorg-dev libgl1-mesa-dev libgl1-mesa-glx mesa-common-dev libglapi-mesa libgbm1 libgl1-mesa-dri libxatracker-dev xvfb - apt-get update --qq -y; - export DEBIAN_FRONTEND=noninteractive; - export DISPLAY=:99l - export LIBGL_ALWAYS_SOFTWARE=1; - xpra --xvfb="Xorg +extension GLX +extension RANDR +extension RENDER -config `pwd`/test/dummy.xorg.conf -logfile ${HOME}/.xpra/xorg.log" start :99; - sleep 3 ; - LIBGL_ALWAYS_SOFTWARE=1 glxinfo; - glxinfo; - cat ${HOME}/.xpra/xorg.log; + sudo bash ./scripts/install-apt.sh; fi \ No newline at end of file diff --git a/src/Core/Error.cpp b/src/Core/Error.cpp index 58aa5c3ca..831e8a020 100644 --- a/src/Core/Error.cpp +++ b/src/Core/Error.cpp @@ -56,7 +56,7 @@ namespace mc { Error::handleError(*this); } - std::string Error::getErrorDump(const std::exception & e, Instance* instance) { + std::string Error::getErrorDump(const std::exception & e, const Instance* instance) { std::stringstream dump; dump << "At "; diff --git a/src/Core/System.cpp b/src/Core/System.cpp index 9bf408b80..358f2b43d 100644 --- a/src/Core/System.cpp +++ b/src/Core/System.cpp @@ -249,7 +249,7 @@ namespace mc { void clearError(const int, const char*) { #ifdef MACE_WINAPI - GetLastError(); + SetLastError(0); #elif defined(MACE_POSIX) errno = 0; #endif @@ -277,6 +277,8 @@ namespace mc { LocalFree(messageBuffer); + SetLastError(0); + throw SystemError(std::to_string(line) + " in " + std::string(file) + ": " + errorMessage + ": Winapi threw error " + std::to_string(lastError) + " with message " + message); #else return; diff --git a/src/Graphics/Context.cpp b/src/Graphics/Context.cpp index 3c2c93562..f0289d211 100644 --- a/src/Graphics/Context.cpp +++ b/src/Graphics/Context.cpp @@ -491,23 +491,20 @@ namespace mc { } void Texture::setData(const void * data, const Size width, const Size height, const Enums::Type type, const Enums::Format format, const Enums::InternalFormat internalFormat, const Index mipmap) { - if (width == 0) { - MACE__THROW(IndexOutOfBounds, "Width of Texture can not be 0!"); - } else if (height == 0) { - MACE__THROW(IndexOutOfBounds, "Height of Texture can not be 0!"); - } - - texture->width = width; - texture->height = height; texture->type = type; texture->format = format; texture->internalFormat = internalFormat; + if (width == 0 || height == 0) { + return; + } + texture->width = width; + texture->height = height; texture->setData(data, mipmap); } void Texture::setData(const void * data, const Index mipmap) { if (isEmpty()) { - MACE__THROW(InvalidState, "Must set width and height with full form of setData() in order to use the shortened form"); + return; } texture->setData(data, mipmap); @@ -559,7 +556,7 @@ namespace mc { } bool Texture::operator==(const Texture& other) const { - return hue == other.hue && texture == other.texture; + return transform == other.transform && hue == other.hue && texture == other.texture; } bool Texture::operator!=(const Texture& other) const { diff --git a/src/Graphics/OGL/OGL33Renderer.cpp b/src/Graphics/OGL/OGL33Renderer.cpp index 5d7b43f69..93d9ac83b 100644 --- a/src/Graphics/OGL/OGL33Renderer.cpp +++ b/src/Graphics/OGL/OGL33Renderer.cpp @@ -60,26 +60,33 @@ namespace mc { const GLenum result = glewInit(); if (result != GLEW_OK) { std::ostringstream errorMessage; - errorMessage << "GLEW failed to initialize: "; + errorMessage << "OpenGL failed to initialize: "; //to convert from GLubyte* to string, we can use the << in ostream. For some reason the //+ operater in std::string can not handle this conversion. errorMessage << glewGetErrorString(result); - + errorMessage << ": "; if (result == GLEW_ERROR_NO_GL_VERSION) { - errorMessage << "\nThis can be a result of an outdated graphics driver. Please ensure that you have OpenGL 3.0+"; + errorMessage << "NO_GL_VERSION: There was no OpenGL version found on this system"; + } else if (result == GLEW_ERROR_GL_VERSION_10_ONLY) { + errorMessage << "GL_VERSION_10_ONLY: The version of OpenGL found on this system is outdated: OpenGL 1.0 found, OpenGL 1.1+ required"; } - MACE__THROW(InitializationFailed, errorMessage.str()); + MACE__THROW(UnsupportedRenderer, errorMessage.str()); + } + + if (!GLEW_VERSION_3_3) { + std::ostringstream errorMessage; + errorMessage << "This system (OpenGL " << glGetString(GL_VERSION) << ")"; + errorMessage << " does not support the required OpenGL version required by this renderer, "; + errorMessage << "OpenGL 3.3"; + MACE__THROW(UnsupportedRenderer, errorMessage.str()); } try { gfx::ogl::checkGLError(__LINE__, __FILE__, "Internal Error: This should be ignored silently, it is a bug with glew"); } catch (...) { //glew sometimes throws errors that can be ignored (GL_INVALID_ENUM) - } - - if (!GLEW_VERSION_3_3) { - std::cerr << "OpenGL 3.3 not found, falling back to a lower version, which may cause undefined results. Try updating your graphics driver to fix this." << std::endl; + //see https://www.khronos.org/opengl/wiki/OpenGL_Loading_Library (section GLEW) saying to ignore a GLEW error } #ifdef MACE_DEBUG diff --git a/src/Graphics/Window.cpp b/src/Graphics/Window.cpp index 4acdc98f6..b7d6dd014 100644 --- a/src/Graphics/Window.cpp +++ b/src/Graphics/Window.cpp @@ -46,9 +46,8 @@ namespace mc { } GLFWwindow* createWindow(const WindowModule::LaunchConfig& config) { - GLFWmonitor* mon = nullptr; if (config.fullscreen) { - mon = glfwGetPrimaryMonitor(); + GLFWmonitor* mon = glfwGetPrimaryMonitor(); const GLFWvidmode* mode = glfwGetVideoMode(mon); glfwWindowHint(GLFW_RED_BITS, mode->redBits); @@ -178,6 +177,9 @@ namespace mc { glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE); glfwWindowHint(GLFW_CLIENT_API, GLFW_OPENGL_API); + + glfwWindowHint(GLFW_STENCIL_BITS, 8); + glfwWindowHint(GLFW_DEPTH_BITS, GLFW_DONT_CARE); #ifdef MACE_DEBUG glfwWindowHint(GLFW_OPENGL_DEBUG_CONTEXT, true); #endif @@ -262,6 +264,8 @@ namespace mc { void WindowModule::threadCallback() { try { + os::clearError(__LINE__, __FILE__); + //typedefs for chrono for readibility purposes using Clock = std::chrono::system_clock; using TimeStamp = std::chrono::time_point; @@ -284,11 +288,11 @@ namespace mc { configureThread(); Entity::init(); - + if (config.fps != 0) { windowDelay = Duration(1000000000L / static_cast(config.fps)); } - + os::checkError(__LINE__, __FILE__, "A system error occurred creating the window"); } catch (const std::exception& e) { Error::handleError(e, instance); @@ -359,7 +363,6 @@ namespace mc { } catch (...) { Error::handleError(MACE__GET_ERROR_NAME(Unknown) ("An unknown error occured while running MACE", __LINE__, __FILE__), instance); } - }//threadCallback void WindowModule::init() { @@ -370,9 +373,9 @@ namespace mc { //release context on this thread, it wll be owned by the seperate rendering thread glfwMakeContextCurrent(nullptr); - windowThread = std::thread(&WindowModule::threadCallback, this); - os::checkError(__LINE__, __FILE__, "A system error occured while trying to init the WindowModule"); + + windowThread = std::thread(&WindowModule::threadCallback, this); } void WindowModule::update() { @@ -395,14 +398,11 @@ namespace mc { os::checkError(__LINE__, __FILE__, "A system error occured while trying to destroy the WindowModule"); - //window can only be destroyed by main thread and with a thread that has a handle to its - glfwMakeContextCurrent(window); glfwDestroyWindow(window); - - //we destroyed the window, now detach it from this thread to be safe glfwMakeContextCurrent(nullptr); - os::checkError(__LINE__, __FILE__, "Internal Error: Error trying to terminate GLFW"); + glfwTerminate(); + os::clearError(__LINE__, __FILE__);//https://github.com/glfw/glfw/issues/1053 }//destroy std::string WindowModule::getName() const { diff --git a/src/Utility/Signal.cpp b/src/Utility/Signal.cpp index 7f34a49c0..ca6a0038e 100644 --- a/src/Utility/Signal.cpp +++ b/src/Utility/Signal.cpp @@ -57,12 +57,20 @@ namespace mc { } void onUnexpected[[noreturn]]() { - std::cerr << "An unexpected error forced the program to abort" << std::endl; + try { + Error::handleError(MACE__GET_ERROR_NAME(Unknown) ("An unexpected error occured", __LINE__, __FILE__)); + } catch (...) { + //this function should never throw an error, ignore all errors and abort anyways + } std::abort(); } void onTerminate[[noreturn]](){ - std::cerr << "The process was forced to terminate" << std::endl; + try { + Error::handleError(MACE__GET_ERROR_NAME(Unknown) ("An exception was thrown somewhere and not caught appropriately", __LINE__, __FILE__)); + } catch (...) { + //this function should never throw an error, ignore all errors and abort anyways + } std::abort(); } } diff --git a/tests/Main.cpp b/tests/Main.cpp index b78628282..195571ae9 100644 --- a/tests/Main.cpp +++ b/tests/Main.cpp @@ -7,6 +7,7 @@ Permission is hereby granted, free of charge, to any person obtaining a copy of The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. */ + #define MACE_DEBUG 1 #define MACE_EXPOSE_ALL 1 #include @@ -20,6 +21,7 @@ The above copyright notice and this permission notice shall be included in all c #include #include + void onUnexpected[[noreturn]](){ std::cerr << "Unexpected exception occured" << std::endl; std::abort(); @@ -38,13 +40,13 @@ int main(int argc, char* const argv[]) { //constant? get it? const int result = Catch::Session().run(argc, argv); - system("pause"); - return result; } catch( const std::exception& e ) { mc::Error::handleError(e); + return -1; } catch (...) { std::cerr << "An unknown exception occured"; + return -1; } return -1;