JAL-2323 render disulfide bond correctly on overview and structure

diff --git a/src/jalview/renderer/seqfeatures/FeatureRenderer.java b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
index b007365..07d97d0 100644 (file)
--- a/src/jalview/renderer/seqfeatures/FeatureRenderer.java
+++ b/src/jalview/renderer/seqfeatures/FeatureRenderer.java
@@ -188,15 +188,20 @@ public class FeatureRenderer extends FeatureRendererModel
   }
 
   /**
-   * This is used by the Molecule Viewer and Overview to get the accurate colour
-   * of the rendered sequence
+   * This is used by Structure Viewers and the Overview Window to get the
+   * feature colour of the rendered sequence, returned as an RGB value
+   * 
+   * @param defaultColour
+   * @param seq
+   * @param column
+   * @return
    */
-  public synchronized int findFeatureColour(int initialCol,
+  public synchronized int findFeatureColour(int defaultColour,
           final SequenceI seq, int column)
   {
     if (!av.isShowSequenceFeatures())
     {
-      return initialCol;
+      return defaultColour;
     }
 
     SequenceFeature[] sequenceFeatures = seq.getSequenceFeatures();
@@ -223,7 +228,7 @@ public class FeatureRenderer extends FeatureRendererModel
 
     if (lastSequenceFeatures == null || sfSize == 0)
     {
-      return initialCol;
+      return defaultColour;
     }
 
     if (jalview.util.Comparison.isGap(lastSeq.getCharAt(column)))
@@ -244,7 +249,7 @@ public class FeatureRenderer extends FeatureRendererModel
 
     if (offscreenImage != null)
     {
-      offscreenImage.setRGB(0, 0, initialCol);
+      offscreenImage.setRGB(0, 0, defaultColour);
       drawSequence(offscreenImage.getGraphics(), lastSeq, column, column, 0);
 
       return offscreenImage.getRGB(0, 0);
@@ -255,7 +260,7 @@ public class FeatureRenderer extends FeatureRendererModel
 
       if (currentColour == null)
       {
-        return initialCol;
+        return defaultColour;
       }
       else
       {
@@ -275,6 +280,19 @@ public class FeatureRenderer extends FeatureRendererModel
 
   int epos;
 
+  /**
+   * Draws the sequence on the graphics context, or just determines the colour
+   * that would be drawn (if flag offscreenrender is true).
+   * 
+   * @param g
+   * @param seq
+   * @param start
+   *          start column (or sequence position in offscreenrender mode)
+   * @param end
+   *          end column (not used in offscreenrender mode)
+   * @param y1
+   *          vertical offset at which to draw on the graphics
+   */
   public synchronized void drawSequence(Graphics g, final SequenceI seq,
           int start, int end, int y1)
   {
@@ -312,12 +330,9 @@ public class FeatureRenderer extends FeatureRendererModel
     }
 
     sfSize = lastSequenceFeatures.length;
-    String type;
-    for (int renderIndex = 0; renderIndex < renderOrder.length; renderIndex++)
+    for (String type : renderOrder)
     {
-      type = renderOrder[renderIndex];
-
-      if (type == null || !showFeatureOfType(type))
+      if (!showFeatureOfType(type))
       {
         continue;
       }
@@ -332,16 +347,16 @@ public class FeatureRenderer extends FeatureRendererModel
           continue;
         }
 
-        if (featureGroups != null
-                && sequenceFeature.featureGroup != null
-                && sequenceFeature.featureGroup.length() != 0
-                && featureGroups.containsKey(sequenceFeature.featureGroup)
-                && !featureGroups.get(sequenceFeature.featureGroup)
-                        .booleanValue())
+        if (featureGroupNotShown(sequenceFeature))
         {
           continue;
         }
 
+        /*
+         * check feature overlaps the visible part of the alignment, 
+         * unless doing offscreenRender (to the Overview window or a 
+         * structure viewer) which is not limited 
+         */
         if (!offscreenRender
                 && (sequenceFeature.getBegin() > epos || sequenceFeature
                         .getEnd() < spos))
@@ -349,35 +364,42 @@ public class FeatureRenderer extends FeatureRendererModel
           continue;
         }
 
+        Color featureColour = getColour(sequenceFeature);
+        boolean isDisulfideBond = "disulfide bond".equals(sequenceFeature.type);
         if (offscreenRender && offscreenImage == null)
         {
-          if (sequenceFeature.begin <= start
-                  && sequenceFeature.end >= start)
+          /*
+           * offscreen mode with no image (image is only needed if transparency 
+           * is applied to feature colours) - just check feature is rendered at 
+           * the requested position (start == sequence position in this mode)
+           */
+          boolean featureIsAtPosition = sequenceFeature.begin <= start
+                  && sequenceFeature.end >= start;
+          if (isDisulfideBond)
+          {
+            featureIsAtPosition = sequenceFeature.begin == start
+                    || sequenceFeature.end == start;
+          }
+          if (featureIsAtPosition)
           {
             // this is passed out to the overview and other sequence renderers
             // (e.g. molecule viewer) to get displayed colour for rendered
             // sequence
-            currentColour = new Integer(getColour(sequenceFeature).getRGB());
+            currentColour = new Integer(featureColour.getRGB());
             // used to be retreived from av.featuresDisplayed
             // currentColour = av.featuresDisplayed
             // .get(sequenceFeatures[sfindex].type);
 
           }
         }
-        else if (sequenceFeature.type.equals("disulfide bond"))
+        else if (isDisulfideBond)
         {
           renderFeature(g, seq, seq.findIndex(sequenceFeature.begin) - 1,
-                  seq.findIndex(sequenceFeature.begin) - 1,
-                  getColour(sequenceFeature)
-                  // new Color(((Integer) av.featuresDisplayed
-                  // .get(sequenceFeatures[sfindex].type)).intValue())
-                  , start, end, y1);
+                  seq.findIndex(sequenceFeature.begin) - 1, featureColour,
+                  start, end, y1);
           renderFeature(g, seq, seq.findIndex(sequenceFeature.end) - 1,
-                  seq.findIndex(sequenceFeature.end) - 1,
-                  getColour(sequenceFeature)
-                  // new Color(((Integer) av.featuresDisplayed
-                  // .get(sequenceFeatures[sfindex].type)).intValue())
-                  , start, end, y1);
+                  seq.findIndex(sequenceFeature.end) - 1, featureColour,
+                  start, end, y1);
 
         }
         else if (showFeature(sequenceFeature))
@@ -388,19 +410,17 @@ public class FeatureRenderer extends FeatureRendererModel
             renderScoreFeature(g, seq,
                     seq.findIndex(sequenceFeature.begin) - 1,
                     seq.findIndex(sequenceFeature.end) - 1,
-                    getColour(sequenceFeature), start, end, y1,
+                    featureColour, start, end, y1,
                     normaliseScore(sequenceFeature));
           }
           else
           {
             renderFeature(g, seq, seq.findIndex(sequenceFeature.begin) - 1,
                     seq.findIndex(sequenceFeature.end) - 1,
-                    getColour(sequenceFeature), start, end, y1);
+                    featureColour, start, end, y1);
           }
         }
