Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Feature 3d graphics #1819

Merged
merged 98 commits into from
@NickHardeman

This PR adds 3d primitives to the OF core. Different dimensions, resolutions and texture coords can be specified. The following 3d primitives have been added/modified:

ofPlanePrimitive
ofSpherePrimitive
ofIcoSpherePrimitive
ofCylinderPrimitive
ofConePrimitive
ofBoxPrimitive

An example app that demonstrates the added functionality is in examples/gl/3DPrimitivesExample
https://github.com/NickHardeman/openFrameworks/tree/feature-3dGraphics/examples/gl/3DPrimitivesExample

The primitives all extend ofNode, so functionality like setPosition and setRotation are inherited.

Functions are available to change the dimensions / resolution of the created primitive. Though it does re-create the mesh.

sphere.setRadius( width );
plane.set( width*1.5, height*1.5 );

Also allows you to get a mesh directly via
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L300-L306

ofMesh ofGetPlaneMesh(float width, float height, int columns, int rows, ofPrimitiveMode mode=OF_PRIMITIVE_TRIANGLE_STRIP);
ofMesh ofGetSphereMesh(float radius, int res, ofPrimitiveMode mode=OF_PRIMITIVE_TRIANGLE_STRIP );
ofMesh ofGetIcosahedronMesh(float radius);
ofMesh ofGetIcoSphereMesh(float radius, int iterations);
ofMesh ofGetCylinderMesh( float radius, float height, int radiusSegments, int heightSegments, int numCapSegments=2, bool bCapped = true, ofPrimitiveMode mode=OF_PRIMITIVE_TRIANGLE_STRIP  );
ofMesh ofGetConeMesh( float radius, float height, int radiusSegments, int heightSegments, ofPrimitiveMode mode=OF_PRIMITIVE_TRIANGLE_STRIP );
ofMesh ofGetBoxMesh( float width, float height, float depth, int resX, int resY, int resZ );

The primitive provides normalized texture coords upon creation, but this can easily be changed with
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L57

void setTexCoords( float u1, float v1, float u2, float v2 ); 

or by just passing in an arb or non-arb texture
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L63

void setTexCoordsFromTexture( ofTexture& inTexture );

The base class also provides useful functions like:
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L78

ofVec3f getFaceNormal(of3dTriangle& tri);
vector<of3dTriangle> getUniqueTriangles();
vector<ofVec3f> getFaceNormals( bool perVetex=false);
void setFromTriangles( vector<of3dTriangle>& tris, bool bUseFaceNormal=false );
void smoothNormals( float angle );

A couple helper functions on each primitive include:
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L128

//ofPlanePrimitive
void resizeToTexture( ofTexture& inTexture, float scale=1.f );

https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L196-L205

// ofCylinderPrimitive
void setTopCapColor( ofColor color );
void setCylinderColor( ofColor color );
void setBottomCapColor( ofColor color );
vector<ofIndexType> getTopCapIndicies();
ofMesh getTopCapMesh();
vector<ofIndexType> getCylinderIndicies();
ofMesh getCylinderMesh();
vector<ofIndexType> getBottomCapIndicies();
ofMesh getBottomCapMesh();

https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L233-L239

// ofConePrimitve
void setTopColor( ofColor color );
void setCapColor( ofColor color );
vector<ofIndexType> getConeIndicies();
ofMesh getConeMesh();
vector<ofIndexType> getCapIndicies();
ofMesh getCapMesh();

https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.h#L274-L286

// ofBoxPrimitive
void setWidth( float a_width );
void setHeight( float a_height );
void setDepth( float a_depth );

void resizeToTexture( ofTexture& inTexture );

vector<ofIndexType> getSideIndicies( int sideIndex );
ofMesh getSideMesh( int sideIndex );

void setResolution( int res ); // same resolution for all sides //
void setResolution( int resX, int resY, int resZ );
void setMode( ofPrimitiveMode mode );
void setSideColor( int sideIndex, ofColor color );

This PR also adds of3dGraphics.h and .cpp. moving 3d primitive drawing like ofSphere and ofBox into separate files.
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/graphics/of3dGraphics.h
This allows for quick drawing and getting/setting of resolutions for cached meshes.

The GL and Cairo renderers also had to be modified to draw an ofPrimitiveBase accordingly
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/gl/ofGLRenderer.cpp#L131-L143

void ofGLRenderer::draw( ofPrimitiveBase& model, ofPolyRenderMode renderType  );

Note: I removed ofBox from the customDraw function inside ofNode since ofBox was conflicting with it, since the primitive extends ofNode.
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/ofNode.h#L141

