From 5dce8323e8ea1c2be5ea8348b574f2c8d5baf934 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sat, 24 Dec 2022 17:04:34 +0000 Subject: [PATCH] KIGFX::REPAINT is not enough if the item isn't in the view. Fixes https://gitlab.com/kicad/code/kicad/issues/11656 Fixes https://gitlab.com/kicad/code/kicad/issues/13217 --- pcbnew/pad.cpp | 6 +-- pcbnew/pcb_base_frame.cpp | 38 ++++++++++------- pcbnew/pcb_edit_frame.cpp | 56 +++++++++++++++++++------- pcbnew/pcb_painter.cpp | 5 ++- pcbnew/tools/zone_filler_tool.cpp | 4 +- pcbnew/widgets/appearance_controls.cpp | 14 +++---- 6 files changed, 83 insertions(+), 40 deletions(-) diff --git a/pcbnew/pad.cpp b/pcbnew/pad.cpp index 64501a78d2..56b31560ad 100644 --- a/pcbnew/pad.cpp +++ b/pcbnew/pad.cpp @@ -249,12 +249,12 @@ bool PAD::FlashLayer( LSET aLayers ) const bool PAD::FlashLayer( int aLayer ) const { - if( aLayer != UNDEFINED_LAYER && !IsOnLayer( static_cast( aLayer ) ) ) - return false; - if( aLayer == UNDEFINED_LAYER ) return true; + if( !IsOnLayer( static_cast( aLayer ) ) ) + return false; + if( GetAttribute() == PAD_ATTRIB::NPTH && IsCopperLayer( aLayer ) ) { if( GetShape() == PAD_SHAPE::CIRCLE && GetDrillShape() == PAD_DRILL_SHAPE_CIRCLE ) diff --git a/pcbnew/pcb_base_frame.cpp b/pcbnew/pcb_base_frame.cpp index 39155280fd..43fb25ae9e 100644 --- a/pcbnew/pcb_base_frame.cpp +++ b/pcbnew/pcb_base_frame.cpp @@ -966,23 +966,25 @@ void PCB_BASE_FRAME::CommonSettingsChanged( bool aEnvVarsChanged, bool aTextVars renderSettings->LoadColors( GetColorSettings( true ) ); renderSettings->LoadDisplayOptions( GetDisplayOptions() ); - GetCanvas()->GetView()->UpdateAllItemsConditionally( KIGFX::REPAINT, - [&]( KIGFX::VIEW_ITEM* aItem ) -> bool + // Note: KIGFX::REPAINT isn't enough for things that go from invisible to visible as + // they won't be found in the view layer's itemset for re-painting. + GetCanvas()->GetView()->UpdateAllItemsConditionally( + [&]( KIGFX::VIEW_ITEM* aItem ) -> int { if( dynamic_cast( aItem ) ) { - return true; // ratsnest display + return KIGFX::ALL; // ratsnest display } else if( dynamic_cast( aItem ) ) { - return true; // track, arc & via clearance display + return KIGFX::REPAINT; // track, arc & via clearance display } else if( dynamic_cast( aItem ) ) { - return true; // pad clearance display + return KIGFX::REPAINT; // pad clearance display } - return false; + return 0; } ); GetCanvas()->GetView()->UpdateAllItems( KIGFX::COLOR ); @@ -1068,8 +1070,10 @@ void PCB_BASE_FRAME::ActivateGalCanvas() void PCB_BASE_FRAME::SetDisplayOptions( const PCB_DISPLAY_OPTIONS& aOptions, bool aRefresh ) { - bool hcChanged = m_displayOptions.m_ContrastModeDisplay != aOptions.m_ContrastModeDisplay; - m_displayOptions = aOptions; + bool hcChanged = m_displayOptions.m_ContrastModeDisplay != aOptions.m_ContrastModeDisplay; + bool hcVisChanged = m_displayOptions.m_ContrastModeDisplay == HIGH_CONTRAST_MODE::HIDDEN + || aOptions.m_ContrastModeDisplay == HIGH_CONTRAST_MODE::HIDDEN; + m_displayOptions = aOptions; EDA_DRAW_PANEL_GAL* canvas = GetCanvas(); KIGFX::PCB_VIEW* view = static_cast( canvas->GetView() ); @@ -1081,21 +1085,27 @@ void PCB_BASE_FRAME::SetDisplayOptions( const PCB_DISPLAY_OPTIONS& aOptions, boo // Vias on a restricted layer set must be redrawn when high contrast mode is changed if( hcChanged ) { - GetCanvas()->GetView()->UpdateAllItemsConditionally( KIGFX::REPAINT, - []( KIGFX::VIEW_ITEM* aItem ) -> bool + // Note: KIGFX::REPAINT isn't enough for things that go from invisible to visible as + // they won't be found in the view layer's itemset for re-painting. + GetCanvas()->GetView()->UpdateAllItemsConditionally( + [&]( KIGFX::VIEW_ITEM* aItem ) -> bool { if( PCB_VIA* via = dynamic_cast( aItem ) ) { - return via->GetViaType() == VIATYPE::BLIND_BURIED + if( via->GetViaType() == VIATYPE::BLIND_BURIED || via->GetViaType() == VIATYPE::MICROVIA - || via->GetRemoveUnconnected(); + || via->GetRemoveUnconnected() ) + { + return hcVisChanged ? KIGFX::ALL : KIGFX::REPAINT; + } } else if( PAD* pad = dynamic_cast( aItem ) ) { - return pad->GetRemoveUnconnected(); + if( pad->GetRemoveUnconnected() ) + return hcVisChanged ? KIGFX::ALL : KIGFX::REPAINT; } - return false; + return 0; } ); } diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index e286aeebcd..687ba6a4fe 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -1161,8 +1161,11 @@ void PCB_EDIT_FRAME::ShowBoardSetupDialog( const wxString& aInitialPage ) if( settings->m_Display.m_PadClearance ) return KIGFX::REPAINT; + // Note: KIGFX::REPAINT isn't enough for things that go from invisible + // to visible as they won't be found in the view layer's itemset for + // re-painting. if( ( GetBoard()->GetVisibleLayers() & maskAndPasteLayers ).any() ) - return KIGFX::REPAINT; + return KIGFX::ALL; } EDA_TEXT* text = dynamic_cast( aItem ); @@ -1276,18 +1279,38 @@ void PCB_EDIT_FRAME::SetActiveLayer( PCB_LAYER_ID aLayer ) GetCanvas()->SetFocus(); // allow capture of hotkeys GetCanvas()->SetHighContrastLayer( aLayer ); - GetCanvas()->GetView()->UpdateAllItemsConditionally( KIGFX::REPAINT, - [&]( KIGFX::VIEW_ITEM* aItem ) -> bool + GetCanvas()->GetView()->UpdateAllItemsConditionally( + [&]( KIGFX::VIEW_ITEM* aItem ) -> int { - if( PCB_VIA* via = dynamic_cast( aItem ) ) + BOARD_ITEM* item = dynamic_cast( aItem ); + + if( !item ) + return 0; + + // Note: KIGFX::REPAINT isn't enough for things that go from invisible to visible + // as they won't be found in the view layer's itemset for re-painting. + if( GetDisplayOptions().m_ContrastModeDisplay == HIGH_CONTRAST_MODE::HIDDEN ) { + if( item->IsOnLayer( oldLayer ) || item->IsOnLayer( aLayer ) ) + return KIGFX::ALL; + } + + if( item->Type() == PCB_VIA_T ) + { + PCB_VIA* via = static_cast( item ); + // Vias on a restricted layer set must be redrawn when the active layer // is changed - return ( via->GetViaType() == VIATYPE::BLIND_BURIED || - via->GetViaType() == VIATYPE::MICROVIA ); + if( via->GetViaType() == VIATYPE::BLIND_BURIED + || via->GetViaType() == VIATYPE::MICROVIA ) + { + return KIGFX::REPAINT; + } } - else if( PAD* pad = dynamic_cast( aItem ) ) + else if( item->Type() == PCB_PAD_T ) { + PAD* pad = static_cast( item ); + // Clearances could be layer-dependent so redraw them when the active layer // is changed if( GetPcbNewSettings()->m_Display.m_PadClearance ) @@ -1298,27 +1321,32 @@ void PCB_EDIT_FRAME::SetActiveLayer( PCB_LAYER_ID aLayer ) if( pad->GetAttribute() == PAD_ATTRIB::SMD ) { if( ( oldLayer == F_Cu || aLayer == F_Cu ) && pad->IsOnLayer( F_Cu ) ) - return true; + return KIGFX::REPAINT; if( ( oldLayer == B_Cu || aLayer == B_Cu ) && pad->IsOnLayer( B_Cu ) ) - return true; + return KIGFX::REPAINT; + } + else if( pad->IsOnLayer( oldLayer ) || pad->IsOnLayer( aLayer ) ) + { + return KIGFX::REPAINT; } - - return true; } } - else if( PCB_TRACK* track = dynamic_cast( aItem ) ) + else if( item->Type() == PCB_TRACE_T || item->Type() == PCB_ARC_T ) { + PCB_TRACK* track = dynamic_cast( item ); + // Clearances could be layer-dependent so redraw them when the active layer // is changed if( GetPcbNewSettings()->m_Display.m_TrackClearance ) { // Tracks aren't particularly expensive to draw, but it's an easy check. - return track->IsOnLayer( oldLayer ) || track->IsOnLayer( aLayer ); + if( track->IsOnLayer( oldLayer ) || track->IsOnLayer( aLayer ) ) + return KIGFX::REPAINT; } } - return false; + return 0; } ); GetCanvas()->Refresh(); diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp index 0695297f8e..fc26921999 100644 --- a/pcbnew/pcb_painter.cpp +++ b/pcbnew/pcb_painter.cpp @@ -1231,7 +1231,7 @@ void PCB_PAINTER::draw( const PAD* aPad, int aLayer ) { drawShape = aPad->FlashLayer( m_pcbSettings.GetPrintLayers() ); } - else if( aPad->FlashLayer( board->GetEnabledLayers() ) ) + else if( aPad->FlashLayer( board->GetVisibleLayers() & board->GetEnabledLayers() ) ) { drawShape = true; } @@ -1483,6 +1483,9 @@ void PCB_PAINTER::draw( const PAD* aPad, int aLayer ) if( IsCopperLayer( activeLayer ) ) flashActiveLayer = aPad->FlashLayer( activeLayer ); + if( !board->GetVisibleLayers().test( activeLayer ) ) + flashActiveLayer = false; + if( flashActiveLayer || aPad->GetDrillSize().x ) { if( aPad->GetAttribute() == PAD_ATTRIB::NPTH ) diff --git a/pcbnew/tools/zone_filler_tool.cpp b/pcbnew/tools/zone_filler_tool.cpp index fc6bdd2676..52bd4c7bdf 100644 --- a/pcbnew/tools/zone_filler_tool.cpp +++ b/pcbnew/tools/zone_filler_tool.cpp @@ -401,7 +401,9 @@ void ZONE_FILLER_TOOL::rebuildConnectivity() void ZONE_FILLER_TOOL::refresh() { - canvas()->GetView()->UpdateAllItemsConditionally( KIGFX::REPAINT, + // Note: KIGFX::REPAINT isn't enough for things that go from invisible to visible as + // they won't be found in the view layer's itemset for re-painting. + canvas()->GetView()->UpdateAllItemsConditionally( KIGFX::ALL, [&]( KIGFX::VIEW_ITEM* aItem ) -> bool { if( PCB_VIA* via = dynamic_cast( aItem ) ) diff --git a/pcbnew/widgets/appearance_controls.cpp b/pcbnew/widgets/appearance_controls.cpp index 5118ea3092..9f4c31e74a 100644 --- a/pcbnew/widgets/appearance_controls.cpp +++ b/pcbnew/widgets/appearance_controls.cpp @@ -1260,15 +1260,15 @@ void APPEARANCE_CONTROLS::setVisibleLayers( LSET aLayers ) { m_frame->GetBoard()->SetVisibleLayers( aLayers ); - view->UpdateAllItemsConditionally( KIGFX::REPAINT, + // Note: KIGFX::REPAINT isn't enough for things that go from invisible to visible as + // they won't be found in the view layer's itemset for repainting. + view->UpdateAllItemsConditionally( KIGFX::ALL, []( KIGFX::VIEW_ITEM* aItem ) -> bool { - if( PCB_VIA* via = dynamic_cast( aItem ) ) - return via->GetRemoveUnconnected(); - else if( PAD* pad = dynamic_cast( aItem ) ) - return pad->GetRemoveUnconnected(); - - return false; + // Items rendered to composite layers (such as LAYER_PAD_TH) must be redrawn + // whether they're optionally flashed or not (as the layer being hidden/shown + // might be the last layer the item is visible on). + return dynamic_cast( aItem ) || dynamic_cast( aItem ); } ); } }