JAL-3383 minor code tidying and commenting
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Thu, 19 Sep 2019 14:25:37 +0000 (15:25 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Thu, 19 Sep 2019 14:25:37 +0000 (15:25 +0100)
15 files changed:
src/jalview/api/FeatureColourI.java
src/jalview/datamodel/Alignment.java
src/jalview/datamodel/AllColsCollection.java
src/jalview/datamodel/HiddenColumns.java
src/jalview/gui/IdCanvas.java
src/jalview/gui/OverviewCanvas.java
src/jalview/gui/OverviewPanel.java
src/jalview/gui/PaintRefresher.java
src/jalview/gui/ScalePanel.java
src/jalview/gui/SeqCanvas.java
src/jalview/renderer/OverviewRenderer.java
src/jalview/renderer/OverviewResColourFinder.java
src/jalview/viewmodel/ViewportRanges.java
test/jalview/renderer/OverviewRendererTest.java
test/jalview/renderer/OverviewResColourFinderTest.java

index c5f6727..44bc321 100644 (file)
@@ -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();
 
index 22d23bc..26379d3 100755 (executable)
@@ -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<SequenceI> sequences;
@@ -61,8 +61,6 @@ public class Alignment implements AlignmentI
 
   private List<AlignedCodonFrame> 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++)
index c65c381..af3fffa 100644 (file)
@@ -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;
 
index 458bde7..48ebb7a 100644 (file)
@@ -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)
index e5a5946..0731bf3 100755 (executable)
@@ -59,7 +59,7 @@ public class IdCanvas extends JPanel implements ViewportListenerI
 
   int imgHeight = 0;
 
-  boolean fastPaint = false;
+  private boolean fastPaint = false;
 
   List<SequenceI> 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;
   }
 }
index de55996..308764b 100644 (file)
@@ -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);
index 5694c3d..9245104 100755 (executable)
@@ -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()
   {
index 953fdc5..326bac0 100755 (executable)
@@ -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();
     }
index b06647d..87f350b 100755 (executable)
@@ -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
index 123e649..35be292 100755 (executable)
@@ -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;
+  }
+
 }
index 873fdac..0768681 100644 (file)
@@ -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));
   }
 
   /**
index e0d6696..8d7980a 100644 (file)
@@ -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;
   }
 }
index a022627..f8decf6 100644 (file)
@@ -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
index 0a77b65..159c814 100644 (file)
@@ -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());
   }
index 0585a07..bf1237d 100644 (file)
@@ -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