af3cab9b22449e7db6b9367ee2416dc2b3d39a3e
tdreszer
  Fri Nov 5 12:26:52 2010 -0700
Fixed bug 32 in 1071. Repeated vis changes in track search had unpredictable consequences.
diff --git src/hg/lib/cart.c src/hg/lib/cart.c
index 8d6a7c6..49218e3 100644
--- src/hg/lib/cart.c
+++ src/hg/lib/cart.c
@@ -2170,31 +2170,31 @@
                     }
                 }
             WARN("cartVarsNoneFoundForTdb: %s=%s",cartVar->name,(char *)cartVar->val);
             slFreeList(&cartVars);
             return FALSE; // Any view cart vars means non-default so do not "shape" composite
             }
         }
     slFreeList(&cartVars);
     }
 return TRUE;
 }
 #endif///ndef COMPOSITE_VIS_SHAPING_PLAN_B
 
 #if defined(COMPOSITE_VIS_SHAPING_PLAN_A) || defined(COMPOSITE_VIS_SHAPING_PLAN_B)
 static int cartTdbParentShapeVis(struct cart *cart,struct trackDb *parent,char *view,struct hash *subVisHash,boolean reshapeFully)
-// This shapes one level of vis (view or container) based upon subtrack specific visibility.  Returns count of subtracks affected
+// This shapes one level of vis (view or container) based upon subtrack specific visibility.  Returns count of tracks affected
 {
 ASSERT(view || (tdbIsContainer(parent) && tdbIsContainerChild(parent->subtracks)));
 struct trackDb *subtrack = NULL;
 char setting[512];
 if (view != NULL)
     safef(setting,sizeof(setting),"%s.%s.vis",parent->parent->track,view);
 else
     safef(setting,sizeof(setting),"%s",parent->track);
 
 enum trackVisibility visMax  = tvHide;
 enum trackVisibility visOrig = tdbVisLimitedByAncestry(cart, parent, FALSE);
 
 // Should walk through children to get max new vis for this parent
 for(subtrack = parent->subtracks;subtrack != NULL;subtrack = subtrack->next)
     {
@@ -2207,90 +2207,103 @@
         }
     else if (!reshapeFully && visOrig != tvHide)
         {
         int fourState = subtrackFourStateChecked(subtrack,cart);
         if (fourStateVisible(fourState)) // subtrack must be visible
             {
             enum trackVisibility visSub = tdbVisLimitedByAncestry(cart, subtrack, FALSE);
             if (tvCompare(visMax, visSub) >= 0)
                 visMax = visSub;
             }
         }
     }
 
 // Now we need to update non-subtrack specific vis/sel in cart
 int countUnchecked=0;
-int countExplicitVis=0;
+int countVisChanged=0;
+// If view, this should always be set, since if a single view needs to be promoted, the composite will go to full.
+if (tdbIsCompositeView(parent))
+    {
+    cartSetString(cart,setting,hStringFromTv(visMax));    // Set this explicitly.  The visOrig may be inherited!
+    countVisChanged++;
+    }
+
 if (visMax != visOrig || reshapeFully)
     {
+    if (!tdbIsCompositeView(parent)) // view vis is always shaped, but composite vis is conditionally shaped.
+        {
     cartSetString(cart,setting,hStringFromTv(visMax));    // Set this explicitly.  The visOrig may be inherited!
+        countVisChanged++;
     if (visOrig == tvHide && tdbIsSuperTrackChild(parent))
         cartTdbOverrideSuperTracks(cart,parent,FALSE);      // deal with superTrack vis! cleanup
+        }
 
     // Now set all subtracks that inherit vis back to visOrig
     for(subtrack = parent->subtracks;subtrack != NULL;subtrack = subtrack->next)
         {
         int fourState = subtrackFourStateChecked(subtrack,cart);
         if (!fourStateChecked(fourState))
             cartRemove(cart,subtrack->track);  // Remove subtrack level vis if it isn't even checked just in case
         else  // subtrack is checked (should include subtrack level vis)
             {
             if (!hashFindVal(subVisHash, subtrack->track))   // if the subtrack doesn't have individual vis AND...
                 {
                 if (reshapeFully || visMax == tvHide)
                     {
                     subtrackFourStateCheckedSet(subtrack, cart,FALSE,fourStateEnabled(fourState)); // uncheck
                     cartRemove(cart,subtrack->track);  // Remove it if it exists, just in case
                     countUnchecked++;
                     }
                 else if (visOrig != tvHide)
                     {
                     if (tdbIsMultiTrack(parent))
                         cartRemove(cart,subtrack->track);  // MultiTrack vis is ALWAYS inherited
                     else
                         cartSetString(cart,subtrack->track,hStringFromTv(visOrig));
-                    countExplicitVis++;
+                    countVisChanged++;
                     }
                 }
             else if (tdbIsMultiTrack(parent))
                 cartRemove(cart,subtrack->track);  // MultiTrack vis is ALWAYS inherited vis and non-selected should not have vis
             }
         }
 
-    if (tdbIsCompositeView(parent))
-        {
-        visOrig = tdbVisLimitedByAncestry(cart, parent->parent, FALSE);
-        cartSetString(cart,parent->parent->track,"full");    // Now set composite to full.
-        if (visOrig == tvHide && tdbIsSuperTrackChild(parent->parent))
-            cartTdbOverrideSuperTracks(cart,parent->parent,FALSE);      // deal with superTrack vis! cleanup
-        }
+    // OUCH!  This cannot be until all the views are dealt with!
+    //if (tdbIsCompositeView(parent))
+    //    {
+    //    visOrig = tdbVisLimitedByAncestry(cart, parent->parent, FALSE);
+    //    cartSetString(cart,parent->parent->track,"full");    // Now set composite to full.
+    //    if (visOrig == tvHide && tdbIsSuperTrackChild(parent->parent))
+    //        cartTdbOverrideSuperTracks(cart,parent->parent,FALSE);      // deal with superTrack vis! cleanup
+    //    }
 
-    WARN("%s visOrig:%s visMax:%s unchecked:%d  explicitVis:%d",parent->track,hStringFromTv(visOrig),hStringFromTv(visMax),countUnchecked,countExplicitVis);
     }
 else if (tdbIsMultiTrack(parent))
     {
     // MultiTrack vis is ALWAYS inherited vis so remove any subtrack specific vis
     struct hashCookie brownie = hashFirst(subVisHash);
     struct hashEl* cartVar = NULL;
     while ((cartVar = hashNext(&brownie)) != NULL)
         {
         if (!endsWith(cartVar->name,"_sel"))
             cartRemove(cart,cartVar->name);
         }
     }
