← Back to team overview

kicad-developers team mailing list archive

Re: PATCH: 3D resolver

 

Hi Wayne,

 The attached patch was made against r6709.

- Cirilo


On Sat, Apr 23, 2016 at 12:52 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> Cirilo,
>
> This patch no longer applies cleanly.  Would you please fix it an repost
> it.
>
> Thanks,
>
> Wayne
>
> On 4/20/2016 5:24 AM, Cirilo Bernardo wrote:
> > The attached patch does the following:
> >
> > 1. Ensures ${KISYS3DMOD} is in the resolver's path list even if
> > it is not defined in KiCad's internal path list (Configure Paths).
> > This fixes a bug which was described on the dev list but not
> > reported to the bug tracker.
> >
> > 2. ENV VARs paths defined by the shell take precedence over
> > what is defined within KiCad; this is consistent with typical
> > UNIX use: ENV_VAR=something pcbnew test.kicad_pcb
> >
> > 3. If a user defines a path via ${ENV_VAR}/som/file.wrl where
> > ENV_VAR is an arbitrary environment variable (as opposed to
> > KISYS3DMOD and others defined via KiCad's Configure Paths)
> > then that ENV_VAR is automatically added to the list of
> > base search paths. Previous behavior was to only include
> > KISYS3DMOD and ENV_VARS defined via Configure Paths.
> >
> > 4. General code cleanup removing functions which were not
> > necessary since wxFileName already performed the required
> > tasks.
> >
> > 5. Fix a bug in the file modification time check where a path to
> > a non-existent file fires a wxASSERT within wxWidgets.
> >
> > - Cirilo
> >
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~kicad-developers
> > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> > Unsubscribe : https://launchpad.net/~kicad-developers
> > More help   : https://help.launchpad.net/ListHelp
> >
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
=== modified file '3d-viewer/3d_cache/3d_cache.cpp'
--- 3d-viewer/3d_cache/3d_cache.cpp	2016-04-21 07:19:08 +0000
+++ 3d-viewer/3d_cache/3d_cache.cpp	2016-04-22 22:44:34 +0000
@@ -225,19 +225,23 @@
     if( mi != m_CacheMap.end() )
     {
         wxFileName fname( full3Dpath );
-        wxDateTime fmdate = fname.GetModificationTime();
         bool reload = false;
 
-        if( fmdate != mi->second->modTime )
+        if( fname.FileExists() )
         {
-            unsigned char hashSum[20];
-            getSHA1( full3Dpath, hashSum );
-            mi->second->modTime = fmdate;
+            wxDateTime fmdate = fname.GetModificationTime();
 
-            if( !isSHA1Same( hashSum, mi->second->sha1sum ) )
+            if( fmdate != mi->second->modTime )
             {
-                mi->second->SetSHA1( hashSum );
-                reload = true;
+                unsigned char hashSum[20];
+                getSHA1( full3Dpath, hashSum );
+                mi->second->modTime = fmdate;
+
+                if( !isSHA1Same( hashSum, mi->second->sha1sum ) )
+                {
+                    mi->second->SetSHA1( hashSum );
+                    reload = true;
+                }
             }
         }
 

=== modified file '3d-viewer/3d_cache/3d_filename_resolver.cpp'
--- 3d-viewer/3d_cache/3d_filename_resolver.cpp	2016-04-21 07:19:08 +0000
+++ 3d-viewer/3d_cache/3d_filename_resolver.cpp	2016-04-22 22:44:34 +0000
@@ -177,6 +177,7 @@
     m_Paths.push_back( lpath );
     wxFileName fndummy;
     wxUniChar psep = fndummy.GetPathSeparator();
+    bool hasKISYS3DMOD = false;
 
     // iterate over the list of internally defined ENV VARs
     // and add existing paths to the resolver
@@ -202,7 +203,15 @@
                 continue;
             }
 
-            fndummy.Assign( mS->second.GetValue(), "" );
+            wxString pathVal;
+
+            // ensure system ENV VARs supercede internally defined vars
+            if( wxGetEnv( mS->first, &pathVal ) && wxDirExists( pathVal ) )
+                fndummy.Assign( pathVal, "" );
+            else
+                fndummy.Assign( mS->second.GetValue(), "" );
+
+            fndummy.Normalize();
 
             if( !fndummy.DirExists() )
             {
@@ -214,9 +223,12 @@
             tmp.Append( mS->first );
             tmp.Append( "}" );
 
+            if( tmp == "${KISYS3DMOD}" )
+                hasKISYS3DMOD = true;
+
             lpath.m_alias =  tmp;
             lpath.m_pathvar = tmp;
-            lpath.m_pathexp = mS->second.GetValue();
+            lpath.m_pathexp = fndummy.GetFullPath();
 
             if( !lpath.m_pathexp.empty() && psep == *lpath.m_pathexp.rbegin() )
                 lpath.m_pathexp.erase( --lpath.m_pathexp.end() );
@@ -227,6 +239,30 @@
         }
     }
 
