Merge branch 'develop' into features/JAL-2446NCList
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Sun, 23 Jul 2017 07:59:37 +0000 (08:59 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Sun, 23 Jul 2017 07:59:37 +0000 (08:59 +0100)
Conflicts:
src/jalview/renderer/seqfeatures/FeatureRenderer.java
src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java

1  2 
src/jalview/api/FeatureColourI.java
src/jalview/renderer/seqfeatures/FeatureRenderer.java
src/jalview/schemes/FeatureColour.java
src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
test/jalview/renderer/seqfeatures/FeatureColourFinderTest.java
test/jalview/schemes/FeatureColourTest.java

@@@ -146,7 -146,7 +146,9 @@@ public interface FeatureColour
    boolean hasThreshold();
  
    /**
--   * Returns the computed colour for the given sequence feature
++   * Returns the computed colour for the given sequence feature. Answers null if
++   * the score of this feature instance is outside the range to render (if any),
++   * i.e. lies below or above a configured threshold.
     * 
     * @param feature
     * @return
    Color getColor(SequenceFeature feature);
  
    /**
--   * Answers true if the feature has a simple colour, or is coloured by label,
--   * or has a graduated colour and the score of this feature instance is within
--   * the range to render (if any), i.e. does not lie below or above any
--   * threshold set.
--   * 
--   * @param feature
--   * @return
--   */
--  boolean isColored(SequenceFeature feature);
--
--  /**
     * Update the min-max range for a graduated colour scheme
     * 
     * @param min
@@@ -21,8 -21,6 +21,8 @@@
  package jalview.renderer.seqfeatures;
  
  import jalview.api.AlignViewportI;
 +import jalview.api.FeatureColourI;
 +import jalview.datamodel.Range;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceI;
  import jalview.util.Comparison;
@@@ -33,7 -31,6 +33,7 @@@ import java.awt.Color
  import java.awt.FontMetrics;
  import java.awt.Graphics;
  import java.awt.Graphics2D;
 +import java.util.List;
  
  public class FeatureRenderer extends FeatureRendererModel
  {
        return null;
      }
  
 -    SequenceFeature[] sequenceFeatures = seq.getSequenceFeatures();
 -
 -    if (sequenceFeatures == null || sequenceFeatures.length == 0)
 -    {
 -      return null;
 -    }
 -
      if (Comparison.isGap(seq.getCharAt(column)))
      {
        /*
         * returning null allows the colour scheme to provide gap colour
 -       * - normally white, but can be customised otherwise
 +       * - normally white, but can be customised
         */
        return null;
      }
        /*
         * simple case - just find the topmost rendered visible feature colour
         */
 -      renderedColour = findFeatureColour(seq, seq.findPosition(column));
 +      renderedColour = findFeatureColour(seq, column);
      }
      else
      {
            final SequenceI seq, int start, int end, int y1,
            boolean colourOnly)
    {
 -    SequenceFeature[] sequenceFeatures = seq.getSequenceFeatures();
 -    if (sequenceFeatures == null || sequenceFeatures.length == 0)
 +    /*
 +     * if columns are all gapped, or sequence has no features, nothing to do
 +     */
 +    Range visiblePositions = seq.findPositions(start+1, end+1);
 +    if (visiblePositions == null || !seq.getFeatures().hasFeatures())
      {
        return null;
      }
                transparency));
      }
  
 -    int startPos = seq.findPosition(start);
 -    int endPos = seq.findPosition(end);
 -
 -    int sfSize = sequenceFeatures.length;
      Color drawnColour = null;
  
      /*
          continue;
        }
  
 -      // loop through all features in sequence to find
 -      // current feature to render
 -      for (int sfindex = 0; sfindex < sfSize; sfindex++)
 +      FeatureColourI fc = getFeatureStyle(type);
 +      List<SequenceFeature> overlaps = seq.getFeatures().findFeatures(
 +              visiblePositions.getBegin(), visiblePositions.getEnd(), type);
 +
 +      filterFeaturesForDisplay(overlaps, fc);
 +
 +      for (SequenceFeature sf : overlaps)
        {
 -        final SequenceFeature sequenceFeature = sequenceFeatures[sfindex];
 -        if (!sequenceFeature.type.equals(type))
 +        Color featureColour = fc.getColor(sf);
++        if (featureColour == null)
+         {
++          // score feature outwith threshold for colouring
+           continue;
+         }
  
          /*
 -         * a feature type may be flagged as shown but the group 
 -         * an instance of it belongs to may be hidden
 +         * if feature starts/ends outside the visible range,
 +         * restrict to visible positions (or if a contact feature,
 +         * to a single position)
           */
 -        if (featureGroupNotShown(sequenceFeature))
 +        int visibleStart = sf.getBegin();
 +        if (visibleStart < visiblePositions.getBegin())
          {
 -          continue;
 +          visibleStart = sf.isContactFeature() ? sf.getEnd()
 +                  : visiblePositions.getBegin();
          }
 -
 -        /*
 -         * check feature overlaps the target range
 -         * TODO: efficient retrieval of features overlapping a range
 -         */
 -        if (sequenceFeature.getBegin() > endPos
 -                || sequenceFeature.getEnd() < startPos)
 +        int visibleEnd = sf.getEnd();
 +        if (visibleEnd > visiblePositions.getEnd())
          {
 -          continue;
 +          visibleEnd = sf.isContactFeature() ? sf.getBegin()
 +                  : visiblePositions.getEnd();
          }
  
 -        Color featureColour = getColour(sequenceFeature);
 -        if (featureColour == null)
 -        {
 -          // score feature outwith threshold for colouring
 -          continue;
 -        }
 +        int featureStartCol = seq.findIndex(visibleStart);
 +        int featureEndCol = sf.begin == sf.end ? featureStartCol : seq
 +                .findIndex(visibleEnd);
 +
