JAL-2388 Fixed inherited&new out-by-one errors; further unit tests

diff --git a/src/jalview/viewmodel/OverviewDimensions.java b/src/jalview/viewmodel/OverviewDimensions.java
index 1f18a10..4a2dd29 100644 (file)
--- a/src/jalview/viewmodel/OverviewDimensions.java
+++ b/src/jalview/viewmodel/OverviewDimensions.java
@@ -138,7 +138,7 @@ public class OverviewDimensions
     {
       x = 0;
     }
-    else if (x >= alwidth)
+    else if (x >= width)
     {
       x = alwidth - 1;
     }
@@ -147,7 +147,7 @@ public class OverviewDimensions
     {
       y = 0;
     }
-    else if (y >= alheight)
+    else if (y >= sequencesHeight)
     {
       y = alheight - 1;
     }
@@ -174,7 +174,8 @@ public class OverviewDimensions
             .findColumnPosition(xAsRes));
 
     // get where end res should be by adding the viewport width on
-    int endRes = xAsRes + vpwidth;
+    // subtract 1 because the width includes endRes
+    int endRes = xAsRes + vpwidth - 1;
 
     // check in case we went off the edge of the alignment
     if (endRes > alwidth)
@@ -182,7 +183,8 @@ public class OverviewDimensions
       // went past the end of the alignment, adjust backwards
       endRes = alwidth;
       // recalc xAsRes backwards from endRes
-      xAsRes = endRes - vpwidth;
+      // add 1 because width includes xAsRes
+      xAsRes = endRes - vpwidth + 1;
     }
 
     //
@@ -193,6 +195,7 @@ public class OverviewDimensions
     int yAsSeq = Math.round((float) y * alheight / sequencesHeight);
 
     // get viewport height in sequences
+    // add 1 because height includes both endSeq and startSeq
     int vpheight = props.getEndSeq() - props.getStartSeq() + 1;
 
     // get where y should be when accounting for hidden rows
@@ -203,7 +206,7 @@ public class OverviewDimensions
             .findIndexWithoutHiddenSeqs(yAsSeq));
 
     // get where end seq should be by adding the viewport height on
-    int endSeq = yAsSeq + vpheight;
+    int endSeq = yAsSeq + vpheight - 1;
 
     // check in case we went off the edge of the alignment
     if (endSeq > alheight)
@@ -250,9 +253,13 @@ public class OverviewDimensions
     boxY = Math.round((float) startSeq * sequencesHeight / alheight);
 
     // boxWidth is the width in residues translated to pixels
+    // since the box includes both the start and end residues, add 1 to the
+    // difference
     boxWidth = Math
             .round((float) (endRes - startRes + 1) * width / alwidth);
     // boxHeight is the height in sequences translated to pixels
+    // since the box includes both the start and end sequences, add 1 to the
+    // difference
     boxHeight = Math.round((float) (endSeq - startSeq + 1)
             * sequencesHeight
             / alheight);

diff --git a/test/jalview/viewmodel/OverviewDimensionsTest.java b/test/jalview/viewmodel/OverviewDimensionsTest.java
index d9d7546..8184b52 100644 (file)
--- a/test/jalview/viewmodel/OverviewDimensionsTest.java
+++ b/test/jalview/viewmodel/OverviewDimensionsTest.java
@@ -118,7 +118,7 @@ public class OverviewDimensionsTest {
     
     av = af.getViewport();
 
-    od = new OverviewDimensions(av.getPosProps(), true);
+
 
     while (av.isCalcInProgress())
     {
@@ -131,10 +131,6 @@ public class OverviewDimensionsTest {
       }
     }
 
-    // Initial box sizing - default path through code
-    od.setBoxPosition(av.getAlignment()
-            .getHiddenSequences(), av.getColumnSelection(), av.getPosProps());
-
     av.showAllHiddenColumns();
     av.showAllHiddenSeqs();
     av.setSelectionGroup(null);
@@ -153,36 +149,42 @@ public class OverviewDimensionsTest {
       }
     }
 
-    mouseClick(od, 0, 0);
-    moveViewport(0, 0);
-
-    viewHeight = av.getEndSeq() - av.getStartSeq();
-    viewWidth = av.getEndRes() - av.getStartRes();
+    viewHeight = av.getEndSeq() - av.getStartSeq() + 1;
+    viewWidth = av.getEndRes() - av.getStartRes() + 1;
 
     // wait for gui to get set up
     // this does actually appear to be necessary
-    while (viewHeight != 17 || viewWidth != 62)
+    while (viewHeight != 18 || viewWidth != 63)
     {
       try
       {
         Thread.sleep(50);
-        av.getAlignPanel().setScrollValues(0, 0);
-        viewHeight = av.getEndSeq() - av.getStartSeq();
-        viewWidth = av.getEndRes() - av.getStartRes();
+        av.getAlignPanel().setScrollValues(0, 1);
+        av.getAlignPanel().setScrollValues(1, 0);
+        viewHeight = av.getEndSeq() - av.getStartSeq() + 1;
+        viewWidth = av.getEndRes() - av.getStartRes() + 1;
       } catch (InterruptedException e)
       {
 
       }
     }
 
