[Panel-devel] Signalplotter for Plasma

[Percy Leonhardt]

On Friday 10 August 2007, Aaron J. Seigo wrote:
> On Monday 06 August 2007, Percy Leonhardt wrote:
> - setTranslatedUnit's apidox says it is about the vertical axis;
> translatedUnit says it is about the horizontal axis. the code says it's the
> horizontal axis. so horizontal axis is indeed what it is supposed to be?

The axis going from top to bottom is the vertical axis, isn't it?

> - seeing as there is no setUnit(const QString&) or unit(), there's no point
> for the word "Translated" in the API is there? =)

Well, not necessarily. Changed to setUnit() and unit().

> - why is backgroundImage a QImage and not a QPixmap?

After reading the documentation for QImage and QPixmap I agree that QPixmap is 
the correct choice. Changed.

> - in some places int is used for plot indexes (e.g. reorderPlots takes a
> QList<int>) and in others uint is (e.g. removePlot takes a uint). it should
> be one or the other for consistency

Changed in some/most places.

> - instead of having 3 lists to iterate through and keep in sync with each
> other (2x QList<QColor> plus a QLinkedist<QList<double> >) it probably
> makes more sense to just have a QList<PlotData> where PlotData is a struct
> consisting of 2 colours and a QList<double>

While the QList<QColor>s contain the color for every plot the QList<double> 
contains the values of every plot for a certain point in time. I was too lazy 
to change the data model so I only combined the 2 colors in a struct and 
stored it in a list.

> - as a Plasma::Widget, it should not reimplement paint() itself, just
> drawWidget()

Changed paint() to paintWidget().

> - the code is pretty inconsistently formatted (e.g. "if (" in some places
> and "if(" in others) and doesn't follow the plasma coding style guide. this
> is a matter of me spending 5 minutes going over the code and fixing that up
> =)

I put a space between every "if(" and "for(", fixed indentation and 
comment-style.

> - i wonder if the ability to move one plot easily rather than all with
> reorderPlots is desirable? e.g. reorderPlot(uint currentIndex, uint
> newIdex).

I still don't know a usecase for the reordering at all but this doesn't mean 
that reordering a single plot makes no sense...

> - are there any numbers of the speed improvement of keeping the graph drawn
> in a background image versus just drawing it all every time? obviously
> keeping it cached will be faster in an absolute sense, i'm curious how much
> faster it is relative to the brute force method...

Sorry, I have no numbers... and I didn't try to get any.

Committed. :-)

	Percy