b8bcfccf5500cd9928c9e8e79eb18dca897dd8e0
galt
  Mon Feb 29 09:47:18 2016 -0800
Fixes #16906. Because composite tracks were flipping between different vis values in an unpredictable way, we have stopped trying to free spaceSavers with old vis values.

diff --git src/hg/hgTracks/simpleTracks.c src/hg/hgTracks/simpleTracks.c
index f8d6005..27e02c4 100644
--- src/hg/hgTracks/simpleTracks.c
+++ src/hg/hgTracks/simpleTracks.c
@@ -368,82 +368,72 @@
     tg->mapsSelf = TRUE;
     }
 }
 
 struct sameItemNode 
 /* sameItem node */
     {
     struct sameItemNode *next; /* next node */
     struct window *window;     /* in which window - can use to detect duplicate keys */
     void *item;
     struct spaceSaver *ss;
     bool done;                 /* have we (reversed the list and) processed it yet? */
     };
 
 
-static struct spaceSaver *findSpaceSaverAndFreeOldOnes(struct track *tg, enum trackVisibility vis)
-/* Find SpaceSaver in list, freeing older ones. Return spaceSaver found or NULL. */
+static struct spaceSaver *findSpaceSaver(struct track *tg, enum trackVisibility vis)
+/* Find SpaceSaver in list. Return spaceSaver found or NULL. */
 {
-boolean found = FALSE;
-struct spaceSaver *result = NULL;
 struct spaceSaver *ss = NULL;
-struct spaceSaver *newSs = NULL; // tg->ss is actually a list of spaceSavers with different viz. with newest on top
-while ((ss = slPopHead(&tg->ss)))  // we needed to keep the old ss around to trigger proper viz changes.
-    {
-    slAddHead(&newSs, ss);
-    if ((int)(ss->vis) == vis)
-	{
-	found = TRUE;
-	result = ss;
-	break;
-	}
-    }
-if (found)
+// tg->ss is actually a list of spaceSavers with different viz. with newest on top
+// We needed to keep the old ss around to trigger proper viz changes.
+// Sometimes vis changes because of limitedVis.
+// Sometimes the parent composite track's vis is being used to override subtrack vis.
+// Since it is not easy to be certain a vis will not be used again, we cannot free old spaceSavers.
+for(ss = tg->ss; ss; ss = ss->next)
     {
-    while ((ss = slPopHead(&tg->ss)))
+    if (ss->vis == vis)
 	{
-	spaceSaverFree(&ss); // free older ones
+	return ss;
 	}
     }
-slReverse(&newSs);
-tg->ss = newSs;
-return result;
+return NULL;
 }
 
 
 int packCountRowsOverflow(struct track *tg, int maxCount,
                           boolean withLabels, boolean allowOverflow, enum trackVisibility vis)
 /* Return packed height. */
 {
 
 // allowOverflow is currently ONLY used by xenoMrna and est tracks.
 //  When true,  the extra rows are kept, printed at the bottom in dense and Last Row: overlow count appears at bottom of leftLabel area.
 //  When false, the extra rows are tossed, the count seems to equal overflow limit + 2, and limitVisibility lowers vis and retries.
 
 // do not calculate if still loading all windows
 if (trackLoadingInProgress) // we pack after all windows are loaded.
     {
     // do not set ss yet
     return 0;  // height of 0 triggers unsetting limitedVis since our data is not all loaded yet and it will get set later.
     }
 
 // do not re-calculate if not needed
 if (tg->ss)
     {
     if (tg->ss->window != currentWindow)
 	errAbort("unexpected current window %lu, expected %lu", (unsigned long) currentWindow, (unsigned long) tg->ss->window);
-    struct spaceSaver *ss = findSpaceSaverAndFreeOldOnes(tg, vis);
+    struct spaceSaver *ss = findSpaceSaver(tg, vis);
     if (ss)
 	return ss->rowCount;
     // Falls thru here if a new visibility is needed, such as full changing to pack after limitVisibility.
     // This will usually be when it is the first window and it is requesting a new vis.
     }
 
 if (currentWindow != windows)  // if not first window
     {
     errAbort("unexpected current window %lu, expected first window %lu", (unsigned long) currentWindow, (unsigned long) windows);
     }
 
 // If we get here, currentWindow should be first window i.e. windows var.
 
 struct slList *item;
 MgFont *font = tl.font;
@@ -649,35 +639,31 @@
 	    if (spaceSaverAddOverflowMulti(ss, rangeList, nodeList, allowOverflow) == NULL)
 		break;
 
 	    }
 	}
     spaceSaverFinish(tg->ss);
     }
 // must assign at end to get final row count
 for(tg=tgSave; tg; tg=tg->nextWindow)
     {
     tg->ss->rowCount   = ss->rowCount;
     }
 tg = tgSave;
 spaceSaverFree(&ss);
 
-ss = findSpaceSaverAndFreeOldOnes(tg, vis);
-if (!ss)
-    errAbort("unexpected error findSpaceSaverAndFreeOldOnes returned NULL at end of pack function");
-
-return ss->rowCount;
+return tg->ss->rowCount;
 }
 
 int packCountRows(struct track *tg, int maxCount, boolean withLabels, enum trackVisibility vis)
 /* Return packed height. */
 {
 return packCountRowsOverflow(tg, maxCount, withLabels, FALSE, vis);
 }
 
 char *getItemDataName(struct track *tg, char *itemName)
 /* Translate an itemName to its itemDataName, using tg->itemDataName if is not
  * NULL. The resulting value should *not* be freed, and it should be assumed
  * that it will only remain valid until the next call of this function.*/
 {
 return (tg->itemDataName != NULL) ? tg->itemDataName(tg, itemName)
     : itemName;