← Back to team overview

kicad-developers team mailing list archive

Re: [RFC] Change to object visibility system for usability/clarity

 

Wayne,

in attachment all 3 patches I have, rebased.

Cheers,
Andrzej


On 02/21/2018 05:45 PM, Wayne Stambaugh wrote:
Andrzej,

This patch does not apply cleanly.  Please rebase and resubmit it when
you get a chance.

Thanks,

Wayne

On 2/21/2018 11:27 AM, Andrzej Wolski wrote:
Wayne,

I have attached that patch to bug report whom which you messaged me,
thought you will see it.

I'm attaching it again in this email.

Cheers,
Andrzej


On 02/21/2018 05:20 PM, Wayne Stambaugh wrote:
Andrzej,

You have not replied to my last message about your original patch that
you sent to the developers mailing list[1].  I need that one so I can
apply and test the rest of your layer visibility patches.  If you want
these to make it into rc1, please send me this patch as an attachment
soon.  I am planning to branch rc1 by Friday at the latest.

Cheers,

Wayne


[1]: https://lists.launchpad.net/kicad-developers/msg34009.html

On 2/19/2018 10:05 AM, Andrzej Wolski wrote:
Patch (in attachment) for "Tracks" turned out to be very simple.
If it gets accepted, I could make more of these.

Andrzej


On 02/19/2018 02:12 PM, Andrzej Wolski wrote:
It would be also nice to have separate controls for:
1. Tracks
2. Drawing primitives (graphic lines, arcs, and circles)
3. Zones
4. Polygons
5. Text (now there is control only for footprint text)
6. Dimensions
7. Plated holes

That would make Render tab (almost) complete.

Andrzej

W dniu 2018-02-18 o 21:00, Jon Evans pisze:
Hi all,

Right now the behavior of the "Layer" and "Render" tabs of the layers
widget are confusing to users, resulting in complaints on the forum
and some bug reports:

https://bugs.launchpad.net/kicad/+bug/1748181
https://bugs.launchpad.net/kicad/+bug/1743890

I could take a crack at fixing this (before or after 5.0 depending on
what the complexity ends up being) but before I write any code I
wanted to propose how I think it should work.

I think the visibility of any object should be the AND of layer
visibility and render visibility.

To get there:

1) In the Render tab, get rid of the distinction between front/back.
For example "Pads Back" and "Pads Front" becomes just "Pads"

2) Change the visibility code so that an object is visible if (a) the
associated Render setting is turned on for the type of object, and
(b) at least one of the layers the object is on is enabled in the
Layers tab.

3) (optionally) Rename "Render" to something more friendly like
"Items" or "Item Types" to make it more clear to the user that this
is where they can turn off the display of various types of items as
opposed to various layerse

If this plan is OK, I will start working out the details of how to
get there.  Right now the Render tab is directly controlling the
visibility of certain "GAL Layers" but unfortunately the set of
objects that appears on one GAL layer is not always equal to the set
of objects that the user would expect to turn on and off, as seen by
the bug reports.  So, there will have to be some additional logic
created to manage these settings beyond just turning on and off
layers in the GAL.

-Jon


_______________________________________________
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

_______________________________________________
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

>From 7b764ae730d80feac0c2c4560359fb2cdaa32b1d Mon Sep 17 00:00:00 2001
From: Andrzej Wolski <awolski.kicad@xxxxxxxxx>
Date: Sat, 17 Feb 2018 19:03:22 +0100
Subject: [PATCH 1/3] Fix pads and footprints rendering switches behavior
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.7.4"

This is a multi-part message in MIME format.
--------------2.7.4
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


Pads and footprints rendering switches in Render tab were working incorrectly, as described in bug report:
https://bugs.launchpad.net/kicad/+bug/1743890

This patch fixes it and makes GAL behave as the legacy canvas.

Fixes: lp:1743890
---
 pcbnew/class_edge_mod.cpp     | 26 ++++++++++++++++++++++++++
 pcbnew/class_edge_mod.h       |  3 +++
 pcbnew/class_pad.cpp          | 22 +++++++++++++++++++++-
 pcbnew/class_text_mod.cpp     | 31 ++++++++++++++++++++++---------
 pcbnew/class_text_mod.h       |  2 ++
 pcbnew/pcb_draw_panel_gal.cpp | 14 --------------
 6 files changed, 74 insertions(+), 24 deletions(-)