NickHardeman added some commits
@NickHardeman NickHardeman added initial primitives, including plane, sphere and icosphere 60d9490
@NickHardeman NickHardeman caching of primitives, moved resolution out of renderer, added draw o…
…f3dModel to GL rederer
6560b6e
@NickHardeman NickHardeman removed drawing of sphere, added draw method with of3dModel as parameter e4fac12
@NickHardeman NickHardeman added draw with of3dModel as a parameter, but does not render anythin…
…g yet
972ae7e
@NickHardeman NickHardeman added draw method with of3dModel as a parameter, removed drawSphere f…
…unction and setSphereResolution
06be8ff
@NickHardeman NickHardeman added draw function with of3dModel as paramter, removed drawSphere fu…
…nction
becb3ec
@NickHardeman NickHardeman removed ofSetSphereResolution and 3d primitive functions. Moved them …
…to of3dGraphics
3829d48
@NickHardeman NickHardeman added normalize and apply tex coords, if the mesh is changed, then it…
… will apply saved tex coords for primitives. Added ofGetIcosahedron method
e6eaa0e
@NickHardeman NickHardeman changed textures and texcoords vector to map for easier setting. Chan…
…ged setTexCoords for index to automatically create tex coords if there were none.
9dc28fd
@NickHardeman NickHardeman added set tex coords from texture. Started cylinder generation code. a16a2b0
@NickHardeman NickHardeman added cylinder mesh and primitive code 27cd7c5
@NickHardeman NickHardeman added cylinder drawing code. Changed caching map to hold pointers of …
…objects, so that they instantiate correctly. Will this leak?
703fe18
@NickHardeman NickHardeman removed scaling to stay with ofNode, added enabling, disabling of nor…
…mals and textures.
b6f5235
@NickHardeman NickHardeman removed scaling since ofNode applies gl transform. only enable GL_NOR…
…MALIZE if model is scaled and it hasNormals enabled.
4df3d24
@NickHardeman NickHardeman fixed scaling of normals in drawNormals function 8ad769c
@NickHardeman NickHardeman added cone primitive and mesh, added mode to plane mesh and primitive ff85308
@NickHardeman NickHardeman added ofBoxPrimitive a26bfad
@NickHardeman NickHardeman added drawing of cone and box 5c07928
@NickHardeman NickHardeman moved setResolution functionality to set function b4fe67f
@NickHardeman NickHardeman added getConeMesh to support triangles 4aee716
@NickHardeman NickHardeman improved get mesh code for cone, added normals and triangle mode 4c92f2d
@NickHardeman NickHardeman removed default colors for ofBoxPrimitive and added colors if setting…
… face color
8e8a43f
@NickHardeman NickHardeman added getTopMesh, getTopIndicies, setTopColor, getCapMesh, getCapIndi…
…cies, setCapColor
fe4d64a
@NickHardeman NickHardeman renamed getFace to getSide to avoid confusion later implementing getF…
…ace functionality to return a triangle, used of3dModel functions in place of writing them out
b51cdb9
@NickHardeman NickHardeman added triangles to cylinder, added setColor, added getMeshes ed80227
@NickHardeman NickHardeman fixed normal for bottom of cylinder 27a746e
@NickHardeman NickHardeman included 3dGraphics in header 90e9d9f
@NickHardeman NickHardeman removed ofBox due to class def errors with ofBox 1ff6894
@NickHardeman NickHardeman added include for 3dGraphics 792d586
@NickHardeman NickHardeman added enable and disable colors in of3dModel cdbde04
@NickHardeman NickHardeman replaced bourke code with more readable code, added triangle strip to…
… sphere, added ogre permission notice
a0e551a
@NickHardeman NickHardeman Adding of3dTriangle 2f40b11
@NickHardeman NickHardeman changed to ofPrimitiveBase 17ac2c9
@NickHardeman NickHardeman added of3dPrimitiveBase 9e9d427
@NickHardeman NickHardeman adding appropriate .cpp and .h files 447b236
@NickHardeman NickHardeman Merge branch 'master' of https://github.com/openframeworks/openFramew…
…orks into 3DGraphics
0e8f3aa
@NickHardeman NickHardeman cleanup comments 4175849
@NickHardeman NickHardeman adding 3dPrimitives Example 7e6d8d0
@NickHardeman NickHardeman Removing Unnecessary images in example 81d2470
@NickHardeman NickHardeman Merge branch 'develop' of https://github.com/openframeworks/openFrame…
…works into 3DGraphics
e9bb475
@arturoc
Owner

wow! looks great, the texture coords scaling seems super useful

