Version: HEAD (using KDE KDE 3.5.1) OS: Linux PROBLEM: Under certain conditions when a new plot is created the associated font is too small relative to the existing plots. STEPS TO REPRODUCE: Start Kst Maximize it Using the data wizard to plot all 4 variables from gyrodata.dat in separate plots, on a 2x2 grid Using the data wizard again plot 1 variable from gyrodata.dat in a new plot in the existing window Ensure that the regrid to 2 columns option is checked in the data wizard RESULTS: The font size of the newly created plot is smaller than the font size of the existing plots EXPECTED RESULTS: The font size of the newly created plot is the same as the font size of the existing plots
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& 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) {