From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Mon, 8 Jun 2020 15:14:08 +0000 (+0100)
Subject: Merge branch 'develop' into feature/JAL-3390hideUnmappedStructure
X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=8a43b8353b89a47002ef17c959ed61281fc4d826;p=jalview.git

Merge branch 'develop' into feature/JAL-3390hideUnmappedStructure

Conflicts:
	gradle.properties
	resources/lang/Messages.properties
	resources/lang/Messages_es.properties
	src/jalview/api/structures/JalviewStructureDisplayI.java
	src/jalview/appletgui/AppletJmol.java
	src/jalview/ext/jmol/JalviewJmolBinding.java
	src/jalview/ext/jmol/JmolCommands.java
	src/jalview/ext/rbvi/chimera/AtomSpecModel.java
	src/jalview/ext/rbvi/chimera/ChimeraCommands.java
	src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
	src/jalview/gui/AlignFrame.java
	src/jalview/gui/AlignmentPanel.java
	src/jalview/gui/AppJmol.java
	src/jalview/gui/ChimeraViewFrame.java
	src/jalview/gui/JalviewChimeraBindingModel.java
	src/jalview/gui/StructureViewerBase.java
	src/jalview/structures/models/AAStructureBindingModel.java
	test/jalview/ext/jmol/JmolCommandsTest.java
	test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
	test/jalview/structures/models/AAStructureBindingModelTest.java
---

8a43b8353b89a47002ef17c959ed61281fc4d826
diff --cc gradle.properties
index f6e9bca,b99cdca..3781991
--- a/gradle.properties
+++ b/gradle.properties
@@@ -1,7 -1,18 +1,17 @@@
+ # Convention for properties.  Read from gradle.properties, use lower_case_underlines for property names.
+ # For properties set within build.gradle, use camelCaseNoSpace.
+ #
+ 
  jalviewDir = .
 -
+ #JAVA_VERSION = 1.8
+ JAVA_VERSION = 11
+ 
+ source_dir = src
+ #source_dir = utils/jalviewjs/test/src
+ 
+ test_source_dir = test
+ #test_source_dir = utils/jalviewjs/test/test
  
- JAVA_VERSION = 1.8
- #JAVA_VERSION = 11
  JALVIEW_VERSION = DEVELOPMENT
  INSTALLATION = Source
  jalview_keystore = keys/.keystore
diff --cc src/jalview/appletgui/AppletJmol.java
index 44a73d9,6665ec8..f73ff88
--- a/src/jalview/appletgui/AppletJmol.java
+++ b/src/jalview/appletgui/AppletJmol.java
@@@ -394,20 -394,19 +394,20 @@@ public class AppletJmol extends Embmenu
  
    void centerViewer()
    {
--    Vector<String> toshow = new Vector<>();
++    Vector<String> toHide = new Vector<>();
      for (int i = 0; i < chainMenu.getItemCount(); i++)
      {
        if (chainMenu.getItem(i) instanceof CheckboxMenuItem)
        {
          CheckboxMenuItem item = (CheckboxMenuItem) chainMenu.getItem(i);
 -        if (item.getState())
 +        if (!item.getState())
          {
--          toshow.addElement(item.getLabel());
++          toHide.addElement(item.getLabel());
          }
        }
      }
-     jmb.setChainsToHide(toshow);
 -    jmb.showChains(toshow);
++    jmb.setChainsToHide(toHide);
 +    jmb.centerViewer();
    }
  
    void closeViewer()
diff --cc src/jalview/ext/jmol/JalviewJmolBinding.java
index 3af7279,eee48df..09f3876
--- a/src/jalview/ext/jmol/JalviewJmolBinding.java
+++ b/src/jalview/ext/jmol/JalviewJmolBinding.java
@@@ -61,6 -39,23 +39,25 @@@ import org.jmol.api.JmolViewer
  import org.jmol.c.CBK;
  import org.jmol.viewer.Viewer;
  
++import jalview.api.AlignViewportI;
+ import jalview.api.AlignmentViewPanel;
+ import jalview.api.FeatureRenderer;
+ import jalview.api.SequenceRenderer;
+ import jalview.bin.Cache;
+ import jalview.datamodel.PDBEntry;
+ import jalview.datamodel.SequenceI;
+ import jalview.gui.IProgressIndicator;
+ import jalview.gui.StructureViewer.ViewerType;
+ import jalview.io.DataSourceType;
+ import jalview.io.StructureFile;
+ import jalview.structure.AtomSpec;
++import jalview.structure.AtomSpecModel;
+ import jalview.structure.StructureCommand;
+ import jalview.structure.StructureCommandI;
+ import jalview.structure.StructureSelectionManager;
+ import jalview.structures.models.AAStructureBindingModel;
+ import javajs.util.BS;
+ 
  public abstract class JalviewJmolBinding extends AAStructureBindingModel
          implements JmolStatusListener, JmolSelectionListener,
          ComponentListener
@@@ -133,46 -115,13 +117,44 @@@
      return getViewerTitle("Jmol", true);
    }
  
 +  /**
 +   * prepare the view for a given set of models/chains. chainList contains strings
 +   * of the form 'pdbfilename:Chaincode'
 +   * 
 +   * @deprecated now only used by applet code
 +   */
 +  @Deprecated
 +  public void centerViewer()
 +  {
 +    StringBuilder cmd = new StringBuilder(128);
 +    int mlength, p;
 +    for (String lbl : chainsToHide)
 +    {
 +      mlength = 0;
 +      do
 +      {
 +        p = mlength;
 +        mlength = lbl.indexOf(":", p);
 +      } while (p < mlength && mlength < (lbl.length() - 2));
 +      // TODO: lookup each pdb id and recover proper model number for it.
 +      cmd.append(":" + lbl.substring(mlength + 1) + " /"
-               + (getModelNum(chainFile.get(lbl))) + " or ");
++              + (getModelIdForFile(getFileForChain(lbl))) + " or ");
 +    }
 +    if (cmd.length() > 0)
 +    {
 +      cmd.setLength(cmd.length() - 4);
 +    }
 +    // todo: revised command is untested - but this method is obsolete anyway
 +    String command = "select *;hide " + cmd + ";cartoon;center " + cmd;
-     evalStateCommand(command);
++    executeCommand(new StructureCommand(command), false);
 +  }
 +
-   public void closeViewer()
+   private String jmolScript(String script)
    {
-     // remove listeners for all structures in viewer
-     getSsm().removeStructureViewerListener(this, this.getStructureFiles());
-     viewer.dispose();
-     lastCommand = null;
-     viewer = null;
-     releaseUIResources();
+     Cache.log.debug(">>Jmol>> " + script);
+     String s = jmolViewer.evalStringQuiet(script); // scriptWait(script); BH
+     Cache.log.debug("<<Jmol<< " + s);
 -
+     return s;
    }
  
    @Override
@@@ -602,28 -191,34 +224,36 @@@
    @Override
    public synchronized String[] getStructureFiles()
    {
 +    if (modelFileNames != null)
 +    {
 +      return modelFileNames;
 +    }
-     if (viewer == null)
+     if (jmolViewer == null)
      {
        return new String[0];
      }
  
 -    if (modelFileNames == null)
 +    List<String> mset = new ArrayList<>();
-     int modelCount = viewer.ms.mc;
++    int modelCount = jmolViewer.ms.mc;
 +    String filePath = null;
 +    for (int i = 0; i < modelCount; ++i)
      {
-       filePath = viewer.ms.getModelFileName(i);
-       if (!mset.contains(filePath))
 -      int modelCount = jmolViewer.ms.mc;
 -      String filePath = null;
 -      List<String> mset = new ArrayList<>();
 -      for (int i = 0; i < modelCount; ++i)
++      /*
++       * defensive check for null as getModelFileName can return null
++       * even when model count ms.mc is > 0
++       */
++      filePath = jmolViewer.ms.getModelFileName(i);
++      if (filePath != null && !mset.contains(filePath))
        {
 -        /*
 -         * defensive check for null as getModelFileName can return null
 -         * even when model count ms.mc is > 0
 -         */
 -        filePath = jmolViewer.ms.getModelFileName(i);
 -        if (filePath != null && !mset.contains(filePath))
 -        {
 -          mset.add(filePath);
 -        }
 -      }
 -      if (!mset.isEmpty())
 -      {
 -        modelFileNames = mset.toArray(new String[mset.size()]);
 +        mset.add(filePath);
        }
      }
-     modelFileNames = mset.toArray(new String[mset.size()]);
+ 
++    if (!mset.isEmpty())
++    {
++      modelFileNames = mset.toArray(new String[mset.size()]);
++    }
 +
      return modelFileNames;
    }
  
@@@ -1417,94 -966,62 +1001,141 @@@
    }
  
    @Override
 +  public void showStructures(AlignViewportI av, boolean refocus)
 +  {
 +    String cmd = buildShowStructuresCommand(av, refocus);
-     evalStateCommand(cmd);
++    executeCommand(new StructureCommand(cmd), false);
 +  }
 +
 +  /**
 +   * Builds a command to show parts of the structure, depending on whether
 +   * <ul>
 +   * <li>all structures or regions mapped to alignment only are shown</li>
 +   * <li>all chains or only selected chains are shown</li>
 +   * </ul>
 +   * 
 +   * @param av
 +   * @param refocus
 +   * @return
 +   */
 +  protected String buildShowStructuresCommand(AlignViewportI av,
 +          boolean refocus)
 +  {
 +    StringBuilder cmd = new StringBuilder(128);
 +    if (!isShowAlignmentOnly())
 +    {
 +      cmd.append("display *");
 +    }
 +    else
 +    {
 +      AtomSpecModel model = getShownResidues(av);
-       String atomSpec = JmolCommands.getAtomSpec(model);
++      String atomSpec = getCommandGenerator().getAtomSpec(model, false);
 +
 +      cmd.append("hide *;display ").append(atomSpec)
 +              .append("; select displayed");
 +    }
 +
 +    /*
 +     * hide any chains not selected to be shown
 +     */
 +    if (!chainsToHide.isEmpty())
 +    {
 +      cmd.append("; hide add ");
 +      boolean firstHide = true;
 +      for (String pdbChain : chainsToHide)
 +      {
 +        String[] toks = pdbChain.split(":");
 +        String chainId = toks[1];
-         int modelNo = getModelNum(chainFile.get(pdbChain));
-         if (modelNo == -1)
++        String modelNo = getModelIdForFile(getFileForChain(pdbChain));
++        if ("".equals(modelNo))
 +        {
 +          continue;
 +        }
 +        if (!firstHide)
 +        {
 +          cmd.append(",");
 +        }
 +        firstHide = false;
 +        cmd.append(":").append(chainId).append("/")
 +                .append(String.valueOf(modelNo)).append(".1");
 +      }
 +    }
 +
 +    cmd.append("; cartoon only");
 +    if (refocus)
 +    {
 +      cmd.append("; zoom 0");
 +    }
 +    return cmd.toString();
 +  }
 +
 +  /**
 +   * Answers a Jmol syntax style structure model specification. Model number 0, 1,
 +   * 2... is formatted as "1.1", "2.1", "3.1" etc.
 +   */
 +  @Override
 +  public String getModelSpec(int model)
 +  {
 +    return String.valueOf(model + 1) + ".1";
 +  }
 +
++  @Override
+   protected String getModelIdForFile(String pdbFile)
+   {
+     if (modelFileNames == null)
+     {
+       return "";
+     }
+     for (int i = 0; i < modelFileNames.length; i++)
+     {
+       if (modelFileNames[i].equalsIgnoreCase(pdbFile))
+       {
+         return String.valueOf(i + 1);
+       }
+     }
+     return "";
+   }
+ 
+   @Override
+   protected ViewerType getViewerType()
+   {
+     return ViewerType.JMOL;
+   }
+ 
+   @Override
+   protected String getModelId(int pdbfnum, String file)
+   {
+     return String.valueOf(pdbfnum + 1);
+   }
+ 
    /**
-    * Sends a command to recentre the display
+    * Returns ".spt" - the Jmol session file extension
+    * 
+    * @return
+    * @see https://chemapps.stolaf.edu/jmol/docs/#writemodel
     */
    @Override
-   public void focusView()
+   public String getSessionFileExtension()
    {
-     /*
-      * don't use evalStateCommand because it ignores a command that is the same
-      * as the last command (why?); but user may have adjusted the display since
-      */
-     viewer.evalString("zoom 0");
+     return ".spt";
+   }
+ 
+   @Override
+   public void selectionChanged(BS arg0)
+   {
+     // TODO Auto-generated method stub
+ 
+   }
+ 
+   @Override
+   public SequenceRenderer getSequenceRenderer(AlignmentViewPanel avp)
+   {
+     return new jalview.gui.SequenceRenderer(avp.getAlignViewport());
+   }
+ 
+   @Override
+   public String getHelpURL()
+   {
+     return "http://wiki.jmol.org"; // BH 2018
    }
  }
diff --cc src/jalview/ext/jmol/JmolCommands.java
index a900701,085fbd5..7dc5224
--- a/src/jalview/ext/jmol/JmolCommands.java
+++ b/src/jalview/ext/jmol/JmolCommands.java
@@@ -45,12 -49,257 +49,271 @@@ import jalview.util.Platform
   * @author JimP
   * 
   */
