From ad6e0dcf682399eb8b7a0ac49d3fcc63f4f991f2 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Mon, 19 Aug 2019 15:43:07 +0100 Subject: [PATCH] JAL-3412 calculateIdWidth - more consistency, constants, tests --- src/jalview/gui/AlignViewport.java | 5 +- src/jalview/gui/AlignmentPanel.java | 55 ++++---- src/jalview/gui/AnnotationLabels.java | 2 +- src/jalview/gui/IdwidthAdjuster.java | 81 +++++------ src/jalview/gui/SplitFrame.java | 19 +-- test/jalview/gui/AlignmentPanelTest.java | 215 +++++++++++++++++------------- 6 files changed, 186 insertions(+), 191 deletions(-) diff --git a/src/jalview/gui/AlignViewport.java b/src/jalview/gui/AlignViewport.java index 61b0d1b..fcae40d 100644 --- a/src/jalview/gui/AlignViewport.java +++ b/src/jalview/gui/AlignViewport.java @@ -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); } } diff --git a/src/jalview/gui/AlignmentPanel.java b/src/jalview/gui/AlignmentPanel.java index ea8fcdc..85c46bd 100644 --- a/src/jalview/gui/AlignmentPanel.java +++ b/src/jalview/gui/AlignmentPanel.java @@ -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) diff --git a/src/jalview/gui/AnnotationLabels.java b/src/jalview/gui/AnnotationLabels.java index 6da6cc3..de67c23 100755 --- a/src/jalview/gui/AnnotationLabels.java +++ b/src/jalview/gui/AnnotationLabels.java @@ -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; diff --git a/src/jalview/gui/IdwidthAdjuster.java b/src/jalview/gui/IdwidthAdjuster.java index 0cffc3b..4ba0699 100755 --- a/src/jalview/gui/IdwidthAdjuster.java +++ b/src/jalview/gui/IdwidthAdjuster.java @@ -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)); } } diff --git a/src/jalview/gui/SplitFrame.java b/src/jalview/gui/SplitFrame.java index 25dedc5..71380c7 100644 --- a/src/jalview/gui/SplitFrame.java +++ b/src/jalview/gui/SplitFrame.java @@ -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 diff --git a/test/jalview/gui/AlignmentPanelTest.java b/test/jalview/gui/AlignmentPanelTest.java index e84b87a..85ca37c 100644 --- a/test/jalview/gui/AlignmentPanelTest.java +++ b/test/jalview/gui/AlignmentPanelTest.java @@ -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)); } } -- 1.7.10.2