From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Mon, 23 Jan 2017 09:11:36 +0000 (+0000)
Subject: JAL-2386 simplify changeColour (doesn't change colour thresholds)
X-Git-Tag: Release_2_10_3b1~357^2~12
X-Git-Url: http://source.jalview.org/gitweb/?p=jalview.git;a=commitdiff_plain;h=56308882b011c6c6ff96786490bbb1c937d3ade3

JAL-2386 simplify changeColour (doesn't change colour thresholds)
---

diff --git a/src/jalview/appletgui/AlignFrame.java b/src/jalview/appletgui/AlignFrame.java
index 9876bb3..4c4c84f 100644
--- a/src/jalview/appletgui/AlignFrame.java
+++ b/src/jalview/appletgui/AlignFrame.java
@@ -2631,30 +2631,7 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener,
   @Override
   public void changeColour(ColourSchemeI cs)
   {
-    // FIXME threshold, conservation, increment belong inside
-    // AlignmentColourScheme and/or ViewStyleI, not ColourSchemeI
-    if (cs != null)
-    {
-      if (viewport.getAbovePIDThreshold())
-      {
-        viewport.setThreshold(SliderPanel.setPIDSliderSource(alignPanel,
-                viewport.getViewportColourScheme(), "Background"));
-      }
-
-      if (viewport.getConservationSelected())
-      {
-        // cs.setConservationApplied(true);
-        viewport.setIncrement(SliderPanel.setConservationSlider(alignPanel,
-                viewport.getViewportColourScheme(), "Background"));
-      }
-      // else
-      // {
-      // cs.setConservationApplied(false);
-      // }
-    }
     viewport.setGlobalColourScheme(cs);
-    viewport.getViewportColourScheme().setConservationApplied(
-            viewport.getConservationSelected());
 
     alignPanel.paintAlignment(true);
   }
diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java
index 5a0d13e..5cc5111 100644
--- a/src/jalview/gui/AlignFrame.java
+++ b/src/jalview/gui/AlignFrame.java
@@ -3346,39 +3346,15 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     if (cs != null)
     {
       ColourMenuHelper.setColourSelected(colourMenu, cs.getSchemeName());
-      // Make sure viewport is up to date w.r.t. any sliders
-      // ? not needed as handled by SliderPanel.valueChanged?
-//       if (viewport.getAbovePIDThreshold())
-//       {
-      // int threshold = SliderPanel.setPIDSliderSource(alignPanel, cs,
-//       "Background");
-       //todo is threshold held in viewport style or in colour scheme?
-//       viewport.setThreshold(threshold);
-      // }
-      //
-      // if (viewport.getConservationSelected())
-      // {
-      // cs.setConservationInc(SliderPanel.setConservationSlider(alignPanel,
-      // cs, "Background"));
-      // }
     }
 
     viewport.setGlobalColourScheme(cs);
-    if (viewport.getConservationSelected())
-    {
-      int conservationInc = SliderPanel.getConservationValue();
-      viewport.getViewportColourScheme()
-              .setConservationInc(conservationInc);
-    }
 
     alignPanel.paintAlignment(true);
   }
 
   /**
-   * DOCUMENT ME!
-   * 
-   * @param e
-   *          DOCUMENT ME!
+   * Show the PID threshold slider panel
    */
   @Override
   protected void modifyPID_actionPerformed()
@@ -3392,10 +3368,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
   }
 
   /**
-   * DOCUMENT ME!
-   * 
-   * @param e
-   *          DOCUMENT ME!
+   * Show the Conservation slider panel
    */
   @Override
   protected void modifyConservation_actionPerformed()
@@ -3418,6 +3391,7 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
     boolean selected = conservationMenuItem.isSelected();
     modifyConservation.setEnabled(selected);
     viewport.setConservationSelected(selected);
+    viewport.getViewportColourScheme().setConservationApplied(selected);
 
     changeColour(viewport.getGlobalColourScheme());
     if (selected)
