JAL-2759 improvements to speed up synchronisation; caching of size
[jalview.git] / src / jalview / datamodel / HiddenColumns.java
index 399e083..0a4d04b 100644 (file)
@@ -24,7 +24,6 @@ import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Iterator;
 import java.util.List;
-import java.util.NoSuchElementException;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 public class HiddenColumns
@@ -33,6 +32,10 @@ public class HiddenColumns
 
   private static final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock();
 
+  private HiddenColumnsCursor cursor = new HiddenColumnsCursor();
+
+  private int numColumns = 0;
+
   /*
    * list of hidden column [start, end] ranges; the list is maintained in
    * ascending start column order
@@ -66,6 +69,7 @@ public class HiddenColumns
           {
             hiddenColumns.add(it.next());
           }
+          cursor.resetCursor(hiddenColumns);
         }
       }
     } finally
@@ -97,6 +101,7 @@ public class HiddenColumns
       if (copy != null)
       {
         hiddenColumns = new ArrayList<>();
+        numColumns = 0;
         Iterator<int[]> it = copy.getBoundedIterator(start, end);
         while (it.hasNext())
         {
@@ -108,8 +113,10 @@ public class HiddenColumns
             hiddenColumns.add(
                     new int[]
             { region[0] - offset, region[1] - offset });
+            numColumns += region[1] - region[0] + 1;
           }
         }
+        cursor.resetCursor(hiddenColumns);
       }
     } finally
     {
@@ -164,17 +171,24 @@ public class HiddenColumns
     try
     {
       LOCK.readLock().lock();
-      int size = 0;
-      if (hiddenColumns != null)
+
+      if (numColumns == 0 && hiddenColumns != null)
       {
-        Iterator<int[]> it = hiddenColumns.iterator();
-        while (it.hasNext())
+        // numColumns is out of date, so recalculate
+        int size = 0;
+        if (hiddenColumns != null)
         {
-          int[] range = it.next();
-          size += range[1] - range[0] + 1;
+          Iterator<int[]> it = hiddenColumns.iterator();
+          while (it.hasNext())
+          {
+            int[] range = it.next();
+            size += range[1] - range[0] + 1;
+          }
         }
+        numColumns = size;
       }
-      return size;
+
+      return numColumns;
     } finally
     {
       LOCK.readLock().unlock();
@@ -263,15 +277,7 @@ public class HiddenColumns
 
       if (hiddenColumns != null)
       {
-        Iterator<int[]> it = hiddenColumns.iterator();
-        while (it.hasNext())
-        {
-          int[] region = it.next();
-          if (result >= region[0])
-          {
-            result += region[1] - region[0] + 1;
-          }
-        }
+        result += cursor.getHiddenOffset(column).getHiddenSoFar();
       }
 
       return result;
@@ -296,43 +302,39 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
       int result = hiddenColumn;
-      int[] region = null;
+
       if (hiddenColumns != null)
       {
-        Iterator<int[]> it = new RegionsIterator(0,
-                hiddenColumn);
-        while (it.hasNext())
-        {
-          region = it.next();
-          if (hiddenColumn > region[1])
-          {
-            result -= region[1] + 1 - region[0];
-          }
-        }
-
-        if (region != null && hiddenColumn >= region[0]
-                && hiddenColumn <= region[1])
+        HiddenCursorPosition cursorPos = cursor
+                .findRegionForColumn(hiddenColumn);
+        int index = cursorPos.getRegionIndex();
+        int hiddenBeforeCol = cursorPos.getHiddenSoFar();
+    
+        // just subtract hidden cols count - this works fine if column is
+        // visible
+        result = hiddenColumn - hiddenBeforeCol;
+    
+        // now check in case column is hidden - it will be in the returned
+        // hidden region
+        if (index < hiddenColumns.size())
         {
-          // Here the hidden column is within a region, so
-          // we want to return the position of region[0]-1, adjusted for any
-          // earlier hidden columns.
-          // Calculate the difference between the actual hidden col position
-          // and region[0]-1, and then subtract from result to convert result
-          // from the adjusted hiddenColumn value to the adjusted region[0]-1
-          // value.
-
-          // However, if the region begins at 0 we cannot return region[0]-1
-          // just return 0
-          if (region[0] == 0)
+          int[] region = hiddenColumns.get(index);
+          if (hiddenColumn >= region[0] && hiddenColumn <= region[1])
           {
-            return 0;
-          }
-          else
-          {
-            return result - (hiddenColumn - region[0] + 1);
+            // actually col is hidden, return region[0]-1
+            // unless region[0]==0 in which case return 0
+            if (region[0] == 0)
+            {
+              result = 0;
+            }
+            else
+            {
+              result = region[0] - 1 - hiddenBeforeCol;
+            }
           }
         }
       }
+
       return result; // return the shifted position after removing hidden
                      // columns.
     } finally
@@ -362,7 +364,8 @@ public class HiddenColumns
       // in case startColumn is in a hidden region, move it to the left
       int start = adjustForHiddenColumns(findColumnPosition(startColumn));
 
-      Iterator<int[]> it = new ReverseRegionsIterator(0, start);
+      Iterator<int[]> it = new ReverseRegionsIterator(0, start,
+              hiddenColumns);
 
       while (it.hasNext() && (distance > 0))
       {
@@ -397,7 +400,8 @@ public class HiddenColumns
    * hidden columns. In otherwords, the next hidden column.
    * 
    * @param alPos
-   *          the (visible) alignmentPosition to find the next hidden column for
+   *          the absolute (visible) alignmentPosition to find the next hidden
+   *          column for
    */
   public int getHiddenBoundaryRight(int alPos)
   {
@@ -406,14 +410,22 @@ public class HiddenColumns
       LOCK.readLock().lock();
       if (hiddenColumns != null)
       {
-        Iterator<int[]> it = hiddenColumns.iterator();
-        while (it.hasNext())
+        int index = cursor.findRegionForColumn(alPos).getRegionIndex();
+        if (index < hiddenColumns.size())
         {
-          int[] region = it.next();
+          int[] region = hiddenColumns.get(index);
           if (alPos < region[0])
           {
             return region[0];
           }
+          else if ((alPos <= region[1])
+                  && (index + 1 < hiddenColumns.size()))
+          {
+            // alPos is within a hidden region, return the next one
+            // if there is one
+            region = hiddenColumns.get(index + 1);
+            return region[0];
+          }
         }
       }
       return alPos;
@@ -428,8 +440,8 @@ public class HiddenColumns
    * hidden columns. In otherwords, the previous hidden column.
    * 
    * @param alPos
-   *          the (visible) alignmentPosition to find the previous hidden column
-   *          for
+   *          the absolute (visible) alignmentPosition to find the previous
+   *          hidden column for
    */
   public int getHiddenBoundaryLeft(int alPos)
   {
@@ -437,16 +449,16 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
 
-      Iterator<int[]> it = new ReverseRegionsIterator(0, alPos);
-      while (it.hasNext())
+      if (hiddenColumns != null)
       {
-        int[] region = it.next();
-        if (alPos > region[1])
+        int index = cursor.findRegionForColumn(alPos).getRegionIndex();
+
+        if (index > 0)
         {
+          int[] region = hiddenColumns.get(index - 1);
           return region[1];
         }
       }
-
       return alPos;
     } finally
     {
@@ -503,6 +515,13 @@ public class HiddenColumns
           added = insertRangeAtRegion(i, start, end);
         } // for
       }
