D17946: Increase draw velocity

Alexander Semke noreply at phabricator.kde.org
Mon Apr 22 08:53:15 BST 2019


asemke added inline comments.

INLINE COMMENTS

> XYCurve.cpp:918
>  	//calculate the scene coordinates
> -	{
> -#ifdef PERFTRACE_CURVES
> -		PERFTRACE(name().toLatin1() + ", XYCurvePrivate::retransform(), map logical points to scene coordinates");
> -#endif
> -		cSystem->mapLogicalToScene(symbolPointsLogical, symbolPointsScene, visiblePoints);
> +	if (symbolsStyle != Symbol::NoSymbols || valuesType != XYCurve::NoValues ) {
> +		{

What is the reason for this if?

> XYCurve.cpp:949
> +			if (startIndex > endIndex) {
> +				int tempIndex = startIndex;
> +				startIndex = endIndex;

use std::swap(startIndex, endIndex)

> XYCurve.cpp:959
> +
> +			cSystem->mapLogicalToScene(startIndex, endIndex, symbolPointsLogical, symbolPointsScene, visiblePoints, scenePointsUsed, minLogicalDiffX, minLogicalDiffY);
> +		}

one tab too much here. Please also break the parameters, let's avoid such long lines.

> XYCurve.cpp:963
>  
> +
> +

remove these empty lines.

> XYCurve.cpp:1052
> + */
> +void XYCurvePrivate::addLine(QPointF p0, QPointF p1, double& minY, double& maxY, bool& overlap, double minLogicalDiffX, double minLogicalDiffY, int& pixelDiff) {
> +	if (samePixel(p0.x() * minLogicalDiffX, p1.x() * minLogicalDiffX, pixelDiff)) {

minLogicalDiffY is not used in this function.

> XYCurve.cpp:1053
> +void XYCurvePrivate::addLine(QPointF p0, QPointF p1, double& minY, double& maxY, bool& overlap, double minLogicalDiffX, double minLogicalDiffY, int& pixelDiff) {
> +	if (samePixel(p0.x() * minLogicalDiffX, p1.x() * minLogicalDiffX, pixelDiff)) {
> +		if (overlap) { // second and so on time pixel are same

samePixel() is only used once here. Let's add this logic directly inside of XYCurvePrivate::addLine().

> XYCurve.cpp:1055
> +		if (overlap) { // second and so on time pixel are same
> +		  if (p0.y() > maxY) {
> +			maxY = p0.y();

we don't use brackets for one-line statements.

> XYCurve.cpp:1113
> +		}
> +
> +	}

remove this empty line.

> XYCurve.cpp:1175
> +		if (startIndex > endIndex) {
> +			int tempIndex = startIndex;
> +			startIndex = endIndex;

use std::swap(startIndex, endIndex)

> XYCurve.cpp:1270
>  			break;
> +			// add last line
> +			if (overlap) {

this part comes after the break. Is this intended?

> XYCurve.cpp:1289
>  			break;
> +			// add last line
> +			if (overlap) {

this part comes after the break. Is this intended?

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D17946

To: Murmele, asemke
Cc: kde-edu, Murmele, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190422/d2297f67/attachment-0001.html>


More information about the kde-edu mailing list