cginternals / globjects

C++ library strictly wrapping OpenGL objects.
https://globjects.org
MIT License
539 stars 59 forks source link

pixelStore bad definition #325

Closed X-Ryl669 closed 7 years ago

X-Ryl669 commented 7 years ago

Currenty, pixelStore is defined as AbstractState::pixelStore(gl::GLenum, gl::GLboolean) which is wrong, IMHO, since, for example, pixelStore(GL_UNPACK_ROW_LENGTH) expect an integer (likely large for big textures and not a boolean stored on an unsigned char).

It should read AbstractState::pixelStore(gl::GLenum, gl::GLint) instead.


diff --git a/source/globjects/include/globjects/AbstractState.h b/source/globjects/include/globjects/AbstractState.h
index eade0dd..2bf052f 100644
--- a/source/globjects/include/globjects/AbstractState.h
+++ b/source/globjects/include/globjects/AbstractState.h
@@ -45,7 +45,7 @@ public:
     void depthRange(const std::array<gl::GLfloat, 2> & range);
     void frontFace(gl::GLenum winding);
     void logicOp(gl::GLenum opcode);
-    void pixelStore(gl::GLenum pname, const gl::GLboolean param);
+    void pixelStore(gl::GLenum pname, const gl::GLint param);
     void pointParameter(gl::GLenum pname, const gl::GLenum param);
     void pointSize(gl::GLfloat size);
     void polygonMode(gl::GLenum face, gl::GLenum mode);
diff --git a/source/globjects/source/AbstractState.cpp b/source/globjects/source/AbstractState.cpp
index 71e5e89..8662c0c 100644
--- a/source/globjects/source/AbstractState.cpp
+++ b/source/globjects/source/AbstractState.cpp
@@ -113,9 +113,9 @@ void AbstractState::logicOp(const GLenum opcode)
     add(new StateSetting(glLogicOp, opcode));
 }

-void AbstractState::pixelStore(const GLenum pname, const GLboolean param)
+void AbstractState::pixelStore(const GLenum pname, const GLint param)
 {
-    auto setting = new StateSetting(static_cast<void(*)(GLenum,GLboolean)>(glPixelStorei), pname, param);
+    auto setting = new StateSetting(static_cast<void(*)(GLenum,GLint)>(glPixelStorei), pname, param);
     setting->type().specializeType(pname);
     add(setting);
 }
scheibel commented 7 years ago

Thanks for reporting and an actual code diff. Upon reading the OpenGL documentation (https://www.opengl.org/sdk/docs/man/html/glPixelStore.xhtml) I noticed that there are actually two valid signatures for glPixelStore: one with int and one with float. The documentation states, that the float interface is used to round the passed float to the nearest integer value or pass it as boolean (where 0.0 is treated as false), depending on the enumeration value of pname. So I extended the interface instead of replacing it. You can have a look at it in PR #326.