some things that i see:

  • of3dPrimitiveType doesn't seem to be used anywhere?

  • what are?
    ofVec4f* getTexCoordsPtr();
    ofVec4f& getTexCoords();
    seem to be related to the scaling of the texcoords but the naming is not very clear, seem like they returned the vector of texture coordinates but just return an ofVec4f

  • perhaps of3dTriangle could be something like ofMeshFace? and instead of using arrays have vectors, that way we could have support for quad faces at some point.

  • ofPrimitiveBase::getFaceNormal is probably better as ofMeshFace::getNormal and then have the faceNormal property in ofMeshFace private (calculated if it's not set yet)

  • ofPrimitiveBase::getUniqueTriangles()/getUniqueFaces() (even if by now we only support triangles) could be moved to ofMesh to make it more generic since it doesn't seem to have any dependency with the particulars of ofPrimitiveBase but only with ofMesh

  • same for any other method that could be moved to ofMesh, which seem most in ofPrimitiveBase, i see ofPrimitiveBase as a wrapper for an ofNode + an ofMesh but most of the functionality seems like could be moved to ofMesh

  • not sure about this one, but probably it makes more sense to have the specific primitives as meshes? then being able to pass that mesh into a primitive? like:

ofPlaneMesh
ofCylinderMesh
ofSphereMesh
...

or

ofMesh::Plane
ofMesh::Cilinder
ofMesh::Sphere

as @elliotwoods suggested sometime ago.

then ofPrimitiveBase would take a pointer to a mesh and the corresponding specialized primitives would create the mesh from the corresponding specialized mesh. but perhaps it makes sense that a primitive always has an associated ofNode?

  • there's a couple of typos in all the classes: indicies should be indices and verticies should be vertices

  • the class variables have all names like _width, _indices, we usually don't do that but instead width, indices and then in the functions parameters we use the _ prefixed version if needed

  • what is:

    int _strides[3][2];
    int _verticies[3][2];

in ofCylinderPrimitive? it seems something related to the caps perhaps it could be renamed of even used as simple variables or 1d arrays to make it clearer?

@bilderbuchi
Owner

for reference, this is work towards #530

@NickHardeman

HI Arturo,

Thanks for the feedback!

of3dPrimitiveType doesn't seem to be used anywhere?
of3dPrimitiveType is used in this file for caching meshes
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/graphics/of3dGraphics.cpp#L15-L69

The idea was to apply it to a primitive, so that it can be identified later, like all stored as pointers in a vector.

The ofGetTexCoords functions return the tex coords (x,y,width, height)

perhaps of3dTriangle could be something like ofMeshFace? and instead of using arrays have vectors, that way we could have support for quad faces at some point.
I used of3dTriangle instead of face because all meshes can be made of triangles, but not all filled meshes can be made from Quads? : /
Would a ofMeshFace be composed of two triangles?
The reason I named it getFaceNormal instead of normal is because it is the normal of the 3 vertices that make up the face and not the normal of each vertex and I thought it was clearer.

I agree that a good amount of the functionality in ofPrimitiveBase could be moved to ofMesh, with only triangles being supported. I originally had support for multiple meshes and that's why I put it in ofPrimitiveBase. But functions like the texture coords might get tricky, as it always starts with normalized texcoords.

I think if all of the mesh generation code was moved into ofMesh, then ofMesh might be a bit hefty. But when passed to a primitive base, the primitive base needs to know which type of primitive it is, so, of3dPrimitiveType could also be passed as a param.
The current functions ofGetPlaneMesh,ofGetSphereMesh .. etc do return a mesh.

RIght now there is the ability to get the mesh with ofGetPlaneMesh...etc. And that can be a primitive. Extending the mesh functionality with functions specific to a primitive, like getSide() for a box seems really beneficial by using ofBoxPrimitive.

there's a couple of typos in all the classes: indicies should be indices and verticies should be vertices
I have been misspelling this for quite some time now. I can update it.

the class variables have all names like _width, _indices, we usually don't do that but instead width, indices and then in the functions parameters we use the _ prefixed version if needed
Ah yes, old habits. That's how I name private members. I can fix it.

what is:
int _strides[3][2];
int _verticies[3][2];

Those are arrays to store the sides of certain primitives like ofCylinder for later use like retrieving sides or settings colors of the bottom or top cap.
https://github.com/NickHardeman/openFrameworks/blob/feature-3dGraphics/libs/openFrameworks/3d/of3dPrimitives.cpp#L1569-L1641
Not sure what else it could be named too. 2d arrays were the easiest for me to understand, not sure a 1d would help. I think it would confuse me. :)

@arturoc
Owner

The reason I named it getFaceNormal instead of normal is because it is the normal of the 3 vertices that make up the face and not the normal of each vertex and I thought it was clearer.

what i meant is that, that method could be inside of3dTriangle/ofMeshFace since it's only touching data from that class

