Skip to content

Commit

Permalink
fixed bugs caused by issue #12
Browse files Browse the repository at this point in the history
The GetLastError() bug is referenced in glfw/glfw#1053

For now, os::clearError() is called after glfwTerminate() as a workaround
to the bug
  • Loading branch information
liavt committed Jul 24, 2017
1 parent ae6e8f3 commit 718eab5
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 58 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down
7 changes: 6 additions & 1 deletion demos/Rotations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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;
}
1 change: 1 addition & 0 deletions demos/Text.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The above copyright notice and this permission notice shall be included in all c
*/
#include <MACE/MACE.h>


using namespace mc;

gfx::Text topLeft, center, topRight, botLeft, botRight;
Expand Down
4 changes: 2 additions & 2 deletions include/MACE/Core/Error.h
Original file line number Diff line number Diff line change
Expand Up @@ -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*)
Expand Down
24 changes: 20 additions & 4 deletions include/MACE/MACE.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <MACE/Core/Core.h>
#include <MACE/Utility/Utility.h>
#include <MACE/Graphics/Graphics.h>

/**
Namespace for everything in MACE. This includes constants, typedefs, tests, classes, and variables.
<p>
Expand All @@ -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 <MACE/Core/Core.h>
#include <MACE/Utility/Utility.h>
#include <MACE/Graphics/Graphics.h>
unsigned int getMACEVersionMinor() {
return MACE_VERSION_MINOR;
}

unsigned int getMACEVersionPatch() {
return MACE_VERSION_PATCH;
}

const char* getMACEVersionString() {
return MACE_STRINGIFY(MACE_VERSION);
}
}

#endif
15 changes: 2 additions & 13 deletions scripts/travis-install.sh
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/Core/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ";

Expand Down
4 changes: 3 additions & 1 deletion src/Core/System.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
17 changes: 7 additions & 10 deletions src/Graphics/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 15 additions & 8 deletions src/Graphics/OGL/OGL33Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 12 additions & 12 deletions src/Graphics/Window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<Clock>;
Expand All @@ -284,11 +288,11 @@ namespace mc {
configureThread();

Entity::init();

if (config.fps != 0) {
windowDelay = Duration(1000000000L / static_cast<long long>(config.fps));
}

os::checkError(__LINE__, __FILE__, "A system error occurred creating the window");
} catch (const std::exception& e) {
Error::handleError(e, instance);
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions src/Utility/Signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
6 changes: 4 additions & 2 deletions tests/Main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <MACE/MACE.h>
Expand All @@ -20,6 +21,7 @@ The above copyright notice and this permission notice shall be included in all c
#include <exception>
#include <iostream>


void onUnexpected[[noreturn]](){
std::cerr << "Unexpected exception occured" << std::endl;
std::abort();
Expand All @@ -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;
Expand Down

0 comments on commit 718eab5

Please sign in to comment.