- public class JmolCommands extends StructureCommands
+ public class JmolCommands extends StructureCommandsBase
  {
 +  private static final String COMMA = ",";
 +
+   private static final StructureCommand SHOW_BACKBONE = new StructureCommand(
+           "select *; cartoons off; backbone");
+ 
+   private static final StructureCommand FOCUS_VIEW = new StructureCommand("zoom 0");
+ 
+   private static final StructureCommand COLOUR_ALL_WHITE = new StructureCommand(
+           "select *;color white;");
+ 
+   private static final StructureCommandI COLOUR_BY_CHARGE = new StructureCommand(
+           "select *;color white;select ASP,GLU;color red;"
+                   + "select LYS,ARG;color blue;select CYS;color yellow");
+ 
+   private static final StructureCommandI COLOUR_BY_CHAIN = new StructureCommand(
+           "select *;color chain");
+ 
+   private static final String PIPE = "|";
+ 
+   private static final String HYPHEN = "-";
+ 
+   private static final String COLON = ":";
+ 
+   private static final String SLASH = "/";
+ 
 -  /**
 -   * {@inheritDoc}
 -   * 
 -   * @return
 -   */
+   @Override
+   public int getModelStartNo()
+   {
+     return 1;
+   }
+ 
+   /**
+    * Returns a string representation of the given colour suitable for inclusion
+    * in Jmol commands
+    * 
+    * @param c
+    * @return
+    */
+   protected String getColourString(Color c)
+   {
+     return c == null ? null
+             : String.format("[%d,%d,%d]", c.getRed(), c.getGreen(),
+                     c.getBlue());
+   }
+ 
+   @Override
+   public StructureCommandI colourByChain()
+   {
+     return COLOUR_BY_CHAIN;
+   }
+ 
+   @Override
+   public List<StructureCommandI> colourByCharge()
+   {
+     return Arrays.asList(COLOUR_BY_CHARGE);
+   }
+ 
+   @Override
+   public List<StructureCommandI> colourByResidues(Map<String, Color> colours)
+   {
+     List<StructureCommandI> cmds = super.colourByResidues(colours);
+     cmds.add(0, COLOUR_ALL_WHITE);
+     return cmds;
+   }
+ 
+   @Override
+   public StructureCommandI setBackgroundColour(Color col)
+   {
+     return new StructureCommand("background " + getColourString(col));
+   }
+ 
+   @Override
+   public StructureCommandI focusView()
+   {
+     return FOCUS_VIEW;
+   }
+ 
+   @Override
+   public List<StructureCommandI> showChains(List<String> toShow)
+   {
+     StringBuilder atomSpec = new StringBuilder(128);
+     boolean first = true;
+     for (String chain : toShow)
+     {
+       String[] tokens = chain.split(":");
+       if (tokens.length == 2)
+       {
+         if (!first)
+         {
+           atomSpec.append(" or ");
+         }
+         first = false;
+         atomSpec.append(":").append(tokens[1]).append(" /").append(tokens[0]);
+       }
+     }
+ 
+     String spec = atomSpec.toString();
+     String command = "select *;restrict " + spec + ";cartoon;center "
+             + spec;
+     return Arrays.asList(new StructureCommand(command));
+   }
+ 
+   /**
+    * Returns a command to superpose atoms in {@code atomSpec} to those in
+    * {@code refAtoms}, restricted to alpha carbons only (Phosphorous for rna).
+    * For example
+    * 
+    * <pre>
+    * compare {2.1} {1.1} SUBSET {(*.CA | *.P) and conformation=1} 
+    *         ATOMS {1-87:A}{2-54:A|61-94:A} ROTATE TRANSLATE 1.0;
+    * </pre>
+    * 
+    * where {@code conformation=1} excludes ALTLOC atom locations, and 1.0 is the
+    * time in seconds to animate the action. For this example, atoms in model 2
+    * are moved towards atoms in model 1.
+    * <p>
+    * The two atomspecs should each be for one model only, but may have more than
+    * one chain. The number of atoms specified should be the same for both
+    * models, though if not, Jmol may make a 'best effort' at superposition.
+    * 
+    * @see https://chemapps.stolaf.edu/jmol/docs/#compare
+    */
+   @Override
+   public List<StructureCommandI> superposeStructures(AtomSpecModel refAtoms,
+           AtomSpecModel atomSpec)
+   {
+     StringBuilder sb = new StringBuilder(64);
+     String refModel = refAtoms.getModels().iterator().next();
+     String model2 = atomSpec.getModels().iterator().next();
+     sb.append(String.format("compare {%s.1} {%s.1}", model2, refModel));
+     sb.append(" SUBSET {(*.CA | *.P) and conformation=1} ATOMS {");
+ 
+     /*
+      * command examples don't include modelspec with atoms, getAtomSpec does;
+      * it works, so leave it as it is for simplicity
+      */
+     sb.append(getAtomSpec(atomSpec, true)).append("}{");
+     sb.append(getAtomSpec(refAtoms, true)).append("}");
+     sb.append(" ROTATE TRANSLATE ");
+     sb.append(getCommandSeparator());
+ 
+     /*
+      * show residues used for superposition as ribbon
+      */
+     sb.append("select ").append(getAtomSpec(atomSpec, false)).append("|");
+     sb.append(getAtomSpec(refAtoms, false)).append(getCommandSeparator())
+             .append("cartoons");
+ 
+     return Arrays.asList(new StructureCommand(sb.toString()));
+   }
+ 
+   @Override
+   public StructureCommandI openCommandFile(String path)
+   {
+     /*
+      * https://chemapps.stolaf.edu/jmol/docs/#script
+      * not currently used in Jalview
+      */
+     return new StructureCommand("script " + path);
+   }
+ 
+   @Override
+   public StructureCommandI saveSession(String filepath)
+   {
+     /*
+      * https://chemapps.stolaf.edu/jmol/docs/#writemodel
+      */
+     return new StructureCommand("write STATE \"" + filepath + "\"");
+   }
+ 
+   @Override
+   protected StructureCommandI colourResidues(String atomSpec, Color colour)
+   {
+     StringBuilder sb = new StringBuilder(atomSpec.length()+20);
+     sb.append("select ").append(atomSpec).append(getCommandSeparator())
+             .append("color").append(getColourString(colour));
+     return new StructureCommand(sb.toString());
+   }
+ 
+   @Override
+   protected String getResidueSpec(String residue)
+   {
+     return residue;
+   }
+ 
+   /**
+    * Generates a Jmol atomspec string like
+    * 
+    * <pre>
 -   * 2-5:A/1.1,8:A/1.1,5-10:B/2.1
++   * (61-64,70)&:A/1.1,(12-25,41-44)&:B/1.1,12:A/2.1
++   * for model 1, chain A, residues 61-64 and 70, chain B residues 12-25 and 41-44, model 2 chain A residue 12
+    * </pre>
+    * 
++   * Note the brackets to group multiple residue ranges for the same chain
++   * (without bracketing, ranges would apply to all chains)
++   * 
+    * Parameter {@code alphaOnly} is not used here - this restriction is made by
+    * a separate clause in the {@code compare} (superposition) command.
+    */
+   @Override
+   public String getAtomSpec(AtomSpecModel model, boolean alphaOnly)
+   {
+     StringBuilder sb = new StringBuilder(128);
+ 
 -    boolean first = true;
++    boolean firstChain = true;
+     for (String modelNo : model.getModels())
+     {
+       for (String chain : model.getChains(modelNo))
+       {
 -        for (int[] range : model.getRanges(modelNo, chain))
++        if (!firstChain)
+         {
 -          if (!first)
 -          {
 -            sb.append(PIPE);
 -          }
 -          first = false;
 -          if (range[0] == range[1])
++          sb.append(COMMA);
++        }
++        firstChain = false;
++        List<int[]> rangeList = model.getRanges(modelNo, chain);
++        if (rangeList.size() > 1)
++        {
++          sb.append("(");
++        }
++        boolean firstRange = true;
++        for (int[] range : rangeList)
++        {
++          if (!firstRange)
+           {
 -            sb.append(range[0]);
++            sb.append(COMMA);
+           }
 -          else
++          firstRange = false;
++          firstChain = false;
++          sb.append(range[0]);
++          if (range[0] != range[1])
+           {
 -            sb.append(range[0]).append(HYPHEN).append(range[1]);
++            sb.append(HYPHEN).append(range[1]);
+           }
 -          sb.append(COLON).append(chain.trim()).append(SLASH);
 -          sb.append(String.valueOf(modelNo)).append(".1");
+         }
++        if (rangeList.size() > 1)
++        {
++          sb.append(")&");
++        }
++        sb.append(COLON).append(chain.trim()).append(SLASH);
++        sb.append(String.valueOf(modelNo)).append(".1");
+       }
+     }
+ 
+     return sb.toString();
+   }
+ 
+   @Override
+   public List<StructureCommandI> showBackbone()
+   {
+     return Arrays.asList(SHOW_BACKBONE);
+   }
+ 
+   @Override
+   public StructureCommandI loadFile(String file)
+   {
+     // https://chemapps.stolaf.edu/jmol/docs/#loadfiles
+     return new StructureCommand("load FILES \"" + 
+             Platform.escapeBackslashes(file) + "\"");
+   }
+ 
    /**
-    * Get commands to colour structure by sequence
+    * Obsolete method, only referenced from
+    * jalview.javascript.MouseOverStructureListener
     * 
     * @param ssm
     * @param files
diff --cc src/jalview/ext/rbvi/chimera/ChimeraCommands.java
index bfec5b2,5beee56..642035f
--- a/src/jalview/ext/rbvi/chimera/ChimeraCommands.java
+++ b/src/jalview/ext/rbvi/chimera/ChimeraCommands.java
@@@ -385,14 -84,13 +84,14 @@@ public class ChimeraCommands extends St
     * </pre>
     * 
     * @param featureMap
 +   * @param binding
     * @return
     */
-   protected static List<String> buildSetAttributeCommands(
-           Map<String, Map<Object, AtomSpecModel>> featureMap,
-           AAStructureBindingModel binding)
+   @Override
+   public List<StructureCommandI> setAttributes(
+           Map<String, Map<Object, AtomSpecModel>> featureMap)
    {
-     List<String> commands = new ArrayList<>();
+     List<StructureCommandI> commands = new ArrayList<>();
      for (String featureType : featureMap.keySet())
      {
        String attributeName = makeAttributeName(featureType);
diff --cc src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
index 8a19d53,460b156..0907695
--- a/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
+++ b/src/jalview/ext/rbvi/chimera/JalviewChimeraBinding.java
@@@ -60,6 -36,24 +36,25 @@@ import ext.edu.ucsf.rbvi.strucviz2.Chim
  import ext.edu.ucsf.rbvi.strucviz2.ChimeraModel;
  import ext.edu.ucsf.rbvi.strucviz2.StructureManager;
  import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType;
++import jalview.api.AlignViewportI;
+ import jalview.api.AlignmentViewPanel;
+ import jalview.api.structures.JalviewStructureDisplayI;
+ import jalview.bin.Cache;
+ import jalview.datamodel.AlignmentI;
+ import jalview.datamodel.PDBEntry;
+ import jalview.datamodel.SearchResultMatchI;
+ import jalview.datamodel.SearchResultsI;
+ import jalview.datamodel.SequenceFeature;
+ import jalview.datamodel.SequenceI;
+ import jalview.gui.StructureViewer.ViewerType;
+ import jalview.httpserver.AbstractRequestHandler;
+ import jalview.io.DataSourceType;
+ import jalview.structure.AtomSpec;
+ import jalview.structure.AtomSpecModel;
+ import jalview.structure.StructureCommand;
+ import jalview.structure.StructureCommandI;
+ import jalview.structure.StructureSelectionManager;
+ import jalview.structures.models.AAStructureBindingModel;
  
  public abstract class JalviewChimeraBinding extends AAStructureBindingModel
  {
@@@ -542,9 -271,13 +272,14 @@@
     * @param pdbfnum
     * @return
     */
 -  protected String getModelSpec(int pdbfnum)
 +  @Override
 +  public String getModelSpec(int pdbfnum)
    {
+     if (pdbfnum < 0 || pdbfnum >= getPdbCount())
+     {
+       return "#" + pdbfnum; // temp hack for ChimeraX
+     }
+ 
      /*
       * For now, the test for having sub-models is whether multiple Chimera
       * models are mapped for the PDB file; the models are returned as a response
@@@ -1059,18 -577,12 +586,11 @@@
    public int sendFeaturesToViewer(AlignmentViewPanel avp)
    {
      // TODO refactor as required to pull up to an interface
-     AlignmentI alignment = avp.getAlignment();
--
-     String[] files = getStructureFiles();
-     if (files == null)
-     {
-       return 0;
-     }
- 
-     StructureMappingcommandSet commandSet = ChimeraCommands
-             .getSetAttributeCommandsForFeatures(avp, this);
-     String[] commands = commandSet.commands;
-     if (commands.length > 10)
+     Map<String, Map<Object, AtomSpecModel>> featureValues = buildFeaturesMap(
+             avp);
+     List<StructureCommandI> commands = getCommandGenerator()
+             .setAttributes(featureValues);
+     if (commands.size() > 10)
      {
        sendCommandsByFile(commands);
      }
@@@ -1247,95 -772,54 +780,130 @@@
      return CHIMERA_FEATURE_GROUP;
    }
  
-   public Hashtable<String, String> getChainFile()
+   @Override
+   public String getModelIdForFile(String pdbFile)
    {
-     return chainFile;
+     List<ChimeraModel> foundModels = chimeraMaps.get(pdbFile);
+     if (foundModels != null && !foundModels.isEmpty())
+     {
+       return String.valueOf(foundModels.get(0).getModelNumber());
+     }
+     return "";
    }
  
-   public List<ChimeraModel> getChimeraModelByChain(String chain)
+   /**
+    * Answers a (possibly empty) list of attribute names in Chimera[X], excluding
+    * any which were added from Jalview
+    * 
+    * @return
+    */
+   public List<String> getChimeraAttributes()
    {
-     return chimeraMaps.get(chainFile.get(chain));
+     List<String> atts = chimeraManager.getAttrList();
+     Iterator<String> it = atts.iterator();
+     while (it.hasNext())
+     {
+       if (it.next().startsWith(ChimeraCommands.NAMESPACE_PREFIX))
+       {
+         /*
+          * attribute added from Jalview - exclude it
+          */
+         it.remove();
+       }
+     }
+     return atts;
    }
  
-   public int getModelNoForChain(String chain)
+   /**
+    * Returns the file extension to use for a saved viewer session file (.py)
+    * 
+    * @return
+    */
+   @Override
+   public String getSessionFileExtension()
    {
-     List<ChimeraModel> foundModels = getChimeraModelByChain(chain);
-     if (foundModels != null && !foundModels.isEmpty())
-     {
-       return foundModels.get(0).getModelNumber();
-     }
-     return -1;
+     return CHIMERA_SESSION_EXTENSION;
+   }
+ 
+   @Override
+   public String getHelpURL()
+   {
+     return "https://www.cgl.ucsf.edu/chimera/docs/UsersGuide";
    }
 +
 +  @Override
 +  public void showStructures(AlignViewportI av, boolean refocus)
 +  {
-     String string = buildShowStructuresCommand(av, refocus);
-     sendChimeraCommand(string, false);
++    StructureCommandI cmd = buildShowStructuresCommand(av, refocus);
++    executeCommand(cmd, false);
 +  }
 +
 +  /**
 +   * Builds a command to show parts of the structure, depending on whether
 +   * <ul>
 +   * <li>all structures or regions mapped to alignment only are shown</li>
 +   * <li>all chains or only selected chains are shown</li>
 +   * </ul>
 +   * 
 +   * @param av
 +   * @param refocus
 +   * @return
 +   */
-   protected String buildShowStructuresCommand(AlignViewportI av,
++  protected StructureCommandI buildShowStructuresCommand(
++          AlignViewportI av,
 +          boolean refocus)
 +  {
++    // TODO refactor using command generator
++    // pull up this method and Jmol variant to base class
 +    StringBuilder cmd = new StringBuilder(128);
 +    cmd.append("~display");
 +
 +    if (isShowAlignmentOnly())
 +    {
 +      AtomSpecModel model = getShownResidues(av);
-       String atomSpec = ChimeraCommands.getAtomSpec(model, this);
++      String atomSpec = getCommandGenerator().getAtomSpec(model, false);
 +      if (!atomSpec.isEmpty())
 +      {
 +        cmd.append("; ~ribbon; ribbon ").append(atomSpec);
 +      }
 +    }
 +    else
 +    {
 +      cmd.append("; ribbon");
 +    }
 +
 +    /*
-      * hide any chains selected not to be shown
++     * hide any chains selected not to be shown (whether mapped to
++     * sequence in the alignment or not)
 +     */
 +    for (String pdbChain : chainsToHide)
 +    {
 +      String chainId = pdbChain.split(":")[1];
-       int modelNo = getModelNoForChain(pdbChain);
-       if (modelNo != -1)
++      String modelNo = getModelIdForFile(getFileForChain(pdbChain));
++      if (!"".equals(modelNo))
 +      {
-         cmd.append("; ~ribbon #").append(String.valueOf(modelNo))
-                 .append(":.").append(chainId);
++        cmd.append("; ~ribbon #").append(modelNo).append(":.")
++                .append(chainId);
 +      }
 +    }
 +    if (refocus)
 +    {
 +      cmd.append("; focus");
 +    }
-     return cmd.toString();
++    return new StructureCommand(cmd.toString());
 +  }
 +
 +  @Override
 +  public int getModelForPdbFile(String fileName, int fileIndex)
 +  {
 +    if (chimeraMaps.containsKey(fileName))
 +    {
 +      List<ChimeraModel> models = chimeraMaps.get(fileName);
 +      if (!models.isEmpty())
 +      {
 +        return models.get(0).getModelNumber();
 +      }
 +    }
 +    return -1;
 +  }
  }
diff --cc src/jalview/gui/AlignmentPanel.java
index 3948f6d,45e4b95..7ee0d80
--- a/src/jalview/gui/AlignmentPanel.java
+++ b/src/jalview/gui/AlignmentPanel.java
@@@ -20,26 -20,29 +20,6 @@@
   */
  package jalview.gui;
  
--import jalview.analysis.AnnotationSorter;
--import jalview.api.AlignViewportI;
--import jalview.api.AlignmentViewPanel;
--import jalview.bin.Cache;
- import jalview.datamodel.AlignmentI;
- import jalview.datamodel.HiddenColumns;
- import jalview.datamodel.SearchResultsI;
- import jalview.datamodel.SequenceFeature;
- import jalview.datamodel.SequenceGroup;
- import jalview.datamodel.SequenceI;
- import jalview.io.HTMLOutput;
- import jalview.jbgui.GAlignmentPanel;
- import jalview.math.AlignmentDimension;
- import jalview.schemes.ResidueProperties;
- import jalview.structure.StructureSelectionManager;
- import jalview.util.Comparison;
- import jalview.util.MessageManager;
- import jalview.viewmodel.ViewportListenerI;
- import jalview.viewmodel.ViewportRanges;
- 
 -import jalview.bin.Jalview;
 -import jalview.datamodel.AlignmentI;
 -import jalview.datamodel.HiddenColumns;
 -import jalview.datamodel.SearchResultsI;
 -import jalview.datamodel.SequenceFeature;
 -import jalview.datamodel.SequenceGroup;
 -import jalview.datamodel.SequenceI;
 -import jalview.gui.ImageExporter.ImageWriterI;
 -import jalview.io.HTMLOutput;
 -import jalview.jbgui.GAlignmentPanel;
 -import jalview.math.AlignmentDimension;
 -import jalview.schemes.ResidueProperties;
 -import jalview.structure.StructureSelectionManager;
 -import jalview.util.Comparison;
 -import jalview.util.ImageMaker;
 -import jalview.util.MessageManager;
 -import jalview.viewmodel.ViewportListenerI;
 -import jalview.viewmodel.ViewportRanges;
 -
  import java.awt.BorderLayout;
  import java.awt.Color;
  import java.awt.Container;
@@@ -64,6 -67,6 +44,29 @@@ import java.util.List
  
  import javax.swing.SwingUtilities;
  
++import jalview.analysis.AnnotationSorter;
++import jalview.api.AlignViewportI;
++import jalview.api.AlignmentViewPanel;
++import jalview.bin.Cache;
++import jalview.bin.Jalview;
++import jalview.datamodel.AlignmentI;
++import jalview.datamodel.HiddenColumns;
++import jalview.datamodel.SearchResultsI;
++import jalview.datamodel.SequenceFeature;
++import jalview.datamodel.SequenceGroup;
++import jalview.datamodel.SequenceI;
++import jalview.gui.ImageExporter.ImageWriterI;
++import jalview.io.HTMLOutput;
++import jalview.jbgui.GAlignmentPanel;
++import jalview.math.AlignmentDimension;
++import jalview.schemes.ResidueProperties;
++import jalview.structure.StructureSelectionManager;
++import jalview.util.Comparison;
++import jalview.util.ImageMaker;
++import jalview.util.MessageManager;
++import jalview.viewmodel.ViewportListenerI;
++import jalview.viewmodel.ViewportRanges;
++
  /**
   * DOCUMENT ME!
   * 
@@@ -1564,6 -1526,6 +1526,7 @@@ public class AlignmentPanel extends GAl
     * 
     * @param b
     */
++  @Override
    public void setSelected(boolean b)
    {
      try
diff --cc src/jalview/gui/ChimeraViewFrame.java
index 08cad53,0e5675c..55fe9da
--- a/src/jalview/gui/ChimeraViewFrame.java
+++ b/src/jalview/gui/ChimeraViewFrame.java
@@@ -89,20 -81,13 +81,12 @@@ public class ChimeraViewFrame extends S
    {
      super.initMenus();
  
-     viewerActionMenu.setText(MessageManager.getString("label.chimera"));
- 
-     viewerColour
-             .setText(MessageManager.getString("label.colour_with_chimera"));
-     viewerColour.setToolTipText(MessageManager
-             .getString("label.let_chimera_manage_structure_colours"));
- 
-     helpItem.setText(MessageManager.getString("label.chimera_help"));
      savemenu.setVisible(false); // not yet implemented
 -    viewMenu.add(fitToWindow);
  
      JMenuItem writeFeatures = new JMenuItem(
-             MessageManager.getString("label.create_chimera_attributes"));
+             MessageManager.getString("label.create_viewer_attributes"));
      writeFeatures.setToolTipText(MessageManager
-             .getString("label.create_chimera_attributes_tip"));
+             .getString("label.create_viewer_attributes_tip"));
      writeFeatures.addActionListener(new ActionListener()
      {
        @Override
@@@ -715,77 -542,13 +541,7 @@@
      return jmb;
    }
  
-   /**
-    * Ask Chimera to save its session to the designated file path, or to a
-    * temporary file if the path is null. Returns the file path if successful,
-    * else null.
-    * 
-    * @param filepath
-    * @see getStateInfo
-    */
-   protected String saveSession(String filepath)
-   {
-     String pathUsed = filepath;
-     try
-     {
-       if (pathUsed == null)
-       {
-         File tempFile = File.createTempFile("chimera", ".py");
-         tempFile.deleteOnExit();
-         pathUsed = tempFile.getPath();
-       }
-       boolean result = jmb.saveSession(pathUsed);
-       if (result)
-       {
-         this.chimeraSessionFile = pathUsed;
-         return pathUsed;
-       }
-     } catch (IOException e)
-     {
-     }
-     return null;
-   }
- 
-   /**
-    * Returns a string representing the state of the Chimera session. This is
-    * done by requesting Chimera to save its session to a temporary file, then
-    * reading the file contents. Returns an empty string on any error.
-    */
-   @Override
-   public String getStateInfo()
-   {
-     String sessionFile = saveSession(null);
-     if (sessionFile == null)
-     {
-       return "";
-     }
-     InputStream is = null;
-     try
-     {
-       File f = new File(sessionFile);
-       byte[] bytes = new byte[(int) f.length()];
-       is = new FileInputStream(sessionFile);
-       is.read(bytes);
-       return new String(bytes);
-     } catch (IOException e)
-     {
-       return "";
-     } finally
-     {
-       if (is != null)
-       {
-         try
-         {
-           is.close();
-         } catch (IOException e)
-         {
-           // ignore
-         }
-       }
-     }
-   }
- 
    @Override
 -  protected void fitToWindow_actionPerformed()
 -  {
 -    jmb.focusView();
 -  }
 -
 -  @Override
    public ViewerType getViewerType()
    {
      return ViewerType.CHIMERA;
diff --cc src/jalview/gui/JalviewChimeraBindingModel.java
index 8fea3f3,49655a4..c66fdfc
--- a/src/jalview/gui/JalviewChimeraBindingModel.java
+++ b/src/jalview/gui/JalviewChimeraBindingModel.java
@@@ -20,6 -20,6 +20,8 @@@
   */
  package jalview.gui;
  
++import javax.swing.JComponent;
++
  import jalview.api.AlignmentViewPanel;
  import jalview.api.structures.JalviewStructureDisplayI;
  import jalview.datamodel.PDBEntry;
@@@ -27,14 -27,12 +29,9 @@@ import jalview.datamodel.SequenceI
  import jalview.ext.rbvi.chimera.JalviewChimeraBinding;
  import jalview.io.DataSourceType;
  import jalview.structure.StructureSelectionManager;
- import jalview.viewmodel.seqfeatures.FeatureRendererModel;
- 
- import javax.swing.SwingUtilities;
  
 -import javax.swing.JComponent;
 -import javax.swing.SwingUtilities;
 -
  public class JalviewChimeraBindingModel extends JalviewChimeraBinding
  {
-   private ChimeraViewFrame cvf;
- 
    public JalviewChimeraBindingModel(ChimeraViewFrame chimeraViewFrame,
            StructureSelectionManager ssm, PDBEntry[] pdbentry,
            SequenceI[][] sequenceIs, DataSourceType protocol)
diff --cc src/jalview/gui/StructureViewerBase.java
index cdfa225,33a122c..900252a
--- a/src/jalview/gui/StructureViewerBase.java
+++ b/src/jalview/gui/StructureViewerBase.java
@@@ -53,8 -33,8 +33,9 @@@ import java.io.FileReader
  import java.io.IOException;
  import java.io.PrintWriter;
  import java.util.ArrayList;
 +import java.util.Collections;
  import java.util.List;
+ import java.util.Random;
  import java.util.Vector;
  
  import javax.swing.ButtonGroup;
@@@ -170,9 -171,9 +174,9 @@@ public abstract class StructureViewerBa
    }
  
    @Override
-   public boolean isUsedforcolourby(AlignmentViewPanel avp)
 -  public boolean isUsedForColourBy(AlignmentViewPanel ap2)
++  public boolean isUsedForColourBy(AlignmentViewPanel avp)
    {
 -    return (_colourwith != null) && _colourwith.contains(ap2);
 +    return (_colourwith != null) && _colourwith.contains(avp);
    }
  
    /**
@@@ -867,13 -794,12 +867,12 @@@
        }
      } catch (Exception e)
      {
--      StringBuffer sp = new StringBuffer();
-       for (AlignmentViewPanel avp : _alignwith)
++      StringBuilder sb = new StringBuilder();
+       for (AlignmentViewPanel alignPanel : _alignwith)
        {
-         sp.append(
-                 "'" + ((AlignmentPanel) avp).alignFrame.getTitle() + "' ");
 -        sp.append("'" + alignPanel.getViewName() + "' ");
++        sb.append("'").append(alignPanel.getViewName()).append("' ");
        }
--      Cache.log.info("Couldn't align structures with the " + sp.toString()
++      Cache.log.info("Couldn't align structures with the " + sb.toString()
                + "associated alignment panels.", e);
      }
      return reply;
@@@ -1111,20 -1045,198 +1118,195 @@@
    }
  
    @Override
 +  public abstract AAStructureBindingModel getBinding();
 +
 +  /**
 +   * Show only the selected chain(s) in the viewer
 +   */
 +  protected void showSelectedChains()
 +  {
 +    setChainsToHide();
 +  
 +    /*
 +     * refresh display without resizing - easier to see what changed
 +     */
 +    getBinding().showStructures(getAlignmentPanel().getAlignViewport(),
 +            false);
 +  }
 +
++  @Override
+   public long startProgressBar(String msg)
+   {
+     // TODO would rather have startProgress/stopProgress as the
+     // IProgressIndicator interface
+     long tm = random.nextLong();
+     if (progressBar != null)
+     {
+       progressBar.setProgressBar(msg, tm);
+     }
+     return tm;
+   }
+ 
+   @Override
+   public void stopProgressBar(String msg, long handle)
+   {
+     if (progressBar != null)
+     {
+       progressBar.setProgressBar(msg, handle);
+     }
+   }
+ 
+   protected IProgressIndicator getProgressIndicator()
+   {
+     return progressBar;
+   }
+ 
+   protected void setProgressIndicator(IProgressIndicator pi)
+   {
+     progressBar = pi;
+   }
+ 
+   protected void setProgressMessage(String message, long id)
+   {
+     if (progressBar != null)
+     {
+       progressBar.setProgressBar(message, id);
+     }
+   }
+ 
+   @Override
+   public void showConsole(boolean show)
+   {
+     // default does nothing
+   }
+ 
+   /**
 -   * Show only the selected chain(s) in the viewer
 -   */
 -  protected void showSelectedChains()
 -  {
 -    List<String> toshow = new ArrayList<>();
 -    for (int i = 0; i < chainMenu.getItemCount(); i++)
 -    {
 -      if (chainMenu.getItem(i) instanceof JCheckBoxMenuItem)
 -      {
 -        JCheckBoxMenuItem item = (JCheckBoxMenuItem) chainMenu.getItem(i);
 -        if (item.isSelected())
 -        {
 -          toshow.add(item.getText());
 -        }
 -      }
 -    }
 -    getBinding().showChains(toshow);
 -  }
 -
 -  /**
+    * Tries to fetch a PDB file and save to a temporary local file. Returns the
+    * saved file path if successful, or null if not.
+    * 
+    * @param processingEntry
+    * @return
+    */
+   protected String fetchPdbFile(PDBEntry processingEntry)
+   {
+     String filePath = null;
+     Pdb pdbclient = new Pdb();
+     AlignmentI pdbseq = null;
+     String pdbid = processingEntry.getId();
+     long handle = System.currentTimeMillis()
+             + Thread.currentThread().hashCode();
+   
+     /*
+      * Write 'fetching PDB' progress on AlignFrame as we are not yet visible
+      */
+     String msg = MessageManager.formatMessage("status.fetching_pdb",
+             new Object[]
+             { pdbid });
+     getAlignmentPanel().alignFrame.setProgressBar(msg, handle);
+     // long hdl = startProgressBar(MessageManager.formatMessage(
+     // "status.fetching_pdb", new Object[]
+     // { pdbid }));
+     try
+     {
+       pdbseq = pdbclient.getSequenceRecords(pdbid);
+     } catch (Exception e)
+     {
+       System.err.println(
+               "Error retrieving PDB id " + pdbid + ": " + e.getMessage());
+     } finally
+     {
+       msg = pdbid + " " + MessageManager.getString("label.state_completed");
+       getAlignmentPanel().alignFrame.setProgressBar(msg, handle);
+       // stopProgressBar(msg, hdl);
+     }
+     /*
+      * If PDB data were saved and are not invalid (empty alignment), return the
+      * file path.
+      */
+     if (pdbseq != null && pdbseq.getHeight() > 0)
+     {
+       // just use the file name from the first sequence's first PDBEntry
+       filePath = new File(pdbseq.getSequenceAt(0).getAllPDBEntries()
+               .elementAt(0).getFile()).getAbsolutePath();
+       processingEntry.setFile(filePath);
+     }
+     return filePath;
+   }
+ 
+   /**
+    * If supported, saves the state of the structure viewer to a temporary file
+    * and returns the file, else returns null
+    * 
+    * @return
+    */
+   public File saveSession()
+   {
+     // TODO: a wait loop to ensure the file is written fully before returning?
+     return getBinding() == null ? null : getBinding().saveSession();
+   }
+ 
+   /**
+    * Close down this instance of Jalview's Chimera viewer, giving the user the
+    * option to close the associated Chimera window (process). They may wish to
+    * keep it open until they have had an opportunity to save any work.
+    * 
+    * @param forceClose
+    *          if true, close any linked Chimera process; if false, prompt first
+    */
+   @Override
+   public void closeViewer(boolean forceClose)
+   {
+     AAStructureBindingModel binding = getBinding();
+     if (binding != null && binding.isViewerRunning())
+     {
+       if (!forceClose)
+       {
+         String viewerName = getViewerName();
+         String prompt = MessageManager
+                 .formatMessage("label.confirm_close_viewer", new Object[]
+                 { binding.getViewerTitle(viewerName, false), viewerName });
+         prompt = JvSwingUtils.wrapTooltip(true, prompt);
+         int confirm = JvOptionPane.showConfirmDialog(this, prompt,
+                 MessageManager.getString("label.close_viewer"),
+                 JvOptionPane.YES_NO_CANCEL_OPTION);
+         /*
+          * abort closure if user hits escape or Cancel
+          */
+         if (confirm == JvOptionPane.CANCEL_OPTION
+                 || confirm == JvOptionPane.CLOSED_OPTION)
+         {
+           return;
+         }
+         forceClose = confirm == JvOptionPane.YES_OPTION;
+       }
+       binding.closeViewer(forceClose);
+     }
+     setAlignmentPanel(null);
+     _aps.clear();
+     _alignwith.clear();
+     _colourwith.clear();
+     // TODO: check for memory leaks where instance isn't finalised because jmb
+     // holds a reference to the window
+     // jmb = null;
+     dispose();
+   }
+ 
+   @Override
+   public void showHelp_actionPerformed()
+   {
+     try
+     {
+       String url = getBinding().getHelpURL();
+       if (url != null)
+       {
+         BrowserLauncher.openURL(url);
+       }
+     } catch (IOException ex)
+     {
+       System.err
+               .println("Show " + getViewerName() + " failed with: "
+                       + ex.getMessage());
+     }
+   }
+ 
  }
diff --cc src/jalview/structure/StructureCommandsBase.java
index 0000000,3c29fd4..e4f7ac1
mode 000000,100644..100644
--- a/src/jalview/structure/StructureCommandsBase.java
+++ b/src/jalview/structure/StructureCommandsBase.java
@@@ -1,0 -1,226 +1,227 @@@
+ package jalview.structure;
+ 
+ import java.awt.Color;
+ import java.util.ArrayList;
+ import java.util.List;
+ import java.util.Map;
+ import java.util.Map.Entry;
+ 
+ /**
+  * A base class holding methods useful to all classes that implement commands
+  * for structure viewers
+  * 
+  * @author gmcarstairs
+  *
+  */
+ public abstract class StructureCommandsBase implements StructureCommandsI
+ {
+   private static final String CMD_SEPARATOR = ";";
++
+   public static final String NAMESPACE_PREFIX = "jv_";
+ 
+   /**
+    * Returns something that separates concatenated commands
+    * 
+    * @return
+    */
+   protected static String getCommandSeparator()
+   {
+     return CMD_SEPARATOR;
+   }
+ 
+   /**
+    * Returns the lowest model number used by the structure viewer
+    * 
+    * @return
+    */
+   @Override
+   public int getModelStartNo()
+   {
+     return 0;
+   }
+ 
+   /**
+    * Helper method to add one contiguous range to the AtomSpec model for the given
+    * value (creating the model if necessary). As used by Jalview, {@code value} is
+    * <ul>
+    * <li>a colour, when building a 'colour structure by sequence' command</li>
+    * <li>a feature value, when building a 'set Chimera attributes from features'
+    * command</li>
+    * </ul>
+    * 
+    * @param map
+    * @param value
+    * @param model
+    * @param startPos
+    * @param endPos
+    * @param chain
+    */
+   public static final void addAtomSpecRange(Map<Object, AtomSpecModel> map,
+           Object value, String model, int startPos, int endPos,
+           String chain)
+   {
+     /*
+      * Get/initialize map of data for the colour
+      */
+     AtomSpecModel atomSpec = map.get(value);
+     if (atomSpec == null)
+     {
+       atomSpec = new AtomSpecModel();
+       map.put(value, atomSpec);
+     }
+   
+     atomSpec.addRange(model, startPos, endPos, chain);
+   }
+ 
+   /**
+    * Makes a structure viewer attribute name for a Jalview feature type by
+    * prefixing it with "jv_", and replacing any non-alphanumeric characters with
+    * an underscore
+    * 
+    * @param featureType
+    * @return
+    */
+   protected String makeAttributeName(String featureType)
+   {
+     StringBuilder sb = new StringBuilder();
+     if (featureType != null)
+     {
+       for (char c : featureType.toCharArray())
+       {
+         sb.append(Character.isLetterOrDigit(c) ? c : '_');
+       }
+     }
+     String attName = NAMESPACE_PREFIX + sb.toString();
+     return attName;
+   }
+ 
+   /**
+    * Traverse the map of colours/models/chains/positions to construct a list of
+    * 'color' commands (one per distinct colour used). The format of each command
+    * is specific to the structure viewer.
+    * <p>
+    * The default implementation returns a single command containing one command
+    * per colour, concatenated.
+    * 
+    * @param colourMap
+    * @return
+    */
+   @Override
+   public List<StructureCommandI> colourBySequence(
+           Map<Object, AtomSpecModel> colourMap)
+   {
+     List<StructureCommandI> commands = new ArrayList<>();
+     StringBuilder sb = new StringBuilder(colourMap.size() * 20);
+     boolean first = true;
+     for (Object key : colourMap.keySet())
+     {
+       Color colour = (Color) key;
+       final AtomSpecModel colourData = colourMap.get(colour);
+       StructureCommandI command = getColourCommand(colourData, colour);
+       if (!first)
+       {
+         sb.append(getCommandSeparator());
+       }
+       first = false;
+       sb.append(command.getCommand());
+     }
+ 
+     commands.add(new StructureCommand(sb.toString()));
+     return commands;
+   }
+ 
+   /**
+    * Returns a command to colour the atoms represented by {@code atomSpecModel}
+    * with the colour specified by {@code colourCode}.
+    * 
+    * @param atomSpecModel
+    * @param colour
+    * @return
+    */
+   protected StructureCommandI getColourCommand(AtomSpecModel atomSpecModel,
+           Color colour)
+   {
+     String atomSpec = getAtomSpec(atomSpecModel, false);
+     return colourResidues(atomSpec, colour);
+   }
+ 
+   /**
+    * Returns a command to colour the atoms described (in viewer command syntax)
+    * by {@code atomSpec} with the colour specified by {@code colourCode}
+    * 
+    * @param atomSpec
+    * @param colour
+    * @return
+    */
+   protected abstract StructureCommandI colourResidues(String atomSpec,
+           Color colour);
+ 
+   @Override
+   public List<StructureCommandI> colourByResidues(
+           Map<String, Color> colours)
+   {
+     List<StructureCommandI> commands = new ArrayList<>();
+     for (Entry<String, Color> entry : colours.entrySet())
+     {
+       commands.add(colourResidue(entry.getKey(), entry.getValue()));
+     }
+     return commands;
+   }
+ 
+   private StructureCommandI colourResidue(String resName, Color col)
+   {
+     String atomSpec = getResidueSpec(resName);
+     return colourResidues(atomSpec, col);
+   }
+ 
+   /**
+    * Helper method to append one start-end range to an atomspec string
+    * 
+    * @param sb
+    * @param start
+    * @param end
+    * @param chain
+    * @param firstPositionForModel
+    */
+   protected void appendRange(StringBuilder sb, int start, int end,
+           String chain, boolean firstPositionForModel, boolean isChimeraX)
+   {
+     if (!firstPositionForModel)
+     {
+       sb.append(",");
+     }
+     if (end == start)
+     {
+       sb.append(start);
+     }
+     else
+     {
+       sb.append(start).append("-").append(end);
+     }
+ 
+     if (!isChimeraX)
+     {
+       sb.append(".");
+       if (!" ".equals(chain))
+       {
+         sb.append(chain);
+       }
+     }
+   }
+ 
+   /**
+    * Returns the atom specifier meaning all occurrences of the given residue
+    * 
+    * @param residue
+    * @return
+    */
+   protected abstract String getResidueSpec(String residue);
+ 
+   @Override
+   public List<StructureCommandI> setAttributes(
+           Map<String, Map<Object, AtomSpecModel>> featureValues)
+   {
+     // default does nothing, override where this is implemented
+     return null;
+   }
+ }
diff --cc src/jalview/structures/models/AAStructureBindingModel.java
index 19b72b9,5949847..7b1485c
--- a/src/jalview/structures/models/AAStructureBindingModel.java
+++ b/src/jalview/structures/models/AAStructureBindingModel.java
@@@ -20,8 -20,22 +20,24 @@@
   */
  package jalview.structures.models;
  
+ import java.awt.Color;
+ import java.io.File;
+ import java.io.IOException;
+ import java.util.ArrayList;
+ import java.util.Arrays;
+ import java.util.BitSet;
++import java.util.Collections;
+ import java.util.HashMap;
++import java.util.Iterator;
+ import java.util.LinkedHashMap;
+ import java.util.List;
+ import java.util.Map;
+ 
+ import javax.swing.SwingUtilities;
+ 
  import jalview.api.AlignViewportI;
  import jalview.api.AlignmentViewPanel;
+ import jalview.api.FeatureRenderer;
  import jalview.api.SequenceRenderer;
  import jalview.api.StructureSelectionManagerProvider;
  import jalview.api.structures.JalviewStructureDisplayI;
@@@ -98,47 -175,6 +178,16 @@@ public abstract class AAStructureBindin
  
    public String fileLoadingError;
  
 +  private boolean showAlignmentOnly;
 +
 +  /*
 +   * a list of chains "pdbid:chainid" to hide in the viewer
 +   */
 +  // TODO make private once deprecated JalviewJmolBinding.centerViewer removed
 +  protected List<String> chainsToHide;
 +
 +  private boolean hideHiddenRegions;
 +
-   protected List<String> chainNames = new ArrayList<>();
- 
-   /**
-    * Data bean class to simplify parameterisation in superposeStructures
-    */
-   protected class SuperposeData
-   {
-     /**
-      * Constructor with alignment width argument
-      * 
-      * @param width
-      */
-     public SuperposeData(int width)
-     {
-       pdbResNo = new int[width];
-     }
- 
-     public String filename;
- 
-     public String pdbId;
- 
-     public String chain = "";
- 
-     public boolean isRna;
- 
-     /*
-      * The pdb residue number (if any) mapped to each column of the alignment
-      */
-     public int[] pdbResNo;
-   }
- 
    /**
     * Constructor
     * 
@@@ -150,7 -186,8 +199,9 @@@
    {
      this.ssm = ssm;
      this.sequence = seqs;
 +    chainsToHide = new ArrayList<>();
+     chainNames = new ArrayList<>();
+     chainFile = new HashMap<>();
    }
  
    /**
@@@ -358,6 -396,25 +412,25 @@@
      return colourBySequence;
    }
  
+   /**
+    * Called when the binding thinks the UI needs to be refreshed after a
+    * structure viewer state change. This could be because structures were
+    * loaded, or because an error has occurred. Default does nothing, override as
+    * required.
+    */
+   public void refreshGUI()
+   {
+   }
+ 
+   /**
+    * Instruct the Jalview binding to update the pdbentries vector if necessary
 -   * prior to matching the jmol view's contents to the list of structure files
++   * prior to matching the viewer's contents to the list of structure files
+    * Jalview knows about. By default does nothing, override as required.
+    */
+   public void refreshPdbEntries()
+   {
+   }
+ 
    public void setColourBySequence(boolean colourBySequence)
    {
      this.colourBySequence = colourBySequence;
@@@ -795,27 -965,44 +981,66 @@@
            AlignmentViewPanel alignment);
  
    /**
 +   * Recolours mapped residues in the structure viewer to match colours in the
-    * given alignment panel. Colours should also be applied to any hidden mapped
-    * residues (so that they are shown correctly if these get unhidden).
++   * given alignment panel, provided colourBySequence is selected. Colours
++   * should also be applied to any hidden mapped residues (so that they are
++   * shown correctly if these get unhidden).
 +   * 
 +   * @param viewPanel
 +   */
-   protected abstract void colourBySequence(AlignmentViewPanel viewPanel);
++  protected void colourBySequence(AlignmentViewPanel viewPanel)
++  {
++
++    if (!colourBySequence || !isLoadingFinished() || getSsm() == null)
++    {
++      return;
++    }
++    Map<Object, AtomSpecModel> colourMap = buildColoursMap(ssm, sequence,
++            viewPanel);
++
++    List<StructureCommandI> colourBySequenceCommands = commandGenerator
++            .colourBySequence(colourMap);
++    executeCommands(colourBySequenceCommands, false, null);
++
++  }
++
++  /**
+    * Sends a command to the structure viewer to colour each chain with a
+    * distinct colour (to the extent supported by the viewer)
+    */
+   public void colourByChain()
+   {
+     colourBySequence = false;
 -
+     // TODO: JAL-628 colour chains distinctly across all visible models
 -
+     executeCommand(commandGenerator.colourByChain(), false,
+             COLOURING_STRUCTURES);
+   }
  
-   public abstract void colourByChain();
+   /**
+    * Sends a command to the structure viewer to colour each chain with a
+    * distinct colour (to the extent supported by the viewer)
+    */
+   public void colourByCharge()
+   {
+     colourBySequence = false;
  
-   public abstract void colourByCharge();
+     executeCommands(commandGenerator.colourByCharge(), false,
+             COLOURING_STRUCTURES);
+   }
  
    /**
-    * Recolours the displayed structures, if they are coloured by sequence, or
-    * 'show only visible alignment' is selected. This supports updating structure
-    * colours on either change of alignment colours, or change to the visible
-    * region of the alignment.
+    * Sends a command to the structure to apply a colour scheme (defined in
+    * Jalview but not necessarily applied to the alignment), which defines a
+    * colour per residue letter. More complex schemes (e.g. that depend on
+    * consensus) cannot be used here and are ignored.
+    * 
+    * @param cs
     */
-   public void updateStructureColours(AlignmentViewPanel alignmentv)
+   public void colourByJalviewColourScheme(ColourSchemeI cs)
    {
-     if (!isLoadingFinished())
+     colourBySequence = false;
+ 
+     if (cs == null || !cs.isSimple())
      {
        return;
      }
@@@ -884,197 -1124,748 +1162,959 @@@
    }
  
    /**
-    * Answers true if regions hidden in the alignment should also be hidden in the
-    * structure viewer, else false (only applies when the structure viewer is
-    * restricted to the alignment only)
+    * Executes one or more structure viewer commands. If a progress message is
+    * provided, it is shown first, and removed after all commands have been run.
     * 
+    * @param getReply
+    * @param msg
+    * @param commands
     * @return
     */
-   public boolean isHideHiddenRegions()
+   protected List<String> executeCommands(boolean getReply, String msg,
+           StructureCommandI[] commands)
    {
-     return hideHiddenRegions;
-   }
+     // todo: tidy this up
  
-   /**
-    * Shows the structures in the viewer, without changing their colouring. This is
-    * to support toggling of whether the whole structure is shown, or only residues
-    * mapped to visible regions of the alignment.
-    * 
-    * @param alignViewportI
-    * @param refocus
-    *                         if true, refit the display to the viewer
-    */
-   public void showStructures(AlignViewportI alignViewportI, boolean refocus)
-   {
-     // override with implementation
+     /*
+      * show progress message if specified
+      */
+     final JalviewStructureDisplayI theViewer = getViewer();
+     final long handle = msg == null ? 0 : theViewer.startProgressBar(msg);
+ 
+     List<String> response = getReply ? new ArrayList<>() : null;
+     try
+     {
+       for (StructureCommandI cmd : commands)
+       {
+         List<String> replies = executeCommand(cmd, getReply, null);
+         if (getReply && replies != null)
+         {
+           response.addAll(replies);
+         }
+       }
+       return response;
+     } finally
+     {
+       if (msg != null)
+       {
+         theViewer.stopProgressBar(null, handle);
+       }
+     }
    }
  
-   @Override
-   public void updateColours(Object source)
+   /**
 -   * Colours any structures associated with sequences in the given alignment as
 -   * coloured in the alignment view, provided colourBySequence is enabled
++   * Recolours the displayed structures, if they are coloured by
++   * sequence, or 'show only visible alignment' is selected. This supports
++   * updating structure colours on either change of alignment colours, or change
++   * to the visible region of the alignment.
+    */
 -  public void colourBySequence(AlignmentViewPanel alignmentv)
++  public void updateStructureColours(AlignmentViewPanel alignmentv)
    {
-     AlignmentViewPanel ap = (AlignmentViewPanel) source;
-     // ignore events from panels not used to colour this view
-     if (!getViewer().isUsedforcolourby(ap))
 -    if (!colourBySequence || !isLoadingFinished() || getSsm() == null)
++    if (!isLoadingFinished())
      {
        return;
      }
-     if (!isLoadingFromArchive())
 -    Map<Object, AtomSpecModel> colourMap = buildColoursMap(ssm, sequence,
 -            alignmentv);
+ 
 -    List<StructureCommandI> colourBySequenceCommands = commandGenerator
 -            .colourBySequence(colourMap);
 -    executeCommands(colourBySequenceCommands, false, null);
++    /*
++     * if structure is not coloured by sequence, but restricted to the alignment,
++     * then redraw it (but don't recolour it) in case hidden regions have changed
++     * (todo: specific messaging for change of hidden region only)
++     */
++    if (!colourBySequence)
 +    {
-       updateStructureColours(ap);
++      if (isShowAlignmentOnly())
++      {
++        showStructures(alignmentv.getAlignViewport(), false);
++      }
++      return;
++    }
++    if (getSsm() == null)
++    {
++      return;
++    }
++    colourBySequence(alignmentv);
+   }
+ 
+   /**
+    * Centre the display in the structure viewer
+    */
+   public void focusView()
+   {
+     executeCommand(commandGenerator.focusView(), false, null);
+   }
+ 
+   /**
+    * Generates and executes a command to show only specified chains in the
+    * structure viewer. The list of chains to show should contain entries
+    * formatted as "pdbid:chaincode".
+    * 
+    * @param toShow
+    */
+   public void showChains(List<String> toShow)
+   {
+     // todo or reformat toShow list entries as modelNo:pdbId:chainCode ?
+ 
+     /*
+      * Reformat the pdbid:chainCode values as modelNo:chainCode
+      * since this is what is needed to construct the viewer command
+      * todo: find a less messy way to do this
+      */
+     List<String> showThese = new ArrayList<>();
+     for (String chainId : toShow)
+     {
+       String[] tokens = chainId.split("\\:");
+       if (tokens.length == 2)
+       {
+         String pdbFile = getFileForChain(chainId);
+         String model = getModelIdForFile(pdbFile);
+         showThese.add(model + ":" + tokens[1]);
+       }
      }
+     executeCommands(commandGenerator.showChains(showThese), false, null);
+   }
+ 
+   /**
+    * Answers the structure viewer's model id given a PDB file name. Returns an
+    * empty string if model id is not found.
+    * 
+    * @param chainId
+    * @return
+    */
+   protected abstract String getModelIdForFile(String chainId);
+ 
+   public boolean hasFileLoadingError()
+   {
+     return fileLoadingError != null && fileLoadingError.length() > 0;
+   }
+ 
+   /**
++   * Sets the flag for whether only mapped visible residues in the alignment
++   * should be visible in the structure viewer
++   * 
++   * @param b
++   */
++  public void setShowAlignmentOnly(boolean b)
++  {
++    showAlignmentOnly = b;
++  }
++
++  /**
++   * Answers true if only residues mapped to the alignment should be shown in the
++   * structure viewer, else false
++   * 
++   * @return
++   */
++  public boolean isShowAlignmentOnly()
++  {
++    return showAlignmentOnly;
++  }
++
++  /**
++   * Sets the flag for hiding regions of structure which are hidden in the
++   * alignment (only applies when the structure viewer is restricted to the
++   * alignment only)
++   * 
++   * @param b
++   */
++  public void setHideHiddenRegions(boolean b)
++  {
++    hideHiddenRegions = b;
++  }
++
++  /**
++   * Answers true if regions hidden in the alignment should also be hidden in the
++   * structure viewer, else false (only applies when the structure viewer is
++   * restricted to the alignment only)
++   * 
++   * @return
++   */
++  public boolean isHideHiddenRegions()
++  {
++    return hideHiddenRegions;
++  }
++
++  /**
++   * Shows the structures in the viewer, without changing their colouring. This is
++   * to support toggling of whether the whole structure is shown, or only residues
++   * mapped to visible regions of the alignment.
++   * 
++   * @param alignViewportI
++   * @param refocus
++   *                         if true, refit the display to the viewer
++   */
++  public void showStructures(AlignViewportI alignViewportI, boolean refocus)
++  {
++    // override with implementation
 +  }
 +
 +  /**
 +   * Sets the list of chains to hide (as "pdbid:chain")
 +   * 
 +   * @param chains
 +   */
 +  public void setChainsToHide(List<String> chains)
 +  {
 +    chainsToHide = chains;
 +  }
 +
 +  /**
 +   * Answers true if the specified structure and chain are selected to be shown in
 +   * the viewer, else false
 +   * 
 +   * @param pdbId
 +   * @param chainId
 +   * @return
 +   */
 +  protected boolean isShowChain(String pdbId, String chainId)
 +  {
 +    if (chainsToHide.isEmpty())
 +    {
 +      return true;
 +    }
 +    return !chainsToHide.contains(pdbId + ":" + chainId);
 +  }
 +
 +  @Override
 +  public abstract String[] getStructureFiles();
 +
 +  /**
 +   * Builds a model of residues mapped from sequences to show on structure, taking
 +   * into account user choices of
 +   * <ul>
 +   * <li>which chains are shown</li>
 +   * <li>whether all structure is shown, or only that mapped to the alignment</li>
 +   * <li>whether hidden regions of the alignment are hidden (excluded) or grayed
 +   * out (included)</li>
 +   * </ul>
 +   * 
 +   * @param av
 +   * @return
 +   */
 +  protected AtomSpecModel getShownResidues(AlignViewportI av)
 +  {
 +    AlignmentI alignment = av.getAlignment();
 +    final int width = alignment.getWidth();
 +  
 +    String[] files = getStructureFiles();
 +  
 +    AtomSpecModel model = new AtomSpecModel();
 +  
 +    for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++)
 +    {
 +      String fileName = files[pdbfnum];
-       final int modelNumber = getModelForPdbFile(fileName, pdbfnum);
++      final String modelId = getModelIdForFile(files[pdbfnum]);
 +      StructureMapping[] mappings = getSsm().getMapping(fileName);
 +  
 +      /*
 +       * Find the first mapped sequence (if any) for this PDB entry which is in
 +       * the alignment
 +       */
 +      final int seqCountForPdbFile = getSequence()[pdbfnum].length;
 +      for (int s = 0; s < seqCountForPdbFile; s++)
 +      {
 +        for (StructureMapping mapping : mappings)
 +        {
 +          final SequenceI theSequence = getSequence()[pdbfnum][s];
 +          if (mapping.getSequence() == theSequence
 +                  && alignment.findIndex(theSequence) > -1)
 +          {
 +            String chainCd = mapping.getChain();
 +            if (!isShowChain(mapping.getPdbId(), chainCd))
 +            {
-               continue;
++              // continue;
 +            }
 +            Iterator<int[]> visible;
 +            if (isShowAlignmentOnly() && isHideHiddenRegions())
 +            {
 +              visible = alignment.getHiddenColumns()
 +                    .getVisContigsIterator(0, width, true);
 +            }
 +            else
 +            {
 +              visible = Collections.singletonList(new int[] { 0, width })
 +                      .iterator();
 +            }
 +            while (visible.hasNext())
 +            {
 +              int[] visibleRegion = visible.next();
 +              int seqStartPos = theSequence.findPosition(visibleRegion[0]);
 +              int seqEndPos = theSequence.findPosition(visibleRegion[1]);
 +              List<int[]> residueRanges = mapping
 +                      .getPDBResNumRanges(seqStartPos, seqEndPos);
 +              if (!residueRanges.isEmpty())
 +              {
 +                for (int[] range : residueRanges)
 +                {
-                   model.addRange(modelNumber, range[0], range[1], chainCd);
++                  model.addRange(modelId, range[0], range[1], chainCd);
 +                }
 +              }
 +            }
 +          }
 +        }
 +      }
 +    }
 +  
 +    return model;
 +  }
 +
 +  /**
 +   * Answers the structure viewer's model number for the given PDB file, or -1 if
 +   * not found
 +   * 
 +   * @param fileName
 +   * @param fileIndex
 +   *                    index of the file in the stored array of file names
 +   * @return
 +   */
 +  public int getModelForPdbFile(String fileName, int fileIndex)
 +  {
 +    return fileIndex;
 +  }
 +
 +  /**
 +   * Answers a default structure model specification which is simply the string
 +   * form of the model number. Override if needed to specify submodels.
 +   * 
 +   * @param model
 +   * @return
 +   */
 +  public String getModelSpec(int model)
 +  {
 +    return String.valueOf(model);
 +  }
 +
 +  /**
-    * Returns a list of chains mapped in this viewer. Note this list is not
-    * currently scoped per structure.
+    * Returns the FeatureRenderer for the given alignment view, or null if
+    * feature display is turned off in the view.
     * 
+    * @param avp
     * @return
     */
-   public List<String> getChainNames()
+   public FeatureRenderer getFeatureRenderer(AlignmentViewPanel avp)
    {
-     return chainNames;
+     AlignmentViewPanel ap = (avp == null) ? getViewer().getAlignmentPanel()
+             : avp;
+     if (ap == null)
+     {
+       return null;
+     }
+     return ap.getAlignViewport().isShowSequenceFeatures()
+             ? ap.getFeatureRenderer()
+             : null;
+   }
+ 
+   protected void setStructureCommands(StructureCommandsI cmd)
+   {
+     commandGenerator = cmd;
    }
  
    /**
-    * Send a command to resize and/or centre the structure display
+    * Records association of one chain id (formatted as "pdbid:chainCode") with
+    * the corresponding PDB file name
+    * 
+    * @param chainId
+    * @param fileName
     */
-   public void focusView()
+   public void addChainFile(String chainId, String fileName)
+   {
+     chainFile.put(chainId, fileName);
+   }
+ 
+   /**
+    * Returns the PDB filename for the given chain id (formatted as
+    * "pdbid:chainCode"), or null if not found
+    * 
+    * @param chainId
+    * @return
+    */
+   protected String getFileForChain(String chainId)
+   {
+     return chainFile.get(chainId);
+   }
+ 
+   @Override
+   public void updateColours(Object source)
+   {
+     AlignmentViewPanel ap = (AlignmentViewPanel) source;
+     // ignore events from panels not used to colour this view
+     if (!getViewer().isUsedForColourBy(ap))
+     {
+       return;
+     }
+     if (!isLoadingFromArchive())
+     {
 -      colourBySequence(ap);
++      updateStructureColours(ap);
+     }
+   }
+ 
+   public StructureCommandsI getCommandGenerator()
+   {
+     return commandGenerator;
+   }
+ 
+   protected abstract ViewerType getViewerType();
+ 
+   /**
+    * Send a structure viewer command asynchronously in a new thread. If the
+    * progress message is not null, display this message while the command is
+    * executing.
+    * 
+    * @param command
+    * @param progressMsg
+    */
+   protected void sendAsynchronousCommand(StructureCommandI command,
+           String progressMsg)
+   {
+     final JalviewStructureDisplayI theViewer = getViewer();
+     final long handle = progressMsg == null ? 0
+             : theViewer.startProgressBar(progressMsg);
+     SwingUtilities.invokeLater(new Runnable()
+     {
+       @Override
+       public void run()
+       {
+         try
+         {
+           executeCommand(command, false, null);
+         } finally
+         {
+           if (progressMsg != null)
+           {
+             theViewer.stopProgressBar(null, handle);
+           }
+         }
+       }
+     });
+ 
+   }
+ 
+   /**
+    * Builds a data structure which records mapped structure residues for each
+    * colour. From this we can easily generate the viewer commands for colour by
+    * sequence. Constructs and returns a map of {@code Color} to
+    * {@code AtomSpecModel}, where the atomspec model holds
+    * 
+    * <pre>
+    *   Model ids
+    *     Chains
+    *       Residue positions
+    * </pre>
+    * 
+    * Ordering is by order of addition (for colours), natural ordering (for
+    * models and chains)
+    * 
+    * @param ssm
+    * @param sequence
+    * @param viewPanel
+    * @return
+    */
+   protected Map<Object, AtomSpecModel> buildColoursMap(
+           StructureSelectionManager ssm, SequenceI[][] sequence,
+           AlignmentViewPanel viewPanel)
+   {
+     String[] files = getStructureFiles();
+     SequenceRenderer sr = getSequenceRenderer(viewPanel);
+     FeatureRenderer fr = viewPanel.getFeatureRenderer();
+     FeatureColourFinder finder = new FeatureColourFinder(fr);
+     AlignViewportI viewport = viewPanel.getAlignViewport();
+     HiddenColumns cs = viewport.getAlignment().getHiddenColumns();
+     AlignmentI al = viewport.getAlignment();
+     Map<Object, AtomSpecModel> colourMap = new LinkedHashMap<>();
+     Color lastColour = null;
+   
+     for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++)
+     {
+       final String modelId = getModelIdForFile(files[pdbfnum]);
+       StructureMapping[] mapping = ssm.getMapping(files[pdbfnum]);
+   
+       if (mapping == null || mapping.length < 1)
+       {
+         continue;
+       }
+   
+       int startPos = -1, lastPos = -1;
+       String lastChain = "";
+       for (int s = 0; s < sequence[pdbfnum].length; s++)
+       {
+         for (int sp, m = 0; m < mapping.length; m++)
+         {
+           final SequenceI seq = sequence[pdbfnum][s];
+           if (mapping[m].getSequence() == seq
+                   && (sp = al.findIndex(seq)) > -1)
+           {
+             SequenceI asp = al.getSequenceAt(sp);
+             for (int r = 0; r < asp.getLength(); r++)
+             {
+               // no mapping to gaps in sequence
+               if (Comparison.isGap(asp.getCharAt(r)))
+               {
+                 continue;
+               }
+               int pos = mapping[m].getPDBResNum(asp.findPosition(r));
+   
+               if (pos < 1 || pos == lastPos)
+               {
+                 continue;
+               }
+   
+               Color colour = sr.getResidueColour(seq, r, finder);
+   
+               /*
+                * darker colour for hidden regions
+                */
+               if (!cs.isVisible(r))
+               {
+                 colour = Color.GRAY;
+               }
+   
+               final String chain = mapping[m].getChain();
+   
+               /*
+                * Just keep incrementing the end position for this colour range
+                * _unless_ colour, PDB model or chain has changed, or there is a
+                * gap in the mapped residue sequence
+                */
+               final boolean newColour = !colour.equals(lastColour);
+               final boolean nonContig = lastPos + 1 != pos;
+               final boolean newChain = !chain.equals(lastChain);
+               if (newColour || nonContig || newChain)
+               {
+                 if (startPos != -1)
+                 {
+                   addAtomSpecRange(colourMap, lastColour, modelId,
+                           startPos, lastPos, lastChain);
+                 }
+                 startPos = pos;
+               }
+               lastColour = colour;
+               lastPos = pos;
+               lastChain = chain;
+             }
+             // final colour range
+             if (lastColour != null)
+             {
+               addAtomSpecRange(colourMap, lastColour, modelId, startPos,
+                       lastPos, lastChain);
+             }
+             // break;
+           }
+         }
+       }
+     }
+     return colourMap;
+   }
+ 
+   /**
+    * todo better refactoring (map lookup or similar to get viewer structure id)
+    * 
+    * @param pdbfnum
+    * @param file
+    * @return
+    */
+   protected String getModelId(int pdbfnum, String file)
+   {
+     return String.valueOf(pdbfnum);
+   }
+ 
+   /**
+    * Saves chains, formatted as "pdbId:chainCode", and lookups from this to the
+    * full PDB file path
+    * 
+    * @param pdb
+    * @param file
+    */
+   public void stashFoundChains(StructureFile pdb, String file)
+   {
+     for (int i = 0; i < pdb.getChains().size(); i++)
+     {
+       String chid = pdb.getId() + ":" + pdb.getChains().elementAt(i).id;
+       addChainFile(chid, file);
+       getChainNames().add(chid);
+     }
+   }
+ 
+   /**
+    * Helper method to add one contiguous range to the AtomSpec model for the given
+    * value (creating the model if necessary). As used by Jalview, {@code value} is
+    * <ul>
+    * <li>a colour, when building a 'colour structure by sequence' command</li>
+    * <li>a feature value, when building a 'set Chimera attributes from features'
+    * command</li>
+    * </ul>
+    * 
+    * @param map
+    * @param value
+    * @param model
+    * @param startPos
+    * @param endPos
+    * @param chain
+    */
+   public static final void addAtomSpecRange(Map<Object, AtomSpecModel> map,
+           Object value,
+           String model, int startPos, int endPos, String chain)
+   {
+     /*
+      * Get/initialize map of data for the colour
+      */
+     AtomSpecModel atomSpec = map.get(value);
+     if (atomSpec == null)
+     {
+       atomSpec = new AtomSpecModel();
+       map.put(value, atomSpec);
+     }
+   
+     atomSpec.addRange(model, startPos, endPos, chain);
+   }
+ 
+   /**
+    * Returns the file extension (including '.' separator) to use for a saved
+    * viewer session file. Default is to return null (not supported), override as
+    * required.
+    * 
+    * @return
+    */
+   public String getSessionFileExtension()
+   {
+     return null;
+   }
+ 
+   /**
+    * If supported, saves the state of the structure viewer to a temporary file
+    * and returns the file. Returns null and logs an error on any failure.
+    * 
+    * @return
+    */
+   public File saveSession()
+   {
+     String prefix = getViewerType().toString();
+     String suffix = getSessionFileExtension();
+     File f = null;
+     try
+     {
+       f = File.createTempFile(prefix, suffix);
+       saveSession(f);
+     } catch (IOException e)
+     {
+       Cache.log.error(String.format("Error saving %s session: %s",
+               prefix, e.toString()));
+     }
+ 
+     return f;
+   }
+ 
+   /**
+    * Saves the structure viewer session to the given file
+    * 
+    * @param f
+    */
+   protected void saveSession(File f)
+   {
+     StructureCommandI cmd = commandGenerator
+             .saveSession(f.getPath());
+     if (cmd != null)
+     {
+       executeCommand(cmd, false);
+     }
+   }
+ 
+   /**
+    * Returns true if the viewer is an external structure viewer for which the
+    * process is still alive, else false (for Jmol, or an external viewer which
+    * the user has independently closed)
+    * 
+    * @return
+    */
+   public boolean isViewerRunning()
    {
+     return false;
+   }
+ 
+   /**
+    * Closes Jalview's structure viewer panel and releases associated resources.
+    * If it is managing an external viewer program, and {@code forceClose} is
+    * true, also shuts down that program.
+    * 
+    * @param forceClose
+    */
+   public void closeViewer(boolean forceClose)
+   {
+     getSsm().removeStructureViewerListener(this, this.getStructureFiles());
+     releaseUIResources();
+ 
+     // add external viewer shutdown in overrides
+     // todo - or can maybe pull up to here
+   }
+ 
+   /**
+    * Returns the URL of a help page for the structure viewer, or null if none is
+    * known
+    * 
+    * @return
+    */
+   public String getHelpURL()
+   {
+     return null;
+   }
+ 
+   /**
+    * <pre>
+    * Helper method to build a map of 
+    *   { featureType, { feature value, AtomSpecModel } }
+    * </pre>
+    * 
+    * @param viewPanel
+    * @return
+    */
+   protected Map<String, Map<Object, AtomSpecModel>> buildFeaturesMap(
+           AlignmentViewPanel viewPanel)
+   {
+     Map<String, Map<Object, AtomSpecModel>> theMap = new LinkedHashMap<>();
+     String[] files = getStructureFiles();
+     if (files == null)
+     {
+       return theMap;
+     }
+ 
+     FeatureRenderer fr = viewPanel.getFeatureRenderer();
+     if (fr == null)
+     {
+       return theMap;
+     }
+   
+     AlignViewportI viewport = viewPanel.getAlignViewport();
+     List<String> visibleFeatures = fr.getDisplayedFeatureTypes();
+   
+     /*
+      * if alignment is showing features from complement, we also transfer
+      * these features to the corresponding mapped structure residues
+      */
+     boolean showLinkedFeatures = viewport.isShowComplementFeatures();
+     List<String> complementFeatures = new ArrayList<>();
+     FeatureRenderer complementRenderer = null;
+     if (showLinkedFeatures)
+     {
+       AlignViewportI comp = fr.getViewport().getCodingComplement();
+       if (comp != null)
+       {
+         complementRenderer = Desktop.getAlignFrameFor(comp)
+                 .getFeatureRenderer();
+         complementFeatures = complementRenderer.getDisplayedFeatureTypes();
+       }
+     }
+     if (visibleFeatures.isEmpty() && complementFeatures.isEmpty())
+     {
+       return theMap;
+     }
+   
+     AlignmentI alignment = viewPanel.getAlignment();
+     SequenceI[][] seqs = getSequence();
+ 
+     for (int pdbfnum = 0; pdbfnum < files.length; pdbfnum++)
+     {
+       String modelId = getModelIdForFile(files[pdbfnum]);
+       StructureMapping[] mapping = ssm.getMapping(files[pdbfnum]);
+   
+       if (mapping == null || mapping.length < 1)
+       {
+         continue;
+       }
+   
+       for (int seqNo = 0; seqNo < seqs[pdbfnum].length; seqNo++)
+       {
+         for (int m = 0; m < mapping.length; m++)
+         {
+           final SequenceI seq = seqs[pdbfnum][seqNo];
+           int sp = alignment.findIndex(seq);
+           StructureMapping structureMapping = mapping[m];
+           if (structureMapping.getSequence() == seq && sp > -1)
+           {
+             /*
+              * found a sequence with a mapping to a structure;
+              * now scan its features
+              */
+             if (!visibleFeatures.isEmpty())
+             {
+               scanSequenceFeatures(visibleFeatures, structureMapping, seq,
+                       theMap, modelId);
+             }
+             if (showLinkedFeatures)
+             {
+               scanComplementFeatures(complementRenderer, structureMapping,
+                       seq, theMap, modelId);
+             }
+           }
+         }
+       }
+     }
+     return theMap;
+   }
+ 
+   /**
+    * Ask the structure viewer to open a session file. Returns true if
+    * successful, else false (or not supported).
+    * 
+    * @param filepath
+    * @return
+    */
+   public boolean openSession(String filepath)
+   {
+     StructureCommandI cmd = getCommandGenerator().openSession(filepath);
+     if (cmd == null)
+     {
+       return false;
+     }
+     executeCommand(cmd, true);
+     // todo: test for failure - how?
+     return true;
+   }
+ 
+   /**
+    * Scans visible features in mapped positions of the CDS/peptide complement, and
+    * adds any found to the map of attribute values/structure positions
+    * 
+    * @param complementRenderer
+    * @param structureMapping
+    * @param seq
+    * @param theMap
+    * @param modelNumber
+    */
+   protected static void scanComplementFeatures(
+           FeatureRenderer complementRenderer,
+           StructureMapping structureMapping, SequenceI seq,
+           Map<String, Map<Object, AtomSpecModel>> theMap,
+           String modelNumber)
+   {
+     /*
+      * for each sequence residue mapped to a structure position...
+      */
+     for (int seqPos : structureMapping.getMapping().keySet())
+     {
+       /*
+        * find visible complementary features at mapped position(s)
+        */
+       MappedFeatures mf = complementRenderer
+               .findComplementFeaturesAtResidue(seq, seqPos);
+       if (mf != null)
+       {
+         for (SequenceFeature sf : mf.features)
+         {
+           String type = sf.getType();
+   
+           /*
+            * Don't copy features which originated from Chimera
+            */
+           if (JalviewChimeraBinding.CHIMERA_FEATURE_GROUP
+                   .equals(sf.getFeatureGroup()))
+           {
+             continue;
+           }
+   
+           /*
+            * record feature 'value' (score/description/type) as at the
+            * corresponding structure position
+            */
+           List<int[]> mappedRanges = structureMapping
+                   .getPDBResNumRanges(seqPos, seqPos);
+   
+           if (!mappedRanges.isEmpty())
+           {
+             String value = sf.getDescription();
+             if (value == null || value.length() == 0)
+             {
+               value = type;
+             }
+             float score = sf.getScore();
+             if (score != 0f && !Float.isNaN(score))
+             {
+               value = Float.toString(score);
+             }
+             Map<Object, AtomSpecModel> featureValues = theMap.get(type);
+             if (featureValues == null)
+             {
+               featureValues = new HashMap<>();
+               theMap.put(type, featureValues);
+             }
+             for (int[] range : mappedRanges)
+             {
+               addAtomSpecRange(featureValues, value, modelNumber, range[0],
+                       range[1], structureMapping.getChain());
+             }
+           }
+         }
+       }
+     }
+   }
+ 
+   /**
+    * Inspect features on the sequence; for each feature that is visible,
+    * determine its mapped ranges in the structure (if any) according to the
+    * given mapping, and add them to the map.
+    * 
+    * @param visibleFeatures
+    * @param mapping
+    * @param seq
+    * @param theMap
+    * @param modelId
+    */
+   protected static void scanSequenceFeatures(List<String> visibleFeatures,
+           StructureMapping mapping, SequenceI seq,
+           Map<String, Map<Object, AtomSpecModel>> theMap, String modelId)
+   {
+     List<SequenceFeature> sfs = seq.getFeatures().getPositionalFeatures(
+             visibleFeatures.toArray(new String[visibleFeatures.size()]));
+     for (SequenceFeature sf : sfs)
+     {
+       String type = sf.getType();
+   
+       /*
+        * Don't copy features which originated from Chimera
+        */
+       if (JalviewChimeraBinding.CHIMERA_FEATURE_GROUP
+               .equals(sf.getFeatureGroup()))
+       {
+         continue;
+       }
+   
+       List<int[]> mappedRanges = mapping.getPDBResNumRanges(sf.getBegin(),
+               sf.getEnd());
+   
+       if (!mappedRanges.isEmpty())
+       {
+         String value = sf.getDescription();
+         if (value == null || value.length() == 0)
+         {
+           value = type;
+         }
+         float score = sf.getScore();
+         if (score != 0f && !Float.isNaN(score))
+         {
+           value = Float.toString(score);
+         }
+         Map<Object, AtomSpecModel> featureValues = theMap.get(type);
+         if (featureValues == null)
+         {
+           featureValues = new HashMap<>();
+           theMap.put(type, featureValues);
+         }
+         for (int[] range : mappedRanges)
+         {
+           addAtomSpecRange(featureValues, value, modelId, range[0],
+                   range[1], mapping.getChain());
+         }
+       }
+     }
+   }
+ 
+   /**
+    * Returns the number of structure files in the structure viewer and mapped to
+    * Jalview. This may be zero if the files are still in the process of loading
+    * in the viewer.
+    * 
+    * @return
+    */
+   public int getMappedStructureCount()
+   {
+     String[] files = getStructureFiles();
+     return files == null ? 0 : files.length;
    }
  }
diff --cc test/jalview/ext/jmol/JmolCommandsTest.java
index 35fd179,21f7e19..c0bb629
--- a/test/jalview/ext/jmol/JmolCommandsTest.java
+++ b/test/jalview/ext/jmol/JmolCommandsTest.java
@@@ -44,104 -47,15 +47,18 @@@ import jalview.structure.StructureSelec
  
  public class JmolCommandsTest
  {
-   private SequenceRenderer sr;
- 
-   private String[] files;
- 
-   private AAStructureBindingModel mockBinding = new AAStructureBindingModel(
-           null, null)
-   {
-     @Override
-     public void releaseReferences(Object svl)
-     {
-     }
- 
-     @Override
-     public void highlightAtoms(List<AtomSpec> atoms)
-     {
-     }
- 
-     @Override
-     public void setJalviewColourScheme(ColourSchemeI cs)
-     {
-     }
- 
-     @Override
-     public String superposeStructures(AlignmentI[] alignments,
-             int[] structureIndices, HiddenColumns[] hiddenCols)
-     {
-       return null;
-     }
- 
-     @Override
-     public void setBackgroundColour(Color col)
-     {
-     }
- 
-     @Override
-     public jalview.api.SequenceRenderer getSequenceRenderer(
-             AlignmentViewPanel alignment)
-     {
-       return sr;
-     }
- 
-     @Override
-     protected void colourBySequence(AlignmentViewPanel avp)
-     {
-     }
- 
-     @Override
-     public void colourByChain()
-     {
-     }
- 
-     @Override
-     public void colourByCharge()
-     {
-     }
- 
-     @Override
-     public FeatureRenderer getFeatureRenderer(AlignmentViewPanel alignment)
-     {
-       return null;
-     }
- 
-     @Override
-     public String[] getStructureFiles()
-     {
-       return files;
-     }
- 
-     @Override
-     public String getModelSpec(int model)
-     {
-       return "#" + String.valueOf(model);
-     }
-   };
+   private JmolCommands testee;
  
    @BeforeClass(alwaysRun = true)
-   public void setUpJvOptionPane()
+   public void setUp()
    {
-     JvOptionPane.setInteractiveMode(false);
-     JvOptionPane.setMockResponse(JvOptionPane.CANCEL_OPTION);
+     testee = new JmolCommands();
    }
  
 -  @Test(groups = { "Functional" })
 +  /**
 +   * Test for the now deprecated version of getColourBySequenceCommand
 +   */
 +  @Test(groups = { "Functional" }, enabled = false)
    public void testGetColourBySequenceCommands_hiddenColumns()
    {
      /*
@@@ -179,16 -93,16 +96,15 @@@
              "B", map, null);
      ssm.addStructureMapping(sm2);
  
-     StructureMappingcommandSet[] commands = JmolCommands
-             .getColourBySequenceCommand(ssm, files, seqs, sr,
-                     af.alignPanel);
 -    String[] commands = testee.colourBySequence(ssm,
 -            files,
 -            seqs, sr, af.alignPanel);
++    String[] commands = testee.colourBySequence(ssm, files, seqs,
++            sr,
++            af.alignPanel);
      assertEquals(commands.length, 2);
-     assertEquals(commands[0].commands.length, 1);
  
-     String chainACommand = commands[0].commands[0];
+     String chainACommand = commands[0];
      // M colour is #82827d == (130, 130, 125) (see strand.html help page)
      assertTrue(
 -            chainACommand.contains("select 21:A/1.1;color[130,130,125]")); // first
 -                                                                           // one
 +            chainACommand.contains("select 21:A/1.1;color[130,130,125]")); // first one
      // H colour is #60609f == (96, 96, 159)
      assertTrue(chainACommand.contains(";select 22:A/1.1;color[96,96,159]"));
      // hidden columns are Gray (128, 128, 128)
@@@ -216,54 -130,83 +132,108 @@@
    public void testGetAtomSpec()
    {
      AtomSpecModel model = new AtomSpecModel();
-     assertEquals(JmolCommands.getAtomSpec(model), "");
+     assertEquals(testee.getAtomSpec(model, false), "");
 -    model.addRange("1", 2, 4, "A");
 -    assertEquals(testee.getAtomSpec(model, false), "2-4:A/1.1");
 -    model.addRange("1", 8, 8, "A");
 -    assertEquals(testee.getAtomSpec(model, false), "2-4:A/1.1|8:A/1.1");
 -    model.addRange("1", 5, 7, "B");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-4:A/1.1|8:A/1.1|5-7:B/1.1");
 -    model.addRange("1", 3, 5, "A");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-7:B/1.1");
 -    model.addRange("2", 1, 4, "B");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-7:B/1.1|1-4:B/2.1");
 -    model.addRange("2", 5, 9, "C");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-7:B/1.1|1-4:B/2.1|5-9:C/2.1");
 -    model.addRange("1", 8, 10, "B");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-10:B/1.1|1-4:B/2.1|5-9:C/2.1");
 -    model.addRange("1", 8, 9, "B");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-10:B/1.1|1-4:B/2.1|5-9:C/2.1");
 -    model.addRange("2", 3, 10, "C"); // subsumes 5-9
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-10:B/1.1|1-4:B/2.1|3-10:C/2.1");
 -    model.addRange("5", 25, 35, " ");
 -    assertEquals(testee.getAtomSpec(model, false),
 -            "2-5:A/1.1|8:A/1.1|5-10:B/1.1|1-4:B/2.1|3-10:C/2.1|25-35:/5.1");
  
 +    /*
 +     * Jalview numbers models from 0, Jmol from 1
 +     */
-     model.addRange(1, 2, 4, "A");
-     assertEquals(JmolCommands.getAtomSpec(model), "2-4:A/2.1");
++    model.addRange("2", 2, 4, "A");
++    assertEquals(testee.getAtomSpec(model, false), "2-4:A/2.1");
 +
-     model.addRange(1, 8, 8, "A");
-     assertEquals(JmolCommands.getAtomSpec(model), "(2-4,8)&:A/2.1");
++    model.addRange("2", 8, 8, "A");
++    assertEquals(testee.getAtomSpec(model, false), "(2-4,8)&:A/2.1");
 +
-     model.addRange(1, 5, 7, "B");
-     assertEquals(JmolCommands.getAtomSpec(model),
++    model.addRange("2", 5, 7, "B");
++    assertEquals(testee.getAtomSpec(model,
++            false),
 +            "(2-4,8)&:A/2.1,5-7:B/2.1");
 +
-     model.addRange(1, 3, 5, "A");
++    model.addRange("2", 3, 5, "A");
 +    // 3-5 merges with 2-4 to make 2-5:
-     assertEquals(JmolCommands.getAtomSpec(model),
++    assertEquals(testee.getAtomSpec(model,
++            false),
 +            "(2-5,8)&:A/2.1,5-7:B/2.1");
 +
-     model.addRange(0, 1, 4, "B");
-     assertEquals(JmolCommands.getAtomSpec(model),
++    model.addRange("1", 1, 4, "B");
++    assertEquals(testee.getAtomSpec(model,
++            false),
 +            "1-4:B/1.1,(2-5,8)&:A/2.1,5-7:B/2.1");
 +
-     model.addRange(0, 5, 9, "C");
-     assertEquals(JmolCommands.getAtomSpec(model),
++    model.addRange("1", 5, 9, "C");
++    assertEquals(testee.getAtomSpec(model,
++            false),
 +            "1-4:B/1.1,5-9:C/1.1,(2-5,8)&:A/2.1,5-7:B/2.1");
 +
-     model.addRange(1, 8, 10, "B");
++    model.addRange("2", 8, 10, "B");
 +    // 8-10 extends 5-7 to make 5-10
-     // note code generates (5-10)&:B which is equivalent to 5-10:B
-     assertEquals(JmolCommands.getAtomSpec(model),
-             "1-4:B/1.1,5-9:C/1.1,(2-5,8)&:A/2.1,(5-10)&:B/2.1");
++    assertEquals(testee.getAtomSpec(model,
++            false),
++            "1-4:B/1.1,5-9:C/1.1,(2-5,8)&:A/2.1,5-10:B/2.1");
 +
-     model.addRange(1, 8, 9, "B");
++    model.addRange("2", 8, 9, "B");
 +    // subsumed by 5-10 so makes no difference
-     assertEquals(JmolCommands.getAtomSpec(model),
-             "1-4:B/1.1,5-9:C/1.1,(2-5,8)&:A/2.1,(5-10)&:B/2.1");
++    assertEquals(testee.getAtomSpec(model,
++            false),
++            "1-4:B/1.1,5-9:C/1.1,(2-5,8)&:A/2.1,5-10:B/2.1");
 +
-     model.addRange(0, 3, 10, "C");
++    model.addRange("1", 3, 10, "C");
 +    // subsumes 5-9 so replaces it
-     assertEquals(JmolCommands.getAtomSpec(model),
-             "1-4:B/1.1,(3-10)&:C/1.1,(2-5,8)&:A/2.1,(5-10)&:B/2.1");
++    assertEquals(testee.getAtomSpec(model,
++            false),
++            "1-4:B/1.1,3-10:C/1.1,(2-5,8)&:A/2.1,5-10:B/2.1");
 +
 +    // empty chain code - e.g. from homology modelling
-     model.addRange(5, 25, 35, " ");
-     assertEquals(JmolCommands.getAtomSpec(model),
-             "1-4:B/1.1,(3-10)&:C/1.1,(2-5,8)&:A/2.1,(5-10)&:B/2.1,25-35:/6.1");
++    model.addRange("6", 25, 35, " ");
++    assertEquals(testee.getAtomSpec(model,
++            false),
++            "1-4:B/1.1,3-10:C/1.1,(2-5,8)&:A/2.1,5-10:B/2.1,25-35:/6.1");
+   }
+ 
+   @Test(groups = { "Functional" })
+   public void testColourBySequence()
+   {
+     Map<Object, AtomSpecModel> map = new LinkedHashMap<>();
+     JmolCommands.addAtomSpecRange(map, Color.blue, "1", 2, 5, "A");
+     JmolCommands.addAtomSpecRange(map, Color.blue, "1", 7, 7, "B");
+     JmolCommands.addAtomSpecRange(map, Color.blue, "1", 9, 23, "A");
+     JmolCommands.addAtomSpecRange(map, Color.blue, "2", 1, 1, "A");
+     JmolCommands.addAtomSpecRange(map, Color.blue, "2", 4, 7, "B");
+     JmolCommands.addAtomSpecRange(map, Color.yellow, "2", 8, 8, "A");
+     JmolCommands.addAtomSpecRange(map, Color.yellow, "2", 3, 5, "A");
+     JmolCommands.addAtomSpecRange(map, Color.red, "1", 3, 5, "A");
+     JmolCommands.addAtomSpecRange(map, Color.red, "1", 6, 9, "A");
+ 
+     // Colours should appear in the Jmol command in the order in which
+     // they were added; within colour, by model, by chain, ranges in start order
+     List<StructureCommandI> commands = testee.colourBySequence(map);
+     assertEquals(commands.size(), 1);
 -    String expected1 = "select 2-5:A/1.1|9-23:A/1.1|7:B/1.1|1:A/2.1|4-7:B/2.1;color[0,0,255]";
 -    String expected2 = "select 3-5:A/2.1|8:A/2.1;color[255,255,0]";
++    String expected1 = "select (2-5,9-23)&:A/1.1,7:B/1.1,1:A/2.1,4-7:B/2.1;color[0,0,255]";
++    String expected2 = "select (3-5,8)&:A/2.1;color[255,255,0]";
+     String expected3 = "select 3-9:A/1.1;color[255,0,0]";
+     assertEquals(commands.get(0).getCommand(),
+             expected1 + ";" + expected2 + ";" + expected3);
+   }
+ 
+   @Test(groups = { "Functional" })
+   public void testSuperposeStructures()
+   {
+     AtomSpecModel ref = new AtomSpecModel();
+     ref.addRange("1", 12, 14, "A");
+     ref.addRange("1", 18, 18, "B");
+     ref.addRange("1", 22, 23, "B");
+     AtomSpecModel toAlign = new AtomSpecModel();
+     toAlign.addRange("2", 15, 17, "B");
+     toAlign.addRange("2", 20, 21, "B");
+     toAlign.addRange("2", 22, 22, "C");
+     List<StructureCommandI> command = testee.superposeStructures(ref,
+             toAlign);
+     assertEquals(command.size(), 1);
 -    String refSpec = "12-14:A/1.1|18:B/1.1|22-23:B/1.1";
 -    String toAlignSpec = "15-17:B/2.1|20-21:B/2.1|22:C/2.1";
++    String refSpec = "12-14:A/1.1,(18,22-23)&:B/1.1";
++    String toAlignSpec = "(15-17,20-21)&:B/2.1,22:C/2.1";
+     String expected = String.format(
+             "compare {2.1} {1.1} SUBSET {(*.CA | *.P) and conformation=1} ATOMS {%s}{%s} ROTATE TRANSLATE ;select %s|%s;cartoons",
+             toAlignSpec, refSpec, toAlignSpec, refSpec);
+     assertEquals(command.get(0).getCommand(), expected);
    }
  
    @Test(groups = "Functional")
diff --cc test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
index d5229c2,b040687..d8eb7bc
--- a/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
+++ b/test/jalview/ext/rbvi/chimera/ChimeraCommandsTest.java
@@@ -138,27 -46,26 +46,26 @@@ public class ChimeraCommandsTes
    }
  
    @Test(groups = { "Functional" })
-   public void testBuildColourCommands()
+   public void testColourBySequence()
    {
 -
      Map<Object, AtomSpecModel> map = new LinkedHashMap<>();
-     ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 2, 5, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 7, 7, "B");
-     ChimeraCommands.addAtomSpecRange(map, Color.blue, 0, 9, 23, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.blue, 1, 1, 1, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.blue, 1, 4, 7, "B");
-     ChimeraCommands.addAtomSpecRange(map, Color.yellow, 1, 8, 8, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.yellow, 1, 3, 5, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.red, 0, 3, 5, "A");
-     ChimeraCommands.addAtomSpecRange(map, Color.red, 0, 6, 9, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.blue, "0", 2, 5, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.blue, "0", 7, 7, "B");
+     ChimeraCommands.addAtomSpecRange(map, Color.blue, "0", 9, 23, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.blue, "1", 1, 1, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.blue, "1", 4, 7, "B");
+     ChimeraCommands.addAtomSpecRange(map, Color.yellow, "1", 8, 8, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.yellow, "1", 3, 5, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.red, "0", 3, 5, "A");
+     ChimeraCommands.addAtomSpecRange(map, Color.red, "0", 6, 9, "A");
  
      // Colours should appear in the Chimera command in the order in which
      // they were added; within colour, by model, by chain, ranges in start order
 +    // all prefixed with #808080 to colour hidden regions (if shown) gray
-     String command = ChimeraCommands.buildColourCommands(map, mockBinding)
-             .get(0);
-     assertEquals(
-             command,
-             "color #808080; color #0000ff #0:2-5.A,9-23.A,7.B|#1:1.A,4-7.B; color #ffff00 #1:3-5.A,8.A; color #ff0000 #0:3-9.A");
+     List<StructureCommandI> commands = testee.colourBySequence(map);
+     assertEquals(commands.size(), 1);
+     assertEquals(commands.get(0).getCommand(),
 -            "color #0000ff #0:2-5.A,9-23.A,7.B|#1:1.A,4-7.B;color #ffff00 #1:3-5.A,8.A;color #ff0000 #0:3-9.A");
++            "color #808080; color #0000ff #0:2-5.A,9-23.A,7.B|#1:1.A,4-7.B;color #ffff00 #1:3-5.A,8.A;color #ff0000 #0:3-9.A");
    }
  
    @Test(groups = { "Functional" })
@@@ -312,55 -214,137 +214,136 @@@
    }
  
    @Test(groups = "Functional")
-   public void testGetAtomSpec()
+   public void testGetAtomSpec_alphaOnly()
    {
      AtomSpecModel model = new AtomSpecModel();
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding), "");
+     assertEquals(testee.getAtomSpec(model, true), "");
+     model.addRange("1", 2, 4, "A");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#1:2-4.A@CA&~@.B-Z&~@.2-9");
+     model.addRange("1", 8, 8, "A");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#1:2-4.A,8.A@CA&~@.B-Z&~@.2-9");
+     model.addRange("1", 5, 7, "B");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#1:2-4.A,8.A,5-7.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("1", 3, 5, "A");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#1:2-5.A,8.A,5-7.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("0", 1, 4, "B");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-7.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("0", 5, 9, "C");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B,5-9.C@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-7.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("1", 8, 10, "B");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B,5-9.C@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-10.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("1", 8, 9, "B");
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B,5-9.C@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-10.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("0", 3, 10, "C"); // subsumes 5-9
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B,3-10.C@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-10.B@CA&~@.B-Z&~@.2-9");
+     model.addRange("5", 25, 35, " "); // empty chain code
+     assertEquals(testee.getAtomSpec(model, true),
+             "#0:1-4.B,3-10.C@CA&~@.B-Z&~@.2-9|#1:2-5.A,8.A,5-10.B@CA&~@.B-Z&~@.2-9|#5:25-35.@CA&~@.B-Z&~@.2-9");
 -
+   }
+ 
+   @Test(groups = "Functional")
+   public void testGetModelStartNo()
+   {
+     assertEquals(testee.getModelStartNo(), 0);
+   }
+ 
+   @Test(groups = "Functional")
+   public void testGetResidueSpec()
+   {
+     assertEquals(testee.getResidueSpec("ALA"), "::ALA");
+   }
+ 
+   @Test(groups = "Functional")
+   public void testShowBackbone()
+   {
+     List<StructureCommandI> cmds = testee.showBackbone();
+     assertEquals(cmds.size(), 1);
+     assertEquals(cmds.get(0).getCommand(),
+             "~display all;~ribbon;chain @CA|P");
+   }
  
-     model.addRange(1, 2, 4, "A");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#1:2-4.A");
+   @Test(groups = "Functional")
+   public void testOpenCommandFile()
+   {
+     assertEquals(testee.openCommandFile("nowhere").getCommand(),
+             "open cmd:nowhere");
+   }
  
-     model.addRange(1, 8, 8, "A");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#1:2-4.A,8.A");
+   @Test(groups = "Functional")
+   public void testSaveSession()
+   {
+     assertEquals(testee.saveSession("somewhere").getCommand(),
+             "save somewhere");
+   }
  
-     model.addRange(1, 5, 7, "B");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#1:2-4.A,8.A,5-7.B");
+   @Test(groups = "Functional")
+   public void testColourByChain()
+   {
+     assertEquals(testee.colourByChain().getCommand(), "rainbow chain");
+   }
  
-     model.addRange(1, 3, 5, "A");
-     // 3-5 combines with 2-4 to make 2-5:
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#1:2-5.A,8.A,5-7.B");
+   @Test(groups = { "Functional" })
+   public void testSetBackgroundColour()
+   {
+     StructureCommandI cmd = testee.setBackgroundColour(Color.PINK);
+     assertEquals(cmd.getCommand(), "set bgColor #ffafaf");
+   }
  
-     model.addRange(0, 1, 4, "B");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B|#1:2-5.A,8.A,5-7.B");
+   @Test(groups = { "Functional" })
+   public void testLoadFile()
+   {
+     StructureCommandI cmd = testee.loadFile("/some/filepath");
+     assertEquals(cmd.getCommand(), "open /some/filepath");
+   }
  
-     model.addRange(0, 5, 9, "C");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B,5-9.C|#1:2-5.A,8.A,5-7.B");
+   @Test(groups = { "Functional" })
+   public void testOpenSession()
+   {
+     StructureCommandI cmd = testee.openSession("/some/filepath");
+     assertEquals(cmd.getCommand(), "open chimera:/some/filepath");
+   }
  
-     model.addRange(1, 8, 10, "B");
-     // 8-10 extends 5-7 to make 5-10
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B,5-9.C|#1:2-5.A,8.A,5-10.B");
+   @Test(groups = "Functional")
+   public void testColourByCharge()
+   {
+     List<StructureCommandI> cmds = testee.colourByCharge();
+     assertEquals(cmds.size(), 1);
+     assertEquals(cmds.get(0)
+             .getCommand(),
+             "color white;color red ::ASP,GLU;color blue ::LYS,ARG;color yellow ::CYS");
+   }
  
-     model.addRange(1, 8, 9, "B");
-     // subsumed range makes no difference
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B,5-9.C|#1:2-5.A,8.A,5-10.B");
+   @Test(groups = "Functional")
+   public void testGetColourCommand()
+   {
+     assertEquals(testee.colourResidues("something", Color.MAGENTA)
+             .getCommand(),
+             "color #ff00ff something");
+   }
  
-     model.addRange(0, 3, 10, "C");
-     // subsumes 5-9 so replaces it
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B,3-10.C|#1:2-5.A,8.A,5-10.B");
+   @Test(groups = "Functional")
+   public void testFocusView()
+   {
+     assertEquals(testee.focusView().getCommand(), "focus");
+   }
  
-     // empty chain code - e.g. from homology modelling
-     model.addRange(5, 25, 35, " ");
-     assertEquals(ChimeraCommands.getAtomSpec(model, mockBinding),
-             "#0:1-4.B,3-10.C|#1:2-5.A,8.A,5-10.B|#5:25-35.");
-   
+   @Test(groups = "Functional")
+   public void testSetAttribute()
+   {
+     AtomSpecModel model = new AtomSpecModel();
+     model.addRange("1", 89, 92, "A");
+     model.addRange("2", 12, 20, "B");
+     model.addRange("2", 8, 9, "B");
+     assertEquals(testee.setAttribute("jv_kd", "27.3", model).getCommand(),
+             "setattr res jv_kd '27.3' #1:89-92.A|#2:8-9.B,12-20.B");
    }
  }
diff --cc test/jalview/ext/rbvi/chimera/JalviewChimeraBindingTest.java
index 2604243,0000000..fde4bb5
mode 100644,000000..100644
--- a/test/jalview/ext/rbvi/chimera/JalviewChimeraBindingTest.java
+++ b/test/jalview/ext/rbvi/chimera/JalviewChimeraBindingTest.java
@@@ -1,164 -1,0 +1,166 @@@
 +package jalview.ext.rbvi.chimera;
 +
 +import static org.testng.Assert.assertEquals;
 +import static org.testng.Assert.assertNotNull;
 +
- import jalview.datamodel.PDBEntry;
- import jalview.datamodel.SequenceI;
- import jalview.gui.AlignFrame;
- import jalview.gui.AlignViewport;
- import jalview.gui.ChimeraViewFrame;
- import jalview.gui.JalviewChimeraBindingModel;
- import jalview.io.DataSourceType;
- import jalview.io.FileLoader;
- import jalview.structure.StructureMapping;
- import jalview.structure.StructureSelectionManager;
- 
 +import java.util.Arrays;
 +import java.util.HashMap;
 +import java.util.List;
 +import java.util.Map;
 +
 +import org.testng.annotations.BeforeTest;
 +import org.testng.annotations.Test;
 +
 +import ext.edu.ucsf.rbvi.strucviz2.ChimeraModel;
 +import ext.edu.ucsf.rbvi.strucviz2.StructureManager.ModelType;
++import jalview.datamodel.PDBEntry;
++import jalview.datamodel.SequenceI;
++import jalview.gui.AlignFrame;
++import jalview.gui.AlignViewport;
++import jalview.gui.ChimeraViewFrame;
++import jalview.gui.JalviewChimeraBindingModel;
++import jalview.io.DataSourceType;
++import jalview.io.FileLoader;
++import jalview.structure.StructureCommandI;
++import jalview.structure.StructureMapping;
++import jalview.structure.StructureSelectionManager;
 +import junit.extensions.PA;
 +
 +public class JalviewChimeraBindingTest
 +{
 +  private AlignFrame af;
 +
 +  @BeforeTest(alwaysRun = true)
 +  public void setup()
 +  {
 +    af = new FileLoader().LoadFileWaitTillLoaded("examples/uniref50.fa",
 +            DataSourceType.FILE);
 +  }
 +
 +  @Test(groups = "Functional")
 +  public void testBuildShowStructuresCommand()
 +  {
 +    AlignViewport av = af.getViewport();
 +    PDBEntry[] pdbs = new PDBEntry[] {};
 +    StructureSelectionManager ssm = new StructureSelectionManager();
 +    SequenceI seq1 = av.getAlignment().findSequenceMatch("FER1_SPIOL")[0];
 +    assertNotNull(seq1);
 +    SequenceI seq2 = av.getAlignment().findSequenceMatch("FER2_ARATH")[0];
 +    assertNotNull(seq2);
 +    SequenceI[][] seqs = new SequenceI[][] { { seq1 }, { seq2 } };
 +    JalviewChimeraBindingModel testee = new JalviewChimeraBindingModel(
 +            new ChimeraViewFrame(),
 +            ssm, pdbs, seqs, null);
 +
 +    /*
 +     * with no structures mapped
 +     */
-     String cmd = testee.buildShowStructuresCommand(av, true);
-     assertEquals(cmd, "~display; ribbon; focus");
++    StructureCommandI cmd = testee.buildShowStructuresCommand(av, true);
++    assertEquals(cmd.getCommand(), "~display; ribbon; focus");
 +    cmd = testee.buildShowStructuresCommand(av, false);
-     assertEquals(cmd, "~display; ribbon");
++    assertEquals(cmd.getCommand(), "~display; ribbon");
 +
 +    /*
 +     * stub out a structure with chains A and B
 +     */
 +    Map<String, List<ChimeraModel>> chimeraMaps = (Map<String, List<ChimeraModel>>) PA
 +            .getValue(testee, "chimeraMaps");
 +    ChimeraModel model0 = new ChimeraModel("1A70", ModelType.PDB_MODEL, 0,
 +            0);
 +    chimeraMaps.put("1a70.pdb", Arrays.asList(model0));
 +    ChimeraModel model1 = new ChimeraModel("4ZHO", ModelType.PDB_MODEL, 1,
 +            0);
 +    chimeraMaps.put("4zho.pdb", Arrays.asList(model1));
 +
 +    Map<String, String> chainFiles = (Map<String, String>) PA
 +            .getValue(testee, "chainFile");
 +    chainFiles.put("1A70:A", "1a70.pdb");
 +    chainFiles.put("1A70:B", "1a70.pdb");
 +    chainFiles.put("4ZHO:B", "4zho.pdb");
 +    chainFiles.put("4ZHO:C", "4zho.pdb");
 +
 +    /*
 +     * map FER1_SPIOL residues 51-100 to residues 1-50 (atoms 1-250) in 1A70
 +     * and residues 110-147 to structure residues 60-97
 +     * (in fact there is no gap, added here for test purposes)
 +     */
 +    HashMap<Integer, int[]> map = new HashMap<>();
 +    for (int pos = 51; pos <= 100; pos++)
 +    {
 +      map.put(pos, new int[] { pos - 50, 5 * (pos - 50) });
 +    }
 +    for (int pos = 110; pos <= 147; pos++)
 +    {
 +      map.put(pos, new int[] { pos - 50, 5 * (pos - 50) });
 +    }
 +    StructureMapping sm1 = new StructureMapping(seq1, "1a70.pdb", "1A70",
 +            "A", map, null);
 +    ssm.addStructureMapping(sm1);
 +
 +    /*
 +     * map FER2_ARATH residues 53-148 to residues 2-97 in 4ZHO
 +     */
 +    map = new HashMap<>();
 +    for (int pos = 53; pos <= 148; pos++)
 +    {
 +      map.put(pos, new int[] { pos - 51, 5 * (pos - 51) });
 +    }
 +    StructureMapping sm2 = new StructureMapping(seq2, "4zho.pdb", "4ZHO",
 +            "B", map, null);
 +    ssm.addStructureMapping(sm2);
 +
 +    /*
 +     * select chain A only (hide chain B)
 +     */
 +    List<String> chainsToHide = (List<String>) PA.getValue(testee, "chainsToHide");
 +    chainsToHide.add("1A70:B");
 +    chainsToHide.add("4ZHO:C");
 +    cmd = testee.buildShowStructuresCommand(av, false);
-     assertEquals(cmd, "~display; ribbon; ~ribbon #0:.B; ~ribbon #1:.C");
++    assertEquals(cmd.getCommand(),
++            "~display; ribbon; ~ribbon #0:.B; ~ribbon #1:.C");
 +
 +    /*
 +     * show alignment only, no chains hidden
 +     */
 +    chainsToHide.clear();
 +    testee.setShowAlignmentOnly(true);
 +    cmd = testee.buildShowStructuresCommand(av, false);
-     assertEquals(cmd,
++    assertEquals(cmd
++            .getCommand(),
 +            "~display; ~ribbon; ribbon #0:1-50.A,60-97.A|#1:2-97.B");
 +
 +    /*
 +     * now with a chain hidden
 +     */
 +    chainsToHide.add("4ZHO:C");
 +    cmd = testee.buildShowStructuresCommand(av, false);
 +    String expected = "~display; ~ribbon; ribbon #0:1-50.A,60-97.A|#1:2-97.B; ~ribbon #1:.C";
-     assertEquals(cmd, expected);
++    assertEquals(cmd.getCommand(), expected);
 +
 +    /*
 +     * hide columns in the mapped region - should not change the command (yet)
 +     */
 +    int fromCol = seq1.findIndex(60); // structure residue 10
 +    int toCol = seq1.findIndex(70); // structure residue 20
 +    av.hideColumns(fromCol - 1, toCol - 1);
 +    cmd = testee.buildShowStructuresCommand(av, false);
-     assertEquals(cmd, expected);
++    assertEquals(cmd.getCommand(), expected);
 +
 +    /*
 +     * select 'hide hidden columns'
 +     * command should now exclude these in both mapped sequences
 +     */
 +    testee.setHideHiddenRegions(true);
 +    cmd = testee.buildShowStructuresCommand(av, false);
 +    expected = "~display; ~ribbon; ribbon #0:1-9.A,21-50.A,60-97.A|#1:2-10.B,22-97.B; ~ribbon #1:.C";
-     assertEquals(cmd, expected);
++    assertEquals(cmd.getCommand(), expected);
 +
 +    /*
 +     * deselect 'show alignment only'
 +     * hide hidden columns is now ignored
 +     */
 +    testee.setShowAlignmentOnly(false);
 +    cmd = testee.buildShowStructuresCommand(av, false);
-     assertEquals(cmd, "~display; ribbon; ~ribbon #1:.C");
++    assertEquals(cmd.getCommand(), "~display; ribbon; ~ribbon #1:.C");
 +  }
 +}
diff --cc test/jalview/structures/models/AAStructureBindingModelTest.java
index 0bb1daf,c1ad03a..aea12b4
--- a/test/jalview/structures/models/AAStructureBindingModelTest.java
+++ b/test/jalview/structures/models/AAStructureBindingModelTest.java
@@@ -20,12 -20,24 +20,24 @@@
   */
  package jalview.structures.models;
  
+ import static org.testng.Assert.assertEquals;
  import static org.testng.Assert.assertFalse;
- import static org.testng.AssertJUnit.assertEquals;
+ import static org.testng.Assert.assertNotNull;
 -import static org.testng.Assert.assertTrue;
 +import static org.testng.AssertJUnit.assertTrue;
  
+ import java.awt.Color;
+ import java.io.IOException;
+ import java.util.Arrays;
+ import java.util.BitSet;
+ import java.util.HashMap;
+ import java.util.List;
+ import java.util.Map;
+ 
+ import org.testng.annotations.BeforeClass;
+ import org.testng.annotations.BeforeMethod;
+ import org.testng.annotations.Test;
+ 
  import jalview.api.AlignmentViewPanel;
- import jalview.api.FeatureRenderer;
  import jalview.api.SequenceRenderer;
  import jalview.datamodel.Alignment;
  import jalview.datamodel.AlignmentI;
@@@ -126,80 -135,61 +135,13 @@@ public class AAStructureBindingModelTes
      // ideally, we would match on the actual data for the 'File' handle for
      // pasted files,
      // see JAL-623 - pasting is still not correctly handled...
--    PDBEntry importedPDB = new PDBEntry("3A6S", "", Type.PDB,
--            "Paste");
--    AAStructureBindingModel binder = new AAStructureBindingModel(
--            new StructureSelectionManager(), new PDBEntry[]
++    PDBEntry importedPDB = new PDBEntry("3A6S", "", Type.PDB, "Paste");
++    AAStructureBindingModel binder = newBindingModel(new PDBEntry[]
              { importedPDB },
              new SequenceI[][]
--            { importedAl.getSequencesArray() }, null)
--    {
--      
--      @Override
--      public void updateColours(Object source)
--      {
--      }
--      
--      @Override
--      public void releaseReferences(Object svl)
--      {
--      }
--      
--      @Override
--      public String[] getStructureFiles()
--      {
--        return null;
--      }
--      
--      @Override
-       public String superposeStructures(AlignmentI[] alignments,
-               int[] structureIndices, HiddenColumns[] hiddenCols)
-       {
-         return null;
-       }
-       
-       @Override
-       public void setJalviewColourScheme(ColourSchemeI cs)
-       {
-       }
-       
-       @Override
-       public void setBackgroundColour(Color col)
-       {
-       }
-       
-       @Override
--      public void highlightAtoms(List<AtomSpec> atoms)
--      {
--      }
--      
--      @Override
--      public SequenceRenderer getSequenceRenderer(AlignmentViewPanel alignment)
--      {
--        return null;
--      }
-       
-       @Override
-       public FeatureRenderer getFeatureRenderer(AlignmentViewPanel alignment)
-       {
-         return null;
-       }
-       
-       @Override
-       protected void colourBySequence(AlignmentViewPanel avp)
-       {
-       }
-       
-       @Override
-       public void colourByCharge()
-       {
-       }
-       
-       @Override
-       public void colourByChain()
-       {
-       }
-     };
 -
 -      @Override
 -      protected List<String> executeCommand(StructureCommandI command,
 -              boolean getReply)
 -      {
 -        return null;
 -      }
 -
 -      @Override
 -      protected String getModelIdForFile(String chainId)
 -      {
 -        return "";
 -      }
++            { importedAl.getSequencesArray() },
++            new StructureSelectionManager(), null);
+ 
 -      @Override
 -      protected ViewerType getViewerType()
 -      {
 -        return null;
 -      }
 -    };
      String[][] chains = binder.getChains();
      assertFalse(chains == null || chains[0] == null,
              "No chains discovered by binding");
@@@ -268,27 -279,12 +231,17 @@@
        }
  
        @Override
-       public void setJalviewColourScheme(ColourSchemeI cs)
-       {
-       }
- 
-       @Override
-       public String superposeStructures(AlignmentI[] als, int[] alm,
-               HiddenColumns[] alc)
-       {
-         return null;
-       }
- 
-       @Override
 +      public void setBackgroundColour(Color col)
 +      {
 +      }
 +
 +      @Override
        public SequenceRenderer getSequenceRenderer(
-               AlignmentViewPanel alignment)
+               AlignmentViewPanel avp)
        {
-         return null;
+         return avp == null ? null
+                 : new jalview.gui.SequenceRenderer(
+                         avp.getAlignViewport());
        }
  
        @Override