Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[V4.1.x] Fix GL_INVALID_ENUM error in CopyTexture #826

Merged
merged 2 commits into from
Aug 4, 2024
Merged

[V4.1.x] Fix GL_INVALID_ENUM error in CopyTexture #826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c4b12d
Properly check for uninitialized texture in CopyTexture class.
kblaschke Jul 12, 2024
9bc89af
Bump libprojectM version to 4.1.2
kblaschke Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ endif()

project(libprojectM
LANGUAGES C CXX
VERSION 4.1.1
VERSION 4.1.2
)

# The API (SO) version for the shared library. Should be incremented whenever the binary interface changes
Expand Down
10 changes: 8 additions & 2 deletions src/libprojectM/Renderer/CopyTexture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ void CopyTexture::Draw(const std::shared_ptr<class Texture>& originalTexture, bo
void CopyTexture::Draw(const std::shared_ptr<class Texture>& originalTexture, const std::shared_ptr<class Texture>& targetTexture,
bool flipVertical, bool flipHorizontal)
{
if (originalTexture == nullptr || originalTexture == targetTexture)
if (originalTexture == nullptr ||
originalTexture->Empty() ||
(targetTexture != nullptr && targetTexture->Empty()) ||
originalTexture == targetTexture)
{
return;
}
Expand Down Expand Up @@ -157,7 +160,10 @@ void CopyTexture::Draw(const std::shared_ptr<class Texture>& originalTexture, co
void CopyTexture::Draw(const std::shared_ptr<class Texture>& originalTexture, Framebuffer& framebuffer, int framebufferIndex,
bool flipVertical, bool flipHorizontal)
{
if (originalTexture == nullptr || framebuffer.GetColorAttachmentTexture(framebufferIndex, 0) == nullptr)
if (originalTexture == nullptr ||
originalTexture->Empty() ||
framebuffer.GetColorAttachmentTexture(framebufferIndex, 0) == nullptr ||
framebuffer.GetColorAttachmentTexture(framebufferIndex, 0)->Empty())
{
return;
}
Expand Down
11 changes: 8 additions & 3 deletions src/libprojectM/Renderer/Texture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Texture::Texture(std::string name, const int width, const int height, const bool
, m_format(GL_RGB)
, m_type(GL_UNSIGNED_BYTE)
{
CreateEmptyTexture();
CreateNewTexture();
}

Texture::Texture(std::string name, int width, int height,
Expand All @@ -29,7 +29,7 @@ Texture::Texture(std::string name, int width, int height,
, m_format(format)
, m_type(type)
{
CreateEmptyTexture();
CreateNewTexture();
}

Texture::Texture(std::string name, const GLuint texID, const GLenum target,
Expand Down Expand Up @@ -99,7 +99,12 @@ auto Texture::IsUserTexture() const -> bool
return m_isUserTexture;
}

void Texture::CreateEmptyTexture()
auto Texture::Empty() const -> bool
{
return m_textureId == 0;
Copy link

@hack-s hack-s Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was considering checking the texture id instead of the target. I ended up checking the target, because that was the value that directly triggered the GL_INVALID_ENUM. Would it be worth it to check both values ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard, just noticed, this is the backport. We should not do anything different here anyways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m_target is, except for the volume noise textures, always set to GL_TEXTURE_2D, even if the texture is empty. I think "target" is a misleading name for the texture's dimensionality, but it's how OpenGL calls it.

What triggered the error was actually m_textureId being zero because the actual texture wasn't initialized at the point when it was first used (copying the previous frame texture into the new preset - naturally, there's no previous texture right after init).

}

void Texture::CreateNewTexture()
{
glGenTextures(1, &m_textureId);
glBindTexture(m_target, m_textureId);
Expand Down
11 changes: 10 additions & 1 deletion src/libprojectM/Renderer/Texture.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,17 @@ class Texture
*/
auto IsUserTexture() const -> bool;

/**
* @brief Returns true if the texture is empty/unallocated.
* @return true if the texture is not yet allocated (e.g. the ID 0), false if the object contains a valid texture.
*/
auto Empty() const -> bool;

private:
void CreateEmptyTexture();
/**
* @brief Creates a new, blank texture with the given size.
*/
void CreateNewTexture();

GLuint m_textureId{0}; //!< The OpenGL texture name/ID.
GLenum m_target{GL_NONE}; //!< The OpenGL texture target, e.g. GL_TEXTURE_2D.
Expand Down
Loading