+    od = new OverviewDimensions(av.getPosProps(), true);
+    // Initial box sizing - default path through code
+    od.setBoxPosition(av.getAlignment().getHiddenSequences(),
+            av.getColumnSelection(), av.getPosProps());
+
+    mouseClick(od, 0, 0);
+    moveViewport(0, 0);
+
     // calculate before hidden columns so we get absolute values
     alheight = av.getAlignment().getHeight();
     alwidth = av.getAlignment().getWidth();
 
     boxWidth = Math.round((float) (av.getEndRes() - av.getStartRes() + 1)
-            / alwidth * od.getWidth());
-    boxHeight = Math.round((float) (av.getEndSeq() - av.getStartSeq())
-            / alheight * od.getHeight());
+            * od.getWidth() / alwidth);
+    boxHeight = Math.round((float) (av.getEndSeq() - av.getStartSeq() + 1)
+            * od.getSequencesHeight() / alheight);
     System.out.println(boxHeight);
 
   }
@@ -326,7 +328,7 @@ public class OverviewDimensionsTest {
 
     // overly large boxX value reset to width-boxWidth
     mouseClick(od, 100, 6);
-    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth());
+    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth() + 1);
     assertEquals(od.getBoxY(), 6);
     assertEquals(od.getBoxWidth(), boxWidth);
     assertEquals(od.getBoxHeight(), boxHeight);
@@ -350,7 +352,7 @@ public class OverviewDimensionsTest {
 
     // click past end of alignment, as above
     mouseClick(od, 3000, 5);
-    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth());
+    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth() + 1);
     assertEquals(od.getBoxWidth(), boxWidth);
     assertEquals(od.getBoxHeight(), boxHeight);
     assertEquals(od.getScrollCol(),
@@ -442,7 +444,7 @@ public class OverviewDimensionsTest {
     // overly large boxX value reset to width-boxWidth
     xpos = 100;
     mouseClick(od, xpos, 5);
-    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth());
+    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth() + 1);
     assertEquals(od.getBoxY(), 5);
     assertEquals(od.getBoxWidth(), boxWidth);
     assertEquals(od.getBoxHeight(), boxHeight);
@@ -545,7 +547,7 @@ public class OverviewDimensionsTest {
     // boxX, scrollCol adjusted back, box width normal
     xpos = 3000;
     mouseClick(od, xpos, 5);
-    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth());
+    assertEquals(od.getBoxX(), od.getWidth() - od.getBoxWidth() + 1);
     assertEquals(od.getBoxY(), 5);
     assertEquals(od.getBoxWidth(), boxWidth);
     assertEquals(od.getBoxHeight(), boxHeight);
@@ -786,7 +788,6 @@ public class OverviewDimensionsTest {
     assertEquals(od.getBoxHeight(), boxHeight);
 
     // move viewport to hidden columns
-    // TODO boxwidth includes hidden in overview panel (why?)
     moveViewport(102, 0);
     assertEquals(od.getBoxX(),
             Math.round((float) 102 * od.getWidth() / alwidth));
@@ -854,10 +855,10 @@ public class OverviewDimensionsTest {
     assertEquals(od.getBoxY(), Math.round ((float)198 * od.getSequencesHeight()
             / alheight));
     assertEquals(od.getBoxWidth(), boxWidth);
-    assertEquals(od.getBoxHeight(), boxHeight
-                    + Math.round((float) (lastHidden - firstHidden + 1)
-                            * od.getSequencesHeight()
- / alheight));
+    assertEquals(
+            od.getBoxHeight(),
+            Math.round((float) (viewHeight + lastHidden - firstHidden + 1)
+                    * od.getSequencesHeight() / alheight));
   }
 
   /**
@@ -1077,7 +1078,7 @@ public class OverviewDimensionsTest {
   private void moveViewportH(int startRes)
   {
     av.setStartRes(startRes);
-    av.setEndRes(startRes + viewWidth);
+    av.setEndRes(startRes + viewWidth - 1);
     od.setBoxPosition(av.getAlignment()
             .getHiddenSequences(), av.getColumnSelection(), av.getPosProps());
   }
@@ -1088,7 +1089,7 @@ public class OverviewDimensionsTest {
   private void moveViewportV(int startSeq)
   {
     av.setStartSeq(startSeq);
-    av.setEndSeq(startSeq + viewHeight);
+    av.setEndSeq(startSeq + viewHeight - 1);
     od.setBoxPosition(av.getAlignment()
             .getHiddenSequences(), av.getColumnSelection(), av.getPosProps());
   }
@@ -1099,9 +1100,9 @@ public class OverviewDimensionsTest {
   private void moveViewport(int startRes, int startSeq)
   {
     av.setStartRes(startRes);
-    av.setEndRes(startRes + viewWidth);
+    av.setEndRes(startRes + viewWidth - 1);
     av.setStartSeq(startSeq);
-    av.setEndSeq(startSeq + viewHeight);
+    av.setEndSeq(startSeq + viewHeight - 1);
     od.setBoxPosition(av.getAlignment()
             .getHiddenSequences(), av.getColumnSelection(), av.getPosProps());
   }
@@ -1120,9 +1121,9 @@ public class OverviewDimensionsTest {
     // int width = av.getEndRes() - av.getStartRes();
     // int height = av.getEndSeq() - av.getStartSeq();
     av.setStartRes(od.getScrollCol());
-    av.setEndRes(od.getScrollCol() + viewWidth);
+    av.setEndRes(od.getScrollCol() + viewWidth - 1);
     av.setStartSeq(od.getScrollRow());
-    av.setEndSeq(od.getScrollRow() + viewHeight);
+    av.setEndSeq(od.getScrollRow() + viewHeight - 1);
     od.setBoxPosition(av.getAlignment()
             .getHiddenSequences(), av.getColumnSelection(), av.getPosProps());
   }