Improved VRML1 conformance to spec

3d-viewer/3d_cache/dialogs/panel_prev_model.cpp

@@ -682,6 +682,7 @@ void PANEL_PREV_3D::updateOrientation( wxCommandEvent &event )
         canvas->Update();
     }
 
+    event.Skip();
     return;
 }
 

3d-viewer/3d_cache/sg/sg_shape.cpp

@@ -230,9 +230,14 @@ bool SGSHAPE::addNode( SGNODE* aNode, bool isChild )
     {
         if( m_Appearance || m_RAppearance )
         {
-            std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+            if( aNode != m_Appearance && aNode != m_RAppearance )
-            std::cerr << " * [BUG] assigning multiple Appearance nodes\n";
+            {
-            return false;
+                std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+                std::cerr << " * [BUG] assigning multiple Appearance nodes\n";
+                return false;
+            }
+
+            return true;
         }
 
         if( isChild )
@@ -253,9 +258,14 @@ bool SGSHAPE::addNode( SGNODE* aNode, bool isChild )
     {
         if( m_FaceSet || m_RFaceSet )
         {
-            std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+            if( aNode != m_FaceSet && aNode != m_RFaceSet )
-            std::cerr << " * [BUG] assigning multiple FaceSet nodes\n";
+            {
-            return false;
+                std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+                std::cerr << " * [BUG] assigning multiple FaceSet nodes\n";
+                return false;
+            }
+
+            return true;
         }
 
         if( isChild )

plugins/3d/vrml/CMakeLists.txt

@@ -4,7 +4,7 @@ include_directories(
     ${CMAKE_CURRENT_SOURCE_DIR}/v2
 )
 
-add_definitions( -DDEBUG_VRML1=3 -DDEBUG_VRML2=3 )
+add_definitions( -DDEBUG_VRML1=2 -DDEBUG_VRML2=2 )
 
 add_library( s3d_plugin_vrml MODULE
         vrml.cpp
@@ -25,6 +25,7 @@ add_library( s3d_plugin_vrml MODULE
         v2/vrml2_switch.cpp
         v1/vrml1_node.cpp
         v1/vrml1_base.cpp
+        v1/vrml1_group.cpp
         v1/vrml1_separator.cpp
         v1/vrml1_material.cpp
         v1/vrml1_matbinding.cpp

plugins/3d/vrml/v1/vrml1_base.cpp

@@ -24,6 +24,7 @@
 #include <iostream>
 
 #include "vrml1_base.h"
+#include "vrml1_group.h"
 #include "vrml1_separator.h"
 #include "vrml1_material.h"
 #include "vrml1_matbinding.h"
@@ -182,11 +183,11 @@ bool WRL1BASE::Read( WRLPROC& proc )
     std::cerr << " * [INFO] Processing node '" << glob << "' ID: " << ntype << "\n";
     #endif
 
-    if( ntype != WRL1_SEPARATOR && ntype != WRL1_SWITCH )
+    if( ntype != WRL1_SEPARATOR && ntype != WRL1_SWITCH && ntype != WRL1_GROUP )
     {
         #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
         std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
-        std::cerr << " * [INFO] bad file - top node is not a Separator or Switch\n";
+        std::cerr << " * [INFO] bad file - top node is not a Separator, Switch, or Group\n";
         #endif
 
         return false;
@@ -201,6 +202,13 @@ bool WRL1BASE::Read( WRLPROC& proc )
 
         break;
 
+    case WRL1_GROUP:
+
+        if( !readGroup( proc, this, NULL ) )
+            return false;
+
+        break;
+
     default:
 
         if( !readSwitch( proc, this, NULL ) )
@@ -412,9 +420,13 @@ bool WRL1BASE::ReadNode( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode )
 
     switch( ntype )
     {
-    //
+    case WRL1_GROUP:
-    // items to be implemented:
+
-    //
+        if( !readGroup( proc, aParent, aNode ) )
+            return false;
+
+        break;
+
     case WRL1_SEPARATOR:
 
         if( !readSeparator( proc, aParent, aNode ) )
@@ -520,6 +532,26 @@ bool WRL1BASE::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
+bool WRL1BASE::readGroup( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode )
+{
+    if( NULL != aNode )
+        *aNode = NULL;
+
+    WRL1GROUP* np = new WRL1GROUP( m_dictionary, aParent );
+
+    if( !np->Read( proc, this ) )
+    {
+        delete np;
+        return false;
+    }
+
+    if( NULL != aNode )
+        *aNode = (WRL1NODE*) np;
+
+    return true;
+}
+
+
 bool WRL1BASE::readSeparator( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode )
 {
     if( NULL != aNode )
@@ -677,7 +709,7 @@ bool WRL1BASE::readShapeHints( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNod
 }
 
 
-SGNODE* WRL1BASE::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1BASE::TranslateToSG( SGNODE* aParent, WRL1STATUS* /*sp*/ )
 {
     #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 2 )
     std::cerr << " * [INFO] Translating VRML1 Base with " << m_Items.size();
@@ -697,5 +729,5 @@ SGNODE* WRL1BASE::TranslateToSG( SGNODE* aParent, bool calcNormals )
         return NULL;
     }
 
-    return (*m_Items.begin())->TranslateToSG( NULL, true );
+    return (*m_Items.begin())->TranslateToSG( NULL, NULL );
 }

plugins/3d/vrml/v1/vrml1_base.h

@@ -51,6 +51,7 @@ private:
     bool implementUse( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
     bool implementDef( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
 
+    bool readGroup( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
     bool readSeparator( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
     bool readSwitch( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
     bool readMaterial( WRLPROC& proc, WRL1NODE* aParent, WRL1NODE** aNode );
@@ -88,7 +89,7 @@ public:
     // functions inherited from WRL1NODE
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool SetParent( WRL1NODE* aParent );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_BASE_H

plugins/3d/vrml/v1/vrml1_coords.cpp

@@ -194,16 +194,18 @@ void WRL1COORDS::GetCoords( WRLVEC3F*& aCoordList, size_t& aListSize )
 }
 
 
-SGNODE* WRL1COORDS::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1COORDS::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
-    if( m_Parent )
+    if( NULL == sp )
     {
-        WRL1STATUS* cp = m_Parent->GetCurrentSettings();
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
-
+        std::cerr << " * [INFO] bad model: no base data given\n";
-        if( NULL != cp )
+        #endif
-            cp->coord = this;
 
+        return NULL;
     }
 
+    sp->coord = this;
+
     return NULL;
 }

plugins/3d/vrml/v1/vrml1_coords.h

@@ -53,7 +53,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 
     void GetCoords( WRLVEC3F*& aCoordList, size_t& aListSize );
 };

plugins/3d/vrml/v1/vrml1_faceset.cpp

@@ -218,33 +218,46 @@ bool WRL1FACESET::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
-    if( m_sgNode )
+    // note: m_sgNode is unused because we cannot manage everything
+    // with a single reused transform due to the fact that VRML1
+    // may use a MatrixTransformation entity which is impossible to
+    // decompose into Rotate,Scale,Transform via an anlytic expression.
+    if( !m_Parent )
     {
-        if( NULL != aParent && !S3D::AddSGNodeRef( aParent, m_sgNode ) )
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
-            return NULL;
+        std::cerr << " * [INFO] bad model: no parent node\n";
-
+        #endif
-        return m_sgNode;
-    }
-
-    if( m_Parent )
-    {
-        WRL1STATUS* cp = m_Parent->GetCurrentSettings();
-
-        if( NULL != cp )
-            m_current = *cp;
-        else
-            return NULL;
 
+        return NULL;
     }
     else
     {
-        return NULL;
+        if( NULL == sp )
+        {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << " * [INFO] bad model: no base data given\n";
+            #endif
+
+            return NULL;
+        }
     }
 
+    m_current = *sp;
+
     if( NULL == m_current.coord || NULL == m_current.mat )
+    {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        if( NULL == m_current.coord )
+            std::cerr << " * [INFO] bad model: no vertex set\n";
+
+        if( NULL == m_current.mat )
+            std::cerr << " * [INFO] bad model: no material set\n";
+        #endif
+
         return NULL;
+    }
 
     WRLVEC3F* pcoords;
     size_t coordsize;
@@ -253,7 +266,13 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
     size_t vsize = coordIndex.size();
 
     if( coordsize < 3 || vsize < 3 )
+    {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << " * [INFO] bad model: coordsize, indexsize = " << coordsize;
+        std::cerr << ", " << vsize << "\n";
+        #endif
         return NULL;
+    }
 
     // 1. create the vertex/normals/colors lists
     std::vector< SGPOINT > vlist;
@@ -270,14 +289,25 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
     case BIND_PER_FACE_INDEXED:
 
         if( matIndex.empty() )
+        {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << " * [INFO] bad model: per face indexed but no indices\n";
+            #endif
             return NULL;
+        }
 
         break;
 
     case BIND_PER_VERTEX_INDEXED:
 
         if( matIndex.size() < 3 )
+        {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << " * [INFO] bad model: per vertex indexed but indexsize = ";
+            std::cerr << matIndex.size() << "\n";
+            #endif
             return NULL;
+        }
 
         break;
 
@@ -302,6 +332,11 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
         if( coordIndex[idx] >= (int)coordsize )
         {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << " * [INFO] bad model: index out of bounds (index = ";
+            std::cerr << coordIndex[idx] << ", npts = " << coordsize << ")\n";
+            #endif
+
             m_current.mat->Reclaim( sgcolor );
             return NULL;
         }
@@ -311,6 +346,11 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
     if( i1 < 0 || i2 < 0 || i3 < 0
         || i1 == i2 || i1 == i3 || i2 == i3 )
     {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << " * [INFO] bad model: defective indices: " << i1;
+        std::cerr << ", " << i2 << ", " << i3 << "\n";
+        #endif
+
         m_current.mat->Reclaim( sgcolor );
         return NULL;
     }
@@ -326,7 +366,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
     {
         // no color list
         // assuming convex polygons, create triangles for the SG node
-        for( idx = 3; idx < vsize; )
+        for( idx = 3; idx <= vsize; )
         {
             switch( m_current.order )
             {
@@ -354,6 +394,10 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
             ++nfaces;
             i2 = i3;
+
+            if( idx == vsize )
+                break;
+
             i3 = coordIndex[idx++];
 
             while( ( i1 < 0 || i2 < 0 || i3 < 0 ) && ( idx < vsize ) )
@@ -372,10 +416,18 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 // to ensure correct handling of the normals
                 if( ( i1 < 0 && i2 < 0 ) || ( i1 < 0 && i3 < 0 ) || ( i2 < 0 && i3 < 0 ) )
                 {
+                    #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+                    std::cerr << " * [INFO] bad model: defective indices: " << i1;
+                    std::cerr << ", " << i2 << ", " << i3 << "\n";
+                    #endif
+
                     m_current.mat->Reclaim( sgcolor );
                     return NULL;
                 }
             }
+
+            if( i1 < 0 || i2 < 0 || i3 < 0 )
+                break;
         }
     }
     else
@@ -441,7 +493,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
         if( matIndex.empty() )
             noidx = true;
 
-        for( idx = 3; idx < vsize; )
+        for( idx = 3; idx <= vsize; )
         {
             switch( m_current.order )
             {
@@ -481,6 +533,10 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
             ++nfaces;
             i2 = i3;
+
+            if( idx == vsize )
+                break;
+
             i3 = coordIndex[idx++];
 
             if( colorPerVertex && i1 >= 0 && i2 >= 0 && i3 >= 0 )
@@ -489,10 +545,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 pc2.SetColor( pc3 );
 
                 if( noidx || cIndex >= cMaxIdx )
-                {
+                    m_current.mat->GetColor( &pc3, cIndex++ );
-                    m_current.mat->GetColor( &pc3, cIndex );
-                    ++cIndex;
-                }
                 else
                     m_current.mat->GetColor( &pc3, matIndex[cIndex++] );
 
@@ -527,7 +580,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                     pc2.SetColor( pc3 );
 
                     if( noidx || cIndex >= cMaxIdx )
-                        m_current.mat->GetColor( &pc3, cIndex );
+                        m_current.mat->GetColor( &pc3, cIndex++ );
                     else
                         m_current.mat->GetColor( &pc3, matIndex[cIndex++] );
 
@@ -536,13 +589,27 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 // any invalid polygons shall void the entire faceset; this is a requirement
                 // to ensure correct handling of the normals
                 if( ( i1 < 0 && i2 < 0 ) || ( i1 < 0 && i3 < 0 ) || ( i2 < 0 && i3 < 0 ) )
+                {
+                    #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+                    std::cerr << " * [INFO] bad model: defective indices: " << i1;
+                    std::cerr << ", " << i2 << ", " << i3 << "\n";
+                    #endif
+
                     return NULL;
+                }
             }
+
+            if( i1 < 0 || i2 < 0 || i3 < 0 )
+                break;
         }
     }
 
     if( lCIdx.empty() )
     {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << " * [INFO] bad model: no points in final index list\n";
+        #endif
+
         m_current.mat->Reclaim( sgcolor );
         return NULL;
     }
@@ -554,7 +621,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
         while( sI != eI )
         {
-            glm::vec4 pt = glm::vec4( pcoords[*sI].x, pcoords[*sI].y, pcoords[*sI].z, 0.0 );
+            glm::vec4 pt = glm::vec4( pcoords[*sI].x, pcoords[*sI].y, pcoords[*sI].z, 1.0 );
             pt = m_current.txmatrix * pt;
             lCPts.push_back( SGPOINT( pt.x, pt.y, pt.z ) );
             ++sI;
@@ -592,6 +659,7 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 lCNorm.push_back( sv );
                 lCNorm.push_back( sv );
                 lCNorm.push_back( sv );
+                sv = S3D::CalcTriNorm( lCPts[i], lCPts[i+2], lCPts[i+1] );
                 lCNorm.push_back( sv );
                 lCNorm.push_back( sv );
                 lCNorm.push_back( sv );
@@ -632,7 +700,5 @@ SGNODE* WRL1FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
         nmColor.SetColorList( lColors.size(), &lColors[0] );
     }
 
-    m_sgNode = fsNode.GetRawPtr();
+    return fsNode.GetRawPtr();
-
-    return m_sgNode;
 }

plugins/3d/vrml/v1/vrml1_faceset.h

@@ -56,7 +56,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_FACESET_H

plugins/3d/vrml/v1/vrml1_group.cpp

@@ -0,0 +1,212 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2015-2016 Cirilo Bernardo <cirilo.bernardo@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+// note: this was copied from the vrml1_separator class. the difference
+// between a separator and a group is that a group propagates its
+// current settings to its parent. While it would be possible to
+// implement the separator as a derived class, it is easy enough to
+// simply duplicate the code
+
+#include <iostream>
+
+#include "vrml1_base.h"
+#include "vrml1_group.h"
+#include "plugins/3dapi/ifsg_all.h"
+
+
+WRL1GROUP::WRL1GROUP( NAMEREGISTER* aDictionary ) : WRL1NODE( aDictionary )
+{
+    m_Type = WRL1_GROUP;
+    return;
+}
+
+
+WRL1GROUP::WRL1GROUP( NAMEREGISTER* aDictionary, WRL1NODE* aParent ) :
+    WRL1NODE( aDictionary )
+{
+    m_Type = WRL1_GROUP;
+    m_Parent = aParent;
+
+    if( NULL != m_Parent )
+        m_Parent->AddChildNode( this );
+
+    return;
+}
+
+
+WRL1GROUP::~WRL1GROUP()
+{
+    #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 2 )
+    std::cerr << " * [INFO] Destroying Group with " << m_Children.size();
+    std::cerr << " children, " << m_Refs.size() << " references and ";
+    std::cerr << m_BackPointers.size() << " backpointers\n";
+    #endif
+    return;
+}
+
+
+// functions inherited from WRL1NODE
+bool WRL1GROUP::Read( WRLPROC& proc, WRL1BASE* aTopNode )
+{
+    if( NULL == aTopNode )
+    {
+        #ifdef DEBUG_VRML1
+        std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+        std::cerr << " * [BUG] aTopNode is NULL\n";
+        #endif
+        return false;
+    }
+
+    size_t line, column;
+    proc.GetFilePosData( line, column );
+
+    char tok = proc.Peek();
+
+    if( proc.eof() )
+    {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+        std::cerr << " * [INFO] bad file format; unexpected eof at line ";
+        std::cerr << line << ", column " << column << "\n";
+        #endif
+        return false;
+    }
+
+    if( '{' != tok )
+    {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << proc.GetError() << "\n";
+        std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+        std::cerr << " * [INFO] bad file format; expecting '{' but got '" << tok;
+        std::cerr  << "' at line " << line << ", column " << column << "\n";
+        #endif
+
+        return false;
+    }
+
+    proc.Pop();
+    std::string glob;
+
+    while( true )
+    {
+        if( proc.Peek() == '}' )
+        {
+            proc.Pop();
+            break;
+        }
+
+        proc.GetFilePosData( line, column );
+
+        if( !aTopNode->ReadNode( proc, this, NULL ) )
+        {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+            std::cerr << " * [INFO] bad file format; unexpected eof at line ";
+            std::cerr << line << ", column " << column << "\n";
+            #endif
+
+            return false;
+        }
+
+        if( proc.Peek() == ',' )
+            proc.Pop();
+
+    }   // while( true ) -- reading contents of Group{}
+
+    return true;
+}
+
+
+SGNODE* WRL1GROUP::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
+{
+    #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 2 )
+    std::cerr << " * [INFO] Translating Group with " << m_Children.size();
+    std::cerr << " children, " << m_Refs.size() << " references and ";
+    std::cerr << m_BackPointers.size() << " backpointers (total ";
+    std::cerr << m_Items.size() << " items)\n";
+    #endif
+
+    std::cerr << "XXX: GROUP!!!!!!!!!!\n";
+
+    if( !m_Parent )
+    {
+        std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+        std::cerr << " * [BUG] Group has no parent\n";
+
+        return NULL;
+    }
+
+    if( WRL1_BASE != m_Parent->GetNodeType() )
+    {
+        if( NULL == sp )
+        {
+            #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+            std::cerr << " * [INFO] bad model: no base data given\n";
+            #endif
+
+            return NULL;
+        }
+    }
+    else
+    {
+        m_current.Init();
+        sp = &m_current;
+    }
+
+    S3D::SGTYPES ptype = S3D::GetSGNodeType( aParent );
+
+    if( NULL != aParent && ptype != S3D::SGTYPE_TRANSFORM )
+    {
+        #ifdef DEBUG_VRML1
+        std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
+        std::cerr << " * [BUG] Group does not have a Transform parent (parent ID: ";
+        std::cerr << ptype << ")\n";
+        #endif
+
+        return NULL;
+    }
+
+    IFSG_TRANSFORM txNode( aParent );
+    bool hasContent = false;
+
+    std::list< WRL1NODE* >::iterator sI = m_Items.begin();
+    std::list< WRL1NODE* >::iterator eI = m_Items.end();
+
+    SGNODE* node = txNode.GetRawPtr();
+
+    while( sI != eI )
+    {
+        if( NULL != (*sI)->TranslateToSG( node, sp ) )
+            hasContent = true;
+
+        ++sI;
+    }
+
+    if( !hasContent )
+    {
+        txNode.Destroy();
+        return NULL;
+    }
+
+    return node;
+}

plugins/3d/vrml/v1/vrml1_group.h

@@ -0,0 +1,52 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2015-2016 Cirilo Bernardo <cirilo.bernardo@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+/**
+ * @file vrml1_group.h
+ */
+
+
+#ifndef VRML1_GROUP_H
+#define VRML1_GROUP_H
+
+#include "vrml1_node.h"
+
+class WRL1BASE;
+class SGNODE;
+
+/**
+ * Class WRL1GROUP
+ */
+class WRL1GROUP : public WRL1NODE
+{
+public:
+    WRL1GROUP( NAMEREGISTER* aDictionary );
+    WRL1GROUP( NAMEREGISTER* aDictionary, WRL1NODE* aNode );
+    virtual ~WRL1GROUP();
+
+    // functions inherited from WRL1NODE
+    bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
+};
+
+#endif  // VRML1_GROUP_H

plugins/3d/vrml/v1/vrml1_matbinding.cpp

@@ -229,16 +229,18 @@ bool WRL1MATBINDING::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1MATBINDING::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1MATBINDING::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
-    if( m_Parent )
+    if( NULL == sp )
     {
-        WRL1STATUS* cp = m_Parent->GetCurrentSettings();
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
-
+        std::cerr << " * [INFO] bad model: no base data given\n";
-        if( NULL != cp )
+        #endif
-            cp->matbind = m_binding;
 
+        return NULL;
     }
 
+    sp->matbind = m_binding;
+
     return NULL;
 }

plugins/3d/vrml/v1/vrml1_matbinding.h

@@ -52,7 +52,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_MATBINDING_H

plugins/3d/vrml/v1/vrml1_material.cpp

@@ -275,17 +275,19 @@ bool WRL1MATERIAL::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1MATERIAL::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1MATERIAL::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
-    if( m_Parent )
+    if( NULL == sp )
     {
-        WRL1STATUS* cp = m_Parent->GetCurrentSettings();
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
-
+        std::cerr << " * [INFO] bad model: no base data given\n";
-        if( NULL != cp )
+        #endif
-            cp->mat = this;
 
+        return NULL;
     }
 
+    sp->mat = this;
+
     return NULL;
 }
 

plugins/3d/vrml/v1/vrml1_material.h

@@ -63,7 +63,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 
     /**
      * Function GetAppearance

plugins/3d/vrml/v1/vrml1_node.cpp

@@ -104,6 +104,7 @@ WRL1NODE::WRL1NODE( NAMEREGISTER* aDictionary )
         nodenames.insert( NODEITEM( "Cylinder", WRL1_CYLINDER ) );
         nodenames.insert( NODEITEM( "DirectionalLight", WRL1_DIRECTIONALLIGHT ) );
         nodenames.insert( NODEITEM( "FontStyle", WRL1_FONTSTYLE ) );
+        nodenames.insert( NODEITEM( "Group", WRL1_GROUP ) );
         nodenames.insert( NODEITEM( "IndexedFaceSet", WRL1_INDEXEDFACESET ) );
         nodenames.insert( NODEITEM( "IndexedLineSet", WRL1_INDEXEDLINESET ) );
         nodenames.insert( NODEITEM( "Info", WRL1_INFO ) );
@@ -350,19 +351,8 @@ const char* WRL1NODE::GetNodeTypeName( WRL1NODES aNodeType ) const
 }
 
 
-WRL1STATUS* WRL1NODE::GetCurrentSettings( void )
-{
-    return &m_current;
-}
-
-
 WRL1NODES WRL1NODE::getNodeTypeID( const std::string aNodeName )
 {
-    // 'Group' is a special case; it has been deprecated
-    // but otherwise is similar enough to Separator.
-    if( !aNodeName.compare( "Group" ) )
-        return WRL1_SEPARATOR;
-
     NODEMAP::iterator it = nodenames.find( aNodeName );
 
     if( nodenames.end() != it )

plugins/3d/vrml/v1/vrml1_node.h

@@ -33,7 +33,6 @@
 #define GLM_FORCE_RADIANS
 #include <glm/gtx/transform.hpp>
 #include <glm/gtc/matrix_transform.hpp>
-#include <glm/gtc/type_ptr.hpp>
 
 #include <list>
 #include <map>
@@ -142,13 +141,6 @@ public:
     // cancel the dictionary pointer; for internal use only
     void cancelDict( void );
 
-    /**
-     * Function GetCurrentSettings
-     * is used by children to retrieve and perhaps modify the
-     * current settings of the parent
-     */
-    WRL1STATUS* GetCurrentSettings( void );
-
     /**
      * Function getNodeTypeID
      * returns the ID based on the given aNodeName or WRL1_INVALID (WRL1_END)
@@ -255,7 +247,7 @@ public:
      * @param aParent is a pointer to the parent SG node
      * @return is non-NULL on success
      */
-    virtual SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals ) = 0;
+    virtual SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp ) = 0;
 };
 
 #endif  // VRML1_NODE_H

plugins/3d/vrml/v1/vrml1_separator.cpp

@@ -131,7 +131,7 @@ bool WRL1SEPARATOR::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
     #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 2 )
     std::cerr << " * [INFO] Translating Separator with " << m_Children.size();
@@ -148,8 +148,8 @@ SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
         return NULL;
     }
 
