Triangle Fan Question

Just a simple question, but I think I know why my rendering is failing so badly…

I’m attempting to render a simple scene of a few buildings. I have the vertices of the scene stored into a single, 1-dimensional array, and they are to be assembled as triangle fans.

Is it incorrect that I am simply calling glDrawArrays(GL_TRIANGLE_FAN, 0, _numVertices)? I’m fearing that I need to create a buffer for EACH and EVERY face of the buildings and then call glDrawArrays individually.

Two questions:

1. Am I correct in my “diagnosis”?

2. If this is really why it’s messing up, is there some way for me to adjust my code so that it will render correctly? Unfortunately, I can’t change the primitive type.

I really appreciate it! I’m still learning OpenGL, and trying my best to get over the hurdles on my own.

You haven’t shown any example or provided any description of what is wrong, which makes any help anyone offers nothing more than speculation (we have to guess what’s wrong, and then guess how to solve it).

But that’s not going to stop me! :^) I do have an idea of what your problem may be. First, you do NOT have to create a buffer for each and every face of the buildings and call glDrawArrays() individually. But, you DO have to use a feature known as “primitive restart” in order to draw more than one triangle fan with a single call to glDrawArrays(). With primitive restart, you insert a special value (that you chose, but have to tell OpenGL which value you chose) to mark the end of each triangle fan in your vertex buffer. Then, glDrawArrays() will “restart” the primitive (i.e., start a new triangle fan) whenever it encounters that value.

1. You’re mostly correct. You can use multiple glDrawArrays on a single VBO by supplying the correct parameters, but with trifans you can’t use a single glDrawArrays for everything.

2. You can do what was suggested above, or you can use indexes with glDrawElements. You don’t need to change your vertex data for this (which is what I suspect you mean by “I can’t change the primitive type”) but the primitive type will become GL_TRIANGLES and each triangle fan will have indexes of 0,1,2 0,2,3 0,3,4 etc. This will probably be faster too as indexes will actually allow your hardware’s vertex cache to work.

Thanks for the responses guys.

Below is the slice of code where things are going awry. Basically my eventual goal is to allow the user to open a file via URL (CityGML file…much like an XML file), send it through a parser to strip out the vertices, then render it.

I’m currently hard-coding in a simple scene consisting of two, side-by-side, rectangular buildings, because four months ago I had never touched OpenGL and I’m still struggling to teach myself.

The trouble I may run into using your suggestion, David, is that scenes could reach upwards of a few hundred thousand vertices, so adding more would just take space, which is valuable on a mobile device (Android).

mhagain, I think I understand your suggestion. If I understand correctly, I would then just run a loop for i < theNumberOfVertices and number the indices as you indicated?

(Also, don’t hesitate to point out any other blatant errors you see…I’m still struggling with generating normals.)

package com.android.aliaga;

import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.FloatBuffer;

import javax.microedition.khronos.egl.EGLConfig;
import javax.microedition.khronos.opengles.GL10;
 
import android.opengl.GLSurfaceView;

public class AliagaRenderer implements GLSurfaceView.Renderer {
    private FloatBuffer _vertexBuffer, _normalBuffer;
 
    private int _nrOfVertices = 0;
    float[][] point;
    float[][] out;
    float[] normalCoords;
    public int detail = GL10.GL_TRIANGLE_FAN;
 
    private float _xAngle;
    private float _yAngle;
    
    public void onSurfaceCreated(GL10 gl, EGLConfig config) {
        gl.glFrontFace(GL10.GL_CCW);
        gl.glCullFace(GL10.GL_BACK);
        gl.glEnable(GL10.GL_CULL_FACE);
        
        initShape();
    }
 
    public void onSurfaceChanged(GL10 gl, int w, int h) {
        gl.glViewport(0, 0, w, h);
    }
 
    public void setXAngle(float angle) {
        _xAngle = angle;
    }
 
    public float getXAngle() {
        return _xAngle;
    }
 
    public void setYAngle(float angle) {
        _yAngle = angle;
    }
 
    public float getYAngle() {
        return _yAngle;
    }
 
