JAL-3412 calculateIdWidth - more consistency, constants, tests
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 19 Aug 2019 14:43:07 +0000 (15:43 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 19 Aug 2019 14:43:07 +0000 (15:43 +0100)
src/jalview/gui/AlignViewport.java
src/jalview/gui/AlignmentPanel.java
src/jalview/gui/AnnotationLabels.java
src/jalview/gui/IdwidthAdjuster.java
src/jalview/gui/SplitFrame.java
test/jalview/gui/AlignmentPanelTest.java

index 61b0d1b..fcae40d 100644 (file)
@@ -891,10 +891,9 @@ public class AlignViewport extends AlignmentViewport
     if (ap != null)
     {
       // modify GUI elements to reflect geometry change
-      Dimension idw = getAlignPanel().getIdPanel().getIdCanvas()
-              .getPreferredSize();
+      Dimension idw = ap.getIdPanel().getIdCanvas().getPreferredSize();
       idw.width = i;
-      getAlignPanel().getIdPanel().getIdCanvas().setPreferredSize(idw);
+      ap.getIdPanel().getIdCanvas().setPreferredSize(idw);
     }
   }
 
index ea8fcdc..85c46bd 100644 (file)
@@ -73,6 +73,11 @@ import javax.swing.SwingUtilities;
 public class AlignmentPanel extends GAlignmentPanel implements
         AdjustmentListener, Printable, AlignmentViewPanel, ViewportListenerI
 {
+  /*
+   * spare space in pixels between sequence id and alignment panel
+   */
+  private static final int ID_WIDTH_PADDING = 4;
+
   public AlignViewport av;
 
   OverviewPanel overviewPanel;
@@ -233,8 +238,6 @@ public class AlignmentPanel extends GAlignmentPanel implements
     getAnnotationPanel().adjustPanelHeight();
 
     Dimension d = calculateIdWidth();
-
-    d.setSize(d.width + 4, d.height);
     getIdPanel().getIdCanvas().setPreferredSize(d);
     hscrollFillerPanel.setPreferredSize(d);
 
@@ -255,7 +258,8 @@ public class AlignmentPanel extends GAlignmentPanel implements
     if (av.getIdWidth() < 0)
     {
       int afwidth = (alignFrame != null ? alignFrame.getWidth() : 300);
-      int maxwidth = Math.max(20, Math.min(afwidth - 200, 2 * afwidth / 3));
+      int idWidth = Math.min(afwidth - 200, 2 * afwidth / 3);
+      int maxwidth = Math.max(IdwidthAdjuster.MIN_ID_WIDTH, idWidth);
       r = calculateIdWidth(maxwidth);
       av.setIdWidth(r.width);
     }
@@ -277,7 +281,7 @@ public class AlignmentPanel extends GAlignmentPanel implements
    * @return Dimension giving the maximum width of the alignment label panel
    *         that should be used.
    */
-  public Dimension calculateIdWidth(int maxwidth)
+  protected Dimension calculateIdWidth(int maxwidth)
   {
     Container c = new Container();
 
@@ -287,19 +291,13 @@ public class AlignmentPanel extends GAlignmentPanel implements
     AlignmentI al = av.getAlignment();
     int i = 0;
     int idWidth = 0;
-    String id;
 
     while ((i < al.getHeight()) && (al.getSequenceAt(i) != null))
     {
       SequenceI s = al.getSequenceAt(i);
-
-      id = s.getDisplayId(av.getShowJVSuffix());
-
-      if (fm.stringWidth(id) > idWidth)
-      {
-        idWidth = fm.stringWidth(id);
-      }
-
+      String id = s.getDisplayId(av.getShowJVSuffix());
+      int stringWidth = fm.stringWidth(id);
+      idWidth = Math.max(idWidth, stringWidth);
       i++;
     }
 
@@ -313,18 +311,16 @@ public class AlignmentPanel extends GAlignmentPanel implements
       while (i < al.getAlignmentAnnotation().length)
       {
         String label = al.getAlignmentAnnotation()[i].label;
-
-        if (fm.stringWidth(label) > idWidth)
-        {
-          idWidth = fm.stringWidth(label);
-        }
-
+        int stringWidth = fm.stringWidth(label);
+        idWidth = Math.max(idWidth, stringWidth);
         i++;
       }
     }
 
-    return new Dimension(
-            maxwidth < 0 ? idWidth : Math.min(maxwidth, idWidth), 12);
+    int w = maxwidth < 0 ? idWidth : Math.min(maxwidth, idWidth);
+    w += ID_WIDTH_PADDING;
+
+    return new Dimension(w, 12);
   }
 
   /**
@@ -1135,21 +1131,22 @@ public class AlignmentPanel extends GAlignmentPanel implements
    *          be returned
    * @return
    */
