From 8be202b71704e4b7e10e3a366446492623a3c322 Mon Sep 17 00:00:00 2001 From: kiramt Date: Thu, 18 Jan 2018 16:30:45 +0000 Subject: [PATCH] JAL-2759 change subtractVisibleColumns and removal of reverse iterator --- src/jalview/datamodel/HiddenColumns.java | 60 +++++----------- src/jalview/datamodel/ReverseRegionsIterator.java | 75 -------------------- .../viewmodel/OverviewDimensionsShowHidden.java | 4 +- test/jalview/datamodel/HiddenColumnsTest.java | 71 +++++++++++++----- 4 files changed, 71 insertions(+), 139 deletions(-) delete mode 100644 src/jalview/datamodel/ReverseRegionsIterator.java diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index 55af3f8..ea8da8d 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -21,6 +21,7 @@ package jalview.datamodel; import java.util.ArrayList; +import java.util.Arrays; import java.util.BitSet; import java.util.Iterator; import java.util.List; @@ -634,12 +635,7 @@ public class HiddenColumns try { LOCK.readLock().lock(); - int num = 0; - if (hasHiddenColumns()) - { - num = hiddenColumns.size(); - } - return num; + return hiddenColumns.size(); } finally { LOCK.readLock().unlock(); @@ -668,16 +664,17 @@ public class HiddenColumns return false; } + Iterator it = this.iterator(); Iterator thatit = that.iterator(); - for (int[] thisRange : hiddenColumns) + while (it.hasNext()) { - int[] thatRange = thatit.next(); - if (thisRange[0] != thatRange[0] || thisRange[1] != thatRange[1]) + if (!(Arrays.equals(it.next(), thatit.next()))) { return false; } } return true; + } finally { LOCK.readLock().unlock(); @@ -769,48 +766,25 @@ public class HiddenColumns /** * Find the visible column which is a given visible number of columns to the - * left of another visible column. i.e. for a startColumn x, the column which - * is distance 1 away will be column x-1. + * left (negative visibleDistance) or right (positive visibleDistance) of + * startColumn. If startColumn is not visible, we use the visible column at + * the left boundary of the hidden region containing startColumn. * * @param visibleDistance - * the number of visible columns to offset by + * the number of visible columns to offset by (left offset = negative + * value; right offset = positive value) * @param startColumn - * the column to start from - * @return the position of the column in the visible alignment + * the position of the column to start from (absolute position) + * @return the position of the column which is away + * (absolute position) */ - public int subtractVisibleColumns(int visibleDistance, int startColumn) + public int offsetByVisibleColumns(int visibleDistance, int startColumn) { try { LOCK.readLock().lock(); - int distance = visibleDistance; - - // in case startColumn is in a hidden region, move it to the left - int start = visibleToAbsoluteColumn(absoluteToVisibleColumn(startColumn)); - - Iterator it = new ReverseRegionsIterator(0, start, - hiddenColumns); - - while (it.hasNext() && (distance > 0)) - { - int[] region = it.next(); - - if (start > region[1]) - { - // subtract the gap to right of region from distance - if (start - region[1] <= distance) - { - distance -= start - region[1]; - start = region[0] - 1; - } - else - { - start = start - distance; - distance = 0; - } - } - } - return start - distance; + int start = absoluteToVisibleColumn(startColumn); + return visibleToAbsoluteColumn(start + visibleDistance); } finally { diff --git a/src/jalview/datamodel/ReverseRegionsIterator.java b/src/jalview/datamodel/ReverseRegionsIterator.java deleted file mode 100644 index b511ab5..0000000 --- a/src/jalview/datamodel/ReverseRegionsIterator.java +++ /dev/null @@ -1,75 +0,0 @@ -package jalview.datamodel; - -import java.util.Iterator; -import java.util.List; - -/** - * A local iterator which reverse iterates over hidden column regions in a - * range. Intended for use ONLY within the HiddenColumns class, because it works - * directly with the hiddenColumns collection without locking (callers should - * lock hiddenColumns). - */ -public class ReverseRegionsIterator implements Iterator -{ - // start position to iterate to - private int start; - - // end position to iterate from - private int end; - - // current index in hiddenColumns - private int currentPosition = 0; - - // current column in hiddenColumns - private int[] nextRegion = null; - - private final List hiddenColumns; - - // Constructor with bounds - ReverseRegionsIterator(int lowerBound, int upperBound, - List hiddenCols) - { - hiddenColumns = hiddenCols; - start = lowerBound; - end = upperBound; - - if (hiddenColumns != null) - { - // iterate until a region overlaps with [start,end] - currentPosition = hiddenColumns.size() - 1; - while (currentPosition >= 0 - && hiddenColumns.get(currentPosition)[1] > end) - { - currentPosition--; - } - if (currentPosition >= 0) - { - nextRegion = hiddenColumns.get(currentPosition); - } - } - } - - @Override - public boolean hasNext() - { - return (hiddenColumns != null) && (nextRegion != null) - && (nextRegion[1] >= start); - } - - @Override - public int[] next() - { - int[] region = nextRegion; - currentPosition--; - if (currentPosition >= 0) - { - nextRegion = hiddenColumns.get(currentPosition); - } - else - { - nextRegion = null; - } - return region; - } - -} diff --git a/src/jalview/viewmodel/OverviewDimensionsShowHidden.java b/src/jalview/viewmodel/OverviewDimensionsShowHidden.java index 9a8c2a8..79e1c47 100644 --- a/src/jalview/viewmodel/OverviewDimensionsShowHidden.java +++ b/src/jalview/viewmodel/OverviewDimensionsShowHidden.java @@ -145,7 +145,7 @@ public class OverviewDimensionsShowHidden extends OverviewDimensions if (ranges.getEndRes() < visAlignWidth) { visXAsRes = hiddenCols.absoluteToVisibleColumn(hiddenCols - .subtractVisibleColumns(vpwidth - 1, alwidth - 1)); + .offsetByVisibleColumns(-(vpwidth - 1), alwidth - 1)); } else { @@ -231,7 +231,7 @@ public class OverviewDimensionsShowHidden extends OverviewDimensions protected int getLeftXFromCentreX(int mousex, HiddenColumns hidden) { int vpx = Math.round((float) mousex * alwidth / width); - return hidden.subtractVisibleColumns(ranges.getViewportWidth() / 2, + return hidden.offsetByVisibleColumns(-ranges.getViewportWidth() / 2, vpx); } diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 3aadcb7..9bbba5f 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -1203,48 +1203,48 @@ public class HiddenColumnsTest public void testSubtractVisibleColumns() { HiddenColumns h = new HiddenColumns(); - int result = h.subtractVisibleColumns(1, 10); + int result = h.offsetByVisibleColumns(-1, 10); assertEquals(9, result); h.hideColumns(7, 9); - result = h.subtractVisibleColumns(4, 10); + result = h.offsetByVisibleColumns(-4, 10); assertEquals(3, result); h.hideColumns(14, 15); - result = h.subtractVisibleColumns(4, 10); + result = h.offsetByVisibleColumns(-4, 10); assertEquals(3, result); - result = h.subtractVisibleColumns(10, 17); + result = h.offsetByVisibleColumns(-10, 17); assertEquals(2, result); - result = h.subtractVisibleColumns(1, 7); + result = h.offsetByVisibleColumns(-1, 7); assertEquals(5, result); - result = h.subtractVisibleColumns(1, 8); + result = h.offsetByVisibleColumns(-1, 8); assertEquals(5, result); - result = h.subtractVisibleColumns(3, 15); + result = h.offsetByVisibleColumns(-3, 15); assertEquals(10, result); ColumnSelection sel = new ColumnSelection(); h.revealAllHiddenColumns(sel); h.hideColumns(0, 30); - result = h.subtractVisibleColumns(31, 0); + result = h.offsetByVisibleColumns(-31, 0); assertEquals(-31, result); HiddenColumns cs = new HiddenColumns(); - // test that without hidden columns, findColumnNToLeft returns + // test that without hidden columns, offsetByVisibleColumns returns // position n to left of provided position - long pos = cs.subtractVisibleColumns(3, 10); + long pos = cs.offsetByVisibleColumns(-3, 10); assertEquals(7, pos); // 0 returns same position - pos = cs.subtractVisibleColumns(0, 10); + pos = cs.offsetByVisibleColumns(0, 10); assertEquals(10, pos); // overflow to left returns negative number - pos = cs.subtractVisibleColumns(3, 0); + pos = cs.offsetByVisibleColumns(-3, 0); assertEquals(-3, pos); // test that with hidden columns to left of result column @@ -1252,21 +1252,21 @@ public class HiddenColumnsTest cs.hideColumns(1, 3); // position n to left of provided position - pos = cs.subtractVisibleColumns(3, 10); + pos = cs.offsetByVisibleColumns(-3, 10); assertEquals(7, pos); // 0 returns same position - pos = cs.subtractVisibleColumns(0, 10); + pos = cs.offsetByVisibleColumns(0, 10); assertEquals(10, pos); // test with one set of hidden columns between start and required position cs.hideColumns(12, 15); - pos = cs.subtractVisibleColumns(8, 17); + pos = cs.offsetByVisibleColumns(-8, 17); assertEquals(5, pos); // test with two sets of hidden columns between start and required position cs.hideColumns(20, 21); - pos = cs.subtractVisibleColumns(8, 23); + pos = cs.offsetByVisibleColumns(-8, 23); assertEquals(9, pos); // repeat last 2 tests with no hidden columns to left of required position @@ -1275,16 +1275,49 @@ public class HiddenColumnsTest // test with one set of hidden columns between start and required position cs.hideColumns(12, 15); - pos = cs.subtractVisibleColumns(8, 17); + pos = cs.offsetByVisibleColumns(-8, 17); assertEquals(5, pos); // test with two sets of hidden columns between start and required position cs.hideColumns(20, 21); - pos = cs.subtractVisibleColumns(8, 23); + pos = cs.offsetByVisibleColumns(-8, 23); assertEquals(9, pos); - } + // test with right (positive) offsets + + // test that without hidden columns, offsetByVisibleColumns returns + // position n to right of provided position + pos = cs.offsetByVisibleColumns(3, 7); + assertEquals(10, pos); + + // test that with hidden columns to left of result column + // behaviour is the same as above + cs.hideColumns(1, 3); + + // test with one set of hidden columns between start and required position + cs.hideColumns(12, 15); + pos = cs.offsetByVisibleColumns(8, 5); + assertEquals(17, pos); + // test with two sets of hidden columns between start and required position + cs.hideColumns(20, 21); + pos = cs.offsetByVisibleColumns(8, 9); + assertEquals(23, pos); + + // repeat last 2 tests with no hidden columns to left of required position + colsel = new ColumnSelection(); + cs.revealAllHiddenColumns(colsel); + + // test with one set of hidden columns between start and required position + cs.hideColumns(12, 15); + pos = cs.offsetByVisibleColumns(8, 5); + assertEquals(17, pos); + + // test with two sets of hidden columns between start and required position + cs.hideColumns(20, 21); + pos = cs.offsetByVisibleColumns(8, 9); + assertEquals(23, pos); + } @Test(groups = "Functional") public void testBoundedIterator() -- 1.7.10.2