From 690524be282b813fed436bce65552716e04bd282 Mon Sep 17 00:00:00 2001 From: kiramt Date: Tue, 30 Jan 2018 10:24:37 +0000 Subject: [PATCH] JAL-2759 Keep numColumns up to date --- src/jalview/datamodel/HiddenColumns.java | 175 +++++++++++-------------- test/jalview/datamodel/HiddenColumnsTest.java | 30 ++++- 2 files changed, 107 insertions(+), 98 deletions(-) diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index fd4a9b5..d2e46c6 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -66,9 +66,6 @@ public class HiddenColumns { private static final int HASH_MULTIPLIER = 31; - private static final int NUMCOLUMNS_RESET = -1; // value of numColumns if it - // needs to be recalculated - private static final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock(); /* @@ -171,8 +168,7 @@ public class HiddenColumns if (!hiddenColumns.isEmpty()) { // set up cursor reset values - HiddenCursorPosition cursorPos = cursor.findRegionForColumn(start, - false); + HiddenCursorPosition cursorPos = cursor.findRegionForColumn(start, false); regionindex = cursorPos.getRegionIndex(); if (regionindex > 0) @@ -185,13 +181,13 @@ public class HiddenColumns } } - /* - * new range follows everything else; check first to avoid looping over whole hiddenColumns collection - */ + // new range follows everything else; check first to avoid looping over + // whole hiddenColumns collection if (hiddenColumns.isEmpty() || start > hiddenColumns.get(hiddenColumns.size() - 1)[1]) { hiddenColumns.add(new int[] { start, end }); + numColumns += end - start + 1; } else { @@ -214,10 +210,6 @@ public class HiddenColumns // a lot of reprocessing in large alignments cursor = new HiddenColumnsCursor(hiddenColumns, previndex, prevHiddenCount); - - // reset the number of columns so they will be recounted - resetNumColumns(); - } finally { LOCK.writeLock().unlock(); @@ -247,6 +239,7 @@ public class HiddenColumns * insert discontiguous preceding range */ hiddenColumns.add(i, new int[] { start, end }); + numColumns += end - start + 1; added = true; } else if (end <= region[1]) @@ -255,7 +248,10 @@ public class HiddenColumns * new range overlaps existing, or is contiguous preceding it - adjust * start column */ + int oldstart = region[0]; region[0] = Math.min(region[0], start); + numColumns += oldstart - region[0]; // new columns are between old and + // adjusted starts added = true; } else if (start <= region[1] + 1) @@ -264,32 +260,57 @@ public class HiddenColumns * new range overlaps existing, or is contiguous following it - adjust * start and end columns */ - region[0] = Math.min(region[0], start); - region[1] = Math.max(region[1], end); + insertRangeAtOverlap(i, start, end, region); + added = true; + } + return added; + } - /* - * also update or remove any subsequent ranges - * that are overlapped - */ - while (i < hiddenColumns.size() - 1) + /** + * Insert a range whose start position overlaps an existing region and/or is + * contiguous to the right of the region + * + * @param i + * index to insert at + * @param start + * start of range to insert + * @param end + * end of range to insert + * @param region + * the overlapped/continued region + */ + private void insertRangeAtOverlap(int i, int start, int end, int[] region) + { + int oldstart = region[0]; + int oldend = region[1]; + region[0] = Math.min(region[0], start); + region[1] = Math.max(region[1], end); + + numColumns += oldstart - region[0]; + numColumns += region[1] - oldend; + + /* + * also update or remove any subsequent ranges + * that are overlapped + */ + int endi = i; + while (endi < hiddenColumns.size() - 1) + { + int[] nextRegion = hiddenColumns.get(endi + 1); + if (nextRegion[0] > end + 1) { - int[] nextRegion = hiddenColumns.get(i + 1); - if (nextRegion[0] > end + 1) - { - /* - * gap to next hidden range - no more to update - */ - break; - } - region[1] = Math.max(nextRegion[1], end); - - // in theory hiddenColumns.subList(i + 1, i + 2).clear() is faster than - // hiddenColumns.remove(i+1) but benchmarking results a bit ambivalent - hiddenColumns.remove(i + 1); + /* + * gap to next hidden range - no more to update + */ + break; } - added = true; + numColumns -= nextRegion[1] - nextRegion[0] + 1; + oldend = nextRegion[1]; + region[1] = Math.max(nextRegion[1], end); + numColumns += region[1] - oldend + 1; + endi++; } - return added; + hiddenColumns.subList(i + 1, endi + 1).clear(); } /** @@ -307,7 +328,7 @@ public class HiddenColumns hideColumns(r[0], r[1]); } cursor = new HiddenColumnsCursor(hiddenColumns); - resetNumColumns(); + } finally { LOCK.writeLock().unlock(); @@ -332,7 +353,7 @@ public class HiddenColumns } hiddenColumns.clear(); cursor = new HiddenColumnsCursor(hiddenColumns); - resetNumColumns(); + numColumns = 0; } finally { @@ -373,15 +394,8 @@ public class HiddenColumns } int colsToRemove = region[1] - region[0] + 1; hiddenColumns.remove(regionIndex); + numColumns -= colsToRemove; - if (hiddenColumns.isEmpty()) - { - resetNumColumns(); - } - else - { - numColumns -= colsToRemove; - } cursor.updateForDeletedRegion(hiddenColumns); } } @@ -438,39 +452,7 @@ public class HiddenColumns */ public int getSize() { - try - { - LOCK.readLock().lock(); - - if (numColumns == NUMCOLUMNS_RESET) - { - // numColumns is out of date, so recalculate - int size = 0; - - for (int[] range : hiddenColumns) - { - size += range[1] - range[0] + 1; - } - - numColumns = size; - } - - return numColumns; - } finally - { - LOCK.readLock().unlock(); - } - } - - /** - * Reset numColumns so that it gets recalculated. Currently the code does not - * recalculate numColumns on hide/reveal as it requires a full sweep of the - * hidden columns collection / smarter updating. Placeholder here if later on - * a recalculation is added. - */ - private void resetNumColumns() - { - numColumns = NUMCOLUMNS_RESET; + return numColumns; } /** @@ -801,7 +783,6 @@ public class HiddenColumns hideColumns(firstSet, lastSet - 1); } cursor = new HiddenColumnsCursor(hiddenColumns); - resetNumColumns(); } finally { LOCK.writeLock().unlock(); @@ -852,7 +833,6 @@ public class HiddenColumns { HiddenCursorPosition pos = cursor.findRegionForColumn(start, false); int index = pos.getRegionIndex(); - int startindex = index; // first index in hiddenColumns to remove if (index != -1 && index != hiddenColumns.size()) { @@ -862,30 +842,35 @@ public class HiddenColumns if (region[0] < start && region[1] >= start) { // region contains start, truncate so that it ends just before start + numColumns -= region[1] - start + 1; region[1] = start - 1; - startindex++; + index++; } - } - - pos = cursor.findRegionForColumn(end, false); - index = pos.getRegionIndex(); - int endindex = index - 1; // last index in hiddenColumns to remove - if (index != -1 && index != hiddenColumns.size()) - { - // regionIndex is the region which either contains end - // or lies to the right of end - int[] region = hiddenColumns.get(index); - if (region[0] <= end && region[1] > end) + int endi = index; + while (endi < hiddenColumns.size()) { - // region contains end, truncate so that it starts just after end - region[0] = end + 1; + region = hiddenColumns.get(endi); + + if (region[1] > end) + { + if (region[0] <= end) + { + // region contains end, truncate so it starts just after end + numColumns -= end - region[0] + 1; + region[0] = end + 1; + } + break; + } + + numColumns -= region[1] - region[0] + 1; + endi++; } + hiddenColumns.subList(index, endi).clear(); + } - hiddenColumns.subList(startindex, endindex + 1).clear(); cursor = new HiddenColumnsCursor(hiddenColumns); - resetNumColumns(); } } finally { diff --git a/test/jalview/datamodel/HiddenColumnsTest.java b/test/jalview/datamodel/HiddenColumnsTest.java index 6730626..45df6da 100644 --- a/test/jalview/datamodel/HiddenColumnsTest.java +++ b/test/jalview/datamodel/HiddenColumnsTest.java @@ -273,6 +273,7 @@ public class HiddenColumnsTest Iterator regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[5, 5]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 1); colsel.hideSelectedColumns(3, al.getHiddenColumns()); regions = cs.iterator(); @@ -280,6 +281,7 @@ public class HiddenColumnsTest // two hidden ranges, in order: assertEquals("[3, 3]", Arrays.toString(regions.next())); assertEquals("[5, 5]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 2); // hiding column 4 expands [3, 3] to [3, 4] // and merges to [5, 5] to make [3, 5] @@ -287,16 +289,19 @@ public class HiddenColumnsTest regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 5]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 3); // clear hidden columns (note they are added to selected) cs.revealAllHiddenColumns(colsel); // it is now actually null but getter returns an empty list assertEquals(0, cs.getNumberOfRegions()); + assertEquals(cs.getSize(), 0); cs.hideColumns(3, 6); regions = cs.iterator(); int[] firstHiddenRange = regions.next(); assertEquals("[3, 6]", Arrays.toString(firstHiddenRange)); + assertEquals(cs.getSize(), 4); // adding a subrange of already hidden should do nothing cs.hideColumns(4, 5); @@ -304,45 +309,53 @@ public class HiddenColumnsTest assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 4); cs.hideColumns(3, 5); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 4); cs.hideColumns(4, 6); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 4); cs.hideColumns(3, 6); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[3, 6]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 4); cs.revealAllHiddenColumns(colsel); cs.hideColumns(2, 4); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[2, 4]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 3); // extend contiguous with 2 positions overlap cs.hideColumns(3, 5); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[2, 5]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 4); // extend contiguous with 1 position overlap cs.hideColumns(5, 6); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[2, 6]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 5); // extend contiguous with overlap both ends: cs.hideColumns(1, 7); regions = cs.iterator(); assertEquals(1, cs.getNumberOfRegions()); assertEquals("[1, 7]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 7); cs.revealAllHiddenColumns(colsel); cs.hideColumns(15, 18); @@ -353,6 +366,7 @@ public class HiddenColumnsTest assertEquals("[2, 4]", Arrays.toString(regions.next())); assertEquals("[7, 9]", Arrays.toString(regions.next())); assertEquals("[15, 18]", Arrays.toString(regions.next())); + assertEquals(cs.getSize(), 10); } /** @@ -570,14 +584,17 @@ public class HiddenColumnsTest // all unhidden if tohide is empty and range covers hidden hc.hideColumns(tohide, 1, 70); assertTrue(!hc.hasHiddenColumns()); + assertEquals(0, hc.getSize()); hc.hideColumns(3, 5); hc.hideColumns(15, 20); hc.hideColumns(45, 60); + assertEquals(25, hc.getSize()); // but not if range does not cover hidden hc.hideColumns(tohide, 23, 40); assertTrue(hc.hasHiddenColumns()); + assertEquals(25, hc.getSize()); // and partial unhide if range partially covers hc.hideColumns(tohide, 1, 17); @@ -595,6 +612,7 @@ public class HiddenColumnsTest assertEquals(60, region[1]); assertFalse(it.hasNext()); + assertEquals(19, hc.getSize()); } @Test(groups = { "Functional" }) @@ -885,7 +903,7 @@ public class HiddenColumnsTest h.hideColumns(tohide); assertTrue(h.equals(h2)); - // when setting bitset, first param is invlusive, second exclusive + // when setting bitset, first param is inclusive, second exclusive tohide.set(3, 6); tohide.set(9); tohide.set(15, 21); @@ -895,24 +913,30 @@ public class HiddenColumnsTest h2.hideColumns(9, 9); h2.hideColumns(15, 20); assertTrue(h.equals(h2)); + assertEquals(h.getSize(), h2.getSize()); tohide.clear(); tohide.set(41); h.hideColumns(tohide, 23, 30); assertTrue(h.equals(h2)); + assertEquals(h.getSize(), h2.getSize()); tohide.set(41); h.hideColumns(tohide, 30, 45); h2.hideColumns(41, 41); assertTrue(h.equals(h2)); + assertEquals(h.getSize(), h2.getSize()); tohide.clear(); tohide.set(25, 28); h.hideColumns(tohide, 17, 50); h2 = new HiddenColumns(); - h2.hideColumns(17, 20); + h2.hideColumns(5, 5); + h2.hideColumns(9, 9); + h2.hideColumns(15, 16); h2.hideColumns(25, 27); - h2.hideColumns(41, 41); + assertTrue(h.equals(h2)); + assertEquals(h.getSize(), h2.getSize()); } @Test(groups = "Functional") -- 1.7.10.2