in general a good OO practice is that data in a class should be accessed/modified by it's methods so if a method only uses data from a different class is probably a signal that it should be in that second class. i would change of3dTriangle to be something like:

class ofMeshFace {
public:
    of3ofMeshFace() {
        bHasNormals = bHasColors = bHasTexcoords = false;
    }


    void setHasColors( bool bColors ) {bHasColors = bColors; }
    void setHasNormals( bool bNormals ) {bHasNormals = bNormals; }
    void setHasTexcoords( bool bTexcoords ) {bHasTexcoords = bTexcoords; }

    bool hasColors() { return bHasColors; }
    bool hasNormals() {return bHasNormals; }
    bool hasTexcoords() { return bHasTexcoords; }

    ofVec3f getFaceNormal(){
         ofVec3f U, V, n;

         U = (points[1]-points[0]);
         V = (points[2]-points[0]);

         n = U.crossed(V);
         n.normalize();
         return n;
    }
    ...

    vector<ofPoint> points;
    vector<ofVec3f> normals;
    vector<ofFloatColor> colors;
    vector<ofVec2f> texcoords;
protected:
    bool bHasNormals, bHasColors, bHasTexcoords;

    ofVec3f faceNormal;
};

probably keeping the result in faceNormal and returning it in case the points haven't changed for which you would need to make the points and probably all the other variables private and add getters and setters

same reason for moving most of the methods from ofPrimitiveBase to ofMesh, if those methods are only using data from ofMesh it seems logical and useful that they should be there.

@NickHardeman

Yeah that makes sense to move the function inside the class.

I like of3dTriangle because it is more clear about what it is, but it is confusing with ofTriangle, since ofTriangle is a draw call. For ofMeshFace, there should probably be some way of identifying what type it is.
For example,
type = OF_PRIMITIVE_MODE_TRIANGLES. But there isn't a primitive mode for quads since the drawing isn't supported in iOS?

I won't be able to get to the other issues until Tuesday. I have only tested on OSX 10.8. But the features use ofMesh for rendering, so it should be ok on other platforms. But nothing is as good as testing! I have an iOS device and can test more when I get back on Tuesday.

@ofTheo
Owner
@arturoc
Owner

i don't think we need a type for the face we can just check the number of vertices

@kylemcdonald

just wanted to say 1. this is awesome, i just browsed your work quickly and ran the example. so good. and 2. from what i saw, i agree with arturo's critiques, with the exception of being so open to quad meshes. i think once you start to generalize the kind of faces in meshes, the code gets unnecessarily complicated, and it's rare to have situations where you need quads that can't be emulated with triangles. i think hardware will continue to move away from quads, and trying to write hyper-general code that allows for them might be an unnecessary early optimization.

@arturoc
Owner

thanks kyle, about the triangle faces, i wasn't thinking about rendering but about working with some algorithms that might rely on quad meshes. i guess is something we probably won't need but in any case the name of3dTriangle sounds confusing to me, i would rename it to something like ofTriangleFace or ofMeshFace: we asume ofMesh is always tri so ofMeshFace is also tri, if at some point we want quad faces we could have ofQuadMesh/ofQuadMeshFace

@kylemcdonald

ah ok -- i agree that ofMeshFace is more fitting, and will always be a triangle, but it's not important to call it a triangle because it's mostly a mesh face :)

@elliotwoods
Collaborator

wow. looks incredible.
i also think quad vectors, etc is more trouble than it's worth
excited to see this stuff making it into oF finally!

@NickHardeman
@NickHardeman

I think I got all of the features / issues resolved. I tried to make my commits descriptive. :)
Take a look and let me know what you think.

@ofTheo
Owner

Looks good to me - +1 to merge.
@arturoc can you give this a quick look over.

@arturoc
Owner

looks really good

  • i would move:

setColorForIndices
getMeshForIndexes

to ofMesh, and that last one should be getMeshForIndices for consistency

  • also: setTexCoords setTexCoodsFromTexture

could be moved to ofMesh? i don't understand fully what they do, but is perhaps the name slightly confusing? it's the same as the one used to actually set the texture coordinates while this seems to be remapping them? perhaps mapTexCoords/remapTexCoords?

  • drawNormals is using immediate mode which won't compile on gles, if you could port it to not use glBegin/glEnd, you can just use ofBeginShape/ofEndShape or even another mesh if you want to cache the results for speed

  • could we have a flag to use an ofVboMesh / ofMesh i think the default should be ofVboMesh with an option to use ofMesh, we can just have both in ofPrimitiveBase and use one of them by looking a flag

apart from that it looks great to me