-         if (sf.isContactFeature())
++        // Color featureColour = getColour(sequenceFeature);
 -        boolean isContactFeature = sequenceFeature.isContactFeature();
++        boolean isContactFeature = sf.isContactFeature();
+         if (isContactFeature)
          {
 -          boolean drawn = renderFeature(g, seq,
 -                  seq.findIndex(sequenceFeature.begin) - 1,
 -                  seq.findIndex(sequenceFeature.begin) - 1, featureColour,
 -                  start, end, y1, colourOnly);
 -          drawn |= renderFeature(g, seq,
 -                  seq.findIndex(sequenceFeature.end) - 1,
 -                  seq.findIndex(sequenceFeature.end) - 1, featureColour,
 -                  start, end, y1, colourOnly);
 +          boolean drawn = renderFeature(g, seq, featureStartCol - 1,
 +                  featureStartCol - 1, featureColour, start, end, y1,
 +                  colourOnly);
 +          drawn |= renderFeature(g, seq, featureEndCol - 1,
 +                  featureEndCol - 1, featureColour, start, end, y1,
 +                  colourOnly);
            if (drawn)
            {
              drawnColour = featureColour;
            }
          }
-         else if (showFeature(sf))
+         else
          {
 +          /*
 +           * showing feature score by height of colour
 +           * is not implemented as a selectable option 
 +           *
            if (av.isShowSequenceFeaturesHeight()
                    && !Float.isNaN(sequenceFeature.score))
            {
            }
            else
            {
 +          */
              boolean drawn = renderFeature(g, seq,
 -                    seq.findIndex(sequenceFeature.begin) - 1,
 -                    seq.findIndex(sequenceFeature.end) - 1, featureColour,
 +                    featureStartCol - 1,
 +                    featureEndCol - 1, featureColour,
                      start, end, y1, colourOnly);
              if (drawn)
              {
                drawnColour = featureColour;
              }
 -          }
 +          /*}*/
          }
        }
      }
    }
  
    /**
 -   * 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.
     * 
    }
  
    /**
 -   * Returns the sequence feature colour rendered at the given sequence
 -   * position, or null if none found. The feature of highest render order (i.e.
 -   * on top) is found, subject to both feature type and feature group being
 -   * visible, and its colour returned.
 +   * Returns the sequence feature colour rendered at the given column position,
 +   * or null if none found. The feature of highest render order (i.e. on top) is
 +   * found, subject to both feature type and feature group being visible, and
 +   * its colour returned.
 +   * <p>
 +   * Note this method does not check for a gap in the column so would return the
 +   * colour for features enclosing a gapped column. Check for gap before calling
 +   * if different behaviour is wanted.
     * 
     * @param seq
 -   * @param pos
 +   * @param column
 +   *          (1..)
     * @return
     */
 -  Color findFeatureColour(SequenceI seq, int pos)
 +  Color findFeatureColour(SequenceI seq, int column)
    {
 -    SequenceFeature[] sequenceFeatures = seq.getSequenceFeatures();
 -    if (sequenceFeatures == null || sequenceFeatures.length == 0)
 -    {
 -      return null;
 -    }
 -  
      /*
       * check for new feature added while processing
       */
          continue;
        }
  
 -      for (int sfindex = 0; sfindex < sequenceFeatures.length; sfindex++)
 +      List<SequenceFeature> overlaps = seq.findFeatures(column, column,
 +              type);
 +      for (SequenceFeature sequenceFeature : overlaps)
        {
 -        SequenceFeature sequenceFeature = sequenceFeatures[sfindex];
 -        if (!sequenceFeature.type.equals(type))
 +        if (!featureGroupNotShown(sequenceFeature))
          {
-           return getColour(sequenceFeature);
 -          continue;
 -        }
 -
 -        if (featureGroupNotShown(sequenceFeature))
 -        {
 -          continue;
 -        }
 -
 -        /*
 -         * check the column position is within the feature range
 -         * (or is one of the two contact positions for a contact feature)
 -         */
 -        boolean featureIsAtPosition = sequenceFeature.begin <= pos
 -                && sequenceFeature.end >= pos;
 -        if (sequenceFeature.isContactFeature())
 -        {
 -          featureIsAtPosition = sequenceFeature.begin == pos
 -                  || sequenceFeature.end == pos;
 -        }
 -        if (featureIsAtPosition)
 -        {
 -          return getColour(sequenceFeature);
++          Color col = getColour(sequenceFeature);
++          if (col != null)
++          {
++            return col;
++          }
          }
        }
      }