-    if( WRL1_BASE != m_Parent->GetNodeType() )
+    if( sp != NULL )
-        m_current = *( m_Parent->GetCurrentSettings() );
+        m_current = *sp;
     else
         m_current.Init();
 
@@ -157,7 +157,7 @@ SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
     if( NULL != aParent && ptype != S3D::SGTYPE_TRANSFORM )
     {
-        #ifdef DEBUG_VRML2
+        #ifdef DEBUG_VRML1
         std::cerr << __FILE__ << ": " << __FUNCTION__ << ": " << __LINE__ << "\n";
         std::cerr << " * [BUG] Separator does not have a Transform parent (parent ID: ";
         std::cerr << ptype << ")\n";
@@ -166,17 +166,6 @@ SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
         return NULL;
     }
 
-    if( m_sgNode )
-    {
-        if( NULL == aParent )
-            return NULL;
-
-        if( !S3D::AddSGNodeRef( aParent, m_sgNode ) )
-            return NULL;
-
-        return m_sgNode;
-    }
-
     IFSG_TRANSFORM txNode( aParent );
     bool hasContent = false;
 
@@ -187,7 +176,7 @@ SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
     while( sI != eI )
     {
-        if( NULL != (*sI)->TranslateToSG( node, calcNormals ) )
+        if( NULL != (*sI)->TranslateToSG( node, &m_current ) )
             hasContent = true;
 
         ++sI;
@@ -199,7 +188,5 @@ SGNODE* WRL1SEPARATOR::TranslateToSG( SGNODE* aParent, bool calcNormals )
         return NULL;
     }
 
