Page layout selection: fix Bug #1405972, and add minor enhancement in page...

Page layout selection: fix Bug #1405972, and add minor enhancement in page layout selection dialog (the new layout is shown after selection).
When the filename is not absolute, the page layout file is now searched first in project folder, and then in kicad template folder, if not found in project.

common/dialogs/dialog_page_settings.cpp

@@ -91,10 +91,12 @@ DIALOG_PAGES_SETTINGS::DIALOG_PAGES_SETTINGS( EDA_DRAW_FRAME* parent ) :
 {
     m_parent   = parent;
     m_screen   = m_parent->GetScreen();
+    m_projectPath = Prj().GetProjectPath();
     m_page_bitmap = NULL;
     m_tb = m_parent->GetTitleBlock();
     m_customFmt = false;
     m_localPrjConfigChanged = false;
+    m_pagelayout = NULL;
 
     initDialog();
 
@@ -105,8 +107,8 @@ DIALOG_PAGES_SETTINGS::DIALOG_PAGES_SETTINGS( EDA_DRAW_FRAME* parent ) :
 
 DIALOG_PAGES_SETTINGS::~DIALOG_PAGES_SETTINGS()
 {
-    if( m_page_bitmap )
     delete m_page_bitmap;
+    delete m_pagelayout;
 }
 
 
@@ -225,7 +227,7 @@ void DIALOG_PAGES_SETTINGS::OnOkClick( wxCommandEvent& event )
         m_screen->SetModify();
         m_parent->GetCanvas()->Refresh();
 
-        if( m_localPrjConfigChanged )
+        if( LocalPrjConfigChanged() )
             m_parent->SaveProjectSettings( true );
 
         EndModal( true );
@@ -409,9 +411,12 @@ bool DIALOG_PAGES_SETTINGS::SavePageSettings()
 
     if( fileName != BASE_SCREEN::m_PageLayoutDescrFileName )
     {
-        if( !fileName.IsEmpty() )
+        wxString fullFileName =
+                    WORKSHEET_LAYOUT::MakeFullFileName( fileName, m_projectPath );
+
+        if( !fullFileName.IsEmpty() )
         {
-            wxString fullFileName = WORKSHEET_LAYOUT::MakeFullFileName( fileName );
+
             if( !wxFileExists( fullFileName ) )
             {
                 wxString msg;
@@ -424,7 +429,7 @@ bool DIALOG_PAGES_SETTINGS::SavePageSettings()
 
         BASE_SCREEN::m_PageLayoutDescrFileName = fileName;
         WORKSHEET_LAYOUT& pglayout = WORKSHEET_LAYOUT::GetTheInstance();
-        pglayout.SetPageLayout( fileName );
+        pglayout.SetPageLayout( fullFileName );
         m_localPrjConfigChanged = true;
     }
 
@@ -663,6 +668,7 @@ void DIALOG_PAGES_SETTINGS::UpdatePageLayoutExample()
         wxString emptyString;
         GRResetPenAndBrush( &memDC );
 
+        WORKSHEET_LAYOUT::SetAltInstance( m_pagelayout );
         DrawPageLayout( &memDC, NULL, pageDUMMY,
                         emptyString, emptyString,
                         m_tb, m_screen->m_NumberOfScreens,
@@ -670,6 +676,7 @@ void DIALOG_PAGES_SETTINGS::UpdatePageLayoutExample()
 
         memDC.SelectObject( wxNullBitmap );
         m_PageLayoutExampleBitmap->SetBitmap( *m_page_bitmap );
+        WORKSHEET_LAYOUT::SetAltInstance( NULL );
 
         // Refresh the dialog.
         Layout();
@@ -782,11 +789,9 @@ void DIALOG_PAGES_SETTINGS::GetCustomSizeMilsFromDialog()
 // Called on .kicad_wks file description selection change
 void DIALOG_PAGES_SETTINGS::OnWksFileSelection( wxCommandEvent& event )
 {
-    wxString pro_dir = wxPathOnly( Prj().GetProjectFullName() );
-
     // Display a file picker dialog
     wxFileDialog fileDialog( this, _( "Select Page Layout Descr File" ),
-                             pro_dir, GetWksFileName(),
+                             m_projectPath, GetWksFileName(),
                              PageLayoutDescrFileWildcard,
                              wxFD_DEFAULT_STYLE | wxFD_FILE_MUST_EXIST );
 
@@ -796,24 +801,30 @@ void DIALOG_PAGES_SETTINGS::OnWksFileSelection( wxCommandEvent& event )
     wxString fileName = fileDialog.GetPath();
 
     // Try to remove the path, if the path is the current working dir,
-    // or the dir of kicad.pro (template)
-    wxString shortFileName = WORKSHEET_LAYOUT::MakeShortFileName( fileName );
-    wxFileName fn = shortFileName;
+    // or the dir of kicad.pro (template), and use a relative path
+    wxString shortFileName = WORKSHEET_LAYOUT::MakeShortFileName( fileName, m_projectPath );
 
     // For Win/Linux/macOS compatibility, a relative path is a good idea
-    if( fn.IsAbsolute() && fileName != GetWksFileName() )
+    if( shortFileName != GetWksFileName() && shortFileName != fileName )
     {
-        fn.MakeRelativeTo( pro_dir );
-
         wxString msg = wxString::Format( _(
                 "The page layout descr filename has changed.\n"
                 "Do you want to use the relative path:\n"
-                "'%s'" ),
-                GetChars( fn.GetFullPath() )
-                );
-        if( IsOK( this, msg ) )
-            shortFileName = fn.GetFullPath();
+                "'%s'\n"
+                "instead of\n"
+                "'%s'" ), GetChars( shortFileName ), GetChars( fileName ) );
+
+        if( !IsOK( this, msg ) )
+            shortFileName = fileName;
     }
 
     SetWksFileName( shortFileName );
+
+    if( m_pagelayout == NULL )
+        m_pagelayout = new WORKSHEET_LAYOUT;
+
+    m_pagelayout->SetPageLayout( fileName );
+
+    GetPageLayoutInfoFromDialog();
+    UpdatePageLayoutExample();
 }

common/dialogs/dialog_page_settings.h

@@ -37,6 +37,7 @@ class DIALOG_PAGES_SETTINGS: public DIALOG_PAGES_SETTINGS_BASE
 private:
     EDA_DRAW_FRAME* m_parent;
     BASE_SCREEN*    m_screen;
+    wxString        m_projectPath;      // the curr project path
     wxArrayString   m_pageFmt;          /// list of page sizes (not translated)
     bool            m_initialized;
     bool            m_localPrjConfigChanged;    /// the page layuout filename was changed
@@ -45,6 +46,8 @@ private:
     PAGE_INFO       m_pageInfo;         /// Temporary page info.
     bool            m_customFmt;        /// true if the page selection is custom
     TITLE_BLOCK     m_tb;               /// Temporary title block (basic inscriptions).
+    WORKSHEET_LAYOUT *m_pagelayout;     // the alternate and temporary page layout shown by the dialog
+                                        // when the initial one is replaced by a new one
 
 public:
     DIALOG_PAGES_SETTINGS( EDA_DRAW_FRAME* parent );

common/page_layout/class_worksheet_layout.cpp

@@ -60,7 +60,8 @@
 
 // The layout shape used in the application
 // It is accessible by WORKSHEET_LAYOUT::GetTheInstance()
-WORKSHEET_LAYOUT wksTheInstance;
+static WORKSHEET_LAYOUT wksTheInstance;
+static WORKSHEET_LAYOUT* wksAltInstance;
 
 WORKSHEET_LAYOUT::WORKSHEET_LAYOUT()
 {
@@ -71,6 +72,28 @@ WORKSHEET_LAYOUT::WORKSHEET_LAYOUT()
     m_bottomMargin = 10.0;  // the bottom page margin in mm
 }
 
+/* static function: returns the instance of WORKSHEET_LAYOUT
+ * used in the application
+ */
+WORKSHEET_LAYOUT& WORKSHEET_LAYOUT::GetTheInstance()
+{
+    if( wksAltInstance )
+        return *wksAltInstance;
+    else
+        return wksTheInstance;
+}
+
+/**
+ * static function: Set an alternate instance of WORKSHEET_LAYOUT
+ * mainly used in page setting dialog
+ * @param aLayout = the alternate page layout.
+ * if null, restore the basic page layout
+ */
+void WORKSHEET_LAYOUT::SetAltInstance( WORKSHEET_LAYOUT* aLayout )
+{
+    wksAltInstance = aLayout;
+}
+
 
 void WORKSHEET_LAYOUT::SetLeftMargin( double aMargin )
 {
@@ -163,10 +186,22 @@ WORKSHEET_DATAITEM* WORKSHEET_LAYOUT::GetItem( unsigned aIdx ) const
 }
 
 
-const wxString WORKSHEET_LAYOUT::MakeShortFileName( const wxString& aFullFileName )
+const wxString WORKSHEET_LAYOUT::MakeShortFileName( const wxString& aFullFileName,
+                                                    const wxString& aProjectPath  )
 {
-    wxFileName  fn = aFullFileName;
     wxString    shortFileName = aFullFileName;
+    wxFileName  fn = aFullFileName;
+
+    if( fn.IsRelative() )
+        return shortFileName;
+
+    if( ! aProjectPath.IsEmpty() && aFullFileName.StartsWith( aProjectPath ) )
+    {
+        fn.MakeRelativeTo( aProjectPath );
+        shortFileName = fn.GetFullPath();
+        return shortFileName;
+    }
+
     wxString    fileName = Kiface().KifaceSearch().FindValidPath( fn.GetFullName() );
 
     if( !fileName.IsEmpty() )
@@ -180,17 +215,34 @@ const wxString WORKSHEET_LAYOUT::MakeShortFileName( const wxString& aFullFileNam
 }
 
 
-const wxString WORKSHEET_LAYOUT::MakeFullFileName( const wxString& aShortFileName )
+const wxString WORKSHEET_LAYOUT::MakeFullFileName( const wxString& aShortFileName,
+                                                   const wxString& aProjectPath )
 {
-    wxFileName  fn = aShortFileName;
-    wxString    fullFileName = aShortFileName;
+    wxString    fullFileName = ExpandEnvVarSubstitutions( aShortFileName );
+
+    if( fullFileName.IsEmpty() )
+        return fullFileName;
+
+    wxFileName fn = fullFileName;
 
-    if( fn.GetPath().IsEmpty() && !fn.GetFullName().IsEmpty() )
+    if( fn.IsAbsolute() )
+        return fullFileName;
+
+    // the path is not absolute: search it in project path, and then in
+    // kicad valid paths
+    if( !aProjectPath.IsEmpty() )
     {
+        fn.MakeAbsolute( aProjectPath );
+
+        if( wxFileExists( fn.GetFullPath() ) )
+            return fn.GetFullPath();
+    }
+
+    fn = fullFileName;
     wxString name = Kiface().KifaceSearch().FindValidPath( fn.GetFullName() );
+
     if( !name.IsEmpty() )
         fullFileName = name;
-    }
 
     return fullFileName;
 }

common/page_layout/page_layout_reader.cpp

@@ -773,9 +773,9 @@ void WORKSHEET_LAYOUT::SetPageLayout( const char* aPageLayout, bool Append )
 
 #include <wx/file.h>
 
-// SetLayout() try to load a custom layout file,
-// currently defined by the environment variable KICAD_WKSFILE
-// (a *.kicad_wks file).
+// SetLayout() try to load the aFullFileName custom layout file,
+// if aFullFileName is empty, try the filename defined by the
+// environment variable KICAD_WKSFILE (a *.kicad_wks filename).
 // if does not exists, loads the default page layout.
 void WORKSHEET_LAYOUT::SetPageLayout( const wxString& aFullFileName, bool Append )
 {
@@ -783,8 +783,6 @@ void WORKSHEET_LAYOUT::SetPageLayout( const wxString& aFullFileName, bool Append
 
     if( !Append )
     {
-        fullFileName = MakeFullFileName( aFullFileName );
-
         if( fullFileName.IsEmpty() )
             wxGetEnv( wxT( "KICAD_WKSFILE" ), &fullFileName );
 
@@ -820,13 +818,15 @@ void WORKSHEET_LAYOUT::SetPageLayout( const wxString& aFullFileName, bool Append
     else
     {
         buffer[filelen]=0;
+
         if( ! Append )
             ClearList();
-        PAGE_LAYOUT_READER_PARSER lp_parser( buffer, fullFileName );
+
+        PAGE_LAYOUT_READER_PARSER pl_parser( buffer, fullFileName );
 
         try
         {
-            lp_parser.Parse( this );
+            pl_parser.Parse( this );
         }
         catch( const IO_ERROR& ioe )
         {

eeschema/eeschema_config.cpp

@@ -477,16 +477,11 @@ bool SCH_EDIT_FRAME::LoadProjectFile()
 
     // Load the page layout decr file, from the filename stored in
     // BASE_SCREEN::m_PageLayoutDescrFileName, read in config project file
-    // If empty, the default descr is loaded
+    // If empty, or not existing, the default descr is loaded
     WORKSHEET_LAYOUT& pglayout = WORKSHEET_LAYOUT::GetTheInstance();
-    wxString pg_fullfilename = ExpandEnvVarSubstitutions( BASE_SCREEN::m_PageLayoutDescrFileName );
-
-    if( !pg_fullfilename.IsEmpty() )
-    {
-        // When the page layout filename is not absolute, therefore
-        // relative to the current project, make it absolute
-        pg_fullfilename = Prj().AbsolutePath( pg_fullfilename  );
-     }
+    wxString pg_fullfilename = WORKSHEET_LAYOUT::MakeFullFileName(
+                                    BASE_SCREEN::m_PageLayoutDescrFileName,
+                                    Prj().GetProjectPath() );
 
     pglayout.SetPageLayout( pg_fullfilename );
 

include/worksheet_shape_builder.h

@@ -550,11 +550,15 @@ public:
      * static function: returns the instance of WORKSHEET_LAYOUT
      * used in the application
      */
-    static WORKSHEET_LAYOUT& GetTheInstance()
-    {
-        extern WORKSHEET_LAYOUT wksTheInstance;
-        return wksTheInstance;
-    }
+    static WORKSHEET_LAYOUT& GetTheInstance();
+
+    /**
+     * static function: Set an alternate instance of WORKSHEET_LAYOUT
+     * mainly used in page setting dialog
+     * @param aLayout = the alternate page layout.
+     * if null, restore the basic page layout
+     */
+    static void SetAltInstance( WORKSHEET_LAYOUT* aLayout = NULL );
 
     // Accessors:
     double GetLeftMargin() { return m_leftMargin; }
@@ -664,19 +668,28 @@ public:
 
     /**
      * @return a short filename  from a full filename:
-     * if the path is the current path, or if the path
+     * if the path is the current project path, or if the path
      * is the same as kicad.pro (in template), returns the shortname
      * else do nothing and returns a full filename
+     * @param aFullFileName = the full filename, which can be a relative
+     * @param aProjectPath = the curr project absolute path (can be empty)
      */
-    static const wxString MakeShortFileName( const wxString& aFullFileName );
+    static const wxString MakeShortFileName( const wxString& aFullFileName,
+                                             const wxString& aProjectPath );
 
     /**
-     * @return a full filename from a short filename,
-     * if the short filename path is void
-     * In this case the path is the same as kicad.pro (in template)
-     * else return the short filename (which have an absolute os relative path
+     * Static function
+     * @return a full filename from a short filename.
+     * @param aShortFileName = the short filename, which can be a relative
+     * @param aProjectPath = the curr project absolute path (can be empty)
+     * or absolute path, and can include env variable reference ( ${envvar} expression )
+     * if the short filename path is relative, it is expected relative to the project path
+     * or (if aProjectPath is empty or if the file does not exist)
+     * relative to kicad.pro (in template)
+     * If aShortFileName is absolute return aShortFileName
      */
-    static const wxString MakeFullFileName( const wxString& aShortFileName );
+    static const wxString MakeFullFileName( const wxString& aShortFileName,
+                                            const wxString& aProjectPath );
 };
 
 #endif      // WORKSHEET_SHAPE_BUILDER_H

pcbnew/pcbnew_config.cpp

@@ -267,17 +267,11 @@ bool PCB_EDIT_FRAME::LoadProjectSettings()
 
     // Load the page layout decr file, from the filename stored in
     // BASE_SCREEN::m_PageLayoutDescrFileName, read in config project file
-    // If empty, the default descr is loaded
+    // If empty, or not existing, the default descr is loaded
     WORKSHEET_LAYOUT& pglayout = WORKSHEET_LAYOUT::GetTheInstance();
-    wxString pg_fullfilename = ExpandEnvVarSubstitutions(
-                                BASE_SCREEN::m_PageLayoutDescrFileName );
-
-    if( !pg_fullfilename.IsEmpty() )
-    {
-        // When the page layout filename is not absolute, therefore
-        // relative to the current project, make it absolute
-        pg_fullfilename = Prj().AbsolutePath( pg_fullfilename  );
-    }
+    wxString pg_fullfilename = WORKSHEET_LAYOUT::MakeFullFileName(
+                                    BASE_SCREEN::m_PageLayoutDescrFileName,
+                                    Prj().GetProjectPath() );
 
     pglayout.SetPageLayout( pg_fullfilename );