JAL-4241 Create sequence mapping before adjusting annotations mmw/bug/JAL-4241-annotation-alignment-fix
authorMateusz Warowny <mmzwarowny@dundee.ac.uk>
Fri, 28 Jul 2023 10:59:16 +0000 (12:59 +0200)
committerMateusz Warowny <mmzwarowny@dundee.ac.uk>
Fri, 28 Jul 2023 10:59:25 +0000 (12:59 +0200)
src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java

index fee2b38..e004bf5 100644 (file)
@@ -702,33 +702,18 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
         {
           ala.graphGroup += graphGroup;
         }
-        SequenceI aseq = null;
-
-        /**
+        SequenceI aseq = (ala.sequenceRef == null) ? null
+                : running.getSeqNames().get(ala.sequenceRef.getName());
+        /*
          * transfer sequence refs and adjust gapmap
+         * Prepare an array for annotations and copy returned annotations to it
+         * according to gapMap if present.
          */
-        if (ala.sequenceRef != null)
-        {
-          SequenceI seq = running.getSeqNames()
-                  .get(ala.sequenceRef.getName());
-          aseq = seq;
-          // This part doesn't do anything. I suppose bad merge.
-          while (seq.getDatasetSequence() != null)
-          {
-            seq = seq.getDatasetSequence();
-          }
-        }
-        /* Prepare an array for annotations and copy returned annotations to it.
-         * Similar process happens in develop:JabawsCalcWorker#constructAnnotationFromScore.
-         * However, JabawsCalcWorker fills the array form the beginning, not
-         * the start position (start - column of the submitted region).
-         * Also, JabawsCalcWorker adds those annotations to an AlignmentAnnotation
-         * retrieved from the current alignment. */
         Annotation[] resAnnot = ala.annotations,
                 gappedAnnot = new Annotation[Math.max(
                         alignViewport.getAlignment().getWidth(),
                         gapMap.length)];
-        for (int p = 0, ap = start; ap < gappedAnnot.length; ap++)
+        for (int p = 0, ap = 0; ap < gappedAnnot.length; ap++)
         {
           if (gapMap != null && gapMap.length > ap && !gapMap[ap])
           {
@@ -739,24 +724,14 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker
             gappedAnnot[ap] = resAnnot[p++];
           }
         }
-        /* Annotations are set in-place on the AlignmentAnnotation object returned by the service.
-         * First, if #updateResultAnnotation is called again the output will be mangled
-         * Second, the mapping of positions to annotations held internally by
-         * the `ala` object is now incorrect. */
         ala.sequenceRef = aseq;
-        ala.annotations = gappedAnnot;
         AlignmentAnnotation newAnnot = getAlignViewport().getAlignment()
                 .updateFromOrCopyAnnotation(ala);
+        newAnnot.annotations = gappedAnnot;
         if (aseq != null)
         {
-
+          newAnnot.createSequenceMapping(aseq, aseq.findPosition(start), false);
           aseq.addAlignmentAnnotation(newAnnot);
-          /* adjustForAlignment is meant to move annotations to their columns
-           * according to the sequenceMapping.
-           * Since the mapping is carried from the alignment returned by the
-           * service it simply maps annotations to consecutive positions 1, 2, 3...
-           * Resulting in the annotation row being always aligned to the beginning
-           * of the alignment. */
           newAnnot.adjustForAlignment();
 
           AlignmentAnnotationUtils.replaceAnnotationOnAlignmentWith(