-  public int getVisibleIdWidth(boolean onscreen)
+  protected int getVisibleIdWidth(boolean onscreen)
   {
     // see if rendering offscreen - check preferences and calc width accordingly
     if (!onscreen && Cache.getDefault("FIGURE_AUTOIDWIDTH", false))
     {
-      return calculateIdWidth(-1).width + 4;
+      return calculateIdWidth(-1).width;
     }
-    Integer idwidth = null;
-    if (onscreen || (idwidth = Cache
-            .getIntegerProperty("FIGURE_FIXEDIDWIDTH")) == null)
+    Integer idwidth = onscreen ? null
+            : Cache.getIntegerProperty("FIGURE_FIXEDIDWIDTH");
+    if (idwidth != null)
     {
-      int w = getIdPanel().getWidth();
-      return (w > 0 ? w : calculateIdWidth().width + 4);
+      return idwidth.intValue() + ID_WIDTH_PADDING;
     }
-    return idwidth.intValue() + 4;
+
+    int w = getIdPanel().getWidth();
+    return (w > 0 ? w : calculateIdWidth().width);
   }
 
   void makeAlignmentImage(jalview.util.ImageMaker.TYPE type, File file)
index 6da6cc3..de67c23 100755 (executable)
@@ -1001,7 +1001,7 @@ public class AnnotationLabels extends JPanel
     int width = getWidth();
     if (width == 0)
     {
-      width = ap.calculateIdWidth().width + 4;
+      width = ap.calculateIdWidth().width;
     }
 
     Graphics2D g2 = (Graphics2D) g;
index 0cffc3b..4ba0699 100755 (executable)
@@ -40,7 +40,7 @@ import javax.swing.JPanel;
 public class IdwidthAdjuster extends JPanel
         implements MouseListener, MouseMotionListener
 {
-  boolean active = false;
+  public static final int MIN_ID_WIDTH = 20;
 
   int oldX = 0;
 
@@ -61,10 +61,9 @@ public class IdwidthAdjuster extends JPanel
   }
 
   /**
-   * DOCUMENT ME!
+   * Action on mouse pressed is to save the start position for any drag
    * 
    * @param evt
-   *          DOCUMENT ME!
    */
   @Override
   public void mousePressed(MouseEvent evt)
@@ -73,121 +72,103 @@ public class IdwidthAdjuster extends JPanel
   }
 
   /**
-   * DOCUMENT ME!
+   * On release of mouse drag to resize the width, if there is a complementary
+   * alignment in a split frame, sets the complement to the same id width and
+   * repaints the split frame. Note this is done whether or not the protein
+   * characters are scaled to codon width.
    * 
    * @param evt
-   *          DOCUMENT ME!
    */
   @Override
   public void mouseReleased(MouseEvent evt)
   {
-    active = false;
     repaint();
 
     /*
-     * If in a SplitFrame with co-scaled alignments, set the other's id width to
-     * match
+     * If in a SplitFrame, set the other's id width to match
      */
     final AlignViewportI viewport = ap.getAlignViewport();
-    if (viewport.getCodingComplement() != null
-            && viewport.isScaleProteinAsCdna())
+    if (viewport.getCodingComplement() != null)
     {
       viewport.getCodingComplement().setIdWidth(viewport.getIdWidth());
       SplitFrame sf = (SplitFrame) ap.alignFrame.getSplitViewContainer();
       sf.repaint();
     }
-
   }
 
   /**
-   * DOCUMENT ME!
+   * When this region is entered, repaints to show a left-right move cursor
    * 
    * @param evt
-   *          DOCUMENT ME!
    */
   @Override
   public void mouseEntered(MouseEvent evt)
   {
-    active = true;
     repaint();
   }
 
