kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #24362
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