Hello! (first post  )

I have written this code to draw strings to my game screen. I am hoping someone can help me find a way to improve it.

The way it works is it loads a 256x256 texture that contain all the characters in ASCII. For now I am using my own ordering...
For example, tiles 0 - 25 are ABCDEFGHIJKLMNOPQRSTUVWXYZ, which is definitely not how t he ASCII system works
Because the ASCII system has 256 characters, that's a lot of work for my artist. So I've just made a simple "look up" method
that switches on a char variable and returns the position in the array where that letter's texture coordinates are.


It renders fine, and performs fine (my fps don't even budge with several strings running).

but I know that game engine design can break down in thousands of areas, so I am taking it one very small area at a time. This is my string drawing code
Please feel free to rip me apart and explain how it could be better. If you have questions about anything just ask (as I have not displayed the entire class).

Code called to draw a single string. Supports line breaks to be a little more optimized than just calling the method again:
public void draw(String text, float x, float y, int orthowidth, int orthoheight, boolean shadow)
{
glMatrixMode(GL_PROJECTION);
glLoadIdentity();
glOrtho(0, orthowidth, orthoheight, 0, 1, -1);

fonttex.bind();
int carriage=0;
int yoffset=0;
int xoffset=0;
char a;
char nextchar;
//char lastchar;

for(int i = 0; i < text.length(); i ++)
{
a = text.charAt(i);
nextchar = 'a';
//lastchar = 'a';

//if(i-1 > 0 ) lastchar = text.charAt(i-1);
if(i+1 < text.length()) nextchar = text.charAt(i+1);

if(a == '\n')
{
carriage++;
glMatrixMode(GL_PROJECTION);
glLoadIdentity();
glOrtho(0, orthowidth, orthoheight, 0, 1, -1);
glTranslated(0,(iCharHeight+1) *carriage,0);//TODO no idea why this works
continue;
}
yoffset=getyspacing(a);
xoffset=getxspacing(a);
if(xoffset <= 0) xoffset=getxspacing(nextchar);

if(a!=' ')
{
glBegin(GL_QUADS);
glTexCoord2f(letters[a].topleftx, letters[a].toplefty);
glVertex2f(x, y + yoffset);

glTexCoord2f(letters[a].toprightx, letters[a].toprighty);
glVertex2f(x + iCharWidth, y + yoffset);

glTexCoord2f(letters[a].bottomrightx, letters[a].bottomrighty);
glVertex2f(x + iCharWidth, y + yoffset + iCharHeight);

glTexCoord2f(letters[a].bottomleftx, letters[a].bottomlefty);
glVertex2f(x, y + yoffset + iCharHeight);
if(shadow)
{
//glBlendColor(0.5f, 0.5f, 0.5f, 1.0f);

//glBlendFunc(GL_SRC_COLOR, GL_);

glTexCoord2f(letters[a].topleftx, letters[a].toplefty);
glVertex2f(x+1, y+1);

glTexCoord2f(letters[a].toprightx, letters[a].toprighty);
glVertex2f(1+x + (iCharWidth),1+ y);

glTexCoord2f(letters[a].bottomrightx, letters[a].bottomrighty);
glVertex2f(1+x + (iCharWidth ),1+y + (iCharHeight));

glTexCoord2f(letters[a].bottomleftx, letters[a].bottomlefty);
glVertex2f(1+x,1+ y + (iCharHeight ));
}
glEnd();
}
glTranslated((iCharWidth-1)-xoffset, 0, 0);
}



       
        glMatrixMode(GL_MODELVIEW);
}

Code called to build the font at startup:
private class texcoord{
float topleftx=0.0f;
float toplefty=0.0f;

float toprightx=0.0f;
float toprighty=0.0f;

float bottomrightx=0.0f;
float bottomrighty=0.0f;

float bottomleftx=0.0f;
float bottomlefty=0.0f;
public texcoord()
{

}
}

public void buildfont()
{
float unitsx = 1.0f / 256;
float unitsy = 1.0f / 256;

letters = new texcoord[256];

for(char i = 0; i < 256; i++)
{
letters[i] = new texcoord();
int letx = getlettertexX(i);
int lety = getlettertexY(i);

float posx = letx * iCharWidth;
float posy = lety * iCharHeight;

letters[i].topleftx = unitsx * posx;
letters[i].toplefty = unitsy * posy;

letters[i].toprightx = unitsx * (posx + iCharWidth);
letters[i].toprighty = unitsy * posy;

letters[i].bottomrightx = unitsx * (posx + iCharWidth);
letters[i].bottomrighty = unitsy * (posy + iCharHeight);

letters[i].bottomleftx = unitsx * posx;
letters[i].bottomlefty = unitsy * (posy + iCharHeight);
//System.out.println("Building Font letter: " + i + )
}
}

here's the look up methods

public int getlettertexX(char a)
{
switch(a)
{
case 'A': return 0;
case 'B': return 1;
case 'C': return 2;
case 'D': return 3;
case 'E': return 4;
case 'F': return 5;
case 'G': return 6;
case 'H': return 7;
case 'I': return 8;
case 'J': return 9;
case 'K': return 10;
case 'L': return 11;
case 'M': return 12;
case 'N': return 13;
case 'O': return 14;
case 'P': return 15;
case 'Q': return 16;
case 'R': return 17;
case 'S': return 18;
case 'T': return 19;
case 'U': return 20;
case 'V': return 21;
case 'W': return 22;
case 'X': return 23;
case 'Y': return 24;
case 'Z': return 25;

case 'a': return 0;
case 'b': return 1;
case 'c': return 2;
case 'd': return 3;
case 'e': return 4;
case 'f': return 5;
case 'g': return 6;
case 'h': return 7;
case 'i': return 8;
case 'j': return 9;
case 'k': return 10;
case 'l': return 11;
case 'm': return 12;
case 'n': return 13;
case 'o': return 14;
case 'p': return 15;
case 'q': return 16;
case 'r': return 17;
case 's': return 18;
case 't': return 19;
case 'u': return 20;
case 'v': return 21;
case 'w': return 22;
case 'x': return 23;
case 'y': return 24;
case 'z': return 25;

case '0': return 0;
case '1': return 1;
case '2': return 2;
case '3': return 3;
case '4': return 4;
case '5': return 5;
case '6': return 6;
case '7': return 7;
case '8': return 8;
case '9': return 9;
case '`': return 10;
case '~': return 11;
case '!': return 12;
case '@': return 13;
case '#': return 14;
case '$': return 15;
case '%': return 16;
case '^': return 17;
case '&': return 18;
case '*': return 19;
case '(': return 20;
case ')': return 21;
case '-': return 22;
case '_': return 23;
case '+': return 24;
case '=': return 25;

case '.': return 0;
case ',': return 1;
case '<': return 2;
case '>': return 3;
case '/': return 4;
case '\\': return 5;
case '?': return 6;
case ':': return 7;
case ';': return 8;
case '"': return 9;
case '\'': return 10;
case '[': return 11;
case ']': return 12;
case '{': return 13;
case '}': return 14;
case '|': return 15;

}
return -1;
}

public int getlettertexY(char a)
{
switch(a)
{
case 'A': return 0;
case 'B': return 0;
case 'C': return 0;
case 'D': return 0;
case 'E': return 0;
case 'F': return 0;
case 'G': return 0;
case 'H': return 0;
case 'I': return 0;
case 'J': return 0;
case 'K': return 0;
case 'L': return 0;
case 'M': return 0;
case 'N': return 0;
case 'O': return 0;
case 'P': return 0;
case 'Q': return 0;
case 'R': return 0;
case 'S': return 0;
case 'T': return 0;
case 'U': return 0;
case 'V': return 0;
case 'W': return 0;
case 'X': return 0;
case 'Y': return 0;
case 'Z': return 0;

case 'a': return 1;
case 'b': return 1;
case 'c': return 1;
case 'd': return 1;
case 'e': return 1;
case 'f': return 1;
case 'g': return 1;
case 'h': return 1;
case 'i': return 1;
case 'j': return 1;
case 'k': return 1;
case 'l': return 1;
case 'm': return 1;
case 'n': return 1;
case 'o': return 1;
case 'p': return 1;
case 'q': return 1;
case 'r': return 1;
case 's': return 1;
case 't': return 1;
case 'u': return 1;
case 'v': return 1;
case 'w': return 1;
case 'x': return 1;
case 'y': return 1;
case 'z': return 1;

case '0': return 2;
case '1': return 2;
case '2': return 2;
case '3': return 2;
case '4': return 2;
case '5': return 2;
case '6': return 2;
case '7': return 2;
case '8': return 2;
case '9': return 2;
case '`': return 2;
case '~': return 2;
case '!': return 2;
case '@': return 2;
case '#': return 2;
case '$': return 2;
case '%': return 2;
case '^': return 2;
case '&': return 2;
case '*': return 2;
case '(': return 2;
case ')': return 2;
case '-': return 2;
case '_': return 2;
case '+': return 2;
case '=': return 2;

case '.': return 3;
case ',': return 3;
case '<': return 3;
case '>': return 3;
case '/': return 3;
case '\\': return 3;
case '?': return 3;
case ':': return 3;
case ';': return 3;
case '"': return 3;
case '\'': return 3;
case '[': return 3;
case ']': return 3;
case '{': return 3;
case '}': return 3;
case '|': return 3;

}
return -1;
}

public int getxspacing(char a)
{
switch(a)
{
case 'A': return 0;
case 'B': return 0;
case 'C': return 0;
case 'D': return 0;
case 'E': return 0;
case 'F': return 0;
case 'G': return 0;
case 'H': return 0;
case 'I': return 0;
case 'J': return 0;
case 'K': return 0;
case 'L': return 0;
case 'M': return 0;
case 'N': return 0;
case 'O': return 0;
case 'P': return 0;
case 'Q': return 0;
case 'R': return 0;
case 'S': return 0;
case 'T': return 0;
case 'U': return 0;
case 'V': return 0;
case 'W': return 0;
case 'X': return 0;
case 'Y': return 0;
case 'Z': return 0;

case 'a': return 0;
case 'b': return 0;
case 'c': return 0;
case 'd': return 0;
case 'e': return 0;
case 'f': return 0;
case 'g': return 0;
case 'h': return 0;
case 'i': return 1;
case 'j': return 0;
case 'k': return 0;
case 'l': return 1;
case 'm': return 0;
case 'n': return 0;
case 'o': return 0;
case 'p': return 0;
case 'q': return 0;
case 'r': return 0;
case 's': return 0;
case 't': return 0;
case 'u': return 0;
case 'v': return 0;
case 'w': return 0;
case 'x': return 0;
case 'y': return 0;
case 'z': return 0;

case '0': return 0;
case '1': return 0;
case '2': return 0;
case '3': return 0;
case '4': return 0;
case '5': return 0;
case '6': return 0;
case '7': return 0;
case '8': return 0;
case '9': return 0;
case '`': return 1;
case '~': return 0;
case '!': return 1;
case '@': return 0;
case '#': return 0;
case '$': return 0;
case '%': return 0;
case '^': return 0;
case '&': return 0;
case '*': return 0;
case '(': return 0;
case ')': return 0;
case '-': return 0;
case '_': return 0;
case '+': return 0;
case '=': return 0;

case '.': return 1;
case ',': return 1;
case '<': return 0;
case '>': return 0;
case '/': return 0;
case '\\': return 0;
case '?': return 0;
case ':': return 1;
case ';': return 1;
case '"': return 0;
case '\'': return 0;
case '[': return 0;
case ']': return 0;
case '{': return 0;
case '}': return 0;
case '|': return 1;
}
return 0;
}

public int getyspacing(char a)
{
final int spacing = 1;
switch(a)
{
case 'A': return 0;
case 'B': return 0;
case 'C': return 0;
case 'D': return 0;
case 'E': return 0;
case 'F': return 0;
case 'G': return 0;
case 'H': return 0;
case 'I': return 0;
case 'J': return 0;
case 'K': return 0;
case 'L': return 0;
case 'M': return 0;
case 'N': return 0;
case 'O': return 0;
case 'P': return 0;
case 'Q': return 0;
case 'R': return 0;
case 'S': return 0;
case 'T': return 0;
case 'U': return 0;
case 'V': return 0;
case 'W': return 0;
case 'X': return 0;
case 'Y': return 0;
case 'Z': return 0;

case 'a': return 0;
case 'b': return 0;
case 'c': return 0;
case 'd': return 0;
case 'e': return 0;
case 'f': return 0;
case 'g': return spacing;
case 'h': return 0;
case 'i': return 0;
case 'j': return spacing -1;
case 'k': return 0;
case 'l': return 0;
case 'm': return 0;
case 'n': return 0;
case 'o': return 0;
case 'p': return spacing;
case 'q': return spacing;
case 'r': return 0;
case 's': return 0;
case 't': return 0;
case 'u': return 0;
case 'v': return 0;
case 'w': return 0;
case 'x': return 0;
case 'y': return spacing;
case 'z': return 0;

case '0': return 0;
case '1': return 0;
case '2': return 0;
case '3': return 0;
case '4': return 0;
case '5': return 0;
case '6': return 0;
case '7': return 0;
case '8': return 0;
case '9': return 0;
case '`': return 0;
case '~': return 0;
case '!': return 0;
case '@': return 0;
case '#': return 0;
case '$': return 0;
case '%': return 0;
case '^': return 0;
case '&': return 0;
case '*': return 0;
case '(': return 0;
case ')': return 0;
case '-': return 0;
case '_': return 0;
case '+': return 0;
case '=': return 0;

case '.': return spacing+1;
case ',': return spacing+1;
case '<': return 0;
case '>': return 0;
case '/': return 0;
case '\\': return 0;
case '?': return 0;
case ':': return 0;
case ';': return 0;
case '"': return 0;
case '\'': return 0;
case '[': return 0;
case ']': return 0;
case '{': return 0;
case '}': return 0;
case '|': return 0;
}
return 0;
}

I would also really appreciate if anyone could tell me how to change the colors of what I'm drawing. The actual font sprite file is just white letters. My artist painted in a shadow for each letter, but I would like to know how to do this in real time (ie: draw the font in red, draw the shadow in dark red). without using multiple texture files.

For color, just bind the OpenGL color! If you're using Slick-Util, there's an object for it; otherwise, glColor#f() will do the job for you.

Also, take a look at Slick's font classes. They're a good resource to look at and do some surprising things.

I'd suggest using a HashMap instead of a switch (that's a lot of conditionals):
// when first start app
letters = new HashMap<Character,Image>();
letters.put('A',image[0]);
letters.put('B',image[1]);
letters.put('C',image[2]);
...
// later when draw
char c; // output
Image i = letters.get(Character.toUpperCase(c));
if(i != null) {
  draw(image[i]);
}
else {
  draw(letters.get('?'));
}