+    // special case: if KISYSMOD is not internally defined but is defined by
+    // the system, then create an entry here
+    wxString envar;
+
+    if( !hasKISYS3DMOD && wxGetEnv( "KISYS3DMOD", &envar ) )
+    {
+        lpath.m_alias = "${KISYS3DMOD}";
+        lpath.m_pathvar = "${KISYS3DMOD}";
+        fndummy.Assign( envar, "" );
+        fndummy.Normalize();
+
+        if( fndummy.DirExists() )
+        {
+            lpath.m_pathexp = fndummy.GetFullPath();
+
+            if( !lpath.m_pathexp.empty() && psep == *lpath.m_pathexp.rbegin() )
+                lpath.m_pathexp.erase( --lpath.m_pathexp.end() );
+
+            if( !lpath.m_pathexp.empty() )
+                m_Paths.push_back( lpath );
+        }
+
+    }
+
     if( !m_ConfigDir.empty() )
         readPathList();
 
@@ -268,6 +304,7 @@
 wxString S3D_FILENAME_RESOLVER::ResolvePath( const wxString& aFileName )
 {
     wxCriticalSectionLocker lock( lock3D_resolver );
+
     if( aFileName.empty() )
         return wxEmptyString;
 
@@ -289,43 +326,40 @@
     tname.Replace( wxT( "/" ), wxT( "\\" ) );
     #endif
 
-    // this case covers full paths and paths relative to
-    // the current working directory (which is not necessarily
+    wxFileName tmpFN( tname );
+
+    // in the case of absolute filenames we don't store a map item
+    if( !aFileName.StartsWith( wxT( "${" ) ) && tmpFN.IsAbsolute() )
+    {
+        if( tmpFN.FileExists() )
+            return tname;
+
+        return wxEmptyString;
+    }
+
+    tmpFN.Normalize();
+
+    // this case covers full paths, leading expanded vars, and paths
+    // relative to the current working directory (which is not necessarily
     // the current project directory)
-    if( wxFileName::FileExists( tname ) )
+    if( tmpFN.FileExists() )
     {
-        wxFileName tmp( tname );
-
-        if( tmp.Normalize() )
-            tname = tmp.GetFullPath();
-
+        tmpFN.Normalize();
+        tname = tmpFN.GetFullPath();
         m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
 
+        // special case: if a path begins with ${ENV_VAR} but is not in the
+        // resolver's path list then add it
+        if( aFileName.StartsWith( "${" ) )
+            checkEnvVarPath( aFileName );
+
         return tname;
     }
 
-    // at this point aFileName:
-    // a. is a legacy ${} shortened name
-    // b. an aliased shortened name
-    // c. cannot be determined
-
+    // if a path begins with ${ENV_VAR} and is not resolved then the file
+    // either does not exist or the ENV_VAR is not defined
     if( aFileName.StartsWith( wxT( "${" ) ) )
     {
-        wxFileName tmp( aFileName );
-
-        if( tmp.Normalize() )
-        {
-            tname = tmp.GetFullPath();
-            m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
-
-            return tname;
-        }
-        else if( resolveVirtualEnv( aFileName, tname ) )
-        {
-            m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
-            return tname;
-        }
-
         if( !( m_errflags & ERRFLG_ENVPATH ) )
         {
             m_errflags |= ERRFLG_ENVPATH;
@@ -338,6 +372,10 @@
         return wxEmptyString;
     }
 
+    // at this point aFileName is:
+    // a. an aliased shortened name or
+    // b. cannot be determined
+
     std::list< S3D_ALIAS >::const_iterator sPL = m_Paths.begin();
     std::list< S3D_ALIAS >::const_iterator ePL = m_Paths.end();
 
@@ -345,49 +383,47 @@
     // note: this is not necessarily the same as the current working
     // directory, which has already been checked. This case accounts
     // for partial paths which do not contain ${KIPRJMOD}.
-    if( !sPL->m_pathexp.empty() )
+    // This check is performed before checking the path relative to
+    // ${KISYS3DMOD} so that users can potentially override a model
+    // within ${KISYS3DMOD}
+    if( !sPL->m_pathexp.empty() && !tname.StartsWith( ":" ) )
     {
-        wxFileName fpath( wxFileName::DirName( sPL->m_pathexp ) );
-        wxString fullPath = fpath.GetPathWithSep() + tname;
+        tmpFN.Assign( sPL->m_pathexp, "" );
+        wxString fullPath = tmpFN.GetPathWithSep() + tname;
 
         if( wxFileName::FileExists( fullPath ) )
         {
-            wxFileName tmp( fullPath );
-
-            if( tmp.Normalize() )
-                tname = tmp.GetFullPath();
-
+            tmpFN.Assign( fullPath );
+            tmpFN.Normalize();
+            tname = tmpFN.GetFullPath();
             m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
 
             return tname;
         }
+
     }
 
-    // ${ENV_VAR} paths have already been checked; skip all but
-    // ${KISYS3DMOD}, since legacy behavior was to check if paths
-    // were relative to ${KISYS3DMOD}
+    // check the partial path relative to ${KISYS3DMOD} (legacy behavior)
+    if( !tname.StartsWith( ":" ) )
+    {
+        wxFileName fpath;
+        wxString fullPath( "${KISYS3DMOD}" );
+        fullPath.Append( fpath.GetPathSeparator() );
+        fullPath.Append( tname );
+        fpath.Assign( fullPath );
+
+        if( fpath.Normalize() && fpath.FileExists() )
+        {
+            tname = fpath.GetFullPath();
+            m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
+            return tname;
+        }
+
+    } while( 0 );
+
+    // ${ENV_VAR} paths have already been checked; skip them
     while( sPL != ePL && sPL->m_alias.StartsWith( "${" ) )
-    {
-        if( sPL->m_alias == "${KISYS3DMOD}" )
-        {
-            wxFileName fpath( wxFileName::DirName( sPL->m_pathexp ) );
-            wxString fullPath = fpath.GetPathWithSep() + tname;
-
-            if( wxFileName::FileExists( fullPath ) )
-            {
-                wxFileName tmp( fullPath );
-
-                if( tmp.Normalize() )
-                    tname = tmp.GetFullPath();
-
-                m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
-
-                return tname;
-            }
-        }
-
         ++sPL;
-    }
 
     // at this point the filename must contain an alias or else it is invalid
     wxString alias;         // the alias portion of the short filename
@@ -424,7 +460,6 @@
                     tname = tmp.GetFullPath();
 
                 m_NameMap.insert( std::pair< wxString, wxString > ( aFileName, tname ) );
-
                 return tname;
             }
         }