@@@ -551,12 -551,12 +551,23 @@@ public class FeatureColour implements F
        return getColour();
      }
  
--    // todo should we check for above/below threshold here?
++    /*
++     * graduated colour case, optionally with threshold
++     * (treating Float.NaN as within visible range here)
++     */
++    float scr = feature.getScore();
++    if (isAboveThreshold() && scr <= threshold)
++    {
++      return null;
++    }
++    if (isBelowThreshold() && scr >= threshold)
++    {
++      return null;
++    }
      if (range == 0.0)
      {
        return getMaxColour();
      }
--    float scr = feature.getScore();
      if (Float.isNaN(scr))
      {
        return getMinColour();
      return (isHighToLow) ? (base + range) : base;
    }
  
--  /**
--   * Answers true if the feature has a simple colour, or is coloured by label,
--   * or has a graduated colour and the score of this feature instance is within
--   * the range to render (if any), i.e. does not lie below or above any
--   * threshold set.
--   * 
--   * @param feature
--   * @return
--   */
--  @Override
--  public boolean isColored(SequenceFeature feature)
--  {
--    if (isColourByLabel() || !isGraduatedColour())
--    {
--      return true;
--    }
--
--    float val = feature.getScore();
--    if (Float.isNaN(val))
--    {
--      return true;
--    }
--    if (Float.isNaN(this.threshold))
--    {
--      return true;
--    }
--
--    if (isAboveThreshold() && val <= threshold)
--    {
--      return false;
--    }
--    if (isBelowThreshold() && val >= threshold)
--    {
--      return false;
--    }
--    return true;
--  }
--
    @Override
    public boolean isSimpleColour()
    {
@@@ -26,7 -26,6 +26,7 @@@ import jalview.api.FeaturesDisplayedI
  import jalview.datamodel.AlignmentI;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceI;
 +import jalview.datamodel.features.SequenceFeatures;
  import jalview.renderer.seqfeatures.FeatureRenderer;
  import jalview.schemes.FeatureColour;
  import jalview.util.ColorUtils;
@@@ -117,10 -116,11 +117,10 @@@ public abstract class FeatureRendererMo
            synchronized (fd)
            {
              fd.clear();
 -            java.util.Iterator<String> fdisp = _fr.getFeaturesDisplayed()
 -                    .getVisibleFeatures();
 -            while (fdisp.hasNext())
 +            for (String type : _fr.getFeaturesDisplayed()
 +                    .getVisibleFeatures())
              {
 -              fd.setVisible(fdisp.next());
 +              fd.setVisible(type);
              }
            }
          }
    }
  
    @Override
 -  public List<SequenceFeature> findFeaturesAtRes(SequenceI sequence, int res)
 +  public List<SequenceFeature> findFeaturesAtColumn(SequenceI sequence, int column)
    {
 -    ArrayList<SequenceFeature> tmp = new ArrayList<SequenceFeature>();
 -    SequenceFeature[] features = sequence.getSequenceFeatures();
 -
 -    if (features != null)
 +    /*
 +     * include features at the position provided their feature type is 
 +     * displayed, and feature group is null or marked for display
 +     */
 +    List<SequenceFeature> result = new ArrayList<SequenceFeature>();
 +    if (!av.areFeaturesDisplayed() || getFeaturesDisplayed() == null)
      {
 -      for (int i = 0; i < features.length; i++)
 -      {
 -        if (!av.areFeaturesDisplayed()
 -                || !av.getFeaturesDisplayed().isVisible(
 -                        features[i].getType()))
 -        {
 -          continue;
 -        }
 +      return result;
 +    }
  
 -        if (features[i].featureGroup != null
 -                && featureGroups != null
 -                && featureGroups.containsKey(features[i].featureGroup)
 -                && !featureGroups.get(features[i].featureGroup)
 -                        .booleanValue())
 -        {
 -          continue;
 -        }
 +    Set<String> visibleFeatures = getFeaturesDisplayed()
 +            .getVisibleFeatures();
 +    String[] visibleTypes = visibleFeatures
 +            .toArray(new String[visibleFeatures.size()]);
 +    List<SequenceFeature> features = sequence.findFeatures(column, column,
 +            visibleTypes);
  
 -        // check if start/end are at res, and if not a contact feature, that res
 -        // lies between start and end
 -        if ((features[i].getBegin() == res || features[i].getEnd() == res)
 -                || (!features[i].isContactFeature()
 -                        && (features[i].getBegin() < res) && (features[i]
 -                        .getEnd() >= res)))
 -        {
 -          tmp.add(features[i]);
 -        }
 +    for (SequenceFeature sf : features)
 +    {
 +      if (!featureGroupNotShown(sf))
 +      {
 +        result.add(sf);
        }
      }
 -    return tmp;
 +    return result;
    }
  
    /**
     * Searches alignment for all features and updates colours
     * 
     * @param newMadeVisible
 -   *          if true newly added feature types will be rendered immediatly
 +   *          if true newly added feature types will be rendered immediately
     *          TODO: check to see if this method should actually be proxied so
     *          repaint events can be propagated by the renderer code
     */
      }
      FeaturesDisplayedI featuresDisplayed = av.getFeaturesDisplayed();
  
 -    ArrayList<String> allfeatures = new ArrayList<String>();
 -    ArrayList<String> oldfeatures = new ArrayList<String>();
 +    Set<String> oldfeatures = new HashSet<String>();
      if (renderOrder != null)
      {
        for (int i = 0; i < renderOrder.length; i++)
          }
        }
      }
 -    if (minmax == null)
 -    {
 -      minmax = new Hashtable<String, float[][]>();
 -    }
  
 -    Set<String> oldGroups = new HashSet<String>(featureGroups.keySet());
      AlignmentI alignment = av.getAlignment();
 +    List<String> allfeatures = new ArrayList<String>();
 +
      for (int i = 0; i < alignment.getHeight(); i++)
      {
        SequenceI asq = alignment.getSequenceAt(i);
 -      SequenceFeature[] features = asq.getSequenceFeatures();
 -
 -      if (features == null)
 -      {
 -        continue;
 -      }
 -
 -      int index = 0;
 -      while (index < features.length)
 +      for (String group : asq.getFeatures().getFeatureGroups(true))
        {
 -        String fgrp = features[index].getFeatureGroup();
 -        oldGroups.remove(fgrp);
 -        if (!featuresDisplayed.isRegistered(features[index].getType()))
 +        boolean groupDisplayed = true;
 +        if (group != null)
          {
 -          if (fgrp != null)
 +          if (featureGroups.containsKey(group))
            {
 -            Boolean groupDisplayed = featureGroups.get(fgrp);
 -            if (groupDisplayed == null)
 -            {
 -              groupDisplayed = Boolean.valueOf(newMadeVisible);
 -              featureGroups.put(fgrp, groupDisplayed);
 -            }
 -            if (!groupDisplayed.booleanValue())
 -            {
 -              index++;
 -              continue;
 -            }
 +            groupDisplayed = featureGroups.get(group);
            }
 -          if (!(features[index].begin == 0 && features[index].end == 0))
 +          else
            {
 -            // If beginning and end are 0, the feature is for the whole sequence
 -            // and we don't want to render the feature in the normal way
 -
 -            if (newMadeVisible
 -                    && !oldfeatures.contains(features[index].getType()))
 -            {
 -              // this is a new feature type on the alignment. Mark it for
 -              // display.
 -              featuresDisplayed.setVisible(features[index].getType());
 -              setOrder(features[index].getType(), 0);
 -            }
 +            groupDisplayed = newMadeVisible;
 +            featureGroups.put(group, groupDisplayed);
            }
          }
 -        if (!allfeatures.contains(features[index].getType()))
 +        if (groupDisplayed)
          {
 -          allfeatures.add(features[index].getType());
 -        }
 -        if (!Float.isNaN(features[index].score))
 -        {
 -          int nonpos = features[index].getBegin() >= 1 ? 0 : 1;
 -          float[][] mm = minmax.get(features[index].getType());
 -          if (mm == null)
 -          {
 -            mm = new float[][] { null, null };
 -            minmax.put(features[index].getType(), mm);
 -          }
 -          if (mm[nonpos] == null)
 -          {
 -            mm[nonpos] = new float[] { features[index].score,
 -                features[index].score };
 -
 -          }
 -          else
 +          Set<String> types = asq.getFeatures().getFeatureTypesForGroups(
 +                  true, group);
 +          for (String type : types)
            {
 -            if (mm[nonpos][0] > features[index].score)
 +            if (!allfeatures.contains(type)) // or use HashSet and no test?
              {
 -              mm[nonpos][0] = features[index].score;
 -            }
 -            if (mm[nonpos][1] < features[index].score)
 -            {
 -              mm[nonpos][1] = features[index].score;
 +              allfeatures.add(type);
              }
 +            updateMinMax(asq, type, true); // todo: for all features?
            }
          }
        }
      }
  
 -    /*
 -     * oldGroups now consists of groups that no longer 
 -     * have any feature in them - remove these
 -     */
 -    for (String grp : oldGroups)
 +    // uncomment to add new features in alphebetical order (but JAL-2575)
 +    // Collections.sort(allfeatures, String.CASE_INSENSITIVE_ORDER);
 +    if (newMadeVisible)
      {
 -      featureGroups.remove(grp);
 +      for (String type : allfeatures)
 +      {
 +        if (!oldfeatures.contains(type))
 +        {
 +          featuresDisplayed.setVisible(type);
 +          setOrder(type, 0);
 +        }
 +      }
      }
  
      updateRenderOrder(allfeatures);
      findingFeatures = false;
    }
  
 +  /**
 +   * Updates the global (alignment) min and max values for a feature type from
 +   * the score for a sequence, if the score is not NaN. Values are stored
 +   * separately for positional and non-positional features.
 +   * 
 +   * @param seq
 +   * @param featureType
 +   * @param positional
 +   */
 +  protected void updateMinMax(SequenceI seq, String featureType,
 +          boolean positional)
 +  {
 +    float min = seq.getFeatures().getMinimumScore(featureType, positional);
 +    if (Float.isNaN(min))
 +    {
 +      return;
 +    }
 +
 +    float max = seq.getFeatures().getMaximumScore(featureType, positional);
 +
 +    /*
 +     * stored values are 
 +     * { {positionalMin, positionalMax}, {nonPositionalMin, nonPositionalMax} }
 +     */
 +    if (minmax == null)
 +    {
 +      minmax = new Hashtable<String, float[][]>();
 +    }
 +    synchronized (minmax)
 +    {
 +      float[][] mm = minmax.get(featureType);
 +      int index = positional ? 0 : 1;
 +      if (mm == null)
 +      {
 +        mm = new float[][] { null, null };
 +        minmax.put(featureType, mm);
 +      }
 +      if (mm[index] == null)
 +      {
 +        mm[index] = new float[] { min, max };
 +      }
 +      else
 +      {
 +        mm[index][0] = Math.min(mm[index][0], min);
 +        mm[index][1] = Math.max(mm[index][1], max);
 +      }
 +    }
 +  }
    protected Boolean firing = Boolean.FALSE;
  
    /**
     * Returns the configured colour for a particular feature instance. This
     * includes calculation of 'colour by label', or of a graduated score colour,
     * if applicable. It does not take into account feature visibility or colour
--   * transparency.
++   * transparency. Returns null for a score feature whose score value lies
++   * outside any colour threshold.
     * 
     * @param feature
     * @return
    public Color getColour(SequenceFeature feature)
    {
      FeatureColourI fc = getFeatureStyle(feature.getType());
 -    return fc.isColored(feature) ? fc.getColor(feature) : null;
 -  }
 -
 -  /**
 -   * Answers true unless the feature has a score value which lies outside a
 -   * minimum or maximum threshold configured for colouring. This method does not
 -   * check feature type or group visibility.
 -   * 
 -   * @param sequenceFeature
 -   * @return
 -   */
 -  protected boolean showFeature(SequenceFeature sequenceFeature)
 -  {
 -    FeatureColourI fc = getFeatureStyle(sequenceFeature.type);
 -    return fc.isColored(sequenceFeature);
 +    return fc.getColor(feature);
    }
  
    /**
-    * Answers true unless the feature has a graduated colour scheme and the
-    * feature value lies outside the current threshold for display
-    * 
-    * @param sequenceFeature
-    * @return
-    */
-   protected boolean showFeature(SequenceFeature sequenceFeature)
-   {
-     FeatureColourI fc = getFeatureStyle(sequenceFeature.type);
-     return fc.isColored(sequenceFeature);
-   }
-   /**
     * Answers true if the feature type is currently selected to be displayed,
     * else false
     * 
    }
  
    /**
 -   * Sets the priority order for features
 +   * Sets the priority order for features, with the highest priority (displayed
 +   * on top) at the start of the data array
     * 
     * @param data
     *          { String(Type), Colour(Type), Boolean(Displayed) }
      {
        return fcols;
      }
 -    Iterator<String> features = getViewport().getFeaturesDisplayed()
 +    Set<String> features = getViewport().getFeaturesDisplayed()
              .getVisibleFeatures();
 -    while (features.hasNext())
 +    for (String feature : features)
      {
 -      String feature = features.next();
        fcols.put(feature, getFeatureStyle(feature));
      }
      return fcols;
    public List<String> getDisplayedFeatureGroups()
    {
      List<String> _gps = new ArrayList<String>();
 -    boolean valid = false;
      for (String gp : getFeatureGroups())
      {
        if (checkGroupVisibility(gp, false))
        {
 -        valid = true;
          _gps.add(gp);
        }
 -      if (!valid)
 +    }
 +    return _gps;
 +  }
 +
 +  /**
 +   * 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();
 +  }
 +
 +  /**
 +   * {@inheritDoc}
 +   */
 +  @Override
 +  public List<SequenceFeature> findFeaturesAtResidue(SequenceI sequence,
 +          int resNo)
 +  {
 +    List<SequenceFeature> result = new ArrayList<SequenceFeature>();
 +    if (!av.areFeaturesDisplayed() || getFeaturesDisplayed() == null)
 +    {
 +      return result;
 +    }
 +
 +    /*
 +     * include features at the position provided their feature type is 
 +     * displayed, and feature group is null or the empty string
 +     * or marked for display
 +     */
 +    Set<String> visibleFeatures = getFeaturesDisplayed()
 +            .getVisibleFeatures();
 +    String[] visibleTypes = visibleFeatures
 +            .toArray(new String[visibleFeatures.size()]);
 +    List<SequenceFeature> features = sequence.getFeatures().findFeatures(
 +            resNo, resNo, visibleTypes);
 +  
 +    for (SequenceFeature sf : features)
 +    {
 +      if (!featureGroupNotShown(sf))
        {
 -        return null;
 +        result.add(sf);
        }
 -      else
 +    }
 +    return result;
 +  }
 +
 +  /**
 +   * Removes from the list of features any that have a feature group that is not
 +   * displayed, or duplicate the location of a feature of the same type (unless
 +   * a graduated colour scheme is applied)
 +   * 
 +   * @param features
 +   * @param fc
 +   */
 +  public void filterFeaturesForDisplay(List<SequenceFeature> features,
 +          FeatureColourI fc)
 +  {
 +    if (features.isEmpty())
 +    {
 +      return;
 +    }
 +    SequenceFeatures.sortFeatures(features, true);
 +    boolean graduated = fc != null && fc.isGraduatedColour();
 +    SequenceFeature lastFeature = null;
 +
 +    Iterator<SequenceFeature> it = features.iterator();
 +    while (it.hasNext())
 +    {
 +      SequenceFeature sf = it.next();
 +      if (featureGroupNotShown(sf))
        {
 -        // gps = new String[_gps.size()];
 -        // _gps.toArray(gps);
 +        it.remove();
 +        continue;
        }
 +
 +      /*
 +       * a feature is redundant for rendering purposes if it has the
 +       * same extent as another (so would just redraw the same colour);
 +       * (checking type and isContactFeature as a fail-safe here, although
 +       * currently they are guaranteed to match in this context)
 +       */
 +      if (!graduated)
 +      {
 +        if (lastFeature != null && sf.getBegin() == lastFeature.getBegin()
 +                && sf.getEnd() == lastFeature.getEnd()
 +                && sf.isContactFeature() == lastFeature.isContactFeature()
 +                && sf.getType().equals(lastFeature.getType()))
 +        {
 +          it.remove();
 +        }
 +      }
 +      lastFeature = sf;
      }
 -    return _gps;
    }
  
  }