-  /**
-   * DOCUMENT ME!
-   * 
-   * @param evt
-   *          DOCUMENT ME!
-   */
   @Override
   public void mouseExited(MouseEvent evt)
   {
-    active = false;
-    repaint();
   }
 
   /**
-   * DOCUMENT ME!
+   * Adjusts the id panel width for a mouse drag left or right (subject to a
+   * minimum of 20 pixels) and repaints the alignment
    * 
    * @param evt
-   *          DOCUMENT ME!
    */
   @Override
   public void mouseDragged(MouseEvent evt)
   {
-    active = true;
-
+    int mouseX = evt.getX();
     final AlignViewportI viewport = ap.getAlignViewport();
     int curwidth = viewport.getIdWidth();
-    int dif = evt.getX() - oldX;
+    int dif = mouseX - oldX;
 
     final int newWidth = curwidth + dif;
-    if ((newWidth > 20) || (dif > 0))
-    {
-      viewport.setIdWidth(newWidth);
 
-      ap.paintAlignment(true, false);
+    /*
+     * don't drag below minimum width
+     */
+    if (newWidth < MIN_ID_WIDTH)
+    {
+      return;
     }
 
     oldX = evt.getX();
+
+    /*
+     * don't drag right if mouse is to the left of the region
+     */
+    if (dif > 0 && mouseX < 0)
+    {
+      return;
+    }
+    viewport.setIdWidth(newWidth);
+    ap.paintAlignment(true, false);
   }
 
-  /**
-   * DOCUMENT ME!
-   * 
-   * @param evt
-   *          DOCUMENT ME!
-   */
   @Override
   public void mouseMoved(MouseEvent evt)
   {
   }
 
-  /**
-   * DOCUMENT ME!
-   * 
-   * @param evt
-   *          DOCUMENT ME!
-   */
   @Override
   public void mouseClicked(MouseEvent evt)
   {
   }
 
   /**
-   * DOCUMENT ME!
+   * Paints this region, showing a left-right move cursor if currently 'active'
    * 
    * @param g
-   *          DOCUMENT ME!
    */
   @Override
   public void paintComponent(Graphics g)
   {
     g.setColor(Color.white);
     g.fillRect(0, 0, getWidth(), getHeight());
-
-    if (active)
-    {
-        setCursor(Cursor.getPredefinedCursor(Cursor.W_RESIZE_CURSOR));
-    }
+    setCursor(Cursor.getPredefinedCursor(Cursor.W_RESIZE_CURSOR));
   }
 }