@@ -721,47 +756,54 @@
 }
 
 
-bool S3D_FILENAME_RESOLVER::resolveVirtualEnv( const wxString& aFileName, wxString& aFullPath )
+void S3D_FILENAME_RESOLVER::checkEnvVarPath( const wxString& aPath )
 {
-    aFullPath.clear();
-
-    if( !aFileName.StartsWith( "${" ) )
-        return false;
-
-    size_t eDelim = aFileName.find( '}' );
-
-    if( eDelim == wxString::npos || eDelim + 2 >= aFileName.length() )
-        return false;
-
-    wxString tPath = aFileName.substr( 0, eDelim + 1 );
-
+    if( !aPath.StartsWith( "${" ) )
+        return;
+
+    size_t pEnd = aPath.find( "}" );
+
+    if( pEnd == wxString::npos )
+        return;
+
+    wxString envar = aPath.substr( 0, pEnd + 1 );
+
+    // check if the alias exists; if not then add it to the end of the
+    // env var section of the path list
     std::list< S3D_ALIAS >::const_iterator sPL = m_Paths.begin();
     std::list< S3D_ALIAS >::const_iterator ePL = m_Paths.end();
 
     while( sPL != ePL )
     {
+        if( sPL->m_alias == envar )
+            return;
+
         if( !sPL->m_alias.StartsWith( "${" ) )
-            return false;
-
-        if( sPL->m_alias == tPath )
-        {
-            tPath.Append( aFileName.substr( eDelim + 2 ) );
-            wxFileName tFile( tPath );
-            tFile.Normalize();
-
-            if( tFile.FileExists() )
-            {
-                aFullPath = tFile.GetFullPath();
-                return true;
-            }
-
-            return false;
-        }
+            break;
 
         ++sPL;
     }
 
-    return false;
+    S3D_ALIAS lpath;
+    lpath.m_alias = envar;
+    lpath.m_pathvar = lpath.m_alias;
+    wxFileName tmpFN( lpath.m_alias, "" );
+    wxUniChar psep = tmpFN.GetPathSeparator();
+    tmpFN.Normalize();
+
+    if( !tmpFN.DirExists() )
+        return;
+
+    lpath.m_pathexp = tmpFN.GetFullPath();
+
+    if( !lpath.m_pathexp.empty() && psep == *lpath.m_pathexp.rbegin() )
+        lpath.m_pathexp.erase( --lpath.m_pathexp.end() );
+
+    if( lpath.m_pathexp.empty() )
+        return;
+
+    m_Paths.insert( sPL, lpath );
+    return;
 }
 
 

=== modified file '3d-viewer/3d_cache/3d_filename_resolver.h'
--- 3d-viewer/3d_cache/3d_filename_resolver.h	2016-04-17 22:35:32 +0000
+++ 3d-viewer/3d_cache/3d_filename_resolver.h	2016-04-22 22:44:34 +0000
@@ -99,12 +99,12 @@
     bool writePathList( void );
 
     /**
-     * Function resolveVirtualEnv
-     * extracts the ${ENV_VAR} component of aFileName and puts a
-     * resolved path in aFullPath if the ${ENV_VAR} exists in the
-     * alias list and the referenced file exists.
+     * Function checkEnvVarPath
+     * checks the ${ENV_VAR} component of a path and adds
+     * it to the resolver's path list if it is not yet in
+     * the list
      */
-    bool resolveVirtualEnv( const wxString& aFileName, wxString& aFullPath );
+    void checkEnvVarPath( const wxString& aPath );
 
 public:
     S3D_FILENAME_RESOLVER();


Follow ups

References