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.

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

gluniform is unsafe with 4x4 matrices of incorrect type #7

Open
ikirill opened this issue Jan 28, 2015 · 4 comments
Open

gluniform is unsafe with 4x4 matrices of incorrect type #7

ikirill opened this issue Jan 28, 2015 · 4 comments

Comments

@ikirill
Copy link

ikirill commented Jan 28, 2015

If you use orthographicprojection, it returns a Float64 matrix, but a shader might expect a GLfloat matrix, not GLdouble, so that gluniform seems to pass a pointer to a matrix of the wrong type to glUniform. This can silently break shaders by setting uniform parameters to unexpected values.

I think maybe gluniform could accept GLProgram and a symbol naming the uniform, instead of the integer for its location, so that it could check the argument type.

@SimonDanisch
Copy link
Member

In orthographicprojection, the type of the matrix depends on the input
types. So if you call it with float32, you will get a float32 matrix, which
is how it should be I suppose.
Additionally, I theoretically check the types supplied for a RenderObject
(I deactivated it though, as I didn't check all types correctly)

2015-01-28 3:54 GMT+01:00 Kirill Ignatiev [email protected]:

If you use orthographicprojection, it returns a Float64 matrix, but a
shader might expect a GLfloat matrix, not GLdouble, so that gluniform seems
to pass a pointer to a matrix of the wrong type to glUniform. This can
silently break shaders by setting uniform parameters to unexpected values.


Reply to this email directly or view it on GitHub
#7.

@ikirill
Copy link
Author

ikirill commented Jan 28, 2015

In orthographicprojection, the type of the matrix depends on the input types. So if you call it with float32, you will get a float32 matrix, which is how it should be I suppose.

Okay, that's my own problem then.

Additionally, I theoretically check the types supplied for a RenderObject (I deactivated it though, as I didn't check all types correctly)

But the commonly used types, Float64/GLfloat, are different between ordinary Julia code and OpenGL, so it would be helpful to check types.

Also, I didn't use RenderObject because it's implemented to reload its uniform parameters when rendered, which makes it trickier to set the uniform parameters. I didn't want to set my coordinate transformation matrix for every single object I had, so I used vertex arrays and loaded uniform parameters by hand, and discovered this.

Like I said, maybe gluniform should accept arguments (GLProgram, Symbol, T) instead of (Int, T), so that it can check that the value T is of the right type for Symbol, because GLProgram keeps track of symbols' types.

@SimonDanisch
Copy link
Member

I see. Yeah that's a pretty bad design of the renderobject. It was only
thought as a temporary solution. In the long run i want to use unform
buffers, which would elimate this problem. I disabled type checking, as i
added quite a few new types and didnt have the time to rewrite the checking
code;)

On Wed, 28 Jan 2015 23:15 Kirill Ignatiev [email protected] wrote:

In orthographicprojection, the type of the matrix depends on the input
types. So if you call it with float32, you will get a float32 matrix, which
is how it should be I suppose.

Okay, that's my own problem then.

Additionally, I theoretically check the types supplied for a RenderObject
(I deactivated it though, as I didn't check all types correctly)

But the commonly used types, Float64/GLfloat, are different between
ordinary Julia code and OpenGL, so it would be helpful to check types.

Also, I didn't use RenderObject because it's implement to reload its
uniform parameters when rendered, which makes it trickier to set the
uniform parameters. I didn't want to set my coordinate transformation
matrix for every single object I had, so I used vertex arrays and loaded
uniform parameters by hand, and discovered this.

Like I said, maybe gluniform should accept arguments (GLProgram, Symbol,
T) instead of (Int, T), so that it can check that the value T is of the
right type for Symbol, because GLProgram keeps track of symbols' types.


Reply to this email directly or view it on GitHub
#7 (comment)
.

@SimonDanisch
Copy link
Member

This is sadly still unresolved! This will be part of my next refactor, which will also fix the biggest performance issue in GLAbstraction! (Not knowing and not checking the type prohibits unrolling the render code, which means all calls don't have type information)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants