haskell-opengl / OpenGL

Haskell bindings to OpenGL
http://www.haskell.org/haskellwiki/OpenGL
BSD 3-Clause "New" or "Revised" License
147 stars 26 forks source link

Geometry shader #34

Closed MaxOw closed 11 years ago

MaxOw commented 11 years ago
MaxOw commented 11 years ago

Yeah, attachShader is sort of a quick fix. I think the best solution would be to add 3 state vars, one for each shader type, e.g: attachedFragmentShaders, attachedVertexShaders, attachedGeometryShaders and mark attachedShaders as deprecated.

MaxOw commented 11 years ago

I think better yet name would be discardingRasterizer, analogically to preservingMatrix...

Laar commented 11 years ago

Just as a thought should discardingRasterizer always reset it to not discarding afterwards, or should it reset to the original state. Both options would have their advantages. Always enabling could lead to counter-intuitive situations but is less expensive (reseting costs one extra gl-call in worst case).

Laar commented 11 years ago

The current shader-API is, if I sense some of the messages on the email list correctly, not really what people want. AttachedShaders would then be the problem. Exporting attachShader and detachShader should make it better for some people (and make it less needlessly "helpfull").

Though then remains the question what to do with attachedShaders (as it breaks with geometry shaders). Apart from attaching shaders it also provides an interface to the already attached shaders, which need to be 'wrapped' into the correct shader type. When redesigning it there are some points to think about:

Splitting it up, or changing it to attachedShaders :: Shader s => StateVar [s] would probably be both a useful and extensible function. But unlike the current one it suffers from bad time complexity if the users want all the shaders types. I currently can only think of one solution to improve time complexity. Though that will introduce a rather undesirable aggregation of existential quantified shader type, both not my favourites.

MaxOw commented 11 years ago

Yes, silly me. discardingRasterizer should definitely reset to original state. I'll fix that right a way.

I tried to make an existential AnyShader type and then exposing Shader class and also marshaling shaderType from GLenum to something prettier but... I don't know, I'm not sure about it, so I guess I'll leave it for someone else to worry about.

dagit commented 11 years ago

Hello,

I see I'm a bit late to the party.

Do you still want me to merge these changes or will you work more on some of the issues that Laar points out?

Thanks!

MaxOw commented 11 years ago

Well, as I said, because I'm not exactly sure how to deal with the attachedShaders, etc. I decided not to touch it and leave it for somebody more competent. Other than that, if you are happy with the changes, go ahead and merge it.

BTW, although it is stated in the package description that it exposes OpenGL version 3.2, it is, quite obviously, slightly different from the truth. So I was wondering if there is somewhere a list of all the things that are still missing? Such a list would definitely make it easier for people to contribute.

schell commented 11 years ago

I've run into the problem of not being able to make my matrices an instance of the Uniform class. If nothing else it would be great to get that change in.

svenpanne commented 11 years ago

I think all parts of this issue are now addressed. In the future it would be nice to have separate issues for separate things, it makes merging/cherry-picking/discussing things easier.