Bug 118457 - legends do not update automatically
Summary: legends do not update automatically
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: general (show other bugs)
Version: 1.x
Platform: unspecified Solaris
: NOR normal
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-16 16:18 UTC by Nicolas Brisset
Modified: 2005-12-20 15:45 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas Brisset 2005-12-16 16:18:41 UTC
Version:           1.2.0_devel (using KDE 3.4.0, compiled sources)
Compiler:          gcc version 3.4.3
OS:                SunOS (sun4u) release 5.8

To reproduce:

- start kst
- load 3 vectors from any source, putting each in a separate plot WITH LEGENDS ON
- Plot dialog/Contents : add the two other curves, hit "Apply changes"
- nothing happens :-(

Expected behavior: the legend updates automatically to reflect the fact that two curves have been added
Comment 1 George Staikos 2005-12-16 16:51:18 UTC
On Friday 16 December 2005 10:18, Nicolas Brisset wrote:
> - start kst
> - load 3 vectors from any source, putting each in a separate plot WITH
> LEGENDS ON - Plot dialog/Contents : add the two other curves, hit "Apply
> changes" - nothing happens :-(
>
> Expected behavior: the legend updates automatically to reflect the fact
> that two curves have been added


  Is the legend actually a child object of the plot you added the curves to?  
If so, it should be adding the curves.  If not, then it is behaving 
correctly.
Comment 2 Nicolas Brisset 2005-12-16 17:34:25 UTC
The legend is a standard one (created by clicking the "Activate" checkbox in the second page of the datawizard), I suppose it qualifies it as a child object of the plot. The legend has not been edited specifically (by adding or removing curves from other plots) so that it *should* really update when plot contents are changed.
I am not sure my answer is very clear, but as it is very easy to reproduce you should probably try it for yourself.
Comment 3 George Staikos 2005-12-16 21:09:04 UTC
  What does the following give in kstcmd:
Kst.windows[0].view.children["yourplotname"].length
Comment 4 Nicolas Brisset 2005-12-19 11:48:56 UTC
> kstcmd
Attached to Kst session kst-26457
kst> zsh: alarm      kstcmd

:-( It crashes before I get a chance to type the command !
Comment 5 George Staikos 2005-12-19 15:58:59 UTC
On Monday 19 December 2005 05:48, Nicolas Brisset wrote:

> > kstcmd
>
> Attached to Kst session kst-26457
> kst> zsh: alarm      kstcmd
>
> :-( It crashes before I get a chance to type the command !


  Can you run it in GDB and get a backtrace please?

Thanks
Comment 6 George Staikos 2005-12-19 17:11:41 UTC
As a workaround you can just drag the legend into a plot so that it is fully contained there.  if adding curves to that plot doesn't automatically add them to the legend, it's a bug.
Comment 7 Nicolas Brisset 2005-12-19 17:38:47 UTC
The suggested workaround does not work: the legend does not update even when it is fully contained in the plot (inside the axes) :-( I fear I have to reopen that one.

Note that if I hide the legend (uncheck it in the plot dialog/appearance tab) and show it again, it gets updated.
Does anyone else see the same problem, or is it somehow linked with Solaris/my setup (even though I doubt it)?

FYI, I don't have the problem on a Linux box where I have a less up-to-date svn version (before borders and margins where introduced, but I don't have the exact svn reference). I suspect this has been introduced by some recent change in the legend code...
Comment 8 George Staikos 2005-12-19 18:10:44 UTC
SVN commit 489752 by staikos:

a great example of why public member variables are evil.  addCurve method was
bypassed in the dialog
BUG: 118457


 M  +2 -2      kst2dplot.cpp  
 M  +2 -2      kstiface_impl.cpp  
 M  +1 -1      kstplotdialog_i.cpp  


--- trunk/extragear/graphics/kst/kst/kst2dplot.cpp #489751:489752
@@ -6525,7 +6525,7 @@
 
 /** find the first legend owned by the plot, or return NULL if there is none */
 KstViewLegendPtr Kst2DPlot::legend() {
-  for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) {
+  for (KstViewObjectList::ConstIterator i = _children.begin(); i != _children.end(); ++i) {
     KstViewLegendPtr vl = kst_cast<KstViewLegend>(*i);
     if (vl) {
       return vl;
@@ -6542,7 +6542,7 @@
     vl = new KstViewLegend;
     appendChild(KstViewObjectPtr(vl), true);
     vl->resizeFromAspect(0.1, 0.1, 0.2, 0.1);
-    for (KstBaseCurveList::Iterator it = Curves.begin(); it != Curves.end(); ++it) {
+    for (KstBaseCurveList::ConstIterator it = Curves.begin(); it != Curves.end(); ++it) {
       vl->addCurve(*it);
     }
   }
--- trunk/extragear/graphics/kst/kst/kstiface_impl.cpp #489751:489752
@@ -249,7 +249,7 @@
   KST::dataObjectList.append(KstDataObjectPtr(vc));
   KST::dataObjectList.lock().writeUnlock();
 
-  plot->Curves.append(KstBaseCurvePtr(vc));
+  plot->addCurve(KstBaseCurvePtr(vc));
 
   _doc->forceUpdate();
   _doc->setModified();
@@ -343,7 +343,7 @@
   KST::dataObjectList.append(KstDataObjectPtr(vc));
   KST::dataObjectList.lock().writeUnlock();
 
-  plot->Curves.append(KstBaseCurvePtr(vc));
+  plot->addCurve(KstBaseCurvePtr(vc));
 
   _doc->forceUpdate();
   _doc->setModified();
--- trunk/extragear/graphics/kst/kst/kstplotdialog_i.cpp #489751:489752
@@ -823,7 +823,7 @@
   for (unsigned i = 0; i < DisplayedCurveList->count(); i++) {
     KstBaseCurveList::Iterator it = curves.findTag(DisplayedCurveList->text(i));
     if (it != curves.end()) {
-      plot->Curves.append(*it);
+      plot->addCurve(*it);
     }
   }
   curves.clear();
Comment 9 Nicolas Brisset 2005-12-20 11:59:53 UTC
I fear you forgot to check the symmetrical case: when a curve is removed, the legend does not update either. 
Reopening the bug...
Comment 10 George Staikos 2005-12-20 15:45:43 UTC
SVN commit 490029 by staikos:

Remove curves from the legend too - another case of not using the accessor.
Also some minor cleanups and a FIXME for something that could be inefficient.
BUG: 118457


 M  +2 -4      kst2dplot.cpp  
 M  +1 -1      kstdatacollection-gui.cpp  
 M  +7 -27     kstiface_impl.cpp  
 M  +4 -1      kstplotdialog_i.cpp  


--- trunk/extragear/graphics/kst/kst/kst2dplot.cpp #490028:490029
@@ -3176,17 +3176,15 @@
 void Kst2DPlot::removeCurve(int id) {
   KstBaseCurvePtr curve = *(Curves.findTag(_curveRemoveMap[id]));
   if (curve) {
-    Curves.remove(curve);
-    setDirty();
+    removeCurve(curve);
     if (_menuView) {
       _menuView->paint();
     }
-    KstApp::inst()->document()->setModified();
   }
 }
 
 
-bool Kst2DPlot::popupMenu (KPopupMenu *menu, const QPoint& pos, KstViewObjectPtr topLevelParent) {
+bool Kst2DPlot::popupMenu(KPopupMenu *menu, const QPoint& pos, KstViewObjectPtr topLevelParent) {
   bool hasEntry = false;
   KstMouseModeType mode;
 
--- trunk/extragear/graphics/kst/kst/kstdatacollection-gui.cpp #490028:490029
@@ -229,7 +229,7 @@
 void KST::removeCurveFromPlots(KstBaseCurve *c) {
   Kst2DPlotList pl = Kst2DPlot::globalPlotList();
   for (Kst2DPlotList::Iterator i = pl.begin(); i != pl.end(); ++i) {
-    (*i)->Curves.remove(c);
+    (*i)->removeCurve(c);
   }
 }
 
--- trunk/extragear/graphics/kst/kst/kstiface_impl.cpp #490028:490029
@@ -660,6 +660,7 @@
   return false;
 }
 
+
 QStringList KstIfaceImpl::plotContents(const QString& name) {
   //iterate through the windows until plot is found
   KstApp *app = KstApp::inst();
@@ -681,6 +682,7 @@
   return QStringList();
 }
 
+
 bool KstIfaceImpl::addCurveToPlot(KMdiChildView *win, const QString& plot, const QString& curve) {
   KstViewWindow *w = dynamic_cast<KstViewWindow*>(win);
 
@@ -693,13 +695,8 @@
         KstBaseCurveList::Iterator ci = bcl.findTag(curve);
         Kst2DPlotPtr p = *(plots.findTag(plot));
         if (p && ci != bcl.end()) {
-          (*ci)->writeLock();
           p->addCurve(*ci);
-          (*ci)->writeUnlock();
-
           _doc->forceUpdate();
-          _doc->setModified();
-
           return true;
         }
       }
@@ -711,18 +708,12 @@
 
 
 bool KstIfaceImpl::addCurveToPlot(const QString& window, const QString& plot, const QString& curve) {
-  KstApp *app = KstApp::inst();
-  KMdiChildView *activewin = app->findWindow(window);
-
-  return addCurveToPlot(activewin, plot, curve);
+  return addCurveToPlot(KstApp::inst()->findWindow(window), plot, curve);
 }
 
 
 bool KstIfaceImpl::addCurveToPlot(const QString& plot, const QString& curve) {
-  KstApp *app = KstApp::inst();
-  KMdiChildView *activewin = app->activeWindow();
-
-  return addCurveToPlot(activewin, plot, curve);
+  return addCurveToPlot(KstApp::inst()->activeWindow(), plot, curve);
 }
 
 
@@ -738,13 +729,8 @@
         KstBaseCurveList bcl = kstObjectSubList<KstDataObject,KstBaseCurve>(KST::dataObjectList);
         KstBaseCurveList::Iterator ci = bcl.findTag(curve);
         if (p && ci != bcl.end()) {
-          (*ci)->readLock();
-          p->Curves.remove(*ci);
-          (*ci)->readUnlock();
-
+          p->removeCurve(*ci);
           _doc->forceUpdate();
-          _doc->setModified();
-
           return true;
         }
       }
@@ -755,18 +741,12 @@
 }
 
 bool KstIfaceImpl::removeCurveFromPlot(const QString& window, const QString& plot, const QString& curve) {
-  KstApp *app = KstApp::inst();
-  KMdiChildView *activewin = app->findWindow(window);
-
-  return removeCurveFromPlot(activewin, plot, curve);
+  return removeCurveFromPlot(KstApp::inst()->findWindow(window), plot, curve);
 }
 
 
 bool KstIfaceImpl::removeCurveFromPlot(const QString& plot, const QString& curve) {
-  KstApp *app = KstApp::inst();
-  KMdiChildView *activewin = app->activeWindow();
-
-  return removeCurveFromPlot(activewin, plot, curve);
+  return removeCurveFromPlot(KstApp::inst()->activeWindow(), plot, curve);
 }
 
 
--- trunk/extragear/graphics/kst/kst/kstplotdialog_i.cpp #490028:490029
@@ -817,9 +817,12 @@
     static_cast<KstViewWindow*>(c)->view()->cleanup(_plotColumns->value());
   }
 
+  // FIXME: be more efficient here.  Only remove the curves that we need, only
+  //        add the curves that we need
+
   // add the curves
   KstBaseCurveList curves = kstObjectSubList<KstDataObject, KstBaseCurve>(KST::dataObjectList);
-  plot->Curves.clear();
+  plot->clearCurves();
   for (unsigned i = 0; i < DisplayedCurveList->count(); i++) {
     KstBaseCurveList::Iterator it = curves.findTag(DisplayedCurveList->text(i));
     if (it != curves.end()) {