Bug 140520

The problem was introduced with changes to datawizard.ui.h in revision 597630. 

In short this change sets the base font size of a plot based on the new grid layout of plots in a window (number of rows and columns). However, already existing plots are left untouched and so the described discrepancy in font size results.

A partial revert of this revision would fix the problem.

Barth, could you add a note about the intent of revision 597630 so that when this bug report is fixed the effect of the revision is maintained.

SVN commit 632990 by arwalker:

BUG:140520 don't modify font size on plots created with data wizard as it results in too many inconsistencies

 M  +2 -1      datawizard.ui.h  


--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui.h #632989:632990
@@ -928,7 +928,8 @@
                 pp->getOrCreateLegend();
 	    }
 	}
-        pp->setPlotLabelFontSizes(fontSize);
+//      see bug report 140520 before enabling the following line.
+//      pp->setPlotLabelFontSizes(fontSize);
 
 	++pit;
     }

I would like to discuss this before this fix...

The code which was commented out prevents the data wizard from guessing at a good font size when creating plots...  

It was guessing wrong when there were already plots in the window.

Telling it to not guess at all seems... worse.  The proper fix is get the wizard to do a better job of guessing.  So lets leave the bug opened until we have done this.

In the mean time, I think the sometimes bad guess behavior is better than always having the font too small... thoughts?

I don't think the datawizard should be guessing at font sizes.

This results in inconsistencies between plots created at different times with the data wizard and plots not created with the data wizard.

If you're worried about font sizes being too small then perhaps this could be fixed in KstPlotLabel::updateAbsFontSize().

From the point of view of a user: it is a wizard.  Its job is to guess.

On Monday 12 February 2007 6:14:52 pm Andrew Walker wrote:
> ------- I don't think the datawizard should be guessing at font sizes.

When the user creates plots (through the wizard or not) they should end up with a default font size that is reasonable. They should get the same font size whether they use the wizard or not. Hence my asertion that it is not the wizard that should be guessing at this font size. Instead this should be done in code that is common to plot creation via the wizard or menu.

I understand and agree with your point that any automagic new-plot generation 
should use a good guess for font size.

The situations that the wizard and command line face (simultaneous creation of 
multiple plots) are different than the other situations (creation of 1 plot, 
sometimes in the presence of existing plots) so whatever we do has to deal 
with that.

In the mean time, I like the pre-fix situation better than the current 
situation.

Can you not get what you are after by increasing the value of the "base font size" in the "Kst Settings" dialog?

No.
Lots of plots -> bigger font (relative to the window).

We find the previous behavior very convenient.

cbn

On Tuesday 13 February 2007 6:00:12 pm Andrew Walker wrote:
[bugs.kde.org quoted mail]

So if I understand correctly the problem you are trying to solve is that the font size is too small for relatively few plots and too large for relatively many plots. 

Does that sound about right?

Created attachment 19709 [details]
Proposed fix

The proposed patch retains the previous behavior (enlarged fonts when creating
multiple plots) only in the case where there are no other plots in the window.

If there are are other plots in the window already, the font size of the first
plot in the window is used for the new plots (this applies to all dialogs).

To do this for everthing but the Wizard, a createPlot method was added to
kstViewWindow.	The wizard, as it deals with multiple plot creation, can't use
it.

The patch also includes a variable name change: a 2dplot list in the
datawizard.ui.h is changed from 'l' to 'vlist' for several unimportant but
obvious reasons.

SVN commit 634383 by netterfield:

BUG: 140520
New plots in windows that already have plots take the font size of the
first plot in the window.



 M  +3 -2      datawizard.ui  
 M  +19 -13    datawizard.ui.h  
 M  +1 -1      kst.cpp  
 M  +1 -1      kstcsddialog_i.cpp  
 M  +2 -1      kstcurvedialog_i.cpp  
 M  +1 -1      ksteqdialog_i.cpp  
 M  +1 -1      ksthsdialog_i.cpp  
 M  +1 -1      kstimagedialog_i.cpp  
 M  +1 -1      kstpsddialog_i.cpp  
 M  +14 -0     kstviewwindow.cpp  
 M  +6 -0      kstviewwindow.h  