diff --git a/src/jalview/viewmodel/AlignmentViewport.java b/src/jalview/viewmodel/AlignmentViewport.java
index 0803773..9f53f02 100644
--- a/src/jalview/viewmodel/AlignmentViewport.java
+++ b/src/jalview/viewmodel/AlignmentViewport.java
@@ -599,7 +599,7 @@ public abstract class AlignmentViewport implements AlignViewportI,
 
   protected boolean ignoreGapsInConsensusCalculation = false;
 
-  protected CollectionColourScheme globalColourScheme;
+  protected CollectionColourSchemeI globalColourScheme;
 
   @Override
   public void setGlobalColourScheme(ColourSchemeI cs)
@@ -614,6 +614,8 @@ public abstract class AlignmentViewport implements AlignViewportI,
 
     /*
      * only instantiate colour scheme once, thereafter update it
+     * this means that any conservation or PID threshold settings
+     * persist when the alignment colour scheme is changed
      */
     if (globalColourScheme == null)
     {
@@ -621,72 +623,38 @@ public abstract class AlignmentViewport implements AlignViewportI,
     }
     globalColourScheme.setColourScheme(cs);
 
-    boolean recalc = false;
+    // boolean recalc = false;
+    // TODO: do threshold and increment belong in ViewStyle or colour scheme?
+    // problem: groups need this but do not currently have a ViewStyle
+
     if (cs != null)
     {
-      recalc = getConservationSelected();
-      if (getAbovePIDThreshold() || cs instanceof PIDColourScheme
+      if (getConservationSelected() || getAbovePIDThreshold()
+              || cs instanceof PIDColourScheme
               || cs instanceof Blosum62ColourScheme)
       {
-        recalc = true;
-        globalColourScheme.setThreshold(viewStyle.getThreshold(),
-                ignoreGapsInConsensusCalculation);
-      }
-      else
-      {
-        globalColourScheme
-                .setThreshold(0, ignoreGapsInConsensusCalculation);
-      }
-      if (recalc)
-      {
-        globalColourScheme.setConsensus(hconsensus);
         globalColourScheme.setConservation(hconservation);
       }
-      globalColourScheme.setConservationApplied(getConservationSelected());
       globalColourScheme.alignmentChanged(alignment, hiddenRepSequences);
     }
+
+    /*
+     * if 'apply colour to all groups' is selected... do so
+     * (but don't transfer any colour threshold settings to groups)
+     */
     if (getColourAppliesToAllGroups())
     {
       for (SequenceGroup sg : getAlignment().getGroups())
       {
-        if (cs == null)
-        {
-          sg.setColourScheme(null);
-          continue;
-        }
-        CollectionColourScheme groupColour = new CollectionColourScheme(cs.getInstance(sg,
-                getHiddenRepSequences()));
-        sg.setGroupColourScheme(groupColour);
-        sg.setConsPercGaps(ConsPercGaps);
-        if (getAbovePIDThreshold() || cs instanceof PIDColourScheme
-                || cs instanceof Blosum62ColourScheme)
-        {
-          groupColour.setThreshold(viewStyle.getThreshold(),
-                  isIgnoreGapsConsensus());
-          recalc = true;
-        }
-        else
-        {
-          groupColour.setThreshold(0, isIgnoreGapsConsensus());
-        }
-
-        if (getConservationSelected())
-        {
-          groupColour.setConservationApplied(true);
-          recalc = true;
-        }
-        else
-        {
-          groupColour.setConservation(null);
-          // sg.cs.setThreshold(0, getIgnoreGapsConsensus());
-        }
-        if (recalc)
-        {
-          sg.recalcConservation();
-        }
-        else
+        /*
+         * retain any colour thresholds per group while
+         * changing choice of colour scheme (JAL-2386)
+         */
+        sg.setColourScheme(cs);
+        if (cs != null)
         {
-          groupColour.alignmentChanged(sg, hiddenRepSequences);
+          sg.getGroupColourScheme()
+                  .alignmentChanged(sg, hiddenRepSequences);
         }
       }
     }
@@ -1880,7 +1848,7 @@ public abstract class AlignmentViewport implements AlignViewportI,
    */
   void resetAllColourSchemes()
   {
-    CollectionColourScheme cs = globalColourScheme;
+    CollectionColourSchemeI cs = globalColourScheme;
     if (cs != null)
     {
       cs.alignmentChanged(alignment, hiddenRepSequences);