-    m_sgNode = node;
-
     return node;
 }

plugins/3d/vrml/v1/vrml1_separator.h

@@ -46,7 +46,7 @@ public:
 
     // functions inherited from WRL1NODE
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_SEPARATOR_H

plugins/3d/vrml/v1/vrml1_shapehints.cpp

@@ -245,19 +245,21 @@ bool WRL1SHAPEHINTS::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1SHAPEHINTS::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1SHAPEHINTS::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
     // note: this is not fully implemented since it is unlikely we shall
     // ever make use of the fields shapeType, faceType, and creaseAngle
 
-    if( m_Parent )
+    if( NULL == sp )
     {
-        WRL1STATUS* cp = m_Parent->GetCurrentSettings();
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
-
+        std::cerr << " * [INFO] bad model: no base data given\n";
-        if( NULL != cp )
+        #endif
-            cp->order = m_order;
 
+        return NULL;
     }
 
+    sp->order = m_order;
+
     return NULL;
 }

plugins/3d/vrml/v1/vrml1_shapehints.h

@@ -52,7 +52,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_SHAPEHINTS_H

plugins/3d/vrml/v1/vrml1_switch.cpp

@@ -184,7 +184,7 @@ bool WRL1SWITCH::Read( WRLPROC& proc, WRL1BASE* aTopNode )
 }
 
 