--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui #634382:634383
@@ -1528,7 +1528,7 @@
     <include location="global" impldecl="in implementation">math.h</include>
     <include location="global" impldecl="in implementation">defaultprimitivenames.h</include>
     <include location="local" impldecl="in implementation">kstvectordefaults.h</include>
-    <include location="local" impldecl="in implementation">kstviewwindow.h</include>
+    <include location="local" impldecl="in declaration">kstviewwindow.h</include>
     <include location="local" impldecl="in implementation">kstcombobox.h</include>
     <include location="local" impldecl="in implementation">kst2dplot.h</include>
     <include location="global" impldecl="in implementation">qdeepcopy.h</include>
@@ -1542,6 +1542,7 @@
     <include location="global" impldecl="in implementation">kdebug.h</include>
     <include location="global" impldecl="in implementation">vectorlistview.h</include>
     <include location="local" impldecl="in implementation">datawizard.ui.h</include>
+    <include location="local" impldecl="in implementation">kstplotlabel.h</include>
 </includes>
 <variables>
     <variable access="private">static const QString&amp; defaultTag;</variable>
@@ -1587,7 +1588,7 @@
 <functions>
     <function returnType="bool">xVectorOk()</function>
     <function returnType="bool">yVectorsOk()</function>
-    <function returnType="double">getFontSize(int count)</function>
+    <function returnType="double">getFontSize(int count, KstViewWindow *w)</function>
     <function>showPage( QWidget * page )</function>
     <function>saveSettings()</function>
     <function>loadSettings()</function>
--- trunk/extragear/graphics/kst/src/libkstapp/datawizard.ui.h #634382:634383
@@ -46,7 +46,7 @@
     _url->setURL(default_source);
     _url->completionObject()->setDir(QDir::currentDirPath());
     _url->setFocus();
-    
+
     // x vector selection
     connect(_xAxisCreateFromField, SIGNAL(toggled(bool)), _xVector, SLOT(setEnabled(bool)));
     connect(_xAxisUseExisting, SIGNAL(toggled(bool)), _xVectorExisting, SLOT(setEnabled(bool)));
@@ -471,7 +471,7 @@
 void DataWizard::finished()
 {
     KstApp *app = KstApp::inst();
-    KstVectorList l;
+    KstVectorList vlist;
     QString name = KST::suggestVectorName(_xVector->currentText());
     QValueList<QColor> colors;
     QColor color;
@@ -642,7 +642,7 @@
 		    _kstDataRange->DoSkip->isChecked() ? _kstDataRange->Skip->value() : 0, 
 		    _kstDataRange->DoSkip->isChecked(),
 		    _kstDataRange->DoFilter->isChecked());
-	    l.append(v);
+	    vlist.append(v);
 	    ++n_curves;
 	    app->slotUpdateProgress(n_steps, ++prg, i18n("Creating vectors..."));
 	    ++it;
@@ -680,7 +680,7 @@
 	return;
     }
 
-    fontSize = qRound(getFontSize(l.count()));
+    fontSize = qRound(getFontSize(vlist.count(), w));
 
     // create the necessary plots
     app->slotUpdateProgress(n_steps, prg, i18n("Creating plots..."));