    public void onDrawFrame(GL10 gl) {
        gl.glMatrixMode(GL10.GL_MODELVIEW);
    	gl.glLoadIdentity();
     
        if(_xAngle > 0 && _xAngle <= 90) {
        		gl.glRotatef(_xAngle, 1.0f, 0.0f, 0.0f);
        }
        if(_xAngle > 90) {
        	gl.glRotatef(90, 1.0f, 0.0f, 0.0f);
        }
        gl.glRotatef(_yAngle, 0.0f, 1.0f, 0.0f);
             
        gl.glMatrixMode(GL10.GL_PROJECTION);
        gl.glLoadIdentity();
        gl.glOrthof(-50.0f, 50.0f, -50.0f, 50.0f, -50.0f, 50.0f);
        
        gl.glClearColor(0.0f, 0.1f, 1.0f, 1.0f);
        gl.glClear(GL10.GL_COLOR_BUFFER_BIT | GL10.GL_DEPTH_BUFFER_BIT);
        
        gl.glEnableClientState(GL10.GL_VERTEX_ARRAY);
        gl.glVertexPointer(3, GL10.GL_FLOAT, 0, _vertexBuffer);
        
        gl.glEnableClientState(GL10.GL_NORMAL_ARRAY);
        gl.glNormalPointer(GL10.GL_FLOAT, 0, _normalBuffer);
        
        gl.glClearColor(0.0f, 0.0f, 0.0f, 0.0f);
        gl.glShadeModel(GL10.GL_SMOOTH);
        gl.glEnable(GL10.GL_LIGHTING);
        
        FloatBuffer light0 = FloatBuffer.allocate(4);
        light0.put(2.0f); light0.put(2.0f); light0.put(0.0f); light0.put(1.0f);
        gl.glLightfv(GL10.GL_LIGHT0, GL10.GL_POSITION, light0);
        
        FloatBuffer light1 = FloatBuffer.allocate(4);
        light1.put(0.5f); light1.put(5.0f); light1.put(2.0f); light1.put(1.0f);
        gl.glLightfv(GL10.GL_LIGHT1, GL10.GL_POSITION, light1);
        
        gl.glEnable(GL10.GL_LIGHT0);
        gl.glEnable(GL10.GL_LIGHT1);
        
        gl.glDrawArrays(detail, 0, _nrOfVertices);
        
        gl.glDisableClientState(GL10.GL_VERTEX_ARRAY);
        gl.glDisableClientState(GL10.GL_NORMAL_ARRAY);
        gl.glFlush();
    } 
    
    public void detail(int d) {
    	detail = d;
    }

