If found some source code that has this in it
Posted 25 December 2012 - 07:53 PM
If found some source code that has this in it
Posted 25 December 2012 - 08:10 PM
for(i; i < sLines.size(); i++)
L. Spiro
Posted 26 December 2012 - 08:36 AM
Also, you can put the int inside the for() statement:
for(int i; i < sLines.size(); i++)
The difference is whether 'i' goes in the scope outside the for() loop or not. Usually you don't want that, but occasionally you do.
The code seems to be converting an std::vector of std::strings to a dynamically allocated raw array of raw strings. That's like going from driving a good car with airbags and your seatbelt on (perfectly safe), to ducktaping yourself to the belly of an elephant (dangerous, messy, and in most situations just plain dumb). Are you sure the code you are converting actually needs to make the std::vector<std::string> to char*[]?
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal
Posted 26 December 2012 - 11:50 AM
In case you use GCC, you can compile with -save-temps to trivially figure out any macro abuse, no matter how convoluted it is. For every file foo.c you will find a file foo.ii generated, which contains the output of the preprocessor (i.e. with everything included, and all macros expanded). I've found this immensely helpful at times.
Posted 26 December 2012 - 01:06 PM
I'm with Servant of the Lord on this. Are you really sure you need to do this?
Sometimes you have to question the quality of your sources. In this case, it's not only converting away from std::string to unsafe raw strings that is a code smell, but the use of macros in this situation also smells nasty. Macro usage like this can conceal or even introduce bugs, and detracts from readability, making it a confusing mess for no real reason other than somebody thought they could save some typing. If they took shortcuts there, the remainder of their code might be smelly, too.
What is it you are trying to accomplish in the larger scheme? Maybe we can help you figure out a less smelly method.
Posted 26 December 2012 - 02:12 PM
Edited by proanim, 26 December 2012 - 02:12 PM.
Posted 26 December 2012 - 02:44 PM
You're mixing things up here.
First of all, fgets() is a C function (included via the header cstdio). You are reading from a file into a C-style array, then creating a C++ string from the line you read. This is pointless. C++ includes functionality for reading lines from files as well, without the limitation of reading to a fixed-length buffer:
#include <fstream>
#include <string>
std::vector<std::string> sLines;
std::ifstream infile("testfile.txt");
while(!infile.eof())
{
std::string s;
std::getline(infile, s);
sLines.push_back(s);
}
infile.close();
getline() will read a string until a newline is encountered, without the maximum size limit of 255 that the fixed-length C-style read has. The allocation is invisibly handled by the string class.
And no, you don't need to free sLines explicitly; it will be freed when it goes out of scope. The only things you need to free are things you explicitly new. That's the beauty of using C++ standard library functionality, rather than C or (as you are doing here) a bastard mix of C and C++.
Now, I take it from the second part of your code that you need sProgram to be an array of pointers to C-style strings (for whatever reason). Rather than explicitly using new/delete you could do something like this (note that this is still somewhat C-ish, and there are other and more concise ways of doing it in straight C++; but now is not the time to introduce lambdas, I think ;) ):
std::vector<const char *> sProgram;
for(unsigned int i=0; i<sLines.size(); ++i)
{
sProgram.push_back(sLines[i].c_str());
}
If you have C++11 you can then call sProgram.data() to get a pointer to the array of const char * held by sProgram, and use that wherever you are using your current sProgram pointer. If you don't have C++11, then the line &sProgram[0] works, though it is, of course, a bit hacky, (and the pointer is invalidated if you do another push_back that causes a reallocation, so be careful). This way, sProgram is also automatically managed (it grows as needed when more lines are added, and is automatically deleted when it goes out of scope) without the explicit new/delete calls. Even better would be to re-work whatever it is you need sProgram for so it doesn't require an array of C-style strings, but this might not always be possible if you are sending it to a third party library or API (such as a shader compiler, which I suspect you are doing).
In this day and age, if you are using a C++ compiler and you are doing explicit allocating/de-allocating of arrays and strings, then you really ought to take a good hard look at what you are doing and why, as there are almost always safer ways to do it that don't require possibly unsafe memory management.
Posted 26 December 2012 - 04:47 PM
Ok i made changes and now same stuff happens only difference is now i can't complete the entire process. There is no error or anything the funtion returns false
bool CShader::loadShader(string sFile, int a_iType)
{
//FILE* fp = fopen(sFile.c_str(), "rt");
//if(!fp)return false;
// get all lines from a file
vector<string> sLines;
//char sLine[255];
//while(fgets(sLine, 255, fp))sLines.push_back(sLine);
//fclose(fp);
ifstream infile(sFile.c_str());
while(!infile.eof())
{
string s;
getline(infile, s);
sLines.push_back(s);
}
infile.close();
//const char** sProgram = new const char*[ESZ(sLines)];
//FOR(i, ESZ(sLines))sProgram[i] = sLines[i].c_str();
//const char** sProgram = new const char* [(int)sLines.size()];
vector<const char *> sProgram;
//for(int i; i < sLines.size(); i++)
// sProgram[i] = sLines[i].c_str();
for(unsigned int i=0; i<sLines.size(); ++i)
sProgram.push_back(sLines[i].c_str());
uiShader = glCreateShader(a_iType);
glShaderSource(uiShader, sLines.size(), &sProgram[0], NULL);
glCompileShader(uiShader);
//delete[] sProgram;
int iCompilationStatus;
glGetShaderiv(uiShader, GL_COMPILE_STATUS, &iCompilationStatus);
if(iCompilationStatus == GL_FALSE)return false;
iType = a_iType;
bLoaded = true;
return 1;
}
What can cause glGetShaderiv(uiShader, GL_COMPILE_STATUS, &iCompilationStatus); to fail ? Shader files that i use with this are basic color shaders and they work with orginal function but now they fail to compile, I am not sure why exactly.
Posted 26 December 2012 - 04:54 PM
Per the manual page on glCompileShader you can call glGetShaderInfoLog to get some information on why the compile might have failed.
Posted 26 December 2012 - 05:41 PM
Why don’t you show the code you are using to get said results?
Also, your way of loading the shader is dreadful (why not just load the file into memory, add a 0 at the end, and pass that directly to ::glShaderSource() instead of wasting time creating an array of char *’s?), but I don’t see any bugs in it.
So why not save a reply and also post the shader code since that seems more likely to be the source of the error?
L. Spiro
Edited by L. Spiro, 26 December 2012 - 05:45 PM.
Posted 26 December 2012 - 06:10 PM
Why do you guys load shader sources line by line? Thats a extra runtime. Shader sources aren't very big, everything can be done with one read:
GLuint ShaderLoader::shaderFromFile(const std::string& name, GLenum type) {
std::fstream file(name.c_str(), std::ios::binary | std::ios::in);
if (!file.good()) {
Logger::getInstance().log(Logger::LOG_ERROR, "Cannot open file %s", name.c_str());
return 0;
}
file.seekg(0, std::ios::end);
int sourceLength = file.tellg();
GLchar* shaderSource = new GLchar[sourceLength + 1];
file.seekg(0, std::ios::beg);
file.read(shaderSource, sourceLength);
shaderSource[sourceLength] = 0;
GLuint shader = glCreateShader(type);
glShaderSource(shader, 1, (const GLchar**)&shaderSource, NULL);
glCompileShader(shader);
delete[] shaderSource;
GLint compileStatus;
glGetShaderiv(shader, GL_COMPILE_STATUS, &compileStatus);
if (compileStatus == GL_FALSE) {
GLint infoLogLength;
glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &infoLogLength);
GLchar* infoLog = new GLchar[infoLogLength + 1];
glGetShaderInfoLog(shader, infoLogLength, NULL, infoLog);
Logger::getInstance().log(Logger::LOG_ERROR, "In file %s: %s", name.c_str(), infoLog);
delete[] infoLog;
}
return shader;
}
Edited by santa01, 26 December 2012 - 06:12 PM.
Posted 26 December 2012 - 07:23 PM
So why not save a reply and also post the shader code since that seems more likely to be the source of the error?
Shaders are working in first version of the function
vertex shader:
#version 330
layout (location = 0) in vec3 inPosition;
layout (location = 1) in vec3 inColor;
smooth out vec3 theColor;
void main()
{
gl_Position = vec4(inPosition, 1.0);
theColor = inColor;
}
fragment shader
#version 330
smooth in vec3 theColor;
out vec4 outputColor;
void main()
{
outputColor = vec4(theColor, 1.0);
}
these are loaded with
// Load shaders and create shader program
shVertex.loadShader("data/shaders/color.vert", GL_VERTEX_SHADER);
shFragment.loadShader("data/shaders/color.frag", GL_FRAGMENT_SHADER);
Posted 26 December 2012 - 08:10 PM
Well all I can get from glGetShaderInfoLog is '00B4A520' or similar ???
Why don’t you show the code you are using to get said results?
Posted 26 December 2012 - 09:49 PM
What L. Spiro is saying, in case it's not obvious, is that you're not showing how you're getting this error log back... Your earlier post shows some good info on how you're compiling your shader, but it doesn't show how you're obtaining that strange error log.
It's kinda like trying to diagnose a patient, if you're a doctor, and he says "I have symptoms" but doesn't say what those symptoms are ![]()
Edited by Cornstalks, 26 December 2012 - 10:00 PM.
Posted 27 December 2012 - 09:17 AM
glShaderSource(uiShader, sLines.size(), &sProgram[0], NULL);
glCompileShader(uiShader);
int infologLength;
int maxLength;
char* infoLog;
glGetShaderiv(uiShader,GL_INFO_LOG_LENGTH, &maxLength);
glGetShaderInfoLog(uiShader, maxLength, &infologLength, infoLog);
ofstream outfile;
outfile.open ("log.txt");
outfile << &infoLog << endl;
outfile.close();
Posted 27 December 2012 - 09:27 AM
Santa01 showed above how to get the log from the shader or program object. Your code has two major problems though:
Posted 27 December 2012 - 09:44 AM
finally, so the errors are this
Fragment shader failed to compile with the following errors: ERROR: 2:1: error(#307) Profile "smooth" is not supported ERROR: 2:1: error(#76) Syntax error: unexpected tokens following #version ERROR: error(#273) 2 compilation errors. No code generated
any ideas?
Posted 27 December 2012 - 10:23 AM
getline(infile, s);should be:
getline(infile, s); s.append( endl );I had assumed originally that ::glShaderSource() would treat each line you passed to it as…a line.
Edited by L. Spiro, 27 December 2012 - 10:36 AM.