JAL-2816 tweaks to feature visibility checks

diff --git a/src/jalview/renderer/seqfeatures/FeatureRenderer.java b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
index 6687e6a..795cd36 100644 (file)
--- a/src/jalview/renderer/seqfeatures/FeatureRenderer.java
+++ b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
@@ -304,7 +304,10 @@ public class FeatureRenderer extends FeatureRendererModel
       List<SequenceFeature> overlaps = seq.getFeatures().findFeatures(
               visiblePositions.getBegin(), visiblePositions.getEnd(), type);
 
-      // filterFeaturesForDisplay(overlaps, fc);
+      if (fc.isSimpleColour())
+      {
+        filterFeaturesForDisplay(overlaps);
+      }
 
       for (SequenceFeature sf : overlaps)
       {

diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
index 6461748..3a97067 100644 (file)
--- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
+++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
@@ -586,7 +586,8 @@ public abstract class FeatureRendererModel
    */
   protected boolean showFeatureOfType(String type)
   {
-    return type == null ? false : av.getFeaturesDisplayed().isVisible(type);
+    return type == null ? false : (av.getFeaturesDisplayed() == null ? true
+            : av.getFeaturesDisplayed().isVisible(type));
   }
 
   @Override
@@ -1000,23 +1001,19 @@ public abstract class FeatureRendererModel
 
   /**
    * Removes from the list of features any that duplicate the location of a
-   * feature of the same type (unless feature is filtered out, or a graduated
-   * colour scheme or colour by label is applied). Should be used only for
-   * features of the same feature colour (which normally implies the same
-   * feature type).
+   * 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.
    * 
    * @param features
-   * @param fc
    */
-  public void filterFeaturesForDisplay(List<SequenceFeature> features,
-          FeatureColourI fc)
+  public void filterFeaturesForDisplay(List<SequenceFeature> features)
   {
     if (features.isEmpty())
     {
       return;
     }
     SequenceFeatures.sortFeatures(features, true);
-    boolean simpleColour = fc == null || fc.isSimpleColour();
     SequenceFeature lastFeature = null;
 
     Iterator<SequenceFeature> it = features.iterator();
@@ -1030,15 +1027,12 @@ public abstract class FeatureRendererModel
        * (checking type and isContactFeature as a fail-safe here, although
        * currently they are guaranteed to match in this context)
        */
-      if (simpleColour)
+      if (lastFeature != null && sf.getBegin() == lastFeature.getBegin()
+              && sf.getEnd() == lastFeature.getEnd()
+              && sf.isContactFeature() == lastFeature.isContactFeature()
+              && sf.getType().equals(lastFeature.getType()))
       {
-        if (lastFeature != null && sf.getBegin() == lastFeature.getBegin()
-                && sf.getEnd() == lastFeature.getEnd()
-                && sf.isContactFeature() == lastFeature.isContactFeature()
-                && sf.getType().equals(lastFeature.getType()))
-        {
-          it.remove();
-        }
+        it.remove();
       }
       lastFeature = sf;
     }

diff --git a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
index d3cddf9..438feba 100644 (file)
--- a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
+++ b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
@@ -264,7 +264,7 @@ public class FeatureRendererTest
     FeatureRenderer fr = new FeatureRenderer(av);
 
     List<SequenceFeature> features = new ArrayList<>();
-    fr.filterFeaturesForDisplay(features, null); // empty list, does nothing
+    fr.filterFeaturesForDisplay(features); // empty list, does nothing
 
     SequenceI seq = av.getAlignment().getSequenceAt(0);
     SequenceFeature sf1 = new SequenceFeature("Cath", "", 6, 8, Float.NaN,
@@ -297,7 +297,7 @@ public class FeatureRendererTest
      * filter out duplicate (co-located) features
      * note: which gets removed is not guaranteed
      */
-    fr.filterFeaturesForDisplay(features, new FeatureColour(Color.blue));
+    fr.filterFeaturesForDisplay(features);
     assertEquals(features.size(), 3);
     assertTrue(features.contains(sf1) || features.contains(sf4));
     assertFalse(features.contains(sf1) && features.contains(sf4));
@@ -306,58 +306,17 @@ public class FeatureRendererTest
     assertTrue(features.contains(sf5));
 
     /*
-     * hide group 3 - sf3 is removed, sf2 is retained
-     */
-    fr.setGroupVisibility("group3", false);
-    features = seq.getSequenceFeatures();
-    fr.filterFeaturesForDisplay(features, new FeatureColour(Color.blue));
-    assertEquals(features.size(), 3);
-    assertTrue(features.contains(sf1) || features.contains(sf4));
-    assertFalse(features.contains(sf1) && features.contains(sf4));
-    assertTrue(features.contains(sf2));
-    assertFalse(features.contains(sf3));
-    assertTrue(features.contains(sf5));
-
-    /*
-     * hide group 2, show group 3 - sf2 is removed, sf3 is retained
+     * hide groups 2 and 3 makes no difference to this method
      */
     fr.setGroupVisibility("group2", false);
-    fr.setGroupVisibility("group3", true);
+    fr.setGroupVisibility("group3", false);
     features = seq.getSequenceFeatures();
-    fr.filterFeaturesForDisplay(features, null);
+    fr.filterFeaturesForDisplay(features);
     assertEquals(features.size(), 3);
     assertTrue(features.contains(sf1) || features.contains(sf4));
     assertFalse(features.contains(sf1) && features.contains(sf4));
-    assertFalse(features.contains(sf2));
-    assertTrue(features.contains(sf3));
-    assertTrue(features.contains(sf5));
-
-    /*
-     * no filtering of co-located features with graduated colour scheme
-     * filterFeaturesForDisplay does _not_ check colour threshold
-     * sf2 is removed as its group is hidden
-     */
-    features = seq.getSequenceFeatures();
-    fr.filterFeaturesForDisplay(features, new FeatureColour(Color.black,
-            Color.white, 0f, 1f));
-    assertEquals(features.size(), 4);
-    assertTrue(features.contains(sf1));
-    assertTrue(features.contains(sf3));
-    assertTrue(features.contains(sf4));
-    assertTrue(features.contains(sf5));
-
-    /*
-     * co-located features with colour by label
-     * should not get filtered
-     */
-    features = seq.getSequenceFeatures();
-    FeatureColour fc = new FeatureColour(Color.black);
-    fc.setColourByLabel(true);
-    fr.filterFeaturesForDisplay(features, fc);
-    assertEquals(features.size(), 4);
-    assertTrue(features.contains(sf1));
-    assertTrue(features.contains(sf3));
-    assertTrue(features.contains(sf4));
+    assertTrue(features.contains(sf2) || features.contains(sf3));
+    assertFalse(features.contains(sf2) && features.contains(sf3));
     assertTrue(features.contains(sf5));
   }
 }