 private void initShape() {
	    float[] coords = {
	    		-37.7409625706199f, -17.9680353881589f, -5.77529135625809e-015f, -37.7409625706199f, 20.9752646118411f, -5.77529135625809e-015f, -15.8699625706199f, 20.9752646118411f, -5.77529135625809e-015f, -15.8699625706199f, -17.9680353881589f, -5.77529135625809e-015f, -37.7409625706199f, -17.9680353881589f, -5.77529135625809e-015f, -37.7409625706199f, 20.9752646118411f, 9.690299999999f, -37.7409625706199f, -17.9680353881589f, 9.690299999999f, -15.8699625706199f, -17.9680353881589f, 9.690299999999f, -15.8699625706199f, 20.9752646118411f, 9.690299999999f, -37.7409625706199f, 20.9752646118411f, 9.690299999999f, -37.7409625706199f, 20.9752646118411f, -5.77529135625809e-015f, -37.7409625706199f, -17.9680353881589f, -5.77529135625809e-015f, -37.7409625706199f, -17.9680353881589f, 9.690299999999f, -37.7409625706199f, 20.9752646118411f, 9.690299999999f, -37.7409625706199f, 20.9752646118411f, -5.77529135625809e-015f, -15.8699625706199f, 20.9752646118411f, -5.77529135625809e-015f, -37.7409625706199f, 20.9752646118411f, -5.77529135625809e-015f, -37.7409625706199f, 20.9752646118411f, 9.690299999999f, -15.8699625706199f, 20.9752646118411f, 9.690299999999f, -15.8699625706199f, 20.9752646118411f, -5.77529135625809e-015f, -15.8699625706199f, -17.9680353881589f, -5.77529135625809e-015f, -15.8699625706199f, 20.9752646118411f, -5.77529135625809e-015f, -15.8699625706199f, 20.9752646118411f, 9.690299999999f, -15.8699625706199f, -17.9680353881589f, 9.690299999999f, -15.8699625706199f, -17.9680353881589f, -5.77529135625809e-015f, -37.7409625706199f, -17.9680353881589f, -5.77529135625809e-015f, -15.8699625706199f, -17.9680353881589f, -5.77529135625809e-015f, -15.8699625706199f, -17.9680353881589f, 9.690299999999f, -37.7409625706199f, -17.9680353881589f, 9.690299999999f, -37.7409625706199f, -17.9680353881589f, -5.77529135625809e-015f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, 21.2264989808161f, -8.99301431951953e-029f, 21.2655717707151f, 21.2264989808161f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, 21.2264989808161f, 9.6903f, -0.605428229284897f, -17.7168010191839f, 9.6903f, 21.2655717707151f, -17.7168010191839f, 9.6903f, 21.2655717707151f, 21.2264989808161f, 9.6903f, -0.605428229284897f, 21.2264989808161f, 9.6903f, -0.605428229284897f, 21.2264989808161f, -8.99301431951953e-029f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, -17.7168010191839f, 9.6903f, -0.605428229284897f, 21.2264989808161f, 9.6903f, -0.605428229284897f, 21.2264989808161f, -8.99301431951953e-029f, 21.2655717707151f, 21.2264989808161f, -8.99301431951953e-029f, -0.605428229284897f, 21.2264989808161f, -8.99301431951953e-029f, -0.605428229284897f, 21.2264989808161f, 9.6903f, 21.2655717707151f, 21.2264989808161f, 9.6903f, 21.2655717707151f, 21.2264989808161f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, -8.99301431951953e-029f, 21.2655717707151f, 21.2264989808161f, -8.99301431951953e-029f, 21.2655717707151f, 21.2264989808161f, 9.6903f, 21.2655717707151f, -17.7168010191839f, 9.6903f, 21.2655717707151f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, 9.6903f, -0.605428229284897f, -17.7168010191839f, 9.6903f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, -8.99301431951953e-029f, 21.2655717707151f, -17.7168010191839f, 9.6903f, -0.605428229284897f, -17.7168010191839f, 9.6903f, -0.605428229284897f, -17.7168010191839f, -8.99301431951953e-029f
	    };
	    
	    _nrOfVertices = coords.length;
	    
	    ByteBuffer vbb = ByteBuffer.allocateDirect(coords.length * 4);
	    vbb.order(ByteOrder.nativeOrder());
	    _vertexBuffer = vbb.asFloatBuffer();
	    _vertexBuffer.put(coords);
	    _vertexBuffer.position(0);
	    
	    // Fills vertex array with coordinates
	    point = new float[_nrOfVertices/3][3];
	    int i = 0;
	    int j = 0;
	    while(i<_nrOfVertices) {
	    	point[j][0] = coords[i++]; // x
	    	point[j][1] = coords[i++]; // y
	    	point[j++][2] = coords[i++]; // z
	    }
	    
	    // Normal function called
	    normalCoords = new float[(_nrOfVertices/3)];
	    normal();
	    
	    ByteBuffer nb = ByteBuffer.allocateDirect(normalCoords.length*4);
	    nb.order(ByteOrder.nativeOrder());
	    _normalBuffer = nb.asFloatBuffer();
	    _normalBuffer.put(normalCoords);
	    _normalBuffer.position(0);
	}
 
	void normal() {
 		float[] v1 = new float[3];
 		float[] v2 = new float[3];

 		int i = 0;
 		int j = 0;
 		while(i<(_nrOfVertices/3)) {
 			v1[0] = point[i+1][0] - point[i][0];
 			v1[1] = point[i+1][1] - point[i][1];
 			v1[2] = point[i+1][2] - point[i][2];
 			
 			v2[0] = point[i+2][0] - point[i][0];
 			v2[1] = point[i+2][1] - point[i][1];
 			v2[2] = point[i+2][2] - point[i][2];
 			
 			normalCoords[j++] = (v1[1] * v2[2]) - (v1[2] * v2[1]);
 			normalCoords[j++] = -(v1[2] * v2[0]) - (v1[0] * v2[2]);
 			normalCoords[j++] = (v1[0] * v2[1]) - (v1[1] * v2[0]);
 			
 			// Normalize normals to unit length
 			float x = normalCoords[j-3];
 			float y = normalCoords[j-2];
 			float z = normalCoords[j-1];
 			
 			float len = (float)(Math.sqrt((x*x)+(y*y)+(z*z)));
 			if(len == 0.0f)
 				len = 1.0f;
 			
 			normalCoords[j-3] /= len;
 			normalCoords[j-2] /= len;
 			normalCoords[j-1] /= len;
 			
 			i += 3;
 		}
 	}
}