--------------2.7.4
Content-Type: text/x-patch; name="0001-Fix-pads-and-footprints-rendering-switches-behavior.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Fix-pads-and-footprints-rendering-switches-behavior.patch"

diff --git a/pcbnew/class_edge_mod.cpp b/pcbnew/class_edge_mod.cpp
index da78be8..6c1ae3e 100644
--- a/pcbnew/class_edge_mod.cpp
+++ b/pcbnew/class_edge_mod.cpp
@@ -49,6 +49,8 @@
 #include <class_module.h>
 #include <class_edge_mod.h>
 
+#include <view/view.h>
+
 #include <stdio.h>
 
 EDGE_MODULE::EDGE_MODULE( MODULE* parent, STROKE_T aShape ) :
@@ -321,6 +323,12 @@ void EDGE_MODULE::Flip( const wxPoint& aCentre )
     SetLayer( FlipLayer( GetLayer() ) );
 }
 
+bool EDGE_MODULE::IsParentFlipped() const
+{
+    if( GetParent() &&  GetParent()->GetLayer() == B_Cu )
+        return true;
+    return false;
+}
 
 void EDGE_MODULE::Mirror( wxPoint aCentre, bool aMirrorAroundXAxis )
 {
@@ -394,3 +402,21 @@ void EDGE_MODULE::Move( const wxPoint& aMoveVector )
 
     SetDrawCoord();
 }
+
+unsigned int EDGE_MODULE::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
+{
+    const int HIDE = std::numeric_limits<unsigned int>::max();
+
+    if( !aView )
+        return 0;
+
+    // Handle Render tab switches
+    if( !IsParentFlipped() && !aView->IsLayerVisible( LAYER_MOD_FR ) )
+        return HIDE;
+
+    if( IsParentFlipped() && !aView->IsLayerVisible( LAYER_MOD_BK ) )
+        return HIDE;
+
+    // Other layers are shown without any conditions
+    return 0;
+}
diff --git a/pcbnew/class_edge_mod.h b/pcbnew/class_edge_mod.h
index 571b692..38dcac1 100644
--- a/pcbnew/class_edge_mod.h
+++ b/pcbnew/class_edge_mod.h
@@ -88,6 +88,8 @@ public:
      */
     void Flip( const wxPoint& aCentre ) override;
 
+    bool IsParentFlipped() const;
+
     void SetStart0( const wxPoint& aPoint )     { m_Start0 = aPoint; }
     const wxPoint& GetStart0() const            { return m_Start0; }
 
@@ -126,6 +128,7 @@ public:
 
     EDA_ITEM* Clone() const override;
 
+    virtual unsigned int ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const override;
 
 #if defined(DEBUG)
     void Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
diff --git a/pcbnew/class_pad.cpp b/pcbnew/class_pad.cpp
index cdbef32..379cf45 100644
--- a/pcbnew/class_pad.cpp
+++ b/pcbnew/class_pad.cpp
@@ -36,6 +36,7 @@
 #include <base_units.h>
 #include <bitmaps.h>
 
+#include <view/view.h>
 #include <pcbnew.h>
 
 #include <class_board.h>
@@ -1274,6 +1275,25 @@ void D_PAD::ViewGetLayers( int aLayers[], int& aCount ) const
 
 unsigned int D_PAD::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
 {
+    const int HIDE = std::numeric_limits<unsigned int>::max();
+
+    // Handle Render tab switches
+    if( ( GetAttribute() == PAD_ATTRIB_STANDARD || GetAttribute() == PAD_ATTRIB_HOLE_NOT_PLATED )
+         && !aView->IsLayerVisible( LAYER_PADS_TH ) )
+        return HIDE;
+
+    if( !IsFlipped() && !aView->IsLayerVisible( LAYER_MOD_FR ) )
+        return HIDE;
+
+    if( IsFlipped() && !aView->IsLayerVisible( LAYER_MOD_BK ) )
+        return HIDE;
+
+    if( IsFrontLayer( ( PCB_LAYER_ID )aLayer ) && !aView->IsLayerVisible( LAYER_PAD_FR ) )
+        return HIDE;
+
+    if( IsBackLayer( ( PCB_LAYER_ID )aLayer ) && !aView->IsLayerVisible( LAYER_PAD_BK ) )
+        return HIDE;
+
     // Netnames will be shown only if zoom is appropriate
     if( IsNetnameLayer( aLayer ) )
     {
@@ -1282,7 +1302,7 @@ unsigned int D_PAD::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
         // Pad sizes can be zero briefly when someone is typing a number like "0.5"
         // in the pad properties dialog
         if( divisor == 0 )
-            return UINT_MAX;
+            return HIDE;
 
         return ( Millimeter2iu( 100 ) / divisor );
     }
diff --git a/pcbnew/class_text_mod.cpp b/pcbnew/class_text_mod.cpp
index df45cf2..2728d74 100644
--- a/pcbnew/class_text_mod.cpp
+++ b/pcbnew/class_text_mod.cpp
@@ -145,6 +145,13 @@ void TEXTE_MODULE::Flip( const wxPoint& aCentre )
     SetLocalCoord();
 }
 
