Increase draw velocity
ClosedPublic

Authored by Murmele on Jan 3 2019, 5:57 PM.

Details

Summary
  • Increase draw velocity by removing points which are shown on the same pixel in the scene
  • Don't retransform curve if it is not visible

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Murmele created this revision.Jan 3 2019, 5:57 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptJan 3 2019, 5:57 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
Murmele requested review of this revision.Jan 3 2019, 5:57 PM
Murmele edited the summary of this revision. (Show Details)
asemke added inline comments.Jan 4 2019, 6:06 PM
src/backend/worksheet/plots/cartesian/XYCurve.cpp
931

this is not required since we check this already in line 892.

965

this logic is not quite correct since we can have DateTime on the x-axis and Numeric or Integer on the y-axis or the other way around.

975

wouldn't an 'or' be even better here?

Please put 'continue' to the next line.

Murmele updated this revision to Diff 48689.Jan 4 2019, 7:08 PM
Murmele updated this revision to Diff 48692.Jan 4 2019, 7:44 PM
Murmele updated this revision to Diff 48693.Jan 4 2019, 7:49 PM
Murmele updated this revision to Diff 49827.Jan 18 2019, 3:51 PM
Murmele updated this revision to Diff 49828.Jan 18 2019, 4:01 PM
Murmele updated this revision to Diff 49831.Jan 18 2019, 6:33 PM
Murmele updated this revision to Diff 56637.Apr 20 2019, 8:22 PM

rebase to master

Murmele updated this revision to Diff 56649.Apr 21 2019, 9:43 AM
Murmele updated this revision to Diff 56650.Apr 21 2019, 10:20 AM
asemke added inline comments.Apr 22 2019, 7:53 AM
src/backend/worksheet/plots/cartesian/XYCurve.cpp
918 ↗(On Diff #48693)

What is the reason for this if?

949 ↗(On Diff #48693)

use std::swap(startIndex, endIndex)

959 ↗(On Diff #48693)

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

963 ↗(On Diff #48693)

remove these empty lines.

1052 ↗(On Diff #48693)

minLogicalDiffY is not used in this function.

1053 ↗(On Diff #48693)

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

1055 ↗(On Diff #48693)

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

1113 ↗(On Diff #48693)

remove this empty line.

1175 ↗(On Diff #48693)

use std::swap(startIndex, endIndex)

1270 ↗(On Diff #48693)

this part comes after the break. Is this intended?

1289 ↗(On Diff #48693)

this part comes after the break. Is this intended?

Murmele marked 10 inline comments as done.Apr 22 2019, 1:01 PM
Murmele added inline comments.
src/backend/worksheet/plots/cartesian/XYCurve.cpp
918 ↗(On Diff #48693)

Because the logical points are not needed, if no symbols nor values are visible. To save computational power

1270 ↗(On Diff #48693)

no

1289 ↗(On Diff #48693)

no

Murmele marked 4 inline comments as done.Apr 22 2019, 1:01 PM
Murmele updated this revision to Diff 56741.Apr 22 2019, 1:06 PM

reworked the code from comments

The patch look ok. Please check the behavior caused by if (symbolsStyle != Symbol::NoSymbols || valuesType != XYCurve::NoValues ) again and land this nice improvement.

src/backend/worksheet/plots/cartesian/XYCurve.cpp
918 ↗(On Diff #48693)

symbolPointsLogical is also used in updateErrorBars(), updateDropLines() and in updateFilling(). So, even if we don't show any symbols and values, we still need these points for error bars, for the drop lines and for the filling below/above the curve.

asemke accepted this revision.Apr 22 2019, 7:45 PM
This revision is now accepted and ready to land.Apr 22 2019, 7:45 PM
Murmele updated this revision to Diff 56789.Apr 23 2019, 6:13 AM
Murmele updated this revision to Diff 56863.Apr 24 2019, 6:43 AM
  • add casts to suppress warnings
  • remove condition to not recalculate logical points, because they are needed in some cases
This revision was automatically updated to reflect the committed changes.