libs/openFrameworks/gl/ofGLRenderer.cpp
@@ -127,6 +128,21 @@ void ofGLRenderer::draw(ofMesh & vertexData, ofPolyRenderMode renderType, bool u
}
//----------------------------------------------------------
+void ofGLRenderer::draw( ofPrimitiveBase& model, ofPolyRenderMode renderType ) {
+
+ if(model.hasScaling() && model.hasNormalsEnabled()) {
+ glEnable( GL_NORMALIZE );
+ }
+
+ draw( model.getMesh(), renderType, model.getMesh().usingColors(), model.getMesh().usingTextures(), model.getMesh().usingNormals() );
+
+ if(model.hasScaling() && model.hasNormalsEnabled()) {
+ glDisable( GL_NORMALIZE );
@arturoc Owner
arturoc added a note

also we shouldn't assume that GL_NORMALIZE is always disabled, if someone enables it and tries to draw a primitive it'll get disabled. by now we can use glGet...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@damiannz

WRT quads, i can imagine some way of indexing quads or n-gons (as an extra layer in additional to triangles) could be useful for things like subdivision surfaces or making nice tetrahedral meshes.

@NickHardeman

I moved setColorForIndices and getMeshForIndices to ofMesh.

I renamed setTexCoords and setTexCoodsFromTexture to mapTexCoords and mapTexCoordsFromTexture inside ofPrimitiveBase.
I don't think this can be moved to ofMesh, since it relies on having the texture coordinates initially. If it doesn't have them mapped, then these functions don't know how to remap them.

drawNormals is using immediate mode which won't compile on gles.
I fixed this just by using a temporary mesh, since this is mostly for debug purposes.

could we have a flag to use an ofVboMesh / ofMesh i think the default should be ofVboMesh with an option to use ofMesh.
I switched ofMesh to ofVboMesh to test and there are several issues when changing / altering primitive. I think if a user wants to utilize a vbo, they can create the primitive how they would like by using the primitive base to manipulate the mesh and then use getMesh() to set it to a vbo mesh.

I added a check to see if GL_NORMALIZE was enabled and leave it enabled if it was prior to the draw function call.

I added the appropriate files to the OF + iPhone static lib and also added an example for iOS. I tested it on the iPad 2 and it runs without a hitch.

I have an android, but have never had the time to successfully get OF running on it, so I haven't been able to test.

@arturoc
Owner

great, what problems are you having with vboMesh? in principle it should behave the same as an ofMesh, is it a bug in ofVboMesh or some limitation that we could fix?

@NickHardeman

This is how the models look with ofMesh
Screen Shot 2013-01-30 at 11 57 27 PM

But changing ofMesh to ofVboMesh results in this lighting. Could be that the normals aren't getting updated?
Screen Shot 2013-01-30 at 11 57 08 PM

@damiannz

great code but OW MY EYES, can you please change the lighting colours so they're not 'programmer colours'? try a desaturated red/orange for the key light and a super light blue for the backlight.

@arturoc
Owner

Nick, i'm fixing the vbo problem and saw that there's this set/getResolution methods in the base class that seem kind of confusing to me. From what i see the meaning of the resolution is totally different in each derived class so having it in the base class doesn't seem useful unless you know which primitive you are using.

To put an example, something like this would be useful if we had a collection of primitives in a vector for example and we could go through them setting the same value and that will give similar results for each primitive but as it is the values change a lot.

It also makes the implementation harder to understand since in every primitive resolution.x,y,z mean totally different things and most of them doesn't even use the 3 values, it could even be that in the future some primitive needed more than 3. so i think it would be better to just remove those methods and variable from the base class and just add specifc methods and variables for each class? like:

sphere.setResolution(res)

plane.setColums()
plane.setRows()

or/and

plane.setResolution(columns,rows)

cylinder.setResolution(horizontalSegments,verticalSegments,capSegments)
...

sorry for being such a pain in the ass :) but there's lot's of code here, a new API... so better to get things right

will send you a PR with the fix for the vboMesh in a bit

@NickHardeman

Yeah that makes sense for readability. I can go through and make it more understandable.

@arturoc
Owner

thanks!

@NickHardeman

Thanks for all of the vbo updates, I just tested it and it works great.

@NickHardeman

Hi Arturo,

I made some functions that are more clear for each primitive for setting the resolutions. Like for plane, setColumns and setRows, etc...

I noticed that for ofMeshFace there are vectors instead of arrays. This change slowed my app from running at ~120fps down to ~32fps. I switched them back to arrays and ran it through a leak checker and watched activity monitor, with no red flags.

Is there a reason that it can't be arrays? It has a significant speed advantage.

@arturoc
Owner

the problem with those arrays is that when you try to copy an ofMeshFace you don't get a copy of those arrays but a reference to the original so if the original is deleted the arrays in the copies will be pointing to invalid memory, for example this function in ofMesh:

vector<ofMeshFace> ofMesh::getUniqueFaces()

is returning a vector of ofMeshFace by copy, the ofMeshFace is created internally an put in a vector but as soon as that function returns the memory of those arrays isn't valid anymore so the returned vector will have variables pointing to invalid memory. this can even work and it's indeed going to be faster than using vectors because you are avoiding lots of copies but the memory is not valid and at some point if you keep that vector around long enough you'll get a crash

there's some code in the new methods in ofMesh that can be optimized, for example that method, getUniqueFaces, is returning a copy which is slow, if it's not used very often it can be ok but if it's something that will be used a lot it'll slow down things a lot. one solution would be to create a cache internally and return it by reference, another to change the method to something like:

void ofMesh::getUniqueFaces(vector<ofMeshFace> & faces)

where you pass the vector from outside reducing the number of allocations if you create that vector only once and reuse it. or even a combination of the 2, where ofMesh has a cache that it's updated only if the vertices, colors... change and it's copied to a vector passed from outside.

also when constructing that vector you are using push_back which is also slow, something that will make it faster is to resize it before the loop to the number of indices and then access the faces in the vector directly avoiding lot's of copies and movements in memory:

triangles.resize(indices.size()/3);
for(int j = 0; j < indices.size(); j += 3) {
    ofMeshFace & tri = triangles[j];
    for(int k = 0; k < 3; k++) {
        index = indices[j+k];
        tri.setVertex( k, verts[index] );
        if(bHasNormals)
            tri.normals[k] = normals[index];
        if(bHasTexcoords)
            tri.texCoords[k] = tcoords[index];
        if(bHasColors)
            tri.colors[k] = colors[index];
        tri.setHasColors(bHasColors);
        tri.setHasNormals(bHasNormals);
        tri.setHasTexcoords(bHasTexcoords);
        // only calculate the normal after all vertices have been set //
        if(k == 2) tri.calculateFaceNormal();
    }
}

that alone should speed up things a lot in that method. there's some other similar stuff in the rest of the new methods that can be optimized, i can take a look later

@arturoc
Owner

btw, the dirty flag for the normal face is a good idea but then the vertices, need to be private if not you can change them by accesing the data directly instead of through the set method and the flag won't be updated. i thouhgt of adding something similar but then the structure of that class gets way more complicated since you need get/sets for every collection.... but if it'll make things faster/cleaner lets do it

@arturoc
Owner

ignore that, i was totally convinced that this:

float data[3]

will behave as:

float * data;

when copying but it actually makes a full copy of all the data, so leave it as arrays if it's faster

@kalwalt

awesome works @NickHardeman ! when it will be merged on the develop branch?

@ofTheo
Owner

great - so is this ready to go?

@arturoc
Owner

i think it's ready in terms of implementation, just a couple of things:

  • ofPrimitiveBase -> of3dPrimitive? the rest of of*Base are usually abstract classes not meant to be used outside the core but this is actually someone might want to use by itself

  • ofCylinder/ofCone/ofBox... should be ofDrawCylinder/ofDrawCone/ofDrawBox... as we spoke yesterday in the dev meeting, i think better to add them with the new syntax instead of having to deprecate the old one in a couple of weeks

  • Nick, can you rename the variables from _mesh to mesh? we don't use the _ for members in the core

Also i've just sent you a PR with some minor optimizations, apart from that i think it's ready

@NickHardeman NickHardeman Merge pull request #3 from arturoc/3dprimitives
3dprimitives optimizations + fix warnings
1562805
@NickHardeman

Hi Arturo,

I can change it to of3dPrimitive and make the other changes.
I merged your pull request. Thanks :)