-SGNODE* WRL1SWITCH::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1SWITCH::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
     #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 2 )
     std::cerr << " * [INFO] Translating Switch with " << m_Children.size();
@@ -199,15 +199,18 @@ SGNODE* WRL1SWITCH::TranslateToSG( SGNODE* aParent, bool calcNormals )
     if( whichChild < 0 || whichChild >= (int)m_Items.size() )
         return NULL;
 
+    if( sp == NULL )
+    {
+        m_current.Init();
+        sp = &m_current;
+    }
+
     std::list< WRL1NODE* >::iterator ip = m_Items.begin();
     std::advance( ip, whichChild );
 
     IFSG_TRANSFORM txNode( aParent );
 
-    if( WRL1_BASE != m_Parent->GetNodeType() )
+    SGNODE* np = (*ip)->TranslateToSG( aParent, sp );
-        m_current = *( m_Parent->GetCurrentSettings() );
-    else
-        m_current.Init();
 
-    return (*ip)->TranslateToSG( aParent, calcNormals );
+    return np;
 }

plugins/3d/vrml/v1/vrml1_switch.h

@@ -49,7 +49,7 @@ public:
 
     // functions inherited from WRL1NODE
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_SWITCH_H

plugins/3d/vrml/v1/vrml1_transform.cpp