+bool TEXTE_MODULE::IsParentFlipped() const
+{
+    if( GetParent() &&  GetParent()->GetLayer() == B_Cu )
+        return true;
+    return false;
+}
+
 
 void TEXTE_MODULE::Mirror( const wxPoint& aCentre, bool aMirrorAroundXAxis )
 {
@@ -473,27 +480,33 @@ void TEXTE_MODULE::ViewGetLayers( int aLayers[], int& aCount ) const
 
 unsigned int TEXTE_MODULE::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
 {
-    const int MAX = std::numeric_limits<unsigned int>::max();
+    const int HIDE = std::numeric_limits<unsigned int>::max();
 
     if( !aView )
         return 0;
 
+    // Handle Render tab switches
     if( ( m_Type == TEXT_is_VALUE || m_Text == wxT( "%V" ) )
             && !aView->IsLayerVisible( LAYER_MOD_VALUES ) )
-        return MAX;
+        return HIDE;
 
     if( ( m_Type == TEXT_is_REFERENCE || m_Text == wxT( "%R" ) )
             && !aView->IsLayerVisible( LAYER_MOD_REFERENCES ) )
-        return MAX;
+        return HIDE;
+
+    if( !IsParentFlipped() && !aView->IsLayerVisible( LAYER_MOD_FR ) )
+        return HIDE;
+
+    if( IsParentFlipped() && !aView->IsLayerVisible( LAYER_MOD_BK ) )
+        return HIDE;
 
-    if( IsFrontLayer( m_Layer ) && ( !aView->IsLayerVisible( LAYER_MOD_TEXT_FR ) ||
-                                     !aView->IsLayerVisible( LAYER_MOD_FR ) ) )
-        return MAX;
+    if( IsFrontLayer( m_Layer ) && !aView->IsLayerVisible( LAYER_MOD_TEXT_FR ) )
+        return HIDE;
 
-    if( IsBackLayer( m_Layer ) && ( !aView->IsLayerVisible( LAYER_MOD_TEXT_BK ) ||
-                                    !aView->IsLayerVisible( LAYER_MOD_BK ) ) )
-        return MAX;
+    if( IsBackLayer( m_Layer ) && !aView->IsLayerVisible( LAYER_MOD_TEXT_BK ) )
+        return HIDE;
 
+    // Other layers are shown without any conditions
     return 0;
 }
 
diff --git a/pcbnew/class_text_mod.h b/pcbnew/class_text_mod.h
index eb4318e..11a42e8 100644
--- a/pcbnew/class_text_mod.h
+++ b/pcbnew/class_text_mod.h
@@ -104,6 +104,8 @@ public:
     /// Flip entity during module flip
     void Flip( const wxPoint& aCentre ) override;
 
+    bool IsParentFlipped() const;
+
     /// Mirror text position in footprint edition
     /// the text itself is not mirrored, and the layer not modified,
     /// only position is mirrored.
diff --git a/pcbnew/pcb_draw_panel_gal.cpp b/pcbnew/pcb_draw_panel_gal.cpp
index 15ea7c5..d9476fc 100644
--- a/pcbnew/pcb_draw_panel_gal.cpp
+++ b/pcbnew/pcb_draw_panel_gal.cpp
@@ -446,26 +446,12 @@ void PCB_DRAW_PANEL_GAL::setDefaultLayerDeps()
     m_view->SetRequired( LAYER_PADS_NETNAMES, LAYER_PADS_TH );
 
     // Front modules
-    m_view->SetRequired( LAYER_PAD_FR, LAYER_MOD_FR );
     m_view->SetRequired( LAYER_MOD_TEXT_FR, LAYER_MOD_FR );
     m_view->SetRequired( LAYER_PAD_FR_NETNAMES, LAYER_PAD_FR );
-    m_view->SetRequired( F_Adhes, LAYER_PAD_FR );
-    m_view->SetRequired( F_Paste, LAYER_PAD_FR );
-    m_view->SetRequired( F_Mask, LAYER_PAD_FR );
-    m_view->SetRequired( F_CrtYd, LAYER_MOD_FR );
-    m_view->SetRequired( F_Fab, LAYER_MOD_FR );
-    m_view->SetRequired( F_SilkS, LAYER_MOD_FR );
 
     // Back modules
-    m_view->SetRequired( LAYER_PAD_BK, LAYER_MOD_BK );
     m_view->SetRequired( LAYER_MOD_TEXT_BK, LAYER_MOD_BK );
     m_view->SetRequired( LAYER_PAD_BK_NETNAMES, LAYER_PAD_BK );
-    m_view->SetRequired( B_Adhes, LAYER_PAD_BK );
-    m_view->SetRequired( B_Paste, LAYER_PAD_BK );
-    m_view->SetRequired( B_Mask, LAYER_PAD_BK );
-    m_view->SetRequired( B_CrtYd, LAYER_MOD_BK );
-    m_view->SetRequired( B_Fab, LAYER_MOD_BK );
-    m_view->SetRequired( B_SilkS, LAYER_MOD_BK );
 
     m_view->SetLayerTarget( LAYER_GP_OVERLAY , KIGFX::TARGET_OVERLAY );
     m_view->SetLayerDisplayOnly( LAYER_GP_OVERLAY ) ;

--------------2.7.4--


>From 7c7df2ae98513f33a5d1345f9d3f8132824e9b5d Mon Sep 17 00:00:00 2001
From: Andrzej Wolski <awolski.kicad@xxxxxxxxx>
Date: Sat, 17 Feb 2018 23:53:44 +0100
Subject: [PATCH 2/3] Do not draw pads on hidden copper layers.
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.7.4"

This is a multi-part message in MIME format.
--------------2.7.4
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit

---
 pcbnew/class_pad.cpp          | 5 +++++
 pcbnew/pcb_draw_panel_gal.cpp | 2 ++
 2 files changed, 7 insertions(+)


--------------2.7.4
Content-Type: text/x-patch; name="0002-Do-not-draw-pads-on-hidden-copper-layers.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0002-Do-not-draw-pads-on-hidden-copper-layers.patch"

diff --git a/pcbnew/class_pad.cpp b/pcbnew/class_pad.cpp
index 379cf45..75540aa 100644
--- a/pcbnew/class_pad.cpp
+++ b/pcbnew/class_pad.cpp
@@ -1276,6 +1276,7 @@ void D_PAD::ViewGetLayers( int aLayers[], int& aCount ) const
 unsigned int D_PAD::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
 {
     const int HIDE = std::numeric_limits<unsigned int>::max();
+    BOARD* board = GetBoard();
 
     // Handle Render tab switches
     if( ( GetAttribute() == PAD_ATTRIB_STANDARD || GetAttribute() == PAD_ATTRIB_HOLE_NOT_PLATED )
@@ -1294,6 +1295,10 @@ unsigned int D_PAD::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
     if( IsBackLayer( ( PCB_LAYER_ID )aLayer ) && !aView->IsLayerVisible( LAYER_PAD_BK ) )
         return HIDE;
 
+    // Only draw the pad if at least one of the layers it crosses is being displayed
+    if( board && !( board->GetVisibleLayers() & GetLayerSet() ).any() )
+        return HIDE;
+
     // Netnames will be shown only if zoom is appropriate
     if( IsNetnameLayer( aLayer ) )
     {
diff --git a/pcbnew/pcb_draw_panel_gal.cpp b/pcbnew/pcb_draw_panel_gal.cpp
index d9476fc..d4f7f2a 100644
--- a/pcbnew/pcb_draw_panel_gal.cpp
+++ b/pcbnew/pcb_draw_panel_gal.cpp
@@ -446,10 +446,12 @@ void PCB_DRAW_PANEL_GAL::setDefaultLayerDeps()
     m_view->SetRequired( LAYER_PADS_NETNAMES, LAYER_PADS_TH );
 
     // Front modules
+    m_view->SetRequired( LAYER_PAD_FR, F_Cu );
     m_view->SetRequired( LAYER_MOD_TEXT_FR, LAYER_MOD_FR );
     m_view->SetRequired( LAYER_PAD_FR_NETNAMES, LAYER_PAD_FR );
 
     // Back modules
+    m_view->SetRequired( LAYER_PAD_BK, B_Cu );
     m_view->SetRequired( LAYER_MOD_TEXT_BK, LAYER_MOD_BK );
     m_view->SetRequired( LAYER_PAD_BK_NETNAMES, LAYER_PAD_BK );
 

--------------2.7.4--


>From 049a234af0e0f855e7870137069caaf6fef8b504 Mon Sep 17 00:00:00 2001
From: Andrzej Wolski <awolski.kicad@xxxxxxxxx>
Date: Mon, 19 Feb 2018 15:48:22 +0100
Subject: [PATCH 3/3] Add tracks display control to Render tab
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.7.4"

This is a multi-part message in MIME format.
--------------2.7.4
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit

---
 pcbnew/class_track.cpp      | 9 ++++++++-
 pcbnew/pcb_layer_widget.cpp | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)


--------------2.7.4
Content-Type: text/x-patch; name="0003-Add-tracks-display-control-to-Render-tab.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0003-Add-tracks-display-control-to-Render-tab.patch"

diff --git a/pcbnew/class_track.cpp b/pcbnew/class_track.cpp
index 1d5e3e7..1c5f69b 100644
--- a/pcbnew/class_track.cpp
+++ b/pcbnew/class_track.cpp
@@ -45,6 +45,7 @@
 #include <base_units.h>
 #include <msgpanel.h>
 #include <bitmaps.h>
+#include <view/view.h>
 
 /**
  * Function ShowClearance
@@ -670,7 +671,8 @@ void TRACK::Draw( EDA_DRAW_PANEL* panel, wxDC* aDC, GR_DRAWMODE aDrawMode,
     auto frame = static_cast<PCB_BASE_FRAME*> ( panel->GetParent() );
     auto color = frame->Settings().Colors().GetLayerColor( m_Layer );
 
-    if( brd->IsLayerVisible( m_Layer ) == false && !( aDrawMode & GR_HIGHLIGHT ) )
+    if( ( !brd->IsLayerVisible( m_Layer ) || !brd->IsElementVisible( LAYER_TRACKS ) )
+        && !( aDrawMode & GR_HIGHLIGHT ) )
         return;
 
 #ifdef USE_WX_OVERLAY
@@ -798,6 +800,11 @@ void TRACK::ViewGetLayers( int aLayers[], int& aCount ) const
 
 unsigned int TRACK::ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const
 {
+    const int HIDE = std::numeric_limits<unsigned int>::max();
+
+    if( !aView->IsLayerVisible( LAYER_TRACKS ) )
+        return HIDE;
+
     // Netnames will be shown only if zoom is appropriate
     if( IsNetnameLayer( aLayer ) )
     {
diff --git a/pcbnew/pcb_layer_widget.cpp b/pcbnew/pcb_layer_widget.cpp
index f874e25..a2a184b 100644
--- a/pcbnew/pcb_layer_widget.cpp
+++ b/pcbnew/pcb_layer_widget.cpp
@@ -60,6 +60,7 @@ const LAYER_WIDGET::ROW PCB_LAYER_WIDGET::s_render_rows[] = {
 #define NOCOLOR COLOR4D::UNSPECIFIED    // specify rows that do not have a color selector icon
 
          // text                id                      color       tooltip
+    RR( _( "Tracks" ),          LAYER_TRACKS,         NOCOLOR,  _( "Show tracks" ) ),
     RR( _( "Through Via" ),     LAYER_VIA_THROUGH,    WHITE,    _( "Show through vias" ) ),
     RR( _( "Bl/Buried Via" ),   LAYER_VIA_BBLIND,     WHITE,    _( "Show blind or buried vias" )  ),
     RR( _( "Micro Via" ),       LAYER_VIA_MICROVIA,   WHITE,    _( "Show micro vias") ),

--------------2.7.4--



Follow ups

References