@arturoc
Owner

thanks!

@arturoc
Owner

btw, ofBox was there before so probably we will need to keep both versions, ofBox and ofDrawBox

@NickHardeman

Ok, I can make both versions, since ofSphere and ofCone were also in there.

@NickHardeman

Hi Arturo,

I made all of the changes mentioned above and added some OF_DEPRECATED_MSG warnings for ofBox, ofSphere and ofCone. Changing them to ofDrawPlane, ofDrawSphere , etc.

libs/openFrameworks/3d/of3dPrimitives.cpp
((398 lines not shown))
+}
+
+//----------------------------------------------------------
+void ofSpherePrimitive::setResolution( int res ) {
+ _resolution = res;
+ ofPrimitiveMode mode = OF_PRIMITIVE_TRIANGLE_STRIP;
+
+ mode = getMesh().getMode();
+
+ set(getRadius(), getResolution(), mode );
+}
+
+//----------------------------------------------------------
+void ofSpherePrimitive::setMode( ofPrimitiveMode mode ) {
+ ofPrimitiveMode currMode = OF_PRIMITIVE_TRIANGLE_STRIP;
+ mode = getMesh().getMode();
@arturoc Owner
arturoc added a note

i was doing some optimizations and noticed this: as it is, it's always setting the currentMode to triangle strip, removing it makes the sphere in the example go crazy because before it was always changing the mode and recreating the mesh each frame so the strength was applied always to the original sphere. i've fixed it by lowering the strength- just so you know why the mods in the example. going to send you a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@arturoc
Owner

@ofTheo so i think with those last changes it is ready, even with that i'd wait till 0.7.4 is out and then merge into develop so people can test it for a release cycle before it's official. i think this is big enough to wait a little before releasing it, mostly if we are going to release 074 in a few days. but as you prefer

@ofTheo
Owner

Thanks @arturoc - I'll take a good look over it tomorrow during the bugfix drive. Will it also test it on Win CB and VS.
It might make sense to wait till after 0.7.4 - though would love to have this in for obvious reasons :)