Or if you don't need things like '.' and ',' being the same:
char c; // output
int i = Character.toUpperCase(c) - 'A';
draw(image[i])


I'd suggest using a HashMap instead of a switch (that's a lot of conditionals):

Code: [Select]

// when first start app
letters = new HashMap<Character,Image>();
letters.put('A',image[0]);
letters.put('B',image[1]);
letters.put('C',image[2]);
...
// later when draw
char c; // output
Image i = letters.get(Character.toUpperCase(c));
if(i != null) {
  draw(image[i]);
}
else {
  draw(letters.get('?'));
}

Or if you don't need things like '.' and ',' being the same:

Code: [Select]

char c; // output
int i = Character.toUpperCase(c) - 'A';
draw(image[i])


The switch approach would be a lot faster than the HashMap and if it makes more sense then I'd rather go with it.

Quote from: jediTofu on September 23, 2011, 03:47:31

I'd suggest using a HashMap instead of a switch (that's a lot of conditionals):

Code: [Select]

// when first start app
letters = new HashMap<Character,Image>();
letters.put('A',image[0]);
letters.put('B',image[1]);
letters.put('C',image[2]);
...
// later when draw
char c; // output
Image i = letters.get(Character.toUpperCase(c));
if(i != null) {
  draw(image[i]);
}
else {
  draw(letters.get('?'));
}