@@@ -16,7 -16,6 +16,7 @@@ import jalview.io.FileLoader
  import jalview.schemes.FeatureColour;
  
  import java.awt.Color;
 +import java.util.List;
  
  import org.testng.annotations.BeforeMethod;
  import org.testng.annotations.BeforeTest;
@@@ -71,10 -70,13 +71,10 @@@ public class FeatureColourFinderTes
    @BeforeMethod(alwaysRun = true)
    public void setUpBeforeTest()
    {
 -    SequenceFeature[] sfs = seq.getSequenceFeatures();
 -    if (sfs != null)
 +    List<SequenceFeature> sfs = seq.getSequenceFeatures();
 +    for (SequenceFeature sf : sfs)
      {
 -      for (SequenceFeature sf : sfs)
 -      {
 -        seq.deleteFeature(sf);
 -      }
 +      seq.deleteFeature(sf);
      }
      fr.findAllFeatures(true);
  
      FeatureColourFinder finder2 = new FeatureColourFinder(null);
      assertTrue(finder2.noFeaturesDisplayed());
    }
+   @Test(groups = "Functional")
+   public void testFindFeatureColour_graduatedWithThreshold()
+   {
+     seq.addSequenceFeature(new SequenceFeature("kd", "hydrophobicity", 2,
+             2, 0f, "KdGroup"));
+     seq.addSequenceFeature(new SequenceFeature("kd", "hydrophobicity", 4,
+             4, 5f, "KdGroup"));
+     seq.addSequenceFeature(new SequenceFeature("kd", "hydrophobicity", 7,
+             7, 10f, "KdGroup"));
+   
+     /*
+      * graduated colour from 0 to 10
+      * above threshold value of 5
+      */
+     Color min = new Color(100, 50, 150);
+     Color max = new Color(200, 0, 100);
+     FeatureColourI fc = new FeatureColour(min, max, 0, 10);
+     fc.setAboveThreshold(true);
+     fc.setThreshold(5f);
+     fr.setColour("kd", fc);
+     fr.featuresAdded();
+     av.setShowSequenceFeatures(true);
+   
+     /*
+      * position 2, column 1, score 0 - below threshold - default colour
+      */
+     Color c = finder.findFeatureColour(Color.blue, seq, 1);
+     assertEquals(c, Color.blue);
+     /*
+      * position 4, column 3, score 5 - at threshold - default colour
+      */
+     c = finder.findFeatureColour(Color.blue, seq, 3);
+     assertEquals(c, Color.blue);
+   
+     /*
+      * position 7, column 9, score 10 - maximum colour in range
+      */
+     c = finder.findFeatureColour(Color.blue, seq, 9);
+     assertEquals(c, max);
+     /*
+      * now colour below threshold of 5
+      */
+     fc.setBelowThreshold(true);
+     /*
+      * position 2, column 1, score 0 - min colour
+      */
+     c = finder.findFeatureColour(Color.blue, seq, 1);
+     assertEquals(c, min);
+     /*
+      * position 4, column 3, score 5 - at threshold - default colour
+      */
+     c = finder.findFeatureColour(Color.blue, seq, 3);
+     assertEquals(c, Color.blue);
+     /*
+      * position 7, column 9, score 10 - above threshold - default colour
+      */
+     c = finder.findFeatureColour(Color.blue, seq, 9);
+     assertEquals(c, Color.blue);
+   }
  }