@@ -269,18 +269,22 @@ bool WRL1TRANSFORM::AddChildNode( WRL1NODE* aNode )
 }
 
 
-SGNODE* WRL1TRANSFORM::TranslateToSG( SGNODE* aParent, bool calcNormals )
+SGNODE* WRL1TRANSFORM::TranslateToSG( SGNODE* aParent, WRL1STATUS* sp )
 {
     if( NULL == m_Parent )
         return NULL;
 
-    if( WRL2_BASE == m_Parent->GetNodeType() )
+    if( WRL1_BASE == m_Parent->GetNodeType() )
         return NULL;
 
-    WRL1STATUS* cp = m_Parent->GetCurrentSettings();
+    if( NULL == sp )
+    {
+        #if defined( DEBUG_VRML1 ) && ( DEBUG_VRML1 > 1 )
+        std::cerr << " * [INFO] bad model: no base data given\n";
+        #endif
 
-    if( NULL == cp )
         return NULL;
+    }
 
     // rotation
     float rX, rY, rZ, rW;
@@ -313,7 +317,7 @@ SGNODE* WRL1TRANSFORM::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
     // resultant transform:
     // tx' = tM * cM * rM * srM * sM * nsrM * ncM
-    cp->txmatrix = cp->txmatrix * tM * cM * rM * srM * sM * nsrM * ncM;
+    sp->txmatrix = sp->txmatrix * tM * cM * rM * srM * sM * nsrM * ncM;
 
     return NULL;
 }