Or if you don't need things like '.' and ',' being the same:

Code: [Select]

char c; // output
int i = Character.toUpperCase(c) - 'A';
draw(image[i])


The switch approach would be a lot faster than the HashMap and if it makes more sense then I'd rather go with it.

Not sure how a switch would be faster.  A switch is a bunch of if statements (or a bunch of gotos/jumps in assembly).  A HashMap is accessing an array.  Accessing an array is faster than doing if statements.

Quote from: concerto on September 23, 2011, 11:42:57

Quote from: jediTofu on September 23, 2011, 03:47:31

I'd suggest using a HashMap instead of a switch (that's a lot of conditionals):

Code: [Select]

// when first start app
letters = new HashMap<Character,Image>();
letters.put('A',image[0]);
letters.put('B',image[1]);
letters.put('C',image[2]);
...
// later when draw
char c; // output
Image i = letters.get(Character.toUpperCase(c));
if(i != null) {
  draw(image[i]);
}
else {
  draw(letters.get('?'));
}

Or if you don't need things like '.' and ',' being the same:

Code: [Select]

char c; // output
int i = Character.toUpperCase(c) - 'A';
draw(image[i])


The switch approach would be a lot faster than the HashMap and if it makes more sense then I'd rather go with it.

Not sure how a switch would be faster.  A switch is a bunch of if statements (or a bunch of gotos/jumps in assembly).  A HashMap is accessing an array.  Accessing an array is faster than doing if statements.