@@@ -22,6 -22,6 +22,7 @@@ package jalview.schemes
  
  import static org.testng.AssertJUnit.assertEquals;
  import static org.testng.AssertJUnit.assertFalse;
++import static org.testng.AssertJUnit.assertNull;
  import static org.testng.AssertJUnit.assertTrue;
  import static org.testng.AssertJUnit.fail;
  
@@@ -84,67 -84,60 +85,11 @@@ public class FeatureColourTes
    }
  
    @Test(groups = { "Functional" })
    public void testGetColor_simpleColour()
    {
      FeatureColour fc = new FeatureColour(Color.RED);
 -    assertEquals(Color.RED, fc.getColor(new SequenceFeature()));
 +    assertEquals(Color.RED,
 +            fc.getColor(new SequenceFeature("Cath", "", 1, 2, 0f, null)));
    }
  
    @Test(groups = { "Functional" })
    }
  
    @Test(groups = { "Functional" })
--  public void testGetColor_belowThreshold()
++  public void testGetColor_aboveBelowThreshold()
    {
      // gradient from [50, 150] from WHITE(255, 255, 255) to BLACK(0, 0, 0)
      FeatureColour fc = new FeatureColour(Color.WHITE, Color.BLACK, 50f,
      SequenceFeature sf = new SequenceFeature("type", "desc", 0, 20, 70f,
              null);
      fc.setThreshold(100f); // ignore for now
      assertEquals(new Color(204, 204, 204), fc.getColor(sf));
  
      fc.setAboveThreshold(true); // feature lies below threshold
--    assertFalse(fc.isColored(sf));
--    assertEquals(new Color(204, 204, 204), fc.getColor(sf));
++    assertNull(fc.getColor(sf));
++
++    fc.setBelowThreshold(true);
++    fc.setThreshold(70f);
++    assertNull(fc.getColor(sf)); // feature score == threshold - hidden
++    fc.setThreshold(69f);
++    assertNull(fc.getColor(sf)); // feature score > threshold - hidden
    }
  
    /**