Hello Michael,
On 11/15/2011 01:07 PM, Michael Raab wrote:
I've tried both patches. These are the results:
thanks for testing!
1. osg::calcFaceNormals(geo) is still buggy. Indices size is still
increasing
in general that can not be avoided. It is possible that a position is
part of multiple faces, but for each a different normal must be used.
This can only be achieved by having a separate index for the normal (or
duplicating positions/other vertex attribs).
What the code did not get right (and the patch attempted to fix) is that
if there already was a separate index for normals it was not reused,
this happens now.
and result normals are still wrong.
yes, I see that now. The problem is specific to strip/fan primitives
where, for the first face of the primitive, the normal was only set on
the last vertex. I've fixed that in the attached patch.
Please note that for non-planar fans/strips it's not really possible to
have face normals, it would require splitting the fan/strip primitive
into independent triangles/quads.
Calling it twice now destroys the geometry :)
hmm, I don't see that here, pushing the button sequence 'a', '7', '3',
'3', '3', ... produces only a change after the first '3' afterwards the
rendering and log output remain the same. Can you give more details on
how the geometry gets destroyed in your case?
2. osg::calcVertexTangents(geo) seems to work fine now
ok, thanks.
3. calling calcVertexNormals(geo, angle) first, then
calcVertexNormal(geo), then calcVertexNormals(geo, angle) again
and so on also increases the indices size
I suspect this is a similar issue with not reusing the normal index, but
instead just inserting a new one.
to make debugging easier ,please find attached our test
application./(See attached file: main.cpp)/
great, thank you for sending this!
Cheers,
Carsten
Index: Source/System/NodeCores/Drawables/Geometry/OSGGeoFunctions.cpp
===================================================================
RCS file: /cvsroot/opensg/OpenSG/Source/System/NodeCores/Drawables/Geometry/OSGGeoFunctions.cpp,v
retrieving revision 1.75
diff -u -p -r1.75 OSGGeoFunctions.cpp
--- Source/System/NodeCores/Drawables/Geometry/OSGGeoFunctions.cpp 12 Oct 2011 08:57:24 -0000 1.75
+++ Source/System/NodeCores/Drawables/Geometry/OSGGeoFunctions.cpp 16 Nov 2011 00:10:47 -0000
@@ -754,8 +754,8 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcV
OSG_SYSTEMLIB_DLLMAPPING void OSG::calcFaceNormals(GeometryPtr geo)
{
- if(geo->getPositions() == NullFC ||
- geo->getPositions()->size() == 0)
+ if(geo->getPositions() == NullFC ||
+ geo->getPositions()->size() == 0 )
{
FINFO(("Geo without positions in calcFaceNormals()\n"));
return;
@@ -765,7 +765,6 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
GeoNormalsPtr newNormals = GeoNormals3f::create();
Vec3f normal;
- FaceIterator faceIter = geo->beginFaces();
GeoIndicesPtr oldIndex = geo->getIndices();
if(oldIndex != NullFC)
@@ -774,10 +773,32 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
if(geo->getMFIndexMapping()->size() > 0)
{
//MULTI INDEXED
- beginEditCP(newIndex);
+ MFUInt16 &oldIndexMap = *geo->editMFIndexMapping();
+ UInt32 oldIMSize = oldIndexMap.size();
+ UInt32 newIMSize = oldIndexMap.size();
+ Int16 oldNIdx = geo->calcMappingIndex(Geometry::MapNormal);
+ Int16 newNIdx = -1;
+
+ if(oldNIdx < 0)
+ {
+ // no pre-existing normals, add new index for normals
+ newNIdx = oldIndexMap.size();
+ newIMSize = oldIMSize + 1;
+ }
+ else if(geo->getIndexMapping(oldNIdx) != Geometry::MapNormal)
+ {
+ // index for normal also indexes other property,
+ // add new index only for normals
+ newNIdx = oldIndexMap.size();
+ newIMSize = oldIMSize + 1;
+ }
+ else
+ {
+ // normals indexed with unique index, reuse it
+ newNIdx = oldNIdx;
+ }
- MFUInt16 &oldIndexMap = (*geo->editMFIndexMapping());
- UInt32 oldIMSize = oldIndexMap.size();
+ beginEditCP(newIndex);
for(UInt32 i = 0; i < oldIndex->getSize() / oldIMSize; ++i)
{
for(UInt32 k = 0; k < oldIMSize; ++k)
@@ -785,13 +806,26 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
newIndex->push_back(oldIndex->getValue(i * oldIMSize + k));
}
- newIndex->push_back(0); //placeholder for normal index
+ if(newIMSize != oldIMSize)
+ {
+ // placeholder for new normal
+ newIndex->push_back(0);
+ }
}
+ Int32 primIdx = -1;
+ FaceIterator faceIter = geo->beginFaces();
+ FaceIterator faceEnd = geo->endFaces ();
+
beginEditCP(newNormals);
- for(UInt32 faceCnt = 0; faceIter != geo->endFaces();
- ++faceIter, ++faceCnt)
+ for(UInt32 faceCnt = 0; faceIter != faceEnd; ++faceIter, ++faceCnt)
{
+ // for strip/fan primitives we need to set the normal on all
+ // vertices of the first face, but on subsequent faces only
+ // on the new vertices - see below where this is used
+ bool primStart = (faceIter.PrimitiveIterator::getIndex() > primIdx);
+ primIdx = faceIter.PrimitiveIterator::getIndex();
+
if(faceIter.getLength() == 3)
{
//Face is a Triangle
@@ -829,23 +863,52 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
switch(faceIter.getType())
{
case GL_TRIANGLE_STRIP:
- base = faceIter.getIndexIndex(2); //get last point's position in index field
- newIndex->setValue(faceCnt, base + (base / oldIMSize) + oldIMSize);
+ if(primStart)
+ {
+ base = faceIter.getIndexIndex(0);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+
+ base = faceIter.getIndexIndex(1);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+ }
+
+ base = faceIter.getIndexIndex(2);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
break;
case GL_TRIANGLE_FAN:
- base = faceIter.getIndexIndex(2); //get last point's position in index field
- newIndex->setValue(faceCnt, base + (base / oldIMSize) + oldIMSize);
+ if(primStart)
+ {
+ base = faceIter.getIndexIndex(0);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+
+ base = faceIter.getIndexIndex(1);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+ }
+
+ base = faceIter.getIndexIndex(2);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
break;
case GL_QUAD_STRIP:
- base = faceIter.getIndexIndex(3); //get last point's position in index field
- newIndex->setValue(faceCnt, base + (base / oldIMSize) + oldIMSize);
+ if(primStart)
+ {
+ base = faceIter.getIndexIndex(0);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+
+ base = faceIter.getIndexIndex(1);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+ }
+
+ base = faceIter.getIndexIndex(2);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
+
+ base = faceIter.getIndexIndex(3);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
break;
default:
for(UInt32 i = 0; i < faceIter.getLength(); ++i)
{
base = faceIter.getIndexIndex(i);
- newIndex->setValue(faceCnt,
- base + (base / oldIMSize) + oldIMSize);
+ newIndex->setValue(faceCnt, (base / oldIMSize) * newIMSize + newNIdx);
}
break;
}
@@ -855,17 +918,22 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
endEditCP(newIndex);
beginEditCP(geo);
+ geo->setNormals(newNormals);
+ geo->setIndices(newIndex);
- Int16 ni;
- ni = geo->calcMappingIndex(Geometry::MapNormal);
- if(ni != -1)
+ if(oldNIdx < 0)
+ {
+ // no pre-existing normals, add new mapping
+ oldIndexMap.push_back(Geometry::MapNormal);
+ }
+ else if(oldIMSize != newIMSize)
{
- oldIndexMap[ni] = oldIndexMap[ni] &~Geometry::MapNormal;
+ // normal index was used to index other property as well,
+ // unshare them
+ oldIndexMap[oldNIdx] &= ~Geometry::MapNormal;
+ oldIndexMap.push_back(Geometry::MapNormal);
}
- oldIndexMap.push_back(Geometry::MapNormal);
- geo->setNormals(newNormals);
- geo->setIndices(newIndex);
endEditCP(geo);
return;
}
@@ -874,8 +942,20 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
//SINGLE INDEXED or NON INDEXED
//UInt32 pointCnt = 0;
newNormals->resize(geo->getPositions()->getSize());
- for(; faceIter != geo->endFaces(); ++faceIter)
+
+ Int32 primIdx = -1;
+ FaceIterator faceIter = geo->beginFaces();
+ FaceIterator faceEnd = geo->endFaces ();
+
+ for(; faceIter != faceEnd; ++faceIter)
{
+ // for strip/fan primitives we need to set the normal on all
+ // vertices of the first face, but on subsequent faces only
+ // on the new vertices - see below where this is used
+ bool primStart = (faceIter.PrimitiveIterator::getIndex() > primIdx);
+ primIdx = faceIter.PrimitiveIterator::getIndex();
+
+
if(faceIter.getLength() == 3)
{
//Face is a Triangle
@@ -902,12 +982,31 @@ OSG_SYSTEMLIB_DLLMAPPING void OSG::calcF
switch(faceIter.getType())
{
case GL_TRIANGLE_STRIP:
+ if(primStart)
+ {
+ newNormals->setValue(normal, faceIter.getPositionIndex(0));
+ newNormals->setValue(normal, faceIter.getPositionIndex(1));
+ }
+
newNormals->setValue(normal, faceIter.getPositionIndex(2));
break;
case GL_TRIANGLE_FAN:
+ if(primStart)
+ {
+ newNormals->setValue(normal, faceIter.getPositionIndex(0));
+ newNormals->setValue(normal, faceIter.getPositionIndex(1));
+ }
+
newNormals->setValue(normal, faceIter.getPositionIndex(2));
break;
case GL_QUAD_STRIP:
+ if(primStart)
+ {
+ newNormals->setValue(normal, faceIter.getPositionIndex(0));
+ newNormals->setValue(normal, faceIter.getPositionIndex(1));
+ }
+
+ newNormals->setValue(normal, faceIter.getPositionIndex(2));
newNormals->setValue(normal, faceIter.getPositionIndex(3));
break;
default:
------------------------------------------------------------------------------
RSA(R) Conference 2012
Save $700 by Nov 18
Register now
http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________
Opensg-users mailing list
Opensg-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensg-users
