matus-chochlik / oglplus

OGLplus is a collection of open-source, cross-platform libraries which implement an object-oriented facade over the OpenGL® (version 3 and higher) and also OpenAL® (version 1.1) and EGL (version 1.4) C-language APIs. It provides wrappers which automate resource and object management and make the use of these libraries in C++ safer and more convenient.
http://oglplus.org/
Boost Software License 1.0
494 stars 71 forks source link

GLSLSource::FromFile problem #49

Closed Senryoku closed 10 years ago

Senryoku commented 10 years ago

Hi, I am new to oglplus, and having some troubles with GLSLSource::FromFile(), the resulting GLSLSource instance doesn't appear to be valid : I get a compilation error for the shader after passing it to ShaderOps::Source(). However this issue only appears on one computer (Windows 7 with AMD GPU), after some research and digging into the sources, it seems that the value returned by

GLint InputStreamGLSLSrcWrap::_check_and_get_size(std::istream& in)

and therefore GLSLSource::Lengths() is wrong : Using an hard coded value for the length and manually passing the tree arguments to ShaderOps::Source solves the problem.

The original source looks like this :

oglplus::VertexShader m_vs;
m_vs.Source( GLSLSource::FromFile("Path.glsl") );
m_vs.Compile();

I read that counting characters from an istream cannot be completely reliable, could this be a bug, or am I doing something wrong ? (I checked my shader file, rewrote it from scratch to avoid any copy/paste bull**\ etc...)

Thanks !

matus-chochlik commented 10 years ago

Hi, thanks for reporting this. Could you try the following and let me know if it worked? In include/oglplus/auxiliary/glsl_source.hpp replace std::istream by std::ifstream in InputStreamGLSLSrcWrap constructor and _check_and_get_size and do the same also in implement/oglplus/auxiliary/glsl_source.ipp. Checking the size of a file stream IMO should be reliable.

Senryoku commented 10 years ago

Thanks for the quick answer ! Same problem using an ifstream. CRLF seems to mess it up : I think each new line is counted twice. (I'm using MinGW btw). (see http://stackoverflow.com/questions/1631288/better-way-to-determine-length-of-a-stdistream)

matus-chochlik commented 10 years ago

Hmm, that's strange. I get what the problem can be with a stdin, pipe or socket istreams, but WTH does it not work with regular files? Anyway, I'll reimplement it during the weekend and let you know.

matus-chochlik commented 10 years ago

OK, I've done the following: The size is determined as before, but the data are not read by a single call to istream::read but instead by using std::istreambuf_iterator. This should be both efficient if the size can be determined reliably and robust even if it cannot. The change has been push to develop (commit f3285562dcc5321b09da5c439abd524e290ae0dc).

HTH

matus-chochlik commented 10 years ago

I've made some further changes to _check_and_get_size, hopefully it'll work now with any istream properly.

Senryoku commented 10 years ago

Your changes did the trick, loading from a file seems to work flawlessly for me now. Thanks a lot !