From 74d5ca6390288f6cd6cb445cccc728802806a29a Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 19 Sep 2019 15:25:37 +0100 Subject: [PATCH] JAL-3383 minor code tidying and commenting --- src/jalview/api/FeatureColourI.java | 5 ++- src/jalview/datamodel/Alignment.java | 10 ++--- src/jalview/datamodel/AllColsCollection.java | 4 +- src/jalview/datamodel/HiddenColumns.java | 7 +++ src/jalview/gui/IdCanvas.java | 44 ++++++++----------- src/jalview/gui/OverviewCanvas.java | 15 +++---- src/jalview/gui/OverviewPanel.java | 29 ++---------- src/jalview/gui/PaintRefresher.java | 4 +- src/jalview/gui/ScalePanel.java | 3 -- src/jalview/gui/SeqCanvas.java | 44 ++++++------------- src/jalview/renderer/OverviewRenderer.java | 13 +++--- src/jalview/renderer/OverviewResColourFinder.java | 46 ++++++++++---------- src/jalview/viewmodel/ViewportRanges.java | 19 +------- test/jalview/renderer/OverviewRendererTest.java | 2 + .../renderer/OverviewResColourFinderTest.java | 4 +- 15 files changed, 94 insertions(+), 155 deletions(-) diff --git a/src/jalview/api/FeatureColourI.java b/src/jalview/api/FeatureColourI.java index c5f6727..44bc321 100644 --- a/src/jalview/api/FeatureColourI.java +++ b/src/jalview/api/FeatureColourI.java @@ -64,9 +64,10 @@ public interface FeatureColourI Color getNoColour(); /** - * Answers true if the feature has a single colour + * Answers true if the feature has a single colour, i.e. if isColourByLabel() + * and isGraduatedColour() both answer false; else answers false * - * @return true iff not (isColourByLabel() || isGraduatedColour()) + * @return */ boolean isSimpleColour(); diff --git a/src/jalview/datamodel/Alignment.java b/src/jalview/datamodel/Alignment.java index 22d23bc..26379d3 100755 --- a/src/jalview/datamodel/Alignment.java +++ b/src/jalview/datamodel/Alignment.java @@ -42,13 +42,13 @@ import java.util.Vector; /** * Data structure to hold and manipulate a multiple sequence alignment - */ -/** - * @author JimP * + * @author JimP */ public class Alignment implements AlignmentI { + private static final SequenceGroup[] NO_GROUPS = new SequenceGroup[0]; + private Alignment dataset; private List sequences; @@ -61,8 +61,6 @@ public class Alignment implements AlignmentI private List codonFrameList; - private static final SequenceGroup[] noGroups = new SequenceGroup[0]; - /* * persistent object to hold result of findAllGroups(SequenceI) */ @@ -419,7 +417,7 @@ public class Alignment implements AlignmentI int gSize = groups.size(); if (gSize == 0) { - return noGroups; + return NO_GROUPS; } groupsForSequence.clear(); for (int i = 0; i < gSize; i++) diff --git a/src/jalview/datamodel/AllColsCollection.java b/src/jalview/datamodel/AllColsCollection.java index c65c381..af3fffa 100644 --- a/src/jalview/datamodel/AllColsCollection.java +++ b/src/jalview/datamodel/AllColsCollection.java @@ -27,9 +27,9 @@ import java.util.Iterator; public class AllColsCollection implements AlignmentColsCollectionI { - int start; + final int start; - int end; + final int end; HiddenColumns hidden; diff --git a/src/jalview/datamodel/HiddenColumns.java b/src/jalview/datamodel/HiddenColumns.java index 458bde7..48ebb7a 100644 --- a/src/jalview/datamodel/HiddenColumns.java +++ b/src/jalview/datamodel/HiddenColumns.java @@ -88,6 +88,13 @@ public class HiddenColumns private BitSet hiddenBitSet; + /** + * Returns a BitSet with a set bit for each hidden column (0, 1, ...). This is + * valid at the time of calling, but will not reflect any changes made + * afterwards. + * + * @return + */ public BitSet getBitset() { if (hiddenBitSet == null) diff --git a/src/jalview/gui/IdCanvas.java b/src/jalview/gui/IdCanvas.java index e5a5946..0731bf3 100755 --- a/src/jalview/gui/IdCanvas.java +++ b/src/jalview/gui/IdCanvas.java @@ -59,7 +59,7 @@ public class IdCanvas extends JPanel implements ViewportListenerI int imgHeight = 0; - boolean fastPaint = false; + private boolean fastPaint = false; List searchResults; @@ -579,45 +579,35 @@ public class IdCanvas extends JPanel implements ViewportListenerI @Override public void propertyChange(PropertyChangeEvent evt) { - // BH just clarifying logic String propertyName = evt.getPropertyName(); - switch (propertyName) { + switch (propertyName) + { case ViewportRanges.STARTSEQ: fastPaint((int) evt.getNewValue() - (int) evt.getOldValue()); - return; + break; case ViewportRanges.STARTRES: if (av.getWrapAlignment()) { fastPaint((int) evt.getNewValue() - (int) evt.getOldValue()); } - return; + break; case ViewportRanges.STARTRESANDSEQ: fastPaint(((int[]) evt.getNewValue())[1] - ((int[]) evt.getOldValue())[1]); - return; + break; case ViewportRanges.MOVE_VIEWPORT: repaint(); - return; - case ViewportRanges.ENDRES: - case ViewportRanges.ENDSEQ: - // ignore ?? - return; + break; + default: } -// BH 2019.07.27 was: -// if (propertyName.equals(ViewportRanges.STARTSEQ) -// || (av.getWrapAlignment() -// && propertyName.equals(ViewportRanges.STARTRES))) -// { -// fastPaint((int) evt.getNewValue() - (int) evt.getOldValue()); -// } -// else if (propertyName.equals(ViewportRanges.STARTRESANDSEQ)) -// { -// fastPaint(((int[]) evt.getNewValue())[1] -// - ((int[]) evt.getOldValue())[1]); -// } -// else if (propertyName.equals(ViewportRanges.MOVE_VIEWPORT)) -// { -// repaint(); - // } + } + + /** + * Clears the flag that allows a 'fast paint' on the next repaint, so + * requiring a full repaint + */ + public void setNoFastPaint() + { + fastPaint = false; } } diff --git a/src/jalview/gui/OverviewCanvas.java b/src/jalview/gui/OverviewCanvas.java index de55996..308764b 100644 --- a/src/jalview/gui/OverviewCanvas.java +++ b/src/jalview/gui/OverviewCanvas.java @@ -51,19 +51,19 @@ public class OverviewCanvas extends JPanel // Can set different properties in this seqCanvas than // main visible SeqCanvas - private SequenceRenderer sr; + private final SequenceRenderer sr; - private jalview.renderer.seqfeatures.FeatureRenderer fr; + private final jalview.renderer.seqfeatures.FeatureRenderer fr; private OverviewDimensions od; private OverviewRenderer or = null; - private AlignViewportI av; + private final AlignViewportI av; - private OverviewResColourFinder cf; + private final OverviewResColourFinder cf; - private ProgressPanel progressPanel; + private final ProgressPanel progressPanel; private boolean showSequenceFeatures; @@ -71,9 +71,9 @@ public class OverviewCanvas extends JPanel private jalview.api.FeatureRenderer featureRenderer; - private OverviewPanel panel; + private final OverviewPanel panel; - private boolean showProgress; + private final boolean showProgress; public OverviewCanvas(OverviewPanel panel, OverviewDimensions overviewDims, @@ -176,7 +176,6 @@ public class OverviewCanvas extends JPanel synchronized void finalizeDraw(BufferedImage miniMe) { - if (showProgress && or != null) { or.removePropertyChangeListener(progressPanel); diff --git a/src/jalview/gui/OverviewPanel.java b/src/jalview/gui/OverviewPanel.java index 5694c3d..9245104 100755 --- a/src/jalview/gui/OverviewPanel.java +++ b/src/jalview/gui/OverviewPanel.java @@ -110,11 +110,6 @@ public class OverviewPanel extends JPanel av.getRanges().addPropertyChangeListener(this); - // without this the overview window does not size to fit the overview canvas - // BH - no,no! - This does not include the progressPanel! - // BH the problem was that OverviewCanvas.setPreferredSize() had not been set. - // setPreferredSize(new Dimension(od.getWidth(), od.getHeight())); - addComponentListener(new ComponentAdapter() { @Override @@ -122,7 +117,6 @@ public class OverviewPanel extends JPanel { resizePanel(); } - }); addMouseMotionListener(new MouseMotionAdapter() @@ -168,7 +162,6 @@ public class OverviewPanel extends JPanel Cursor.getPredefinedCursor(Cursor.CROSSHAIR_CURSOR)); } } - }); addMouseListener(new MouseAdapter() @@ -176,7 +169,6 @@ public class OverviewPanel extends JPanel @Override public void mousePressed(MouseEvent evt) { - if (Platform.isWinRightButton(evt)) { showPopupMenu(evt); @@ -225,17 +217,7 @@ public class OverviewPanel extends JPanel { draggingBox = false; } - }); - - // /* - // * Javascript does not call componentResized on initial display, - // * so do the update here - // */ - // if (Platform.isJS()) - // { - // updateOverviewImage(); - // } } protected void resizePanel() @@ -268,10 +250,6 @@ public class OverviewPanel extends JPanel od.setHeight(h - ph); updateOverviewImage(); } - // BH 2019.07.29 this is unnecessary -- it is what layout managers are - // for: - // setPreferredSize(new Dimension(od.getWidth(), od.getHeight() + - // ph)); } } @@ -295,7 +273,7 @@ public class OverviewPanel extends JPanel } - /* + /** * Displays the popup menu and acts on user input */ protected void showPopupMenu(MouseEvent e) @@ -320,8 +298,9 @@ public class OverviewPanel extends JPanel popup.show(this, e.getX(), e.getY()); } - /* - * Toggle overview display between showing hidden columns and hiding hidden columns + /** + * Toggle overview display between showing hidden columns and hiding hidden + * columns */ protected void toggleHiddenColumns() { diff --git a/src/jalview/gui/PaintRefresher.java b/src/jalview/gui/PaintRefresher.java index 953fdc5..326bac0 100755 --- a/src/jalview/gui/PaintRefresher.java +++ b/src/jalview/gui/PaintRefresher.java @@ -128,13 +128,13 @@ public class PaintRefresher { // BH 2019.04.22 fixes JS problem of repaint() consolidation // that occurs in JavaScript but not Java [JAL-3226] - ((IdCanvas) comp).fastPaint = false; + ((IdCanvas) comp).setNoFastPaint(); } else if (comp instanceof SeqCanvas) { // BH 2019.04.22 fixes JS problem of repaint() consolidation // that occurs in JavaScript but not Java [JAL-3226] - ((SeqCanvas) comp).fastPaint = false; + ((SeqCanvas) comp).setNoFastPaint(); } comp.repaint(); } diff --git a/src/jalview/gui/ScalePanel.java b/src/jalview/gui/ScalePanel.java index b06647d..87f350b 100755 --- a/src/jalview/gui/ScalePanel.java +++ b/src/jalview/gui/ScalePanel.java @@ -436,9 +436,6 @@ public class ScalePanel extends JPanel @Override public void paintComponent(Graphics g) { - - // super.paintComponent(g); // BH 2019 - /* * shouldn't get called in wrapped mode as the scale above is * drawn instead by SeqCanvas.drawNorthScale diff --git a/src/jalview/gui/SeqCanvas.java b/src/jalview/gui/SeqCanvas.java index 123e649..35be292 100755 --- a/src/jalview/gui/SeqCanvas.java +++ b/src/jalview/gui/SeqCanvas.java @@ -75,7 +75,7 @@ public class SeqCanvas extends JPanel implements ViewportListenerI private final SequenceRenderer seqRdr; - boolean fastPaint = false; + private boolean fastPaint = false; private boolean fastpainting = false; @@ -385,8 +385,6 @@ public class SeqCanvas extends JPanel implements ViewportListenerI availWidth -= (availWidth % charWidth); availHeight -= (availHeight % charHeight); - // BH 2019 can't possibly fastPaint if either width or height is 0 - if (availWidth == 0 || availHeight == 0) { return; @@ -398,28 +396,6 @@ public class SeqCanvas extends JPanel implements ViewportListenerI int endRes = ranges.getEndRes(); int endSeq = ranges.getEndSeq(); - // [JAL-3226] problem that JavaScript (or Java) may consolidate multiple - // repaint() requests in unpredictable ways. In this case, the issue was - // that in response to a CTRL-C/CTRL-V paste request, in Java a fast - // repaint request preceded two full requests, thus resulting - // in a full request for paint. In constrast, in JavaScript, the three - // requests were bundled together into one, so the fastPaint flag was - // still present for the second and third request. - // - // This resulted in incomplete painting. - // - // The solution was to set seqCanvas.fastPaint and idCanvas.fastPaint false - // in PaintRefresher when the target to be painted is one of those two - // components. - // - // BH 2019.04.22 - // - // An initial idea; can be removed once we determine this issue is closed: - // if (av.isFastPaintDisabled()) - // { - // fastPaint = false; - // } - Rectangle vis, clip; if (img != null && (fastPaint @@ -527,7 +503,7 @@ public class SeqCanvas extends JPanel implements ViewportListenerI /** * Using the current font, determine fields labelWidthEast and labelWidthWest, - * and return the number of residues that can fill the remaining width. + * and return the number of residues that can fill the remaining width * * @param width * the width in pixels (possibly including scales) @@ -1710,11 +1686,6 @@ public class SeqCanvas extends JPanel implements ViewportListenerI // typically scroll, but possibly just the end changed fastPaint(0, (int) evt.getNewValue() - (int) evt.getOldValue()); return; - case ViewportRanges.ENDRES: - case ViewportRanges.ENDSEQ: - // meaning second event along with "START" -- ENDONLY,NOTSTART - // TODO: ignore?? - return; case ViewportRanges.STARTRES: // meaning STARTOREND scrollX = (int) evt.getNewValue() - (int) evt.getOldValue(); @@ -1735,6 +1706,8 @@ public class SeqCanvas extends JPanel implements ViewportListenerI } break; + default: + return; } ViewportRanges vpRanges = av.getRanges(); @@ -2248,4 +2221,13 @@ public class SeqCanvas extends JPanel implements ViewportListenerI return labelWidthWest; } + /** + * Clears the flag that allows a 'fast paint' on the next repaint, so + * requiring a full repaint + */ + public void setNoFastPaint() + { + fastPaint = false; + } + } diff --git a/src/jalview/renderer/OverviewRenderer.java b/src/jalview/renderer/OverviewRenderer.java index 873fdac..0768681 100644 --- a/src/jalview/renderer/OverviewRenderer.java +++ b/src/jalview/renderer/OverviewRenderer.java @@ -51,7 +51,7 @@ import javax.swing.Timer; public class OverviewRenderer { // transparency of hidden cols/seqs overlay - private final float TRANSPARENCY = 0.5f; + private static final float TRANSPARENCY = 0.5f; public static final String UPDATE = "OverviewUpdate"; @@ -67,9 +67,7 @@ public class OverviewRenderer private Timer timer; - private boolean isJS = Platform.isJS(); - - private int delay = (isJS ? 1 : 0); + private int delay = (Platform.isJS() ? 1 : 0); private int seqIndex; @@ -480,6 +478,7 @@ public class OverviewRenderer sendProgressUpdate(1, 1, 0, 0); } } + panel.overviewDone(miniMe); } @@ -536,9 +535,9 @@ public class OverviewRenderer int icol) { return (seq == null || icol >= seq.getLength() - ? resColFinder.GAP_COLOUR - : resColFinder.getResidueColourInt(true, shader, allGroups, seq, - icol, finder)); + ? resColFinder.gapColourInt + : resColFinder.getResidueColourInt(true, shader, allGroups, seq, + icol, finder)); } /** diff --git a/src/jalview/renderer/OverviewResColourFinder.java b/src/jalview/renderer/OverviewResColourFinder.java index e0d6696..8d7980a 100644 --- a/src/jalview/renderer/OverviewResColourFinder.java +++ b/src/jalview/renderer/OverviewResColourFinder.java @@ -36,11 +36,15 @@ public class OverviewResColourFinder extends ResidueColourFinder public static final Color OVERVIEW_DEFAULT_HIDDEN = Color.darkGray .darker(); - final int GAP_COLOUR; // default colour to use at gaps + final Color gapColour; // colour to use for gaps - final int RESIDUE_COLOUR; // default colour to use at residues + final int gapColourInt; - final Color HIDDEN_COLOUR; // colour for hidden regions + final Color residueColour; // colour to use for uncoloured residues + + final int residueColourInt; + + final Color hiddenColour; // colour for hidden regions boolean useLegacy = false; @@ -67,21 +71,20 @@ public class OverviewResColourFinder extends ResidueColourFinder { if (useLegacyColouring) { - GAP_COLOUR = Color.white.getRGB(); - RESIDUE_COLOUR = Color.lightGray.getRGB(); + gapColour = Color.white; + residueColour = Color.lightGray; } else { - GAP_COLOUR = gapCol.getRGB(); - RESIDUE_COLOUR = Color.white.getRGB(); + gapColour = gapCol; + residueColour = Color.WHITE; } - HIDDEN_COLOUR = hiddenCol; + gapColourInt = gapColour.getRGB(); + residueColourInt = residueColour.getRGB(); + hiddenColour = hiddenCol; } @Override - /** - * for Test suite only. - */ public Color getBoxColour(ResidueShaderI shader, SequenceI seq, int i) { return new Color(getBoxColourInt(shader, seq, i)); @@ -96,22 +99,24 @@ public class OverviewResColourFinder extends ResidueColourFinder boolean isGap = Comparison.isGap(currentChar); if (shader.getColourScheme() == null) { - return (isGap ? GAP_COLOUR : RESIDUE_COLOUR); + return (isGap ? gapColourInt : residueColourInt); } - return (isGap && !shader.getColourScheme().hasGapColour() ? GAP_COLOUR + return (isGap && !shader.getColourScheme().hasGapColour() ? gapColourInt : shader.findColourInt(currentChar, i, seq)); } - /** - * For test suite only. - */ @Override public Color getResidueColour(boolean showBoxes, ResidueShaderI shader, SequenceGroup[] allGroups, final SequenceI seq, int i, FeatureColourFinder finder) { - return new Color(getResidueColourInt(showBoxes, shader, allGroups, seq, - i, finder)); + Color col = getResidueBoxColour(showBoxes, shader, allGroups, seq, i); + + // if there's a FeatureColourFinder we might override the residue colour + // here with feature colouring + col = finder == null || finder.noFeaturesDisplayed() ? col + : finder.findFeatureColour(col, seq, i); + return col; } @@ -135,9 +140,6 @@ public class OverviewResColourFinder extends ResidueColourFinder return seq.setColor(i, col); } - /** - * For test suite only. - */ @Override protected Color getResidueBoxColour(boolean showBoxes, ResidueShaderI shader, SequenceGroup[] allGroups, SequenceI seq, @@ -169,6 +171,6 @@ public class OverviewResColourFinder extends ResidueColourFinder */ protected Color getHiddenColour() { - return HIDDEN_COLOUR; + return hiddenColour; } } diff --git a/src/jalview/viewmodel/ViewportRanges.java b/src/jalview/viewmodel/ViewportRanges.java index a022627..f8decf6 100644 --- a/src/jalview/viewmodel/ViewportRanges.java +++ b/src/jalview/viewmodel/ViewportRanges.java @@ -142,17 +142,6 @@ public class ViewportRanges extends ViewportProperties return; // BH 2019.07.27 standard check for no changes } - // listeners include: - - // jalview.gui.SeqCanvas[,0,0,568x90,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=] - // STARTRES, STARTRESANDSEQ - // jalview.gui.IdCanvas[,0,0,112x90,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=java.awt.Dimension[width=112,height=0]] - // jalview.gui.ScalePanel[,0,0,594x17,layout=java.awt.FlowLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=] - // jalview.gui.AnnotationPanel[,0,0,0x162,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=java.awt.Dimension[width=1,height=162]] - // jalview.gui.AlignmentPanel[,0,0,706x133,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=16777225,maximumSize=,minimumSize=,preferredSize=java.awt.Dimension[width=220,height=166]] - // jalview.gui.OverviewPanel[,0,0,543x135,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=,flags=9,maximumSize=,minimumSize=,preferredSize=java.awt.Dimension[width=543,height=135]] - - // "STARTRES" is a misnomer here -- really "STARTORENDRES" // note that this could be "no change" if the range is just being expanded changeSupport.firePropertyChange(STARTRES, oldstartres, startRes); @@ -335,7 +324,6 @@ public class ViewportRanges extends ViewportProperties */ public void setStartResAndSeq(int res, int seq) { - // from Overview only int width = getViewportWidth(); int[] oldresvalues = updateStartEndRes(res, res + width - 1); @@ -468,8 +456,6 @@ public class ViewportRanges extends ViewportProperties { vpstart = visHeight - h; } - // System.out.println("ViewportRanges setviewportStartAndHeight " + vpstart - // + " " + start + " " + h + " " + getVisibleAlignmentHeight()); setStartEndSeq(vpstart, vpstart + h - 1); } @@ -661,10 +647,7 @@ public class ViewportRanges extends ViewportProperties } /** - * Set the viewport location so that a position is visible. From - * SeqPanel.scrollToVisible(true) only, from AlignFrame keyboard actions - * SeqPanel.scrollCursor[Row(VK_S)/Column(VK_C)/RowAndColumn(VK_ENTER,COMMA)/Position(VK_P)] - * + * Set the viewport location so that a position is visible * * @param x * column to be visible: absolute position in alignment diff --git a/test/jalview/renderer/OverviewRendererTest.java b/test/jalview/renderer/OverviewRendererTest.java index 0a77b65..159c814 100644 --- a/test/jalview/renderer/OverviewRendererTest.java +++ b/test/jalview/renderer/OverviewRendererTest.java @@ -82,11 +82,13 @@ public class OverviewRendererTest fr.findAllFeatures(true); av.setShowSequenceFeatures(true); fr.setColour("Pfam", new FeatureColour(Color.yellow)); + seq1.resetColors(); assertEquals(or.getColumnColourFromSequence(null, seq1, 0), Color.yellow.getRGB()); // don't show sequence features av.setShowSequenceFeatures(false); + seq1.resetColors(); assertEquals(or.getColumnColourFromSequence(null, seq1, 0), Color.magenta.getRGB()); } diff --git a/test/jalview/renderer/OverviewResColourFinderTest.java b/test/jalview/renderer/OverviewResColourFinderTest.java index 0585a07..bf1237d 100644 --- a/test/jalview/renderer/OverviewResColourFinderTest.java +++ b/test/jalview/renderer/OverviewResColourFinderTest.java @@ -170,7 +170,7 @@ public class OverviewResColourFinderTest av.getResidueShading(), null, seq, 3, null)); } - @Test + @Test(groups = "Functional") public void testGetResidueBoxColour_group() { SequenceI seq = new Sequence("name", "MA--TVLGSPRAPAFF"); @@ -235,7 +235,7 @@ public class OverviewResColourFinderTest av.getResidueShading(), groups, seq, 2, null)); } - @Test + @Test(groups = "Functional") public void testGetBoxColour() { SequenceI seq = new Sequence("name", "MAT--GSPRAPAFF"); // FER1_MAIZE... + a -- 1.7.10.2