JAL-2438 refactored checks for 'no features displayed'

diff --git a/src/jalview/renderer/seqfeatures/FeatureColourFinder.java b/src/jalview/renderer/seqfeatures/FeatureColourFinder.java
index 0c18d39..d4a545b 100644 (file)
--- a/src/jalview/renderer/seqfeatures/FeatureColourFinder.java
+++ b/src/jalview/renderer/seqfeatures/FeatureColourFinder.java
@@ -47,6 +47,10 @@ public class FeatureColourFinder
    * transparency, this class provides a dummy 'offscreen' graphics context
    * where multiple feature colours can be overlaid and the combined colour read
    * back.
+   * <p>
+   * This method is not thread-safe when transparency is applied, since a shared
+   * BufferedImage would be used by all threads to hold the composite colour at
+   * a position. Each thread should use a separate instance of this class.
    * 
    * @param defaultColour
    * @param seq
@@ -57,17 +61,11 @@ public class FeatureColourFinder
   public Color findFeatureColour(Color defaultColour, SequenceI seq,
           int column)
   {
-    if (!((FeatureRendererModel) featureRenderer).hasRenderOrder())
+    if (noFeaturesDisplayed())
     {
       return defaultColour;
     }
 
-    FeaturesDisplayedI displayed = featureRenderer.getFeaturesDisplayed();
-    if (displayed == null || displayed.getVisibleFeatureCount() == 0)
-    {
-      return defaultColour; // nothing to see here folks
-    }
-
     Graphics g = null;
 
     /*
@@ -95,4 +93,31 @@ public class FeatureColourFinder
     }
     return c;
   }
+
+  /**
+   * Answers true if feature display is turned off, or there are no features
+   * configured to be visible
+   * 
+   * @return
+   */
+  boolean noFeaturesDisplayed()
+  {
+    if (!featureRenderer.getViewport().isShowSequenceFeatures())
+    {
+      return true;
+    }
+
+    if (!((FeatureRendererModel) featureRenderer).hasRenderOrder())
+    {
+      return true;
+    }
+
+    FeaturesDisplayedI displayed = featureRenderer.getFeaturesDisplayed();
+    if (displayed == null || displayed.getVisibleFeatureCount() == 0)
+    {
+      return true;
+    }
+
+    return false;
+  }
 }

diff --git a/test/jalview/renderer/seqfeatures/FeatureColourFinderTest.java b/test/jalview/renderer/seqfeatures/FeatureColourFinderTest.java
index 127b6c2..a7f2289 100644 (file)
--- a/test/jalview/renderer/seqfeatures/FeatureColourFinderTest.java
+++ b/test/jalview/renderer/seqfeatures/FeatureColourFinderTest.java
@@ -1,6 +1,8 @@
 package jalview.renderer.seqfeatures;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 
 import jalview.api.FeatureColourI;
 import jalview.datamodel.SequenceFeature;
@@ -384,4 +386,52 @@ public class FeatureColourFinderTest
     c = finder.findFeatureColour(Color.pink, seq, 10);
     assertEquals(c, new Color(102, 223, 70));
   }
+
+  @Test(groups = "Functional")
+  public void testNoFeaturesDisplayed()
+  {
+    /*
+     * no features on alignment to render
+     */
+    assertTrue(finder.noFeaturesDisplayed());
+
+    /*
+     * add a feature
+     * it will be automatically set visible but the viewport
+     * is still configured not to show features
+     */
+    seq.addSequenceFeature(new SequenceFeature("Metal", "Metal", 2, 12,
+            Float.NaN, "MetalGroup"));
+    FeatureColour red = new FeatureColour(Color.red);
+    fr.setColour("Metal", red);
+    fr.featuresAdded();
+    assertTrue(finder.noFeaturesDisplayed());
+
+    /*
+     * turn on feature display
+     */
+    av.setShowSequenceFeatures(true);
+    assertFalse(finder.noFeaturesDisplayed());
+
+    /*
+     * turn off display of Metal
+     */
+    Object[][] data = new Object[1][];
+    data[0] = new Object[] { "Metal", red, false };
+    fr.setFeaturePriority(data);
+    assertTrue(finder.noFeaturesDisplayed());
+
+    /*
+     * turn display of Metal back on
+     */
+    fr.setVisible("Metal");
+    assertFalse(finder.noFeaturesDisplayed());
+
+    /*
+     * turn off MetalGroup - has no effect here since the group of a
+     * sequence feature instance is independent of its type
+     */
+    fr.setGroupVisibility("MetalGroup", false);
+    assertFalse(finder.noFeaturesDisplayed());
+  }
 }