+if (countUnchecked + countVisChanged)
+    WARN("%s visOrig:%s visMax:%s unchecked:%d  Vis changed:%d",parent->track,hStringFromTv(visOrig),hStringFromTv(visMax),countUnchecked,countVisChanged);
 
-return (countUnchecked + countExplicitVis);
+return (countUnchecked + countVisChanged);
 }
 #endif/// defined(COMPOSITE_VIS_SHAPING_PLAN_A) || defined(COMPOSITE_VIS_SHAPING_PLAN_B)
 
 boolean cartTdbTreeMatchSubtrackVis(struct cart *cart,struct trackDb *tdbContainer)
 /* When subtrack vis is set via findTracks, and composite has no cart settings,
    then "shape" composite to match found */
 {
 #if !defined(COMPOSITE_VIS_SHAPING_PLAN_A) && !defined(COMPOSITE_VIS_SHAPING_PLAN_B)
     return FALSE;  // Don't do any shaping
 #else/// if defined(COMPOSITE_VIS_SHAPING_PLAN_A) || defined(COMPOSITE_VIS_SHAPING_PLAN_B)
 if (!tdbIsContainer(tdbContainer))
     return FALSE;  // Don't do any shaping
 
 // First look for subtrack level vis
 char setting[512];
@@ -2307,31 +2320,31 @@
         int fourState = subtrackFourStateChecked(subtrack,cart);
         //if (cartUsualInt(cart,setting,0) != 0) // FIXME: sub level vis is not getting cleared out!!!!
         if (fourStateVisible(fourState))            // subtrack is checked too
             {
             WARN("Subtrack level vis %s=%s",subtrack->track,val);
             hashAdd(subVisHash,subtrack->track,val);
             safef(setting,sizeof(setting),"%s_sel",subtrack->track);
             hashAdd(subVisHash,setting,subtrack);     // Add the "_sel" setting which should also exist.  Point it to subtrack
             }
         }
     }
 slFreeList(&tdbRefList);
 
 if (hashNumEntries(subVisHash) == 0)
     {
-    WARN("No subtrack level vis for %s",tdbContainer->track);
+    //WARN("No subtrack level vis for %s",tdbContainer->track);
     return FALSE;
     }
 
 // Next look for any cart settings other than subtrack vis/sel
 #ifdef COMPOSITE_VIS_SHAPING_PLAN_B
 // New directive means that if composite is hidden, then ignore previous and don't bother checking cart.
 boolean reshapeFully = (tdbVisLimitedByAncestry(cart, tdbContainer, FALSE) == tvHide);
 boolean hasViews = tdbIsCompositeView(tdbContainer->subtracks);
 
 #else///ifndef COMPOSITE_VIS_SHAPING_PLAN_B
 boolean reshapeFully = cartVarsNoneFoundForTdb(cart,subVisHash,tdbContainer);
 boolean hasViews = FALSE;
 tdbView = tdbContainer->subtracks;
 if (tdbIsCompositeView(tdbView))
     {
@@ -2367,30 +2380,37 @@
     return FALSE; // Any view cart vars means non-default so do not "shape" composite
     }
 #endif///ndef COMPOSITE_VIS_SHAPING_PLAN_B
 WARN("reshape: %s",reshapeFully?"Fully":"Incrementally");
 
 // Now shape views and composite to match subtrack specific visibility
 int count = 0;
 if (hasViews)
     {
     for(tdbView = tdbContainer->subtracks;tdbView != NULL; tdbView = tdbView->next )
         {
         char *view = NULL;
         if (tdbIsView(tdbView,&view) )
             count += cartTdbParentShapeVis(cart,tdbView,view,subVisHash,reshapeFully);
         }
+    if (count > 0)
+        {
+        enum trackVisibility visOrig = tdbVisLimitedByAncestry(cart, tdbContainer, FALSE);
+        cartSetString(cart,tdbContainer->track,"full");    // Now set composite to full.
+        if (visOrig == tvHide && tdbIsSuperTrackChild(tdbContainer))
+            cartTdbOverrideSuperTracks(cart,tdbContainer,FALSE);      // deal with superTrack vis! cleanup
+        }
     }
 else // If no views then composite is not set to fuul but to max of subtracks
     count = cartTdbParentShapeVis(cart,tdbContainer,NULL,subVisHash,reshapeFully);
 
 hashFree(&subVisHash);
 return TRUE;
 #endif/// defined(COMPOSITE_VIS_SHAPING_PLAN_A) || defined(COMPOSITE_VIS_SHAPING_PLAN_B)
 }
 
 boolean cartTdbTreeCleanupOverrides(struct trackDb *tdb,struct cart *newCart,struct hash *oldVars)
 /* When container or composite/view settings changes, remove subtrack specific settings
    Returns TRUE if any cart vars are removed */
 {
 boolean anythingChanged = cartTdbOverrideSuperTracks(newCart,tdb,TRUE);
 if (!tdbIsContainer(tdb))
@@ -2435,78 +2455,81 @@
     {
     (void)cartNamesPruneChanged(newCart,oldVars,&changedSettings,TRUE,FALSE);
     if (changedSettings == NULL && !containerVisChanged)
         return anythingChanged;
     }
 
 // Walk through views
 if (hasViews)
     {
     for (tdbView = tdb->subtracks;tdbView != NULL; tdbView = tdbView->next)
         {
         boolean viewVisChanged = FALSE;
         if (!tdbIsView(tdbView,&view))
             break;
 
-        // If just created and if vis is the same as tdb default then vis has not changed
-        safef(setting,sizeof(setting),"%s.%s.vis",tdb->track,view);
-        char *cartVis = cartOptionalString(newCart,setting);
-        char *oldValue = hashFindVal(oldVars,setting);
-        if (cartVis && oldValue == NULL && hTvFromString(cartVis) != tdbView->visibility)
-            viewVisChanged = FALSE;
-
         safef(setting,   sizeof(setting),"%s.%s.",tdb->track,view); // unfortunatly setting name could be containerName.View.???
         char settingAlt[512];
         safef(settingAlt,sizeof(settingAlt),"%s.",tdbView->track);  // or viewTrackName.???
         struct slPair *leftOvers = NULL;
         // Walk through settings that match this view
         while ((oneName = slPopHead(&changedSettings)) != NULL)
             {
             if(startsWith(setting,oneName->name))
                 suffix = oneName->name + strlen(setting);
             else if(startsWith(settingAlt,oneName->name))
                 suffix = oneName->name + strlen(settingAlt);
             else
                 {
                 slAddHead(&leftOvers,oneName);
                 continue;
                 }
 
             if (sameString(suffix,"vis"))
                 {
                 viewVisChanged = TRUE;
                 }
             else if (cartRemoveFromTdbTree(newCart,tdbView,suffix,TRUE) > 0)
                 clensed++;
 
             freeMem(oneName);
             }
+        if (viewVisChanged)
+            {
+            // If just created and if vis is the same as tdb default then vis has not changed
+            safef(setting,sizeof(setting),"%s.%s.vis",tdb->track,view);
+            char *cartVis = cartOptionalString(newCart,setting);
+            char *oldValue = hashFindVal(oldVars,setting);
+            if (cartVis && oldValue == NULL && hTvFromString(cartVis) != tdbView->visibility)
+                viewVisChanged = FALSE;
+            }
+
         if  (containerVisChanged || viewVisChanged)
             { // vis is a special additive case!
             WARN("Removing subtrack vis for %s.%s",tdb->track,view);
             if (cartRemoveFromTdbTree(newCart,tdbView,NULL,TRUE) > 0)
                 clensed++;
             }
         changedSettings = leftOvers;
         }
     }
 
 // Now deal with anything remaining at the container level
 while ((oneName = slPopHead(&changedSettings)) != NULL)
     {
     suffix = oneName->name + strlen(tdb->track) + 1;
     if(cartRemoveFromTdbTree(newCart,tdb,suffix,TRUE) > 0)
         clensed++;
     freeMem(oneName);
     }
-if  (containerVisChanged || !hasViews)
+if  (containerVisChanged && !hasViews)
     { // vis is a special additive case!
     if (cartRemoveFromTdbTree(newCart,tdb,NULL,TRUE) > 0)
         clensed++;
     }
 
 anythingChanged = (anythingChanged || (clensed > 0));
 return anythingChanged;
 }