I didn’t study your code closely. But I do have a few comments. First, my explanation of “primitive restart” wasn’t quite right. It requires indexed primitives; the special value you insert is an index rather than a coordinate value (actually, I didn’t say that it was a coordinate value before, but it was implied because I discussed it incorrectly in the context of glDrawArrays()).

So, here’s a little analysis: you will have several thousand vertices, so you will require long (4 byte) indices per vertex specified (or you will have to split your vertices into several VBOs, none of which has more than 65536 vertices, in order to use unsigned shorts for vertex indices). If you specify triangle primitives (mhagain’s suggestion), you will require three indices per triangle. If you specify triangle fan primitives, you will require one index per triangle plus two indices for the first triangle of each fan plus one index as special primitive restart index per triangle fan except for the first. Unless you specify mostly one triangle triangle fans, triangle fans require less memory than triangle primitives.

I don’t know Android OpenGL details, but if you need to save memory, and you are rendering things like buildings where you want sharp corners at corners rather than the appearance of round corners, then the geometry shader is what you should be looking at for surface normals. If Android supports geometry shaders, you can easily calculate normals on the fly rather than store them in memory. This not only saves a TREMENDOUS amount of memory, but it also prevents needing to rotate normals in the vertex shader and allows reusing vertices for triangles (otherwise, if you specify a normal per vertex you have to specify each vertex for each triangle – you can’t share vertices for triangles that aren’t coplanar, which also means every vertex for every triangle has to be transformed independently). (In the geometry shader, you will calculate normals based on vertices already in eye coordinates, so that the normals will be computed correctly oriented without any rotations necessary.)

Okay, so if I go ahead with your suggestion, how will I know where to insert the primitive restart vertices given an arbitrary vertex array of length [big]? I’m having a hard time working through it in my head - would it be as easy as running a loop and inserting the restart every six numbers (or every two vertices…6/3 for a coordinate) or is it more involved? A GML file, as I said, is formatted just like an XML file, with each “block” containing vertices for one side or aspect of a building. I think I would have to write my parser such that as it runs through the file, each time it reaches the end of a GML block, it inserts the restart coordinate.

I’m not familiar with geometry shaders…I’ll have to look those up. Will it completely eliminate my normal function?

You won’t modify the list of vertices at all, though ideally, you would recognize duplicate vertices and eliminate all but the first of each. What you need to do is build a second list, of indices to the vertices in the vertex list. Following the index to the last vertex of each triangle fan, you would place the special primitive restart index you define. You would need to build both lists simultaneously, so that you know what the structure of vertices forming the triangle fans is.

As it is, though, it sounds like your raw data duplicates every vertex for every side or aspect of a building. That is, if a building has two walls that meet in a corner, it sounds like each wall in your GML file specifies the same vertices for the common edge. In general, on average every point will be specified with more than two vertices. (Many points will be at the junction of three planes (two walls and a roof, for example).) You could reduce the size of your vertex buffer by about 2/3 by eliminating all duplicate vertices. Not only does that same lots of memory, it also saves lots of processing. However, you don’t need to start out trying to find and eliminate any duplicate vertices. You can first get your program working with duplicates, and then go back and worry about optimizing it. This particular optimization is isolated from the rest of your code provided that you calculate surface normals in a geometry shader.

Using the geometry shader won’t eliminate the normal function, it will move it from the CPU to the GPU. The unique thing about the geometry shader is that it is given all the vertices for each triangle. That is, it operates on triangles rather than on vertices or fragments. (The vertex shader only knows about each individual vertex in isolation, and the fragment shader doesn’t know about any triangle vertices.)

Knowing all the vertices of a triangle, all you need to do to calculate the planar surface normal is take the cross product of any two vectors emanating from any triangle corner. You should then normalize it. The same normal would be assigned to each of the three corners of the triangle that the geometry shader emits. That is, the geometry shader would pass through the input triangle vertices to the output and compute the surface normal from them on the fly. The GLSL geometry shader language works on vectors and provides builtin functions to calculate the cross product and to normalize the result. In other words, you can calculate the normalized surface normal in about one line of GLSL code in a geometry shader, like:

vec3  normal;

normal = normalize (cross (gl_in[1].gl_Position.xyz - gl_in[0].gl_Position.xyz, gl_in[2].gl_Position.xyz - gl_in[0].gl_Position.xyz));