@@ -702,7 +702,7 @@
 	}
     } else if (_multiplePlots->isChecked()) {
 	Kst2DPlotPtr p;
-	for (uint i = 0; i < l.count(); ++i) {
+	for (uint i = 0; i < vlist.count(); ++i) {
 	    p = kst_cast<Kst2DPlot>(w->view()->findChild(w->createObject<Kst2DPlot>(KST::suggestPlotName(), false)));
 	    plots.append(p.data());
 	}
@@ -713,7 +713,7 @@
 		QString n = app->newWindow(newName);
 		w = static_cast<KstViewWindow*>(app->findWindow(n));
 	    }
-	    for (uint i = 0; i < l.count(); ++i) {
+	    for (uint i = 0; i < vlist.count(); ++i) {
 		p = kst_cast<Kst2DPlot>(w->view()->findChild(w->createObject<Kst2DPlot>(KST::suggestPlotName(), false)));
 		plots.append(p.data());
 	        p->setXAxisInterpretation(false, KstAxisInterpretation(), KstAxisDisplay());
@@ -753,14 +753,14 @@
 
     // Reorder the vectors if the user wants it
     if (_orderInColumns->isChecked()) {
-	const KstVectorList lOld = l;
+	const KstVectorList lOld = vlist;
 	const int count = lOld.count();
 	const int cols = signed(sqrt(plots.count()));
 	const int rows = cols + (count - cols * cols) / cols;
 	int overflow = count % cols;
 	int row = 0, col = 0;
 	for (int i = 0; i < count; ++i) {
-	    l[row * cols + col] = lOld[i];
+	    vlist[row * cols + col] = lOld[i];
 	    ++row;
 	    if (row >= rows) {
 		if (overflow > 0) {
@@ -776,7 +776,7 @@
     // create the data curves
     app->slotUpdateProgress(n_steps, prg, i18n("Creating curves..."));
     KstViewObjectList::Iterator pit = plots.begin();
-    for (KstVectorList::Iterator it = l.begin(); it != l.end(); ++it) {
+    for (KstVectorList::Iterator it = vlist.begin(); it != vlist.end(); ++it) {
 	if (_radioButtonPlotData->isChecked() || _radioButtonPlotDataPSD->isChecked()) {
 	    name = KST::suggestCurveName((*it)->tag(), false);
 	    Kst2DPlotPtr plot = kst_cast<Kst2DPlot>(*pit);
@@ -837,7 +837,7 @@
 	ptype = 0;
 	app->slotUpdateProgress(n_steps, prg, i18n("Creating PSDs..."));
 	
-	for (KstVectorList::Iterator it = l.begin(); it != l.end(); ++it) {
+	for (KstVectorList::Iterator it = vlist.begin(); it != vlist.end(); ++it) {
 	    if ((*it)->length() > 0) {
 		Kst2DPlotPtr plot;
 		KstViewObjectList::Iterator startPlot = pit;
@@ -928,9 +928,9 @@
                 pp->getOrCreateLegend();
 	    }
 	}
-//      see bug report 140520 before enabling the following line.
-//      pp->setPlotLabelFontSizes(fontSize);
 
+	pp->setPlotLabelFontSizes(fontSize);
+
 	++pit;
     }
 
@@ -1334,10 +1334,16 @@
     updateVectorPageButtons();
 }
 
-double DataWizard::getFontSize(int count) {
+double DataWizard::getFontSize(int count, KstViewWindow *w) {
   double size;
   double rows, cols;
 
+  // if there are already plots in the window, use the the first one's font size.
+  Kst2DPlotList plotList = w->view()->findChildrenType<Kst2DPlot>(false);
+  if (!plotList.isEmpty()) {
+    return(plotList[0]->xTickLabel()->fontSize());
+  }
+
   if (_cycleThrough->isChecked()) {
     count = _plotNumber->value();
   } else if (!_multiplePlots->isChecked()) {
--- trunk/extragear/graphics/kst/src/libkstapp/kst.cpp #634382:634383
@@ -2004,7 +2004,7 @@
     w = dynamic_cast<KstViewWindow*>(activeWindow());
     assert(w);
   }
-  w->createObject<Kst2DPlot>(KST::suggestPlotName(), false);
+  w->createPlot(KST::suggestPlotName(), false);
 }
 
 
--- trunk/extragear/graphics/kst/src/libkstapp/kstcsddialog_i.cpp #634382:634383
@@ -218,7 +218,7 @@
 
     if (_w->_curvePlacement->newPlot()) {
       /* assign image to plot */
-      QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+      QString name = w->createPlot(KST::suggestPlotName());
       if (_w->_curvePlacement->reGrid()) {
         w->view()->cleanup(_w->_curvePlacement->columns());
       }
--- trunk/extragear/graphics/kst/src/libkstapp/kstcurvedialog_i.cpp #634382:634383
@@ -39,6 +39,7 @@
 #include "kstviewwindow.h"
 #include "kstuinames.h"
 #include "vectorselector.h"
+#include "kstplotlabel.h"
 
 const QString& KstCurveDialogI::defaultTag = KGlobal::staticQString("<Auto Name>");
 
@@ -316,7 +317,7 @@
 
       if (_w->_curvePlacement->newPlot()) {
         // assign curve to plot
-        QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+	QString name = w->createPlot(KST::suggestPlotName());
         plot = kst_cast<Kst2DPlot>(w->view()->findChild(name));
         if (_w->_curvePlacement->reGrid()) {
           w->view()->cleanup(_w->_curvePlacement->columns());
--- trunk/extragear/graphics/kst/src/libkstapp/ksteqdialog_i.cpp #634382:634383
@@ -226,7 +226,7 @@
 
     if (_w->_curvePlacement->newPlot()) {
       /* assign curve to plot */
-      QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+      QString name = w->createPlot(KST::suggestPlotName());
       if (_w->_curvePlacement->reGrid()) {
         w->view()->cleanup(_w->_curvePlacement->columns());
       }
--- trunk/extragear/graphics/kst/src/libkstapp/ksthsdialog_i.cpp #634382:634383
@@ -259,7 +259,7 @@
 
     if (_w->_curvePlacement->newPlot()) {
       /* assign curve to plot */
-      QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+      QString name = w->createPlot(KST::suggestPlotName());
       if (_w->_curvePlacement->reGrid()) {
         w->view()->cleanup(_w->_curvePlacement->columns());
       }
--- trunk/extragear/graphics/kst/src/libkstapp/kstimagedialog_i.cpp #634382:634383
@@ -517,7 +517,7 @@
 
     if (_w->_curvePlacement->newPlot()) {
       /* assign image to plot */
-      QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+      QString name = w->createPlot(KST::suggestPlotName());
       if (_w->_curvePlacement->reGrid()) {
         w->view()->cleanup(_w->_curvePlacement->columns());
       }
--- trunk/extragear/graphics/kst/src/libkstapp/kstpsddialog_i.cpp #634382:634383
@@ -223,7 +223,7 @@
 
       if (_w->_curvePlacement->newPlot()) {
         // assign curve to plot
-        QString name = w->createObject<Kst2DPlot>(KST::suggestPlotName());
+        QString name = w->createPlot(KST::suggestPlotName());
         if (_w->_curvePlacement->reGrid()) {
           w->view()->cleanup(_w->_curvePlacement->columns());
         }
--- trunk/extragear/graphics/kst/src/libkstapp/kstviewwindow.cpp #634382:634383
@@ -27,6 +27,7 @@
 
 // application specific includes
 #include "kst2dplot.h"
+#include "kstplotlabel.h"
 #include "kstdoc.h"
 #include "kstsettings.h"
 #include "kstviewwindow.h"
@@ -428,5 +429,18 @@
   KMdiChildView::closeEvent(e);
 }
 
+QString KstViewWindow::createPlot(const QString& suggestedName, bool prompt) {
+  Kst2DPlotList plotList = view()->findChildrenType<Kst2DPlot>(false);
+  QString name = createObject<Kst2DPlot>(suggestedName, prompt);
+  Kst2DPlotPtr plot = kst_cast<Kst2DPlot>(view()->findChild(name));
+
+  // if there are already plots in the window, use the the first one's font size.
+  if (!plotList.isEmpty()) {
+    plot->setPlotLabelFontSizes(plotList[0]->xTickLabel()->fontSize());
+  }
+  return (name);
+}
+
+
 #include "kstviewwindow.moc"
 // vim: ts=2 sw=2 et
--- trunk/extragear/graphics/kst/src/libkstapp/kstviewwindow.h #634382:634383
@@ -99,7 +99,9 @@
 
   public:
     template<class T> QString createObject(const QString& suggestedName = QString::null, bool prompt = false);
+    QString createPlot(const QString& suggestedName = QString::null, bool prompt = false);
 
+
   private:
     void commonConstructor();
 
@@ -109,6 +111,10 @@
 };
 
 
+//FIXME: while this purports to be a general createObject, it presumes
+// in a couple cases that it is creating a plot, As, afaikt it is currently only
+// used to create plots, doing anything about it is not urgent, but if
+// you decide to use this to create anything else, keep it in mind.
 template<class T>
 QString KstViewWindow::createObject(const QString& suggestedName, bool prompt) {