plugins/3d/vrml/v1/vrml1_transform.h

@@ -55,7 +55,7 @@ public:
     bool Read( WRLPROC& proc, WRL1BASE* aTopNode );
     bool AddRefNode( WRL1NODE* aNode );
     bool AddChildNode( WRL1NODE* aNode );
-    SGNODE* TranslateToSG( SGNODE* aParent, bool calcNormals );
+    SGNODE* TranslateToSG( SGNODE* aParent, WRL1STATUS* sp );
 };
 
 #endif  // VRML1_TRANSFORM_H

plugins/3d/vrml/v2/vrml2_faceset.cpp

@@ -646,7 +646,7 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
     if( NULL == color )
     {
         // assuming convex polygons, create triangles for the SG node
-        for( idx = 3; idx < vsize; )
+        for( idx = 3; idx <= vsize; )
         {
             lCIdx.push_back( i1 );
 
@@ -663,6 +663,10 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
 
             ++nfaces;
             i2 = i3;
+
+            if( idx == vsize )
+                break;
+
             i3 = coordIndex[idx++];
 
             while( ( i1 < 0 || i2 < 0 || i3 < 0 ) && ( idx < vsize ) )
@@ -682,6 +686,9 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 if( ( i1 < 0 && i2 < 0 ) || ( i1 < 0 && i3 < 0 ) || ( i2 < 0 && i3 < 0 ) )
                     return NULL;
             }
+
+            if( i1 < 0 || i2 < 0 || i3 < 0 )
+                break;
         }
     }
     else