Look at the bytecode. There are 2 types of switches. If optimized, it can be run as a jump table, which is an array access. A HashMap is a lot more than array access. Look at the source code. It still contains IFs and you have to calculator the hash on top, which might be expensive, depending on the hash function used. If there is collision on the HashMap, it's worse.

Quote from: jediTofu on September 23, 2011, 12:18:09

Quote from: concerto on September 23, 2011, 11:42:57

Quote from: jediTofu on September 23, 2011, 03:47:31

I'd suggest using a HashMap instead of a switch (that's a lot of conditionals):

Code: [Select]

// when first start app
letters = new HashMap<Character,Image>();
letters.put('A',image[0]);
letters.put('B',image[1]);
letters.put('C',image[2]);
...
// later when draw
char c; // output
Image i = letters.get(Character.toUpperCase(c));
if(i != null) {
  draw(image[i]);
}
else {
  draw(letters.get('?'));
}

Or if you don't need things like '.' and ',' being the same:

Code: [Select]

char c; // output
int i = Character.toUpperCase(c) - 'A';
draw(image[i])


The switch approach would be a lot faster than the HashMap and if it makes more sense then I'd rather go with it.

Not sure how a switch would be faster.  A switch is a bunch of if statements (or a bunch of gotos/jumps in assembly).  A HashMap is accessing an array.  Accessing an array is faster than doing if statements.