index 25dedc5..71380c7 100644 (file)
@@ -161,26 +161,21 @@ public class SplitFrame extends GSplitFrame implements SplitContainerI
    */
   public void adjustLayout()
   {
+    final AlignViewport topViewport = ((AlignFrame) getTopFrame()).viewport;
+    final AlignViewport bottomViewport = ((AlignFrame) getBottomFrame()).viewport;
+
     /*
      * Ensure sequence ids are the same width so sequences line up
      */
-    int w1 = ((AlignFrame) getTopFrame()).getViewport().getIdWidth();
-    int w2 = ((AlignFrame) getBottomFrame()).getViewport().getIdWidth();
+    int w1 = topViewport.getIdWidth();
+    int w2 = bottomViewport.getIdWidth();
     int w3 = Math.max(w1, w2);
-    if (w1 != w3)
-    {
-      ((AlignFrame) getTopFrame()).getViewport().setIdWidth(w3);
-    }
-    if (w2 != w3)
-    {
-      ((AlignFrame) getBottomFrame()).getViewport().setIdWidth(w3);
-    }
+    topViewport.setIdWidth(w3);
+    bottomViewport.setIdWidth(w3);
 
     /*
      * Scale protein to either 1 or 3 times character width of dna
      */
-    final AlignViewport topViewport = ((AlignFrame) getTopFrame()).viewport;
-    final AlignViewport bottomViewport = ((AlignFrame) getBottomFrame()).viewport;
     final AlignmentI topAlignment = topViewport.getAlignment();
     final AlignmentI bottomAlignment = bottomViewport.getAlignment();
     AlignmentViewport cdna = topAlignment.isNucleotide() ? topViewport
index e84b87a..85ca37c 100644 (file)
@@ -23,111 +23,23 @@ package jalview.gui;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotEquals;
 
+import jalview.api.AlignViewportI;
 import jalview.bin.Cache;
 import jalview.bin.Jalview;
-import jalview.datamodel.Sequence;
+import jalview.datamodel.AlignmentAnnotation;
 import jalview.datamodel.SequenceI;
 import jalview.io.DataSourceType;
 import jalview.io.FileLoader;
 import jalview.viewmodel.ViewportRanges;
 
+import java.awt.Dimension;
+import java.awt.Font;
+
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 public class AlignmentPanelTest
 {
-  SequenceI seq1 = new Sequence(
-          "Seq1",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq2 = new Sequence(
-          "Seq2",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq3 = new Sequence(
-          "Seq3",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq4 = new Sequence(
-          "Seq4",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq5 = new Sequence(
-          "Seq5",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq6 = new Sequence(
-          "Seq6",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq7 = new Sequence(
-          "Seq7",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq8 = new Sequence(
-          "Seq8",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq9 = new Sequence(
-          "Seq9",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq10 = new Sequence(
-          "Seq10",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq11 = new Sequence(
-          "Seq11",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq12 = new Sequence(
-          "Seq12",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq13 = new Sequence(
-          "Seq13",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq14 = new Sequence(
-          "Seq14",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq15 = new Sequence(
-          "Seq15",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq16 = new Sequence(
-          "Seq16",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq17 = new Sequence(
-          "Seq17",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq18 = new Sequence(
-          "Seq18",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq19 = new Sequence(
-          "Seq19",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq20 = new Sequence(
-          "Seq20",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq21 = new Sequence(
-          "Seq21",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq22 = new Sequence(
-          "Seq22",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
-  SequenceI seq23 = new Sequence(
-          "Seq23",
-          "ABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBACABCABCABCABCABCABCABCABCBACBACBACBAC");
-
   AlignFrame af;
 
   @BeforeMethod(alwaysRun = true)
@@ -158,13 +70,12 @@ public class AlignmentPanelTest
     }
   }
 
-
   /**
    * Test side effect that end residue is set correctly by setScrollValues, with
    * or without hidden columns
    */
   @Test(groups = "Functional")
-  public void TestSetScrollValues()
+  public void testSetScrollValues()
   {
     ViewportRanges ranges = af.getViewport().getRanges();
     af.alignPanel.setScrollValues(0, 0);
@@ -225,7 +136,7 @@ public class AlignmentPanelTest
    * when switching from wrapped back to unwrapped mode (JAL-2739)
    */
   @Test(groups = "Functional")
-  public void TestUpdateLayout_endRes()
+  public void testUpdateLayout_endRes()
   {
     // get details of original alignment dimensions
     ViewportRanges ranges = af.getViewport().getRanges();
@@ -244,6 +155,118 @@ public class AlignmentPanelTest
 
     // endRes back to original value
     assertEquals(ranges.getEndRes(), endres);
+  }
+
+  /**
+   * Test the variant of calculateIdWidth that only recomputes the width if it is
+   * not already saved in the viewport (initial value is -1)
+   */
+  @Test(groups = "Functional")
+  public void testCalculateIdWidth_noArgs()
+  {
+    AlignViewportI av = af.alignPanel.getAlignViewport();
+    av.setShowJVSuffix(true);
+    av.setFont(new Font("Courier", Font.PLAIN, 15), true);
+
+    av.setIdWidth(0);
+    Dimension d = af.alignPanel.calculateIdWidth();
+    assertEquals(d.width, 0);
+    assertEquals(d.height, 0);
+
+    av.setIdWidth(99);
+    d = af.alignPanel.calculateIdWidth();
+    assertEquals(d.width, 99);
+    assertEquals(d.height, 0);
+
+    /*
+     * note 4 pixels padding are added to the longest sequence name width
+     */
+    av.setIdWidth(-1); // force recalculation
+    d = af.alignPanel.calculateIdWidth();
+    assertEquals(d.width, 166); // 4 + pixel width of "Q93Z60_ARATH/1-118"
+    assertEquals(d.height, 12);
+    assertEquals(d.width, av.getIdWidth());
+  }
+
+  /**
+   * Test the variant of calculateIdWidth that computes the longest of any
+   * sequence name or annotation label width
+   */
+  @Test(groups = "Functional")
+  public void testCalculateIdWidth_withMaxWidth()
+  {
+    AlignViewportI av = af.alignPanel.getAlignViewport();
+    av.setShowJVSuffix(true);
+    av.setFont(new Font("Courier", Font.PLAIN, 15), true);
+    av.setShowAnnotation(false);
+    av.setIdWidth(18);
+
+    /*
+     * note 4 pixels 'padding' are added to the longest seq name/annotation label
+     */
+    Dimension d = af.alignPanel.calculateIdWidth(2000);
+    assertEquals(d.width, 166); // 4 + pixel width of "Q93Z60_ARATH/1-118"
+    assertEquals(d.height, 12); // fixed value (not used?)
+    assertEquals(av.getIdWidth(), 18); // not changed by this method
+
+    /*
+     * make the longest sequence name longer
+     */
+    SequenceI seq = af.viewport.getAlignment()
+            .findSequenceMatch("Q93Z60_ARATH")[0];
+    seq.setName(seq.getName() + "MMMMM");
+    d = af.alignPanel.calculateIdWidth(2000);
+    assertEquals(d.width, 211); // 4 + pixel width of "Q93Z60_ARATHMMMMM/1-118"
+    assertEquals(d.height, 12);
+    assertEquals(av.getIdWidth(), 18); // unchanged
+
+    /*
+     * make the longest annotation name even longer
+     * note this is checked even if annotations are not shown
+     */
+    AlignmentAnnotation aa = av.getAlignment().getAlignmentAnnotation()[0];
+    aa.label = "THIS IS A VERY LONG LABEL INDEED";
+    d = af.alignPanel.calculateIdWidth(2000);
+    assertEquals(d.width, 229); // 4 + pixel width of "THIS IS A VERY LONG LABEL INDEED"
+    assertEquals(d.height, 12);
 
+    /*
+     * override with maxwidth
+     * note the 4 pixels padding is added to this value
+     */
+    d = af.alignPanel.calculateIdWidth(213);
+    assertEquals(d.width, 217);
+    assertEquals(d.height, 12);
+  }
+
+  @Test(groups = "Functional")
+  public void testGetVisibleWidth()
+  {
+    /*
+     * width for onscreen rendering is IDPanel width
+     */
+    int w = af.alignPanel.getVisibleIdWidth(true);
+    assertEquals(w, af.alignPanel.getIdPanel().getWidth());
+    assertEquals(w, 115);
+
+    /*
+     * width for offscreen rendering is the same
+     * if no fixed id width is specified in preferences
+     */
+    Cache.setProperty("FIGURE_AUTOIDWIDTH", Boolean.FALSE.toString());
+    Cache.removeProperty("FIGURE_FIXEDIDWIDTH");
+    assertEquals(w, af.alignPanel.getVisibleIdWidth(false));
+
+    /*
+     * preference for fixed id width - note 4 pixels padding is added
+     */
+    Cache.setProperty("FIGURE_FIXEDIDWIDTH", "120");
+    assertEquals(124, af.alignPanel.getVisibleIdWidth(false));
+
+    /*
+     * preference for auto id width overrides fixed width
+     */
+    Cache.setProperty("FIGURE_AUTOIDWIDTH", Boolean.TRUE.toString());
+    assertEquals(115, af.alignPanel.getVisibleIdWidth(false));
   }
 }