From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Tue, 23 Oct 2018 14:31:50 +0000 (+0100)
Subject: JAL-3139 check group visibility when removing 'redundant' features
X-Git-Tag: Release_2_11_0~17^2~85^2
X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=refs%2Fheads%2Fbug%2FJAL-3139colocatedFeatures

JAL-3139 check group visibility when removing 'redundant' features
---

diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
index 553f813..5db8751 100644
--- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
+++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
@@ -1029,11 +1029,11 @@ public abstract class FeatureRendererModel
   }
 
   /**
-   * Removes from the list of features any that duplicate the location of a
-   * feature of the same type. Should be used only for features of the same,
-   * simple, feature colour (which normally implies the same feature type). Does
-   * not check visibility settings for feature type or feature group. No
-   * filtering is done if transparency, or any feature filters, are in force.
+   * Removes from the list of features any whose group is not shown, or that are
+   * visible and duplicate the location of a visible feature of the same type.
+   * Should be used only for features of the same, simple, feature colour (which
+   * normally implies the same feature type). No filtering is done if
+   * transparency, or any feature filters, are in force.
    * 
    * @param features
    */
@@ -1057,6 +1057,11 @@ public abstract class FeatureRendererModel
     while (it.hasNext())
     {
       SequenceFeature sf = it.next();
+      if (featureGroupNotShown(sf))
+      {
+        it.remove();
+        continue;
+      }
 
       /*
        * a feature is redundant for rendering purposes if it has the
@@ -1064,7 +1069,8 @@ public abstract class FeatureRendererModel
        * (checking type and isContactFeature as a fail-safe here, although
        * currently they are guaranteed to match in this context)
        */
-      if (lastFeature != null && sf.getBegin() == lastFeature.getBegin()
+      if (lastFeature != null
+              && sf.getBegin() == lastFeature.getBegin()
               && sf.getEnd() == lastFeature.getEnd()
               && sf.isContactFeature() == lastFeature.isContactFeature()
               && sf.getType().equals(lastFeature.getType()))
diff --git a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
index 11b129e..a9e3754 100644
--- a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
+++ b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
@@ -328,18 +328,18 @@ public class FeatureRendererTest
     SequenceI seq = av.getAlignment().getSequenceAt(0);
     SequenceFeature sf1 = new SequenceFeature("Cath", "", 6, 8, Float.NaN,
             "group1");
-    seq.addSequenceFeature(sf1);
     SequenceFeature sf2 = new SequenceFeature("Cath", "", 5, 11, 2f,
             "group2");
-    seq.addSequenceFeature(sf2);
     SequenceFeature sf3 = new SequenceFeature("Cath", "", 5, 11, 3f,
             "group3");
-    seq.addSequenceFeature(sf3);
     SequenceFeature sf4 = new SequenceFeature("Cath", "", 6, 8, 4f,
             "group4");
-    seq.addSequenceFeature(sf4);
     SequenceFeature sf5 = new SequenceFeature("Cath", "", 6, 9, 5f,
             "group4");
+    seq.addSequenceFeature(sf1);
+    seq.addSequenceFeature(sf2);
+    seq.addSequenceFeature(sf3);
+    seq.addSequenceFeature(sf4);
     seq.addSequenceFeature(sf5);
 
     fr.findAllFeatures(true);
@@ -365,17 +365,17 @@ public class FeatureRendererTest
     assertTrue(features.contains(sf5));
 
     /*
-     * hide groups 2 and 3 makes no difference to this method
+     * features in hidden groups are removed
      */
     fr.setGroupVisibility("group2", false);
     fr.setGroupVisibility("group3", false);
     features = seq.getSequenceFeatures();
     fr.filterFeaturesForDisplay(features);
-    assertEquals(features.size(), 3);
+    assertEquals(features.size(), 2);
     assertTrue(features.contains(sf1) || features.contains(sf4));
     assertFalse(features.contains(sf1) && features.contains(sf4));
-    assertTrue(features.contains(sf2) || features.contains(sf3));
-    assertFalse(features.contains(sf2) && features.contains(sf3));
+    assertFalse(features.contains(sf2));
+    assertFalse(features.contains(sf3));
     assertTrue(features.contains(sf5));
 
     /*