Look at the bytecode. There are 2 types of switches. If optimized, it can be run as a jump table, which is an array access. A HashMap is a lot more than array access. Look at the source code. It still contains IFs and you have to calculator the hash on top, which might be expensive, depending on the hash function used. If there is collision on the HashMap, it's worse.

We'll just agree to disagree    Based on the switch statement above, I just believe a HashMap is better IMO.

Edit:  also the JVM (for a certain system) may not optimize it, while an array will always be stored in the most optimized way.  worst case scenario, those switches will be bad news.  with an array, maybe you could even use some OpenCL

Edit:  I also think a hashmap is more easily maintained than that gigantic switch statement -- but that comes down to coder preference and opinions.  Example:
// called once when first load
public static void createXHash() {
    for(char c = 'A'; c <= 'Z'; ++c) {
      xhash.put(c,0);
    }
    for(char c = 'a'; c <= 'z'; ++c) {
      xhash.put(c,1);
    }
    for(char c = '0'; c <= '='; ++c) {
      xhash.put(c,2);
    }
    for(char c = '.'; c <= '|'; ++c) {
      xhash.put(c,3);
    }
}


Also, for that switch statement, this may be easier to read and maintain:

case '[':
case ']':
case '|':
case ',': return 0;

case '^':
case '&':
case '%': return 1;

Wouldn't an array be better than a hash map?

If you care about performance, you would likely put the results into a vertex array, display list, or VBO, so it probably doesn't matter

Anyway, it seems more important not to do glBegin(GL_QUADS); for every character in the string, or glOrtho for every line.