+      if (!wasAlreadyLocked)
+      {
+        cursor.resetCursor(hiddenColumns);
+
+        // reset the number of columns so they will be recounted
+        numColumns = 0;
+      }
     } finally
     {
       if (!wasAlreadyLocked)
@@ -590,17 +609,17 @@ public class HiddenColumns
     {
       LOCK.readLock().lock();
 
-      Iterator<int[]> it = new RegionsIterator(column, column);
-      while (it.hasNext())
+      int regionindex = cursor.findRegionForColumn(column).getRegionIndex();
+      if (regionindex > -1 && regionindex < hiddenColumns.size())
       {
-        int[] region = it.next();
+        int[] region = hiddenColumns.get(regionindex);
         if (column >= region[0] && column <= region[1])
         {
           return false;
         }
       }
-
       return true;
+
     } finally
     {
       LOCK.readLock().unlock();
@@ -633,7 +652,7 @@ public class HiddenColumns
           StringBuffer visibleSeq = new StringBuffer();
 
           Iterator<int[]> blocks = new VisibleContigsIterator(start,
-                  end + 1, false);
+                  end + 1, hiddenColumns);
 
           while (blocks.hasNext())
           {
@@ -796,7 +815,7 @@ public class HiddenColumns
     int w = 0;
     
     Iterator<int[]> blocks = new VisibleContigsIterator(start, end + 1,
-            false);
+            hiddenColumns);
 
     int copylength;
     int annotationLength;
@@ -893,6 +912,8 @@ public class HiddenColumns
       {
         hideColumns(r[0], r[1]);
       }
+      cursor.resetCursor(hiddenColumns);
+      numColumns = 0;
     } finally
     {
       LOCK.writeLock().unlock();
@@ -907,16 +928,21 @@ public class HiddenColumns
     try
     {
       LOCK.writeLock().lock();
-      Iterator<int[]> it = hiddenColumns.iterator();
-      while (it.hasNext())
+      if (hiddenColumns != null)
       {
-        int[] region = it.next();
-        for (int j = region[0]; j < region[1] + 1; j++)
+        Iterator<int[]> it = hiddenColumns.iterator();
+        while (it.hasNext())
         {
-          sel.addElement(j);
+          int[] region = it.next();
+          for (int j = region[0]; j < region[1] + 1; j++)
+          {
+            sel.addElement(j);
+          }
         }
+        hiddenColumns = null;
+        cursor.resetCursor(hiddenColumns);
+        numColumns = 0;
       }
-      hiddenColumns = null;
     } finally
     {
       LOCK.writeLock().unlock();
@@ -934,29 +960,40 @@ public class HiddenColumns
     try
     {
       LOCK.writeLock().lock();
-      Iterator<int[]> it = new RegionsIterator(start, start);
-      while (it.hasNext())
+
+      if (hiddenColumns != null)
       {
-        int[] region = it.next();
-        if (start == region[0])
+        int regionIndex = cursor.findRegionForColumn(start)
+                .getRegionIndex();
+
+        if (regionIndex != -1 && regionIndex != hiddenColumns.size())
         {
-          for (int j = region[0]; j < region[1] + 1; j++)
+          // regionIndex is the region which either contains start
+          // or lies to the right of start
+          int[] region = hiddenColumns.get(regionIndex);
+          if (start == region[0])
           {
-            sel.addElement(j);
+            for (int j = region[0]; j < region[1] + 1; j++)
+            {
+              sel.addElement(j);
+            }
+            int colsToRemove = region[1] - region[0] + 1;
+            hiddenColumns.remove(regionIndex);
+
+            if (hiddenColumns.isEmpty())
+            {
+              hiddenColumns = null;
+              numColumns = 0;
+            }
+            else
+            {
+              numColumns -= colsToRemove;
+            }
+            cursor.updateForDeletedRegion(hiddenColumns);
+
           }
-          it.remove();
-          break;
-        }
-        else if (start < region[0])
-        {
-          break; // passed all possible matching regions
         }
       }
-
-      if (hiddenColumns.size() == 0)
-      {
-        hiddenColumns = null;
-      }
     } finally
     {
       LOCK.writeLock().unlock();
@@ -1061,6 +1098,8 @@ public class HiddenColumns
 
       }
       hiddenColumns = newhidden;
+      cursor.resetCursor(hiddenColumns);
+      numColumns = 0;
     } finally
     {
       LOCK.writeLock().unlock();
@@ -1164,6 +1203,8 @@ public class HiddenColumns
         lastSet = inserts.nextClearBit(firstSet);
         hideColumns(firstSet, lastSet - 1);
       }
+      cursor.resetCursor(hiddenColumns);
+      numColumns = 0;
     } finally
     {
       LOCK.writeLock().unlock();
@@ -1250,7 +1291,6 @@ public class HiddenColumns
     {
       LOCK.readLock().unlock();
     }
-
   }
 
   /**
@@ -1268,18 +1308,36 @@ public class HiddenColumns
       int adjres = adjustForHiddenColumns(res);
 
       int[] reveal = null;
-      Iterator<int[]> it = new RegionsIterator(adjres - 2,
-              adjres + 2);
-      while (it.hasNext())
+
+      if (hiddenColumns != null)
       {
-        int[] region = it.next();
-        if (adjres + 1 == region[0] || adjres - 1 == region[1])
+        // look for a region ending just before adjres
+        int regionindex = cursor.findRegionForColumn(adjres - 1)
+                .getRegionIndex();
+        if (regionindex < hiddenColumns.size()
+                && hiddenColumns.get(regionindex)[1] == adjres - 1)
         {
-          reveal = region;
-          break;
+          reveal = hiddenColumns.get(regionindex);
+        }
+        // check if the region ends just after adjres
+        else if (regionindex < hiddenColumns.size()
+                && hiddenColumns.get(regionindex)[0] == adjres + 1)
+        {
+          reveal = hiddenColumns.get(regionindex);
+        }
+        // or try the next region
+        else
+        {
+          regionindex++;
+          if (regionindex < hiddenColumns.size()
+                  && hiddenColumns.get(regionindex)[0] == adjres + 1)
+          {
+            reveal = hiddenColumns.get(regionindex);
+          }
         }
       }
       return reveal;
+
     } finally
     {
       LOCK.readLock().unlock();
@@ -1291,7 +1349,14 @@ public class HiddenColumns
    */
   public Iterator<int[]> iterator()
   {
-    return new BoundedHiddenColsIterator();
+    try
+    {
+      LOCK.readLock().lock();
+      return new BoundedHiddenColsIterator(hiddenColumns);
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
   }
 
   /**
@@ -1305,7 +1370,14 @@ public class HiddenColumns
    */
   public Iterator<int[]> getBoundedIterator(int start, int end)
   {
-    return new BoundedHiddenColsIterator(start, end);
+    try
+    {
+      LOCK.readLock().lock();
+      return new BoundedHiddenColsIterator(start, end, hiddenColumns);
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
   }
 
   /**
@@ -1319,7 +1391,14 @@ public class HiddenColumns
    */
   public Iterator<Integer> getBoundedStartIterator(int start, int end)
   {
-    return new BoundedStartRegionIterator(start, end, true);
+    try
+    {
+      LOCK.readLock().lock();
+      return new BoundedStartRegionIterator(start, end, hiddenColumns);
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
   }
 
   /**
@@ -1333,7 +1412,14 @@ public class HiddenColumns
    */
   public Iterator<Integer> getVisibleColsIterator(int start, int end)
   {
-    return new VisibleColsIterator(start, end);
+    try
+    {
+      LOCK.readLock().lock();
+      return new VisibleColsIterator(start, end, hiddenColumns);
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
   }
 
   /**
@@ -1347,8 +1433,14 @@ public class HiddenColumns
    */
   public Iterator<int[]> getVisContigsIterator(int start, int end)
   {
-    // return new VisibleBlocksIterator(start, end, true)
-    return new VisibleContigsIterator(start, end, true);
+    try
+    {
+      LOCK.readLock().lock();
+      return new VisibleContigsIterator(start, end, hiddenColumns);
+    } finally
+    {
+      LOCK.readLock().unlock();
+    }
   }
 
   /**
@@ -1378,596 +1470,15 @@ public class HiddenColumns
     }
     else
     {
-      return new VisibleContigsIterator(start, end + 1, true);
-    }
-  }
-
-  /**
-   * A local iterator which 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).
-   */
-  private class RegionsIterator implements Iterator<int[]>
-  {
-    // start position to iterate from
-    private int start;
-
-    // end position to iterate to
-    private int end;
-
-    // current index in hiddenColumns
-    private int currentPosition = 0;
-
-    // current column in hiddenColumns
-    private int[] nextRegion = null;
-
-    private int[] currentRegion = null;
-
-    private int removedIndex = -1;
-
-    // Constructor with bounds
-    RegionsIterator(int lowerBound, int upperBound)
-    {
-      start = lowerBound;
-      end = upperBound;
-
-      if (hiddenColumns != null)
-      {
-        // iterate until a region overlaps with [start,end]
-        currentPosition = 0;
-        while ((currentPosition < hiddenColumns.size())
-                && (hiddenColumns.get(currentPosition)[1] < start))
-        {
-          currentPosition++;
-        }
-        if (currentPosition < hiddenColumns.size())
-        {
-          nextRegion = hiddenColumns.get(currentPosition);
-        }
-      }
-    }
-
-    @Override
-    public boolean hasNext()
-    {
-      return (hiddenColumns != null) && (nextRegion != null)
-              && (nextRegion[0] <= end);
-    }
-
-    @Override
-    public int[] next()
-    {
-      currentRegion = nextRegion;
-      currentPosition++;
-      if (currentPosition < hiddenColumns.size())
-      {
-        nextRegion = hiddenColumns.get(currentPosition);
-      }
-      else
-      {
-        nextRegion = null;
-      }
-      return currentRegion;
-    }
-
-    @Override
-    public void remove()
-    {
-      if ((currentRegion != null) && (removedIndex != currentPosition))
-      {
-        currentPosition--;
-        hiddenColumns.subList(currentPosition, currentPosition + 1).clear();
-        removedIndex = currentPosition;
-      }
-      else
-      {
-        // already removed element last returned by next()
-        // or next() has not yet been called
-        throw new IllegalStateException();
-      }
-    }
-
-  }
-
-  /**
-   * 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).
-   */
-  private class ReverseRegionsIterator implements Iterator<int[]>
-  {
-    // 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;
-
-    // Constructor with bounds
-    ReverseRegionsIterator(int lowerBound, int upperBound)
-    {
-      init(lowerBound, upperBound);
-    }
-
-    /**
-     * Construct an iterator over hiddenColums bounded at
-     * [lowerBound,upperBound]
-     * 
-     * @param lowerBound
-     *          lower bound to iterate to
-     * @param upperBound
-     *          upper bound to iterate from
-     */
-    private void init(int lowerBound, int upperBound)
-    {
-      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;
-    }
-
-  }
-
-  /**
-   * An iterator which iterates over hidden column regions in a range. Works
-   * with a copy of the hidden columns collection. Intended to be used by
-   * callers OUTSIDE of HiddenColumns.
-   */
-  private class BoundedHiddenColsIterator implements Iterator<int[]>
-  {
-    // start position to iterate from
-    private int start;
-
-    // end position to iterate to
-    private int end;
-
-    // current index in hiddenColumns
-    private int currentPosition = 0;
-
-    // current column in hiddenColumns
-    private int[] currentRegion;
-
-    // local copy or reference to hiddenColumns
-    private List<int[]> localHidden;
-
-    /**
-     * Unbounded constructor
-     */
-    BoundedHiddenColsIterator()
-    {
-      if (hiddenColumns != null)
-      {
-        int last = hiddenColumns.get(hiddenColumns.size() - 1)[1];
-        init(0, last);
-      }
-      else
-      {
-        init(0, 0);
-      }
-    }
-
-    /**
-     * Construct an iterator over hiddenColums bounded at
-     * [lowerBound,upperBound]
-     * 
-     * @param lowerBound
-     *          lower bound to iterate from
-     * @param upperBound
-     *          upper bound to iterate to
-     */
-    BoundedHiddenColsIterator(int lowerBound, int upperBound)
-    {
-      init(lowerBound, upperBound);
-    }
-
-    /**
-     * Construct an iterator over hiddenColums bounded at
-     * [lowerBound,upperBound]
-     * 
-     * @param lowerBound
-     *          lower bound to iterate from
-     * @param upperBound
-     *          upper bound to iterate to
-     */
-    private void init(int lowerBound, int upperBound)
-    {
-      start = lowerBound;
-      end = upperBound;
-
       try
       {
         LOCK.readLock().lock();
-        
-        if (hiddenColumns != null)
-        {
-          localHidden = new ArrayList<>();
-
-          // iterate until a region overlaps with [start,end]
-          int i = 0;
-          while ((i < hiddenColumns.size())
-                  && (hiddenColumns.get(i)[1] < start))
-          {
-            i++;
-          }
-
-          // iterate from start to end, adding each hidden region. Positions are
-          // absolute, and all regions which *overlap* [start,end] are added.
-          while (i < hiddenColumns.size()
-                  && (hiddenColumns.get(i)[0] <= end))
-          {
-            int[] rh = hiddenColumns.get(i);
-            int[] cp = new int[2];
-            System.arraycopy(rh, 0, cp, 0, rh.length);
-            localHidden.add(cp);
-            i++;
-          }
-        }
-      }
-      finally
-      {
-        LOCK.readLock().unlock();
-      }
-    }
-
-    @Override
-    public boolean hasNext()
-    {
-      return (localHidden != null)
-              && (currentPosition < localHidden.size());
-    }
-
-    @Override
-    public int[] next()
-    {
-      currentRegion = localHidden.get(currentPosition);
-      currentPosition++;
-      return currentRegion;
-    }
-  }
-
-  /**
-   * An iterator which iterates over visible start positions of hidden column
-   * regions in a range.
-   */
-  private class BoundedStartRegionIterator implements Iterator<Integer>
-  {
-    // start position to iterate from
-    private int start;
-
-    // end position to iterate to
-    private int end;
-
-    // current index in hiddenColumns
-    private int currentPosition = 0;
-
-    // local copy or reference to hiddenColumns
-    private List<Integer> positions = null;
-
-    /**
-     * Construct an iterator over hiddenColums bounded at
-     * [lowerBound,upperBound]
-     * 
-     * @param lowerBound
-     *          lower bound to iterate from
-     * @param upperBound
-     *          upper bound to iterate to
-     * @param useCopyCols
-     *          whether to make a local copy of hiddenColumns for iteration (set
-     *          to true if calling from outwith the HiddenColumns class)
-     */
-    BoundedStartRegionIterator(int lowerBound, int upperBound,
-            boolean useCopy)
-    {
-      start = lowerBound;
-      end = upperBound;
-      
-      try
-      {
-        if (useCopy)
-        {
-          // assume that if useCopy is false the calling code has locked
-          // hiddenColumns
-          LOCK.readLock().lock();
-        }
-
-        if (hiddenColumns != null)
-        {
-          positions = new ArrayList<>(hiddenColumns.size());
-
-          // navigate to start, keeping count of hidden columns
-          int i = 0;
-          int hiddenSoFar = 0;
-          while ((i < hiddenColumns.size())
-                  && (hiddenColumns.get(i)[0] < start + hiddenSoFar))
-          {
-            int[] region = hiddenColumns.get(i);
-            hiddenSoFar += region[1] - region[0] + 1;
-            i++;
-          }
-
-          // iterate from start to end, adding start positions of each
-          // hidden region. Positions are visible columns count, not absolute
-          while (i < hiddenColumns.size()
-                  && (hiddenColumns.get(i)[0] <= end + hiddenSoFar))
-          {
-            int[] region = hiddenColumns.get(i);
-            positions.add(region[0] - hiddenSoFar);
-            hiddenSoFar += region[1] - region[0] + 1;
-            i++;
-          }
-        }
-        else
-        {
-          positions = new ArrayList<>();
-        }
+        return new VisibleContigsIterator(start, end + 1, hiddenColumns);
       } finally
-      {
-        if (useCopy)
-        {
-          LOCK.readLock().unlock();
-        }
-      }
-    }
-
-    @Override
-    public boolean hasNext()
-    {
-      return (currentPosition < positions.size());
-    }
-
-    /**
-     * Get next hidden region start position
-     * 
-     * @return the start position in *visible* coordinates
-     */
-    @Override
-    public Integer next()
     {
-      int result = positions.get(currentPosition);
-      currentPosition++;
-      return result;
-    }
-  }
-
-  /**
-   * Iterator over the visible *columns* (not regions) as determined by the set
-   * of hidden columns. Uses a local copy of hidden columns.
-   * 
-   * @author kmourao
-   *
-   */
-  private class VisibleColsIterator implements Iterator<Integer>
-  {
-    private int last;
-
-    private int current;
-
-    private int next;
-
-    private List<int[]> localHidden = new ArrayList<>();
-
-    private int nexthiddenregion;
-
-    VisibleColsIterator(int firstcol, int lastcol)
-    {
-      last = lastcol;
-      current = firstcol;
-      next = firstcol;
-      nexthiddenregion = 0;
-
-      LOCK.readLock().lock();
-
-      if (hiddenColumns != null)
-      {
-        int i = 0;
-        for (i = 0; i < hiddenColumns.size()
-                && (current <= hiddenColumns.get(i)[0]); ++i)
-        {
-          if (current >= hiddenColumns.get(i)[0]
-                  && current <= hiddenColumns.get(i)[1])
-          {
-            // current is hidden, move to right
-            current = hiddenColumns.get(i)[1] + 1;
-            next = current;
-            nexthiddenregion = i + 1;
-          }
-        }
-
-        for (i = hiddenColumns.size() - 1; i >= 0
-                && (last >= hiddenColumns.get(i)[1]); --i)
-        {
-          if (last >= hiddenColumns.get(i)[0]
-                  && last <= hiddenColumns.get(i)[1])
-          {
-            // last is hidden, move to left
-            last = hiddenColumns.get(i)[0] - 1;
-          }
-        }
-
-        // make a local copy of the bit we need
-        i = nexthiddenregion;
-        while (i < hiddenColumns.size() && hiddenColumns.get(i)[0] <= last)
-        {
-          int[] region = new int[] { hiddenColumns.get(i)[0],
-              hiddenColumns.get(i)[1] };
-          localHidden.add(region);
-          i++;
-        }
-      }
-
-      LOCK.readLock().unlock();
-    }
-
-    @Override
-    public boolean hasNext()
-    {
-      return next <= last;
-    }
-
-    @Override
-    public Integer next()
-    {
-      if (next > last)
-      {
-        throw new NoSuchElementException();
-      }
-      current = next;
-      if ((localHidden != null)
-              && (nexthiddenregion < localHidden.size()))
-      {
-        // still some more hidden regions
-        if (next + 1 < localHidden.get(nexthiddenregion)[0])
-        {
-          // next+1 is still before the next hidden region
-          next++;
-        }
-        else if ((next + 1 >= localHidden.get(nexthiddenregion)[0])
-                && (next + 1 <= localHidden.get(nexthiddenregion)[1]))
-        {
-          // next + 1 is in the next hidden region
-          next = localHidden.get(nexthiddenregion)[1] + 1;
-          nexthiddenregion++;
-        }
-      }
-      else
-      {
-        // finished with hidden regions, just increment normally
-        next++;
-      }
-      return current;
-    }
-
-    @Override
-    public void remove()
-    {
-      throw new UnsupportedOperationException();
-    }
-  }
-
-  /**
-   * An iterator which iterates over visible regions in a range.
-   */
-  private class VisibleContigsIterator implements Iterator<int[]>
-  {
-    private List<int[]> vcontigs = new ArrayList<>();
-
-    private int currentPosition = 0;
-
-    VisibleContigsIterator(int start, int end, boolean usecopy)
-    {
-      try
-      {
-        if (usecopy)
-        {
-          LOCK.readLock().lock();
-        }
-
-        if (hiddenColumns != null && hiddenColumns.size() > 0)
-        {
-          int vstart = start;
-          int hideStart;
-          int hideEnd;
-
-          for (int[] region : hiddenColumns)
-          {
-            hideStart = region[0];
-            hideEnd = region[1];
-
-            // navigate to start
-            if (hideEnd < vstart)
-            {
-              continue;
-            }
-            if (hideStart > vstart)
-            {
-              int[] contig = new int[] { vstart, hideStart - 1 };
-              vcontigs.add(contig);
-            }
-            vstart = hideEnd + 1;
-
-            // exit if we're past the end
-            if (vstart >= end)
-            {
-              break;
-            }
-          }
-
-          if (vstart < end)
-          {
-            int[] contig = new int[] { vstart, end - 1 };
-            vcontigs.add(contig);
-          }
-        }
-        else
-        {
-          int[] contig = new int[] { start, end - 1 };
-          vcontigs.add(contig);
-        }
-      } finally
-      {
-        if (usecopy)
-        {
-          LOCK.readLock().unlock();
-        }
+        LOCK.readLock().unlock();
       }
     }
-
-    @Override
-    public boolean hasNext()
-    {
-      return (currentPosition < vcontigs.size());
-    }
-
-    @Override
-    public int[] next()
-    {
-      int[] result = vcontigs.get(currentPosition);
-      currentPosition++;
-      return result;
-    }
   }
 
   /**
@@ -2053,7 +1564,7 @@ public class HiddenColumns
 
             i++;
           }
-          if (visSoFar < end - start)
+          if (visSoFar < end - start + 1)
           {
             // the number of visible columns we've accounted for is less than
             // the number specified by end-start; work out the end position of