Fix loading of penstyle data
ClosedPublic

Authored by piggz on Apr 10 2017, 9:01 PM.

Details

Summary

BUG:378561

Test Plan

Create report, insert a label/field/check/text, set linestyle, switch to data view

Diff Detail

Repository
R14 KReport
Branch
3.0
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz created this revision.Apr 10 2017, 9:01 PM
Restricted Application added a project: KReport. · View Herald TranscriptApr 10 2017, 9:01 PM
staniek edited edge metadata.Apr 10 2017, 10:50 PM

I also propose add m_lineWeight->setOption("step", 1.0); just after all occurrences of m_lineWeight = new KProperty("line-weight", 1.0, tr("Line Weight")); so our step is 1.0 and not default (0.01 - inconvenient to use).

src/common/KReportItemLine.cpp
56

Why to cast if KReportLineStyle::width is qreal?

All our 5 KProperty("line-weight") are double and have correct 1.0 value by default.

src/items/check/KReportItemCheck.cpp
58

Same note

src/items/field/KReportItemField.cpp
72

Same note

src/items/label/KReportItemLabel.cpp
64

Same note

src/items/text/KReportItemText.cpp
68

Same note

staniek requested changes to this revision.Apr 10 2017, 10:51 PM
This revision now requires changes to proceed.Apr 10 2017, 10:51 PM
piggz added inline comments.Apr 11 2017, 6:33 AM
src/common/KReportItemLine.cpp
56

in 3.0

m_lineWeight = new KProperty("line-weight", 1, tr("Line Weight"));
  • Fix invalid code loosing information both in 3.0 and in 3.1: ls->setWidth(elemSource.attribute(QLatin1String("report:line-weight"), QLatin1String("0")).toInt()); -- should be ls->setWidth(elemSource.attribute(QLatin1String("report:line-weight"), QLatin1String("0.0")).toDouble()); right?
src/common/KReportItemLine.cpp
56

This is true. My question would be: what's better: to suddenly start supporting qreal in 3.1 or "fixing" the support for qreal in 3.0.2 which would be used for a while since its release before 3.1 replaces it.

So make the change earlier or later?

User that redesigns a report using 3.0.2 loses the precision back to integer. Because we have code like elemSource.attribute(QLatin1String("report:line-weight"), QLatin1String("0")).toInt() in 3.0 though.

How about making the change for 3.0.2 calling it a fix.

piggz updated this revision to Diff 13334.Apr 11 2017, 6:51 PM
piggz edited edge metadata.
  • Improvements. Use qreal instead of int, and dont use QPen. Set step value of 1.0.

Missing bits.

I do realize we would be confident about completeness only after autotests are added :)

src/common/KReportItemLine.cpp
56

(int) not needed here?

75

Missing changes also here: 1.0 and step

92

Missing change toReal

staniek requested changes to this revision.Apr 11 2017, 7:09 PM
This revision now requires changes to proceed.Apr 11 2017, 7:09 PM
piggz marked 3 inline comments as done.Apr 11 2017, 7:11 PM
piggz updated this revision to Diff 13336.Apr 11 2017, 7:18 PM
piggz edited edge metadata.
  • Bring improvements to Line item
piggz updated this revision to Diff 13337.Apr 11 2017, 7:21 PM
  • use static_cast instead of c style cast
staniek accepted this revision.Apr 11 2017, 7:59 PM

Perfect!

This revision is now accepted and ready to land.Apr 11 2017, 7:59 PM
piggz closed this revision.Apr 11 2017, 8:06 PM