From 02bdd4e842db62a0c9c3a36bbddd2b4f7249df92 Mon Sep 17 00:00:00 2001 From: kiramt Date: Wed, 4 Oct 2017 13:19:46 +0100 Subject: [PATCH] JAL-2674 Simplify locateVisibleBoundsOfSequence Take advantage of sequence cursor changes where possible --- src/jalview/datamodel/HiddenColumns.java | 76 +++++++------ src/jalview/renderer/ScaleRenderer.java | 18 ++-- test/jalview/datamodel/HiddenColumnsTest.java | 141 +++++-------------------- 3 files changed, 82 insertions(+), 153 deletions(-) diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index a6b4f82..09f2ec5 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -693,10 +693,9 @@ public class HiddenColumns * extent of the sequence * * @param seq - * @return int[] { visible start, visible end, first seqpos, last seqpos, - * alignment index for seq start, alignment index for seq end } + * @return int[] { visible start, first seqpos, last seqpos } */ - public int[] locateVisibleBoundsOfSequence(SequenceI seq) + public int locateVisibleBoundsOfSequence(SequenceI seq) { try { @@ -705,9 +704,7 @@ public class HiddenColumns if (hiddenColumns == null || hiddenColumns.size() == 0) { - int ifpos = seq.findIndex(seq.getStart()) - 1; - int ilpos = seq.findIndex(seq.getEnd()) - 1; - return new int[] { ifpos, ifpos, ilpos }; + return seq.findIndex(seq.getStart()) - 1; } // Simply walk along the sequence whilst watching for hidden column @@ -718,20 +715,12 @@ public class HiddenColumns int hideEnd = -1; int visPrev = 0; int visNext = 0; - int firstP = -1; - int lastP = -1; + boolean foundStart = false; for (int p = 0; spos <= seq.getEnd() && p < seq.getLength(); p++) { if (!Comparison.isGap(seq.getCharAt(p))) { - // keep track of first/last column - // containing sequence data regardless of visibility - if (firstP == -1) - { - firstP = p; - } - lastP = p; // update hidden region start/end while (hideEnd < p && regions.hasNext()) { @@ -746,13 +735,10 @@ public class HiddenColumns hideStart = seq.getLength(); } // update visible boundary for sequence - if (p < hideStart) + if ((p < hideStart) && (!foundStart)) { - if (!foundStart) - { start = p; foundStart = true; - } } // look for next sequence position spos++; @@ -760,10 +746,10 @@ public class HiddenColumns } if (foundStart) { - return new int[] { findColumnPosition(start), firstP, lastP }; + return findColumnPosition(start); } // otherwise, sequence was completely hidden - return new int[] { visPrev, firstP, lastP }; + return visPrev; } finally { LOCK.readLock().unlock(); @@ -1916,8 +1902,10 @@ public class HiddenColumns } /** - * An iterator which iterates over visible regions in a range as visible - * column positions. + * An iterator which iterates over visible regions in a range. The range is + * specified in terms of visible column positions. Provides a special + * "endsAtHidden" indicator to allow callers to determine if the final visible + * column is adjacent to a hidden region. */ public class VisibleBlocksVisBoundsIterator implements Iterator { @@ -1927,8 +1915,19 @@ public class HiddenColumns private boolean endsAtHidden = false; + /** + * Constructor for iterator over visible regions in a range. + * + * @param start + * start position in terms of visible column position + * @param end + * end position in terms of visible column position + * @param usecopy + * whether to use a local copy of hidden columns + */ VisibleBlocksVisBoundsIterator(int start, int end, boolean usecopy) { + /* actually this implementation always uses a local copy but this may change in future */ try { if (usecopy) @@ -1942,7 +1941,6 @@ public class HiddenColumns int blockEnd = end; int hiddenSoFar = 0; int visSoFar = 0; - endsAtHidden = false; // iterate until a region begins within (start,end] int i = 0; @@ -1955,34 +1953,47 @@ public class HiddenColumns } blockStart += hiddenSoFar; // convert start to absolute position - blockEnd += hiddenSoFar; // convert end too in + blockEnd += hiddenSoFar; // convert end to absolute position - // iterate from start to end, adding each hidden region. Positions are - // absolute, and all regions which *overlap* [start,end] are used. + // iterate from start to end, adding each visible region. Positions + // are + // absolute, and all hidden regions which overlap [start,end] are + // used. while (i < hiddenColumns.size() && (hiddenColumns.get(i)[0] <= blockEnd)) { int[] region = hiddenColumns.get(i); + // end position of this visible region is either just before the + // start of the next hidden region, or the absolute position of + // 'end', whichever is lowest blockEnd = Math.min(blockEnd, region[0] - 1); - int[] contig = new int[] { blockStart, blockEnd }; - vcontigs.add(contig); + vcontigs.add(new int[] { blockStart, blockEnd }); visSoFar += blockEnd - blockStart + 1; + + // next visible region starts after this hidden region blockStart = region[1] + 1; hiddenSoFar += region[1] - region[0] + 1; + + // reset blockEnd to absolute position of 'end', assuming we've now + // passed all hidden regions before end blockEnd = end + hiddenSoFar; i++; } if (visSoFar < end - start) { + // the number of visible columns we've accounted for is less than + // the number specified by end-start; work out the end position of + // the last visible region blockEnd = blockStart + end - start - visSoFar; - int[] contig = new int[] { blockStart, blockEnd }; - vcontigs.add(contig); + vcontigs.add(new int[] { blockStart, blockEnd }); + // if the last visible region ends at the next hidden region, set + // endsAtHidden=true if (i < hiddenColumns.size() && hiddenColumns.get(i)[0] - 1 == blockEnd) { @@ -1993,8 +2004,7 @@ public class HiddenColumns else { // there are no hidden columns, return a single visible contig - int[] contig = new int[] { start, end }; - vcontigs.add(contig); + vcontigs.add(new int[] { start, end }); endsAtHidden = false; } } finally diff --git a/src/jalview/renderer/ScaleRenderer.java b/src/jalview/renderer/ScaleRenderer.java index d310705..6d4edd9 100644 --- a/src/jalview/renderer/ScaleRenderer.java +++ b/src/jalview/renderer/ScaleRenderer.java @@ -68,17 +68,23 @@ public class ScaleRenderer int scalestartx = (startx / 10) * 10; SequenceI refSeq = av.getAlignment().getSeqrep(); - int refSp = 0, refStartI = 0, refEndI = -1; + int refSp = 0; + int refStartI = 0; + int refEndI = -1; if (refSeq != null) { - // find bounds and set origin appopriately + // find bounds and set origin appropriately // locate first visible position for this sequence - int[] refbounds = av.getAlignment().getHiddenColumns() + refSp = av.getAlignment().getHiddenColumns() .locateVisibleBoundsOfSequence(refSeq); - refSp = refbounds[0]; - refStartI = refbounds[1]; - refEndI = refbounds[2]; + refStartI = refSeq.findIndex(refSeq.getStart()) - 1; + + int seqlength = refSeq.getLength(); + // get sequence position past the end of the sequence + int pastEndPos = refSeq.findPosition(seqlength + 1); + refEndI = refSeq.findIndex(pastEndPos - 1) - 1; + scalestartx = refSp + ((scalestartx - refSp) / 10) * 10; } diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 6f1260c..bc1e7b7 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -329,29 +329,17 @@ public class HiddenColumnsTest assertEquals(2, seq.findIndex(seq.getStart())); // no hidden columns - assertEquals( - Arrays.toString(new int[] - { seq.findIndex(seq.getStart()) - 1, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(seq.findIndex(seq.getStart()) - 1, cs.locateVisibleBoundsOfSequence(seq)); // hidden column on gap after end of sequence - should not affect bounds colsel.hideSelectedColumns(13, al.getHiddenColumns()); - assertEquals( - Arrays.toString(new int[] - { seq.findIndex(seq.getStart()) - 1, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(seq.findIndex(seq.getStart()) - 1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); // hidden column on gap before beginning of sequence - should vis bounds by // one colsel.hideSelectedColumns(0, al.getHiddenColumns()); - assertEquals( - Arrays.toString(new int[] - { seq.findIndex(seq.getStart()) - 2, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(seq.findIndex(seq.getStart()) - 2,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); // hide columns around most of sequence - leave one residue remaining @@ -361,191 +349,117 @@ public class HiddenColumnsTest cs.getVisibleSequenceStrings(0, 5, new SequenceI[] { seq })[0]); - assertEquals( - Arrays.toString( - new int[] - { 1, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(1, cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); // hide whole sequence - should just get location of hidden region // containing sequence cs.hideColumns(1, 11); - assertEquals( - Arrays.toString( - new int[] - { 0, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 15); - assertEquals(Arrays - .toString(new int[] - { 0, seq.findIndex(seq.getStart()) - 1, - seq.findIndex(seq.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq))); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); SequenceI seq2 = new Sequence("RefSeq2", "-------A-SD-ASD--E---"); cs.revealAllHiddenColumns(colsel); cs.hideColumns(7, 17); - assertEquals( - Arrays.toString( - new int[] - { 0, seq2.findIndex(seq2.getStart()) - 1, - seq2.findIndex(seq2.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq2))); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq2)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(3, 17); - assertEquals( - Arrays.toString( - new int[] - { 0, seq2.findIndex(seq2.getStart()) - 1, - seq2.findIndex(seq2.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq2))); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq2)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(3, 19); - assertEquals( - Arrays.toString( - new int[] - { 0, seq2.findIndex(seq2.getStart()) - 1, - seq2.findIndex(seq2.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq2))); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq2)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 0); - int[] test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 1); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 1, 1, 11 }), - Arrays.toString(test)); + assertEquals(1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 2); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(1, 1); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 2, 1, 11 }), - Arrays.toString(test)); + assertEquals(2,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(1, 2); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 1, 1, 11 }), - Arrays.toString(test)); + assertEquals(1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(1, 3); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 1, 1, 11 }), - Arrays.toString(test)); + assertEquals(1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 2); cs.hideColumns(5, 6); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 2); cs.hideColumns(5, 6); cs.hideColumns(9, 10); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 2); cs.hideColumns(7, 11); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(2, 4); cs.hideColumns(7, 11); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 1, 1, 11 }), - Arrays.toString(test)); + assertEquals(1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(2, 4); cs.hideColumns(7, 12); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 1, 1, 11 }), - Arrays.toString(test)); + assertEquals(1,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(1, 11); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 12); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 4); cs.hideColumns(6, 12); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 1); cs.hideColumns(3, 12); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 0, 1, 11 }), - Arrays.toString(test)); + assertEquals(0,cs.locateVisibleBoundsOfSequence(seq)); - // These tests cover different behaviour to original - // locateVisibleBoundsOfSequence - // Previously first values of each were 3,9 and 6 respectively. cs.revealAllHiddenColumns(colsel); cs.hideColumns(3, 14); cs.hideColumns(17, 19); - assertEquals( - Arrays.toString( - new int[] - { 3, seq2.findIndex(seq2.getStart()) - 1, - seq2.findIndex(seq2.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq2))); + assertEquals(3,cs.locateVisibleBoundsOfSequence(seq2)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(3, 7); cs.hideColumns(9, 14); cs.hideColumns(17, 19); - assertEquals( - Arrays.toString( - new int[] - { 9, seq2.findIndex(seq2.getStart()) - 1, - seq2.findIndex(seq2.getEnd()) - 1 }), - Arrays.toString(cs.locateVisibleBoundsOfSequence(seq2))); + assertEquals(9,cs.locateVisibleBoundsOfSequence(seq2)); cs.revealAllHiddenColumns(colsel); cs.hideColumns(0, 1); cs.hideColumns(3, 4); cs.hideColumns(6, 8); cs.hideColumns(10, 12); - test = cs.locateVisibleBoundsOfSequence(seq); - assertEquals(Arrays.toString(new int[] { 6, 1, 11 }), - Arrays.toString(test)); + assertEquals(6, cs.locateVisibleBoundsOfSequence(seq)); } @@ -560,7 +474,6 @@ public class HiddenColumnsTest cs.hideInsertionsFor(al.getSequenceAt(0)); assertEquals("G", "" + al.getSequenceAt(0).getCharAt(cs.adjustForHiddenColumns(9))); - } @Test(groups = { "Functional" }) -- 1.7.10.2