@@ -746,7 +753,7 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
         // assuming convex polygons, create triangles for the SG node
         int cMaxIdx = (int) colorIndex.size();
 
-        for( idx = 3; idx < vsize; )
+        for( idx = 3; idx <= vsize; )
         {
             lCIdx.push_back( i1 );
             lColors.push_back( pc1 );
@@ -787,6 +794,9 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 pc3.SetColor( tc.x, tc.y, tc.z );
             }
 
+            if( idx == vsize )
+                break;
+
             i3 = coordIndex[idx++];
 
             while( ( i1 < 0 || i2 < 0 || i3 < 0 ) && ( idx < vsize ) )
@@ -839,6 +849,9 @@ SGNODE* WRL2FACESET::TranslateToSG( SGNODE* aParent, bool calcNormals )
                 if( ( i1 < 0 && i2 < 0 ) || ( i1 < 0 && i3 < 0 ) || ( i2 < 0 && i3 < 0 ) )
                     return NULL;
             }
+
+            if( i1 < 0 || i2 < 0 || i3 < 0 )
+                break;
         }
     }
 

plugins/3d/vrml/vrml.cpp

@@ -208,8 +208,7 @@ SCENEGRAPH* Load( char const* aFileName )
             std::cout << " * [INFO] load completed\n";
             #endif
 
-            // for now we recalculate all normals per-vertex per-face
+            scene = (SCENEGRAPH*)bp->TranslateToSG( NULL, NULL );
-            scene = (SCENEGRAPH*)bp->TranslateToSG( NULL, true );
         }
 
         delete bp;

plugins/3d/vrml/wrltypes.h

@@ -55,6 +55,7 @@ enum WRL1NODES
     WRL1_CYLINDER,
     WRL1_DIRECTIONALLIGHT,
     WRL1_FONTSTYLE,
+    WRL1_GROUP,
     WRL1_INDEXEDFACESET,
     WRL1_INDEXEDLINESET,
     WRL1_INFO,