From 84b456a763b49a2cd9361ba2ea6357b9fca0d1b3 Mon Sep 17 00:00:00 2001 From: SArpnt <36542932+SArpnt@users.noreply.github.com> Date: Mon, 20 Sep 2021 19:15:03 -0400 Subject: [PATCH 1/2] almost finished texture changes --- include/cinnabar-render/assets.hpp | 13 +++-- include/cinnabar-render/texture.hpp | 15 +++--- src/cinnabar-render-demo/main.cpp | 10 ++++ src/cinnabar-render/asset_manager.cpp | 27 ++++++++-- src/cinnabar-render/texture.cpp | 76 +++++++++++++-------------- 5 files changed, 87 insertions(+), 54 deletions(-) diff --git a/include/cinnabar-render/assets.hpp b/include/cinnabar-render/assets.hpp index e1708654..3fec6701 100644 --- a/include/cinnabar-render/assets.hpp +++ b/include/cinnabar-render/assets.hpp @@ -15,12 +15,15 @@ namespace ce { geom = "", frag = ""; }; - struct TextureFile { - unsigned char* data = NULL; - int + struct TextureFormat { + GLsizei width = 0, - height = 0, - channelCount = 0; + height = 0; + GLint internalColorSpace = 0; // TODO: is there a better value for this? GL_NONE exists but doesn't seem correct + }; + struct TextureFile { + unsigned char* data = NULL; // TODO: what type should this actually be? this has been a void* in some areas + TextureFormat format; }; struct MaterialFile { glm::vec4 diff --git a/include/cinnabar-render/texture.hpp b/include/cinnabar-render/texture.hpp index afcc7ea5..b2f4d56d 100644 --- a/include/cinnabar-render/texture.hpp +++ b/include/cinnabar-render/texture.hpp @@ -7,18 +7,19 @@ namespace ce { class Texture { public: - Texture(std::string filename, GLenum type = GL_TEXTURE_2D) - : Texture(ce::assetManager::getTextureFile(filename), type){}; - Texture(TextureFile textureFile, GLenum type = GL_TEXTURE_2D); - Texture(const void* data, GLsizei width, GLsizei height, GLenum color_space = GL_RGBA, GLenum type = GL_TEXTURE_2D); + Texture(std::string filename, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D) // TODO: default colorSpace should change based on TextureFile.channelCount (GL_RED, GL_RG, GL_RGB, GL_RGBA) + : Texture(ce::assetManager::getTextureFile(filename), colorSpace, target){}; + Texture(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D); ~Texture(); void bind(), unbind(), activate(int slot); private: GLuint m_texture; - int m_width, m_height, m_channelCount; - GLenum m_type; - bool loadData(const void* data, GLsizei width, GLsizei height, GLenum color_space = GL_RGBA, GLenum type = GL_TEXTURE_2D); + TextureFormat m_format; + GLenum m_colorSpace; + GLenum m_target; + + bool loadData(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D); }; } diff --git a/src/cinnabar-render-demo/main.cpp b/src/cinnabar-render-demo/main.cpp index 3d46fc82..62b62ab3 100644 --- a/src/cinnabar-render-demo/main.cpp +++ b/src/cinnabar-render-demo/main.cpp @@ -150,6 +150,16 @@ int main(int argc, char* argv[]) { window->swapBuffers(); + // error check + // TODO: make this into some function + while (true) { + GLenum tmp = glGetError(); + if (tmp == GL_NO_ERROR) + break; + else + LOG_ERROR("Uncaught GL error: 0x%04x", tmp); + } + // framerate cap time->waitUntilDelta(deltaTimeMin); } diff --git a/src/cinnabar-render/asset_manager.cpp b/src/cinnabar-render/asset_manager.cpp index b3880d1c..be1db3cc 100644 --- a/src/cinnabar-render/asset_manager.cpp +++ b/src/cinnabar-render/asset_manager.cpp @@ -43,12 +43,32 @@ ce::ShaderFile ce::assetManager::getShaderFile(std::string vert, std::string geo ce::TextureFile ce::assetManager::getTextureFile(std::string path) { // stbi_set_flip_vertically_on_load(1); TextureFile textureFile; + int channelCount; textureFile.data = stbi_load( (defaults::RESOURCE_FOLDER + "/" + defaults::TEXTURE_FOLDER + "/" + path).c_str(), - &textureFile.width, - &textureFile.height, - &textureFile.channelCount, + &textureFile.format.width, + &textureFile.format.height, + &channelCount, 0); + + switch (channelCount) { + case 1: + textureFile.format.internalColorSpace = GL_RED; + break; + case 2: + textureFile.format.internalColorSpace = GL_RG; + break; + case 3: + textureFile.format.internalColorSpace = GL_RGB; + break; + case 4: + textureFile.format.internalColorSpace = GL_RGBA; + break; + default: + LOG_WARN("Unsupported texture channel count: %i", channelCount); + textureFile.data == NULL; + } + if (textureFile.data == NULL) { LOG_WARN("Failed to load texture: %s", path.c_str()); if (path == defaults::TEXTURE_MISSING) @@ -57,6 +77,7 @@ ce::TextureFile ce::assetManager::getTextureFile(std::string path) { return getTextureFile(defaults::TEXTURE_MISSING); } else LOG_SUCCESS("Loaded texture: %s", path.c_str()); + return textureFile; } diff --git a/src/cinnabar-render/texture.cpp b/src/cinnabar-render/texture.cpp index 2538fb78..560645ed 100644 --- a/src/cinnabar-render/texture.cpp +++ b/src/cinnabar-render/texture.cpp @@ -4,36 +4,21 @@ #include -ce::Texture::Texture(TextureFile textureFile, GLenum type) - : m_width(0), m_height(0), m_channelCount(0), m_type(type) { - m_width = textureFile.width; - m_height = textureFile.height; - m_channelCount = textureFile.channelCount; +ce::Texture::Texture(TextureFile textureFile, GLenum colorSpace, GLenum target) { glGenTextures(1, &m_texture); bind(); - glTexParameteri(type, GL_TEXTURE_WRAP_S, GL_REPEAT); - glTexParameteri(type, GL_TEXTURE_WRAP_T, GL_REPEAT); - glTexParameteri(type, GL_TEXTURE_MAG_FILTER, GL_LINEAR); - glTexParameteri(type, GL_TEXTURE_MIN_FILTER, GL_REPEAT); + glTexParameteri(target, GL_TEXTURE_WRAP_S, GL_REPEAT); // TODO: proper system for setting texture parameters + glTexParameteri(target, GL_TEXTURE_WRAP_T, GL_REPEAT); + glTexParameteri(target, GL_TEXTURE_MAG_FILTER, GL_LINEAR); + glTexParameteri(target, GL_TEXTURE_MIN_FILTER, GL_REPEAT); - - if (this->loadData(textureFile.data, textureFile.width, textureFile.height, textureFile.channelCount == 3 ? GL_RGB : GL_RGBA, type)) { - LOG_SUCCESS("Loaded texture"); - } else { - LOG_ERROR("Failed to load texture"); - } - ce::assetManager::freeTextureFile(textureFile); -} - -ce::Texture::Texture(const void* data, GLsizei width, GLsizei height, GLenum color_space, GLenum type) - : m_width(0), m_height(0), m_channelCount(0), m_type(type) { - - if (this->loadData(data, width, height, color_space, type)) { + if (this->loadData(textureFile, colorSpace, target)) { LOG_SUCCESS("Loaded texture"); } else { LOG_ERROR("Failed to load texture"); } + ce::assetManager::freeTextureFile(textureFile); // TODO: what if the TextureFile is used more than once? this should only be called if a string is passed in for the constructor } ce::Texture::~Texture() { @@ -41,8 +26,8 @@ ce::Texture::~Texture() { } void ce::Texture::bind() { - glBindTexture(m_type, m_texture); - glEnable(m_type); + glBindTexture(m_target, m_texture); + glEnable(m_target); } void ce::Texture::activate(int slot = 0) { @@ -51,24 +36,37 @@ void ce::Texture::activate(int slot = 0) { } void ce::Texture::unbind() { - glDisable(m_type); + glDisable(m_target); glActiveTexture(0); - glBindTexture(m_type, 0); + glBindTexture(m_target, 0); } -bool ce::Texture::loadData(const void* data, GLsizei width, GLsizei height, GLenum color_space, GLenum type) { - m_width = width; - m_height = height; +bool ce::Texture::loadData(TextureFile textureFile, GLenum colorSpace, GLenum target) { + m_format = textureFile.format; // TODO: are these variables needed, and are they redundantly set elsewhere? + m_target = target; + if (colorSpace) + m_colorSpace = colorSpace; + else + switch (m_format.internalColorSpace) { + case GL_RED: + case GL_RG: + case GL_RGB: + case GL_RGBA: + m_colorSpace = m_format.internalColorSpace; + break; - bool out = false; - bind(); + // TODO: add formats GL_RED_INTEGER, GL_RG_INTEGER, GL_RGB_INTEGER, GL_RGBA_INTEGER, GL_STENCIL_INDEX, GL_DEPTH_COMPONENT, GL_DEPTH_STENCIL + default: + LOG_WARN("Unrecognized internal color space: %i", textureFile.format.internalColorSpace); + } - if (data) { - glTexImage2D(type, 0, color_space, m_width, m_height, 0, color_space, - GL_UNSIGNED_BYTE, data); - glGenerateMipmap(type); - out = true; - } - unbind(); - return out; + if (textureFile.data) { + bind(); + glTexImage2D(m_target, 0, m_format.internalColorSpace, m_format.width, m_format.height, 0, m_colorSpace, + GL_UNSIGNED_BYTE, textureFile.data); + glGenerateMipmap(m_target); + unbind(); + return true; + } else + return false; } From 008c6323ab37433b370d274a4629b6cc6748b13f Mon Sep 17 00:00:00 2001 From: SArpnt <36542932+SArpnt@users.noreply.github.com> Date: Mon, 20 Sep 2021 19:38:49 -0400 Subject: [PATCH 2/2] finished Texture changes --- include/cinnabar-render/assets.hpp | 9 +++------ include/cinnabar-render/texture.hpp | 14 +++++++++----- src/cinnabar-render/asset_manager.cpp | 12 ++++++------ src/cinnabar-render/texture.cpp | 17 ++++++----------- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/include/cinnabar-render/assets.hpp b/include/cinnabar-render/assets.hpp index 3fec6701..eaa19fa1 100644 --- a/include/cinnabar-render/assets.hpp +++ b/include/cinnabar-render/assets.hpp @@ -15,15 +15,12 @@ namespace ce { geom = "", frag = ""; }; - struct TextureFormat { + struct TextureFile { + unsigned char* data = NULL; // TODO: what type should this actually be? this has been a void* in some areas GLsizei width = 0, height = 0; - GLint internalColorSpace = 0; // TODO: is there a better value for this? GL_NONE exists but doesn't seem correct - }; - struct TextureFile { - unsigned char* data = NULL; // TODO: what type should this actually be? this has been a void* in some areas - TextureFormat format; + GLint internalColorSpace = 0; // TODO: is there a better value for this? GL_NONE exists but doesn't seem correct (also make changes in Texture) }; struct MaterialFile { glm::vec4 diff --git a/include/cinnabar-render/texture.hpp b/include/cinnabar-render/texture.hpp index b2f4d56d..dd71572c 100644 --- a/include/cinnabar-render/texture.hpp +++ b/include/cinnabar-render/texture.hpp @@ -7,19 +7,23 @@ namespace ce { class Texture { public: - Texture(std::string filename, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D) // TODO: default colorSpace should change based on TextureFile.channelCount (GL_RED, GL_RG, GL_RGB, GL_RGBA) - : Texture(ce::assetManager::getTextureFile(filename), colorSpace, target){}; - Texture(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D); + Texture(std::string filename, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D) { + TextureFile textureFile = ce::assetManager::getTextureFile(filename); + init(textureFile, colorSpace, target); + ce::assetManager::freeTextureFile(textureFile); + }; + Texture(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D) { + init(textureFile, colorSpace, target); + }; ~Texture(); void bind(), unbind(), activate(int slot); private: GLuint m_texture; - TextureFormat m_format; - GLenum m_colorSpace; GLenum m_target; + void init(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D); bool loadData(TextureFile textureFile, GLenum colorSpace = 0, GLenum target = GL_TEXTURE_2D); }; } diff --git a/src/cinnabar-render/asset_manager.cpp b/src/cinnabar-render/asset_manager.cpp index be1db3cc..12e44fcc 100644 --- a/src/cinnabar-render/asset_manager.cpp +++ b/src/cinnabar-render/asset_manager.cpp @@ -46,23 +46,23 @@ ce::TextureFile ce::assetManager::getTextureFile(std::string path) { int channelCount; textureFile.data = stbi_load( (defaults::RESOURCE_FOLDER + "/" + defaults::TEXTURE_FOLDER + "/" + path).c_str(), - &textureFile.format.width, - &textureFile.format.height, + &textureFile.width, + &textureFile.height, &channelCount, 0); switch (channelCount) { case 1: - textureFile.format.internalColorSpace = GL_RED; + textureFile.internalColorSpace = GL_RED; break; case 2: - textureFile.format.internalColorSpace = GL_RG; + textureFile.internalColorSpace = GL_RG; break; case 3: - textureFile.format.internalColorSpace = GL_RGB; + textureFile.internalColorSpace = GL_RGB; break; case 4: - textureFile.format.internalColorSpace = GL_RGBA; + textureFile.internalColorSpace = GL_RGBA; break; default: LOG_WARN("Unsupported texture channel count: %i", channelCount); diff --git a/src/cinnabar-render/texture.cpp b/src/cinnabar-render/texture.cpp index 560645ed..83e49d44 100644 --- a/src/cinnabar-render/texture.cpp +++ b/src/cinnabar-render/texture.cpp @@ -4,8 +4,7 @@ #include -ce::Texture::Texture(TextureFile textureFile, GLenum colorSpace, GLenum target) { - +void ce::Texture::init(TextureFile textureFile, GLenum colorSpace, GLenum target) { glGenTextures(1, &m_texture); bind(); glTexParameteri(target, GL_TEXTURE_WRAP_S, GL_REPEAT); // TODO: proper system for setting texture parameters @@ -18,7 +17,6 @@ ce::Texture::Texture(TextureFile textureFile, GLenum colorSpace, GLenum target) } else { LOG_ERROR("Failed to load texture"); } - ce::assetManager::freeTextureFile(textureFile); // TODO: what if the TextureFile is used more than once? this should only be called if a string is passed in for the constructor } ce::Texture::~Texture() { @@ -42,27 +40,24 @@ void ce::Texture::unbind() { } bool ce::Texture::loadData(TextureFile textureFile, GLenum colorSpace, GLenum target) { - m_format = textureFile.format; // TODO: are these variables needed, and are they redundantly set elsewhere? m_target = target; - if (colorSpace) - m_colorSpace = colorSpace; - else - switch (m_format.internalColorSpace) { + if (!colorSpace) + switch (textureFile.internalColorSpace) { case GL_RED: case GL_RG: case GL_RGB: case GL_RGBA: - m_colorSpace = m_format.internalColorSpace; + colorSpace = textureFile.internalColorSpace; break; // TODO: add formats GL_RED_INTEGER, GL_RG_INTEGER, GL_RGB_INTEGER, GL_RGBA_INTEGER, GL_STENCIL_INDEX, GL_DEPTH_COMPONENT, GL_DEPTH_STENCIL default: - LOG_WARN("Unrecognized internal color space: %i", textureFile.format.internalColorSpace); + LOG_WARN("Unrecognized internal color space: %i", textureFile.internalColorSpace); } if (textureFile.data) { bind(); - glTexImage2D(m_target, 0, m_format.internalColorSpace, m_format.width, m_format.height, 0, m_colorSpace, + glTexImage2D(m_target, 0, textureFile.internalColorSpace, textureFile.width, textureFile.height, 0, colorSpace, GL_UNSIGNED_BYTE, textureFile.data); glGenerateMipmap(m_target); unbind();