@bilderbuchi
Owner

I just tested the 3DPrimitivesExample on Ubuntu and it looks good.
I couldn't see any problems, except that when you switch from mode 3 to mode 0, the spheres don't reset back to "normal", but keep the exploded geo. probably just a glitch in the logic.

arturoc and others added some commits
@arturoc arturoc Merge branch 'develop' into 3dprimitives
Conflicts:
	libs/openFrameworks/gl/ofGLRenderer.cpp
788ea36
@NickHardeman NickHardeman Merge pull request #5 from arturoc/3dprimitives
merge develop to fix merging
9536a03
@arturoc arturoc Merge branch 'develop' into 3dprimitives
Conflicts:
	libs/openFrameworks/3d/ofMesh.h
	libs/openFrameworks/types/ofBaseTypes.h
	libs/openFrameworksCompiled/project/osx/openFrameworksLib.xcodeproj/project.pbxproj
f938329
@arturoc arturoc ofBaseTypes: fixes for 3dprimitives, broke in previous merges 604b914
@NickHardeman NickHardeman Merge pull request #6 from arturoc/3dprimitives
merge develop, again
0ad93b6
@arturoc arturoc merged commit 555543c into from
@arturoc
Owner

yay! thanks!

@NickHardeman
@bilderbuchi
Owner

did you guys fix this? I guess not, judging from the commit log.

@arturoc
Owner

@bilderbuchi actually no, but i think it's not really important and it will require to keep a copy of the original mesh making the code more complicated so i think it's better as it is even if it might look weird when you run the example

@quadbyte

I'm really enjoying the new primitives; however I miss the ability to draw a wireframe box without the faces splited in triangles. Do you guys plan to bring this back ?

@tgfrerer
Collaborator

hey @quadbyte - this is unlikely to happen. the reason is less openFrameworks than openGL. From OpenGL 3.0 onwards, quads (drawing rectangles on the GPU) have been deprecated on favour of triangles. If you wanted to draw wireframe rectangles without inner diagonal you'd have to use glLine(s) and draw individual lines.

@elliotwoods
Collaborator

in my opinion

ofNoFill();
ofSetLineWidth(1.0f);
ofBox(..);

Should draw a box without diagonals, whilst ofBoxPrimitive should just display its triangles without any special treatment.

@arturoc
Owner

probably we should add a flag or one more primitive (will be faster) that draws as lines instead of triangles.

@quadbyte

Wireframe/Lines primitives would be nice, not only for boxes, but also for cylinder, cone, etc...

@cguotju

I'm drawing Wireframes on ios, but some lines are missing. (iPad 3rd, ios 7)

@admsyn
Collaborator

@ginrain can you open a new issue, and include a screenshot of what you see?

@julapy
Collaborator

this issue was already logged #1744 and #1766

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 21, 2012
  1. @NickHardeman
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman

    added draw method with of3dModel as a parameter, removed drawSphere f…

    NickHardeman authored
    …unction and setSphereResolution
  6. @NickHardeman
  7. @NickHardeman
  8. @NickHardeman

    added normalize and apply tex coords, if the mesh is changed, then it…

    NickHardeman authored
    … will apply saved tex coords for primitives. Added ofGetIcosahedron method
  9. @NickHardeman

    changed textures and texcoords vector to map for easier setting. Chan…

    NickHardeman authored
    …ged setTexCoords for index to automatically create tex coords if there were none.
Commits on Sep 22, 2012
  1. @NickHardeman
