From 947c73f23b100ab6dccee2473124a8ae93e52a43 Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Wed, 15 Jun 2016 10:26:41 +0200 Subject: [PATCH] Gerbview: try to fix a crash (hard to reproduce) which someting happens when closing gerbview. Fix incorrect calculation of bounding boxes. --- gerbview/class_gbr_layout.cpp | 11 ++++- gerbview/class_gerber_draw_item.cpp | 11 ++++- gerbview/class_gerber_file_image.cpp | 2 + gerbview/class_gerber_file_image_list.cpp | 55 +++++++++-------------- gerbview/class_gerber_file_image_list.h | 15 ++----- gerbview/class_gerbview_layer_widget.cpp | 4 +- gerbview/clear_gbr_drawlayers.cpp | 26 ++--------- gerbview/draw_gerber_screen.cpp | 4 -- gerbview/gerbview_frame.cpp | 12 ++--- gerbview/toolbars_gerber.cpp | 2 +- 10 files changed, 60 insertions(+), 82 deletions(-) diff --git a/gerbview/class_gbr_layout.cpp b/gerbview/class_gbr_layout.cpp index 86cd9dbc3d..7715e3bba5 100644 --- a/gerbview/class_gbr_layout.cpp +++ b/gerbview/class_gbr_layout.cpp @@ -59,6 +59,7 @@ bool GBR_LAYOUT::IsLayerPrintable( int aLayer ) const EDA_RECT GBR_LAYOUT::ComputeBoundingBox() { EDA_RECT bbox; + bool first_item = true; for( int layer = 0; layer < GERBER_DRAWLAYERS_COUNT; ++layer ) { @@ -68,7 +69,15 @@ EDA_RECT GBR_LAYOUT::ComputeBoundingBox() continue; for( GERBER_DRAW_ITEM* item = gerber->GetItemsList(); item; item = item->Next() ) - bbox.Merge( item->GetBoundingBox() ); + { + if( first_item ) + { + bbox = item->GetBoundingBox(); + first_item = false; + } + else + bbox.Merge( item->GetBoundingBox() ); + } } SetBoundingBox( bbox ); diff --git a/gerbview/class_gerber_draw_item.cpp b/gerbview/class_gerber_draw_item.cpp index 348a7f992d..e5593eaf68 100644 --- a/gerbview/class_gerber_draw_item.cpp +++ b/gerbview/class_gerber_draw_item.cpp @@ -207,8 +207,15 @@ const EDA_RECT GERBER_DRAW_ITEM::GetBoundingBox() const bbox.Inflate( m_Size.x / 2, m_Size.y / 2 ); - bbox.SetOrigin( GetABPosition( bbox.GetOrigin() ) ); - bbox.SetEnd( GetABPosition( bbox.GetEnd() ) ); + // calculate the corners coordinates in current gerber axis orientations + wxPoint org = GetABPosition( bbox.GetOrigin() ); + wxPoint end = GetABPosition( bbox.GetEnd() ); + + // Set the corners position: + bbox.SetOrigin( org ); + bbox.SetEnd( end ); + bbox.Normalize(); + return bbox; } diff --git a/gerbview/class_gerber_file_image.cpp b/gerbview/class_gerber_file_image.cpp index a21f826715..6d21aa1e69 100644 --- a/gerbview/class_gerber_file_image.cpp +++ b/gerbview/class_gerber_file_image.cpp @@ -107,6 +107,8 @@ GERBER_FILE_IMAGE::GERBER_FILE_IMAGE( int aLayer ) GERBER_FILE_IMAGE::~GERBER_FILE_IMAGE() { + m_Drawings.DeleteAll(); + for( unsigned ii = 0; ii < DIM( m_Aperture_List ); ii++ ) { delete m_Aperture_List[ii]; diff --git a/gerbview/class_gerber_file_image_list.cpp b/gerbview/class_gerber_file_image_list.cpp index fb68710449..d9f64d7959 100644 --- a/gerbview/class_gerber_file_image_list.cpp +++ b/gerbview/class_gerber_file_image_list.cpp @@ -47,17 +47,13 @@ GERBER_FILE_IMAGE_LIST::GERBER_FILE_IMAGE_LIST() m_GERBER_List.push_back( NULL ); } + GERBER_FILE_IMAGE_LIST::~GERBER_FILE_IMAGE_LIST() { - ClearList(); - - for( unsigned layer = 0; layer < m_GERBER_List.size(); ++layer ) - { - delete m_GERBER_List[layer]; - m_GERBER_List[layer] = NULL; - } + DeleteAllImages(); } + GERBER_FILE_IMAGE* GERBER_FILE_IMAGE_LIST::GetGbrImage( int aIdx ) { if( (unsigned)aIdx < m_GERBER_List.size() ) @@ -66,11 +62,10 @@ GERBER_FILE_IMAGE* GERBER_FILE_IMAGE_LIST::GetGbrImage( int aIdx ) return NULL; } -/** - * creates a new, empty GERBER_FILE_IMAGE* at index aIdx +/* creates a new, empty GERBER_FILE_IMAGE* at index aIdx * or at the first free location if aIdx < 0 - * @param aIdx = the location to use ( 0 ... GERBER_DRAWLAYERS_COUNT-1 ) - * @return true if the index used, or -1 if no room to add image + * aIdx = the index of graphic layer to use, or -1 to uses the first free graphic layer + * return the index actually used, or -1 if no room to add image */ int GERBER_FILE_IMAGE_LIST::AddGbrImage( GERBER_FILE_IMAGE* aGbrImage, int aIdx ) { @@ -80,7 +75,7 @@ int GERBER_FILE_IMAGE_LIST::AddGbrImage( GERBER_FILE_IMAGE* aGbrImage, int aIdx { for( idx = 0; idx < (int)m_GERBER_List.size(); idx++ ) { - if( !IsUsed( idx ) ) + if( m_GERBER_List[idx] == NULL ) break; } } @@ -94,23 +89,24 @@ int GERBER_FILE_IMAGE_LIST::AddGbrImage( GERBER_FILE_IMAGE* aGbrImage, int aIdx } -// remove all loaded data in list, but do not delete empty images -// (can be reused) -void GERBER_FILE_IMAGE_LIST::ClearList() +void GERBER_FILE_IMAGE_LIST::DeleteAllImages() { - for( unsigned layer = 0; layer < m_GERBER_List.size(); ++layer ) - ClearImage( layer ); + for( unsigned idx = 0; idx < m_GERBER_List.size(); ++idx ) + DeleteImage( idx ); } -// remove the loaded data of image aIdx, but do not delete it -void GERBER_FILE_IMAGE_LIST::ClearImage( int aIdx ) + +void GERBER_FILE_IMAGE_LIST::DeleteImage( int aIdx ) { - if( aIdx >= 0 && aIdx < (int)m_GERBER_List.size() && m_GERBER_List[aIdx] ) - { - m_GERBER_List[aIdx]->InitToolTable(); - m_GERBER_List[aIdx]->ResetDefaultValues(); - m_GERBER_List[aIdx]->m_InUse = false; - } + // Ensure the index is valid: + if( aIdx < 0 || aIdx >= int( m_GERBER_List.size() ) ) + return; + + // delete image aIdx + GERBER_FILE_IMAGE* gbr_image = GetGbrImage( aIdx ); + + delete gbr_image; + m_GERBER_List[ aIdx ] = NULL; } // Build a name for image aIdx which can be used in layers manager @@ -127,7 +123,7 @@ const wxString GERBER_FILE_IMAGE_LIST::GetDisplayName( int aIdx ) // if a X2 FileFunction info is found // or (if no FileFunction info) // * - if( gerber && IsUsed(aIdx ) ) + if( gerber ) { wxFileName fn( gerber->m_FileName ); wxString filename = fn.GetFullName(); @@ -168,14 +164,7 @@ const wxString GERBER_FILE_IMAGE_LIST::GetDisplayName( int aIdx ) return name; } -// return true if image is used (loaded and not cleared) -bool GERBER_FILE_IMAGE_LIST::IsUsed( int aIdx ) -{ - if( aIdx >= 0 && aIdx < (int)m_GERBER_List.size() ) - return m_GERBER_List[aIdx] != NULL && m_GERBER_List[aIdx]->m_InUse; - return false; -} // Helper function, for std::sort. // Sort loaded images by Z order priority, if they have the X2 FileFormat info diff --git a/gerbview/class_gerber_file_image_list.h b/gerbview/class_gerber_file_image_list.h index f6c5e5b6fb..6a10cb112a 100644 --- a/gerbview/class_gerber_file_image_list.h +++ b/gerbview/class_gerber_file_image_list.h @@ -28,7 +28,6 @@ #include #include -//#include #include #include @@ -83,15 +82,15 @@ public: /** - * remove all loaded data in list + * remove all loaded data in list, and delete all images. Memory is freed */ - void ClearList(); + void DeleteAllImages(); /** - * remove the loaded data of image aIdx + * delete the loaded data of image aIdx. Memory is freed * @param aIdx = the index ( 0 ... GERBER_DRAWLAYERS_COUNT-1 ) */ - void ClearImage( int aIdx ); + void DeleteImage( int aIdx ); /** * @return a name for image aIdx which can be used in layers manager @@ -106,12 +105,6 @@ public: */ const wxString GetDisplayName( int aIdx ); - /** - * @return true if image is used (loaded and with items) - * @param aIdx = the index ( 0 ... GERBER_DRAWLAYERS_COUNT-1 ) - */ - bool IsUsed( int aIdx ); - /** * Sort loaded images by Z order priority, if they have the X2 FileFormat info * (SortImagesByZOrder updates the graphic layer of these items) diff --git a/gerbview/class_gerbview_layer_widget.cpp b/gerbview/class_gerbview_layer_widget.cpp index 4fbd972887..2e77c6b5af 100644 --- a/gerbview/class_gerbview_layer_widget.cpp +++ b/gerbview/class_gerbview_layer_widget.cpp @@ -307,11 +307,11 @@ void GERBER_LAYER_WIDGET::OnRenderEnable( int aId, bool isEnabled ) /* * Virtual Function useAlternateBitmap * return true if bitmaps shown in Render layer list - * must be alternate bitmaps, or false to use "normal" bitmaps + * must be alternate bitmap (when a gerber iùmage is loaded), or false to use "normal" bitmap */ bool GERBER_LAYER_WIDGET::useAlternateBitmap(int aRow) { - return g_GERBER_List.IsUsed( aRow ); + return g_GERBER_List.GetGbrImage( aRow ) != NULL; } /* diff --git a/gerbview/clear_gbr_drawlayers.cpp b/gerbview/clear_gbr_drawlayers.cpp index 139790ea47..3ed9504d71 100644 --- a/gerbview/clear_gbr_drawlayers.cpp +++ b/gerbview/clear_gbr_drawlayers.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2013 Jean-Pierre Charras, jp.charras at wanadoo.fr + * Copyright (C) 2016 Jean-Pierre Charras, jp.charras at wanadoo.fr * Copyright (C) 1992-2016 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or @@ -47,22 +47,10 @@ bool GERBVIEW_FRAME::Clear_DrawLayers( bool query ) return false; } - for( int layer = 0; layer < GERBER_DRAWLAYERS_COUNT; ++layer ) - { - GERBER_FILE_IMAGE* gerber = g_GERBER_List.GetGbrImage( layer ); - - if( gerber == NULL ) // Graphic layer not yet used - continue; - - gerber->m_Drawings.DeleteAll(); - } - - g_GERBER_List.ClearList(); + g_GERBER_List.DeleteAllImages(); GetGerberLayout()->SetBoundingBox( EDA_RECT() ); - SetScreen( new GBR_SCREEN( GetPageSettings().GetSizeIU() ) ); - setActiveLayer( 0 ); m_LayersManager->UpdateLayerIcons(); syncLayerBox(); @@ -82,15 +70,9 @@ void GERBVIEW_FRAME::Erase_Current_DrawLayer( bool query ) SetCurItem( NULL ); - GERBER_FILE_IMAGE* gerber = g_GERBER_List.GetGbrImage( layer ); + g_GERBER_List.DeleteImage( layer ); - if( gerber ) // gerber == NULL should not occur - gerber->m_Drawings.DeleteAll(); - - g_GERBER_List.ClearImage( layer ); - - GetScreen()->SetModify(); - m_canvas->Refresh(); m_LayersManager->UpdateLayerIcons(); syncLayerBox(); + m_canvas->Refresh(); } diff --git a/gerbview/draw_gerber_screen.cpp b/gerbview/draw_gerber_screen.cpp index b2eede142d..d43c9230dd 100644 --- a/gerbview/draw_gerber_screen.cpp +++ b/gerbview/draw_gerber_screen.cpp @@ -144,8 +144,4 @@ void GERBVIEW_FRAME::RedrawActiveWindow( wxDC* DC, bool EraseBg ) m_canvas->CallMouseCapture( DC, wxDefaultPosition, false ); m_canvas->DrawCrossHair( DC ); - - // Display the filename and the layer name (found in the gerber files, if any) - // relative to the active layer - UpdateTitleAndInfo(); } diff --git a/gerbview/gerbview_frame.cpp b/gerbview/gerbview_frame.cpp index 3dff05ac18..9ab1f05b84 100644 --- a/gerbview/gerbview_frame.cpp +++ b/gerbview/gerbview_frame.cpp @@ -247,11 +247,11 @@ double GERBVIEW_FRAME::BestZoom() bbox.SetSize( wxSize( Mils2iu( pagesize.x ), Mils2iu( pagesize.y ) ) ); } + // Compute best zoom: wxSize size = m_canvas->GetClientSize(); - - double x = (double) bbox.GetWidth() * 1.1 / (double) size.x; - double y = (double) bbox.GetHeight() * 1.1 / (double) size.y; - double best_zoom = std::max( x, y ); + double x = (double) bbox.GetWidth() / (double) size.x; + double y = (double) bbox.GetHeight() / (double) size.y; + double best_zoom = std::max( x, y ) * 1.1; SetScrollCenterPosition( bbox.Centre() ); @@ -373,10 +373,10 @@ int GERBVIEW_FRAME::getNextAvailableLayer( int aLayer ) const { GERBER_FILE_IMAGE* gerber = g_GERBER_List.GetGbrImage( layer ); - if( gerber == NULL || gerber->m_FileName.IsEmpty() ) + if( gerber == NULL ) // this graphic layer is available: use it return layer; - ++layer; + ++layer; // try next graphic layer if( layer >= GERBER_DRAWLAYERS_COUNT ) layer = 0; diff --git a/gerbview/toolbars_gerber.cpp b/gerbview/toolbars_gerber.cpp index 3507ceb22c..8284c03346 100644 --- a/gerbview/toolbars_gerber.cpp +++ b/gerbview/toolbars_gerber.cpp @@ -319,7 +319,7 @@ void GERBVIEW_FRAME::OnUpdateSelectDCode( wxUpdateUIEvent& aEvent ) void GERBVIEW_FRAME::OnUpdateLayerSelectBox( wxUpdateUIEvent& aEvent ) { - if( m_SelLayerBox && (m_SelLayerBox->GetSelection() != getActiveLayer()) ) + if( m_SelLayerBox->GetSelection() != getActiveLayer() ) { m_SelLayerBox->SetSelection( getActiveLayer() ); }