From 88db0f1de5180c572667bcce46747b4829d8d491 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Tue, 7 Nov 2017 11:44:50 +0000 Subject: [PATCH] JAL-2816 tweaks to feature visibility checks --- .../renderer/seqfeatures/FeatureRenderer.java | 5 +- .../seqfeatures/FeatureRendererModel.java | 28 ++++------ .../renderer/seqfeatures/FeatureRendererTest.java | 55 +++----------------- 3 files changed, 22 insertions(+), 66 deletions(-) diff --git a/src/jalview/renderer/seqfeatures/FeatureRenderer.java b/src/jalview/renderer/seqfeatures/FeatureRenderer.java index 6687e6a..795cd36 100644 --- a/src/jalview/renderer/seqfeatures/FeatureRenderer.java +++ b/src/jalview/renderer/seqfeatures/FeatureRenderer.java @@ -304,7 +304,10 @@ public class FeatureRenderer extends FeatureRendererModel List 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 --- 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 features, - FeatureColourI fc) + public void filterFeaturesForDisplay(List features) { if (features.isEmpty()) { return; } SequenceFeatures.sortFeatures(features, true); - boolean simpleColour = fc == null || fc.isSimpleColour(); SequenceFeature lastFeature = null; Iterator 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 --- 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 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)); } } -- 1.7.10.2