Commits on Sep 24, 2012
  1. @NickHardeman
  2. @NickHardeman

    added cylinder drawing code. Changed caching map to hold pointers of …

    NickHardeman authored
    …objects, so that they instantiate correctly. Will this leak?
  3. @NickHardeman
  4. @NickHardeman

    removed scaling since ofNode applies gl transform. only enable GL_NOR…

    NickHardeman authored
    …MALIZE if model is scaled and it hasNormals enabled.
Commits on Sep 25, 2012
  1. @NickHardeman
Commits on Sep 30, 2012
  1. @NickHardeman
Commits on Oct 1, 2012
  1. @NickHardeman

    added ofBoxPrimitive

    NickHardeman authored
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman
  7. @NickHardeman
  8. @NickHardeman

    renamed getFace to getSide to avoid confusion later implementing getF…

    NickHardeman authored
    …ace functionality to return a triangle, used of3dModel functions in place of writing them out
  9. @NickHardeman
  10. @NickHardeman
  11. @NickHardeman
  12. @NickHardeman
  13. @NickHardeman
  14. @NickHardeman
Commits on Oct 2, 2012
  1. @NickHardeman

    replaced bourke code with more readable code, added triangle strip to…

    NickHardeman authored
    … sphere, added ogre permission notice
Commits on Jan 18, 2013
  1. @NickHardeman

    Adding of3dTriangle

    NickHardeman authored
  2. @NickHardeman
  3. @NickHardeman

    added of3dPrimitiveBase

    NickHardeman authored
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman

    cleanup comments

    NickHardeman authored
  7. @NickHardeman
  8. @NickHardeman
  9. @NickHardeman
  10. @NickHardeman
  11. @NickHardeman

    Updated example to rebuild necessary primitives if they are not in th…

    NickHardeman authored
    …e supported mode for pulling out their meshes
  12. @NickHardeman
  13. @NickHardeman
Commits on Jan 26, 2013
  1. @NickHardeman
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman
  7. @NickHardeman
Commits on Jan 30, 2013
  1. @NickHardeman
  2. @NickHardeman

    vertical sync

    NickHardeman authored
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman

    update function calls

    NickHardeman authored
  7. @NickHardeman
  8. @NickHardeman
  9. @NickHardeman

    Check for GL_NORMALIZE

    NickHardeman authored
  10. @NickHardeman
Commits on Jan 31, 2013
  1. @arturoc
  2. @NickHardeman

    Merge pull request #1 from arturoc/3dprimitives

    NickHardeman authored
    of3dPrimitive: fix vboMesh rendering + make it optional and default
  3. @NickHardeman
  4. @arturoc
  5. @NickHardeman

    Merge pull request #2 from arturoc/3dprimitives

    NickHardeman authored
    ofMeshFace: arrays -> vectors to avoid memory problems + face normal as method
Commits on Feb 2, 2013
  1. @NickHardeman
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman
  7. @NickHardeman
  8. @NickHardeman
  9. @NickHardeman
  10. @NickHardeman
Commits on Feb 3, 2013
  1. @NickHardeman

    clean up comments

    NickHardeman authored
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @NickHardeman
Commits on Feb 4, 2013
  1. @arturoc
  2. @NickHardeman

    Merge pull request #3 from arturoc/3dprimitives

    NickHardeman authored
    3dprimitives optimizations + fix warnings
Commits on Feb 5, 2013
  1. @NickHardeman
  2. @NickHardeman
  3. @NickHardeman
  4. @NickHardeman
  5. @NickHardeman
  6. @arturoc
  7. @arturoc
Commits on Feb 8, 2013
  1. @NickHardeman

    Merge pull request #4 from arturoc/3dprimitives

    NickHardeman authored
    3dprimitives
  2. @NickHardeman

    fix to iOS example

    NickHardeman authored
Commits on Mar 6, 2013
  1. @arturoc

    Merge branch 'develop' into 3dprimitives

    arturoc authored
    Conflicts:
    	libs/openFrameworks/gl/ofGLRenderer.cpp
  2. @NickHardeman

    Merge pull request #5 from arturoc/3dprimitives

    NickHardeman authored
    merge develop to fix merging
Commits on Mar 7, 2013
  1. @arturoc

    Merge branch 'develop' into 3dprimitives

    arturoc authored
    Conflicts:
    	libs/openFrameworks/3d/ofMesh.h
    	libs/openFrameworks/types/ofBaseTypes.h
    	libs/openFrameworksCompiled/project/osx/openFrameworksLib.xcodeproj/project.pbxproj
  2. @arturoc
  3. @NickHardeman

    Merge pull request #6 from arturoc/3dprimitives

    NickHardeman authored
    merge develop, again
Something went wrong with that request. Please try again.