Review Request 114692: Better Plotter2D

[Percy Camilo Triveño Aucahuasi]

Hi,

On 29/12/13 19:45, Aleix Pol Gonzalez wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114692/
>
>
> Ship it!
>
> In general it looks good to me, once this issues have been sorted out, you can commit. (But please, sort them out)
>
>
> analitzagui/plotsview2d.h
> <https://git.reviewboard.kde.org/r/114692/diff/1/?file=227422#file227422line127>
> (Diff revision 1)
>
> public slots:
>
> 	
>
> 	116 	
>
>      void  zoomIn()  {  Plotter2D::zoomIn(true);  }
>
> Seems like you want these methods virtual.
>
>
> analitzaplot/plotter2d.cpp
> <https://git.reviewboard.kde.org/r/114692/diff/1/?file=227426#file227426line882>
> (Diff revision 1)
>
> void Plotter2D::drawFunctions(QPaintDevice *qpd)
>
> 492 	
>
>          QPointF  ultim(toWidget(vect[0]));
>
> 	825 	
>
>          QPointF  ultim(toWidget(vect.at(0)));
>
> Why did you change that?
>

at() can be faster than operator[](), because it never causes a deep 
copy to occur.


>
> analitzaplot/plotter2d.cpp
> <https://git.reviewboard.kde.org/r/114692/diff/1/?file=227426#file227426line1154>
> (Diff revision 1)
>
> void Plotter2D::setXAxisLabel(const QString &label)
>
> 745 	
>
>      m_ticksFormat  =  tsfmt;
>
> 	1091 	
>
>      scaleViewport(ZoomInFactor,  QPoint(m_size.width()*0.5,  m_size.height()*0.5),  repaint);
>
> This changes behavior. Why?
>

The last one was buggy, if you see the description in the previous 
version it says: "FIXME:Bad solution" the one that I'm proposing is the 
standard behavior for zooming and I think is bug free.

>
> analitzaplot/plottingenums.h
> <https://git.reviewboard.kde.org/r/114692/diff/1/?file=227428#file227428line37>
> (Diff revision 1)
>
> enum CoordinateSystem {
>
> 	
>
> 35 	
>
> enum  CoordinateSystem  {
>
> 	37 	
>
> enum  CoordinateSystem
>
> Unrelated changes, please don't commit those brace movements
>

No, that is a good change, we need to have uniform code style, please 
don't comment about non important aspects of the patch.

>
> - Aleix Pol Gonzalez
>

Percy

>
> On December 28th, 2013, 8:09 a.m. UTC, Percy Camilo Triveño Aucahuasi wrote:
>
> Review request for KDE Edu and Aleix Pol Gonzalez.
> By Percy Camilo Triveño Aucahuasi.
>
> /Updated Dec. 28, 2013, 8:09 a.m./
>
> *Repository: * analitza
>
>
>   Description
>
> - Fix many bugs (ticks, labels, zooming, etc,)
> - Add polar angles and polar axis.
> - Add option to draw the angles in radians, degrees and gradians.
> - Add new grid styles (some similar to KmPlot like crosses)
> - Better API to manage colors and to set render options (like drawticks, etc.)
> - Many minor fixes/improvements too.
>
>
>   Testing
>
> All test pass, all analitzaplot demos run ok
>
>
>   Diffs
>
>   * analitzaplot/plottingenums.h (f0c4c1c)
>   * analitzaplot/private/utils/mathutils.h (08630b8)
>   * analitzaplot/private/utils/mathutils.cpp (d44ed6d)
>   * analitzagui/plotsview2d.h (b08a716)
>   * analitzagui/plotsview2d.cpp (f66f22d)
>   * analitzaplot/plotitem.h (efe942c)
>   * analitzaplot/plotter2d.h (ac52684)
>   * analitzaplot/plotter2d.cpp (48085f1)
>   * analitzaplot/plotter3d.h (85c3378)
>
> View Diff <https://git.reviewboard.kde.org/r/114692/diff/>
>