-
       }
-
     }
 
     if (transparency != 1.0f && g != null)
@@ -412,6 +432,24 @@ public class FeatureRenderer extends FeatureRendererModel
   }
 
   /**
+   * Answers true if the feature belongs to a feature group which is not
+   * currently displayed, else false
+   * 
+   * @param sequenceFeature
+   * @return
+   */
+  protected boolean featureGroupNotShown(
+          final SequenceFeature sequenceFeature)
+  {
+    return featureGroups != null
+            && sequenceFeature.featureGroup != null
+            && sequenceFeature.featureGroup.length() != 0
+            && featureGroups.containsKey(sequenceFeature.featureGroup)
+            && !featureGroups.get(sequenceFeature.featureGroup)
+                    .booleanValue();
+  }
+
+  /**
    * Called when alignment in associated view has new/modified features to
    * discover and display.
    * 

diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
index 4ac4804..7ff9adf 100644 (file)
--- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
+++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
@@ -564,9 +564,16 @@ public abstract class FeatureRendererModel implements
     return fc.isColored(sequenceFeature);
   }
 
+  /**
+   * Answers true if the feature type is currently selected to be displayed,
+   * else false
+   * 
+   * @param type
+   * @return
+   */
   protected boolean showFeatureOfType(String type)
   {
-    return av.getFeaturesDisplayed().isVisible(type);
+    return type == null ? false : av.getFeaturesDisplayed().isVisible(type);
   }
 
   @Override