Add support for custom page width/height
ClosedPublic

Authored by piggz on Mar 30 2016, 9:56 AM.

Details

Summary

Add support for custom page sizes, specified in user units/points

Test Plan

Created report with 10x10cm size. Displays ok in designer and viewer

Printed report from Kexi with custom size to PDF. Seems to work ok.

Diff Detail

Repository
R14 KReport
Branch
customsize-piggz
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz updated this revision to Diff 3024.Mar 30 2016, 9:56 AM
piggz retitled this revision from to Add support for custom page width/height.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptMar 30 2016, 9:56 AM
staniek edited edge metadata.Mar 31 2016, 11:35 AM

Let's use spaces instead of tabs...

@piggz What's the status for this commit?

src/common/KReportPageOptions.cpp
214

Lines 214, 215 to long, break them.

src/wrtembed/KReportDesigner.cpp
320

Line too long

733

Let's use QPrinterInfo::defaultPrinter().defaultPageSize() as the default.

733

Lines too long..

734

Let's have custom-page-size property (tr("Custom Page Size")) of type SizeF.

735

TODO: later let's have this default unit defined somewhere in a single place.

795

TODO future idea: If p is page size or custom page size or margin, and the margin is too big let's change margin (one or both) so the effective page area is at least 5mmx5mm.

LibreOffice Writer does this:

873

remove debug..

882

remove or disable debug here and elsewhere..

src/wrtembed/KReportDesignerSection.cpp
273

Do we want this to prevent calling KReportDesigner::changeSet() and emitting the propertySetChanged() signal? If so can't we change:

void KReportDesigner::changeSet(KPropertySet *s)
{
...
    d->itmset = s;
    emit propertySetChanged();
}

to

void KReportDesigner::changeSet(KPropertySet *s)
{
...
    if (d->itmset != s) {
        d->itmset = s;
        emit propertySetChanged();
    }
}

And we're done?

Any chances to make basic improvements and push it for 3.0.0?

staniek requested changes to this revision.Jul 12 2017, 10:04 PM
This revision now requires changes to proceed.Jul 12 2017, 10:04 PM
piggz marked 7 inline comments as done.Jul 21 2017, 9:02 PM
piggz added inline comments.
src/wrtembed/KReportDesigner.cpp
733

at least on my system, defaultPageSize() returns an invalid size;

piggz updated this revision to Diff 16985.Jul 21 2017, 9:06 PM
piggz edited edge metadata.
  • Add size to printed page
  • Get custom size to work in kreportexample
  • Address comments
piggz edited the test plan for this revision. (Show Details)Jul 21 2017, 9:12 PM
staniek requested changes to this revision.Jul 21 2017, 10:17 PM
staniek added inline comments.
src/wrtembed/KReportDesigner.cpp
730

Is it good time to remove this code?

795

Moved to T6557

This revision now requires changes to proceed.Jul 21 2017, 10:17 PM
piggz marked an inline comment as done.Aug 8 2017, 7:40 PM
piggz updated this revision to Diff 17899.Aug 8 2017, 7:41 PM
piggz edited edge metadata.
  • Remove unused code

When changing page CustomPageSize->Width property in the KReportExample's property editor, it is interpreted as Height. And coversely, Height is interpreted as Width.

staniek requested changes to this revision.Aug 12 2017, 12:25 PM
This revision now requires changes to proceed.Aug 12 2017, 12:25 PM
staniek updated this revision to Diff 18055.Aug 12 2017, 12:39 PM
staniek edited edge metadata.

Update to master

staniek requested changes to this revision.Aug 12 2017, 12:40 PM
This revision now requires changes to proceed.Aug 12 2017, 12:40 PM
piggz added a comment.Aug 12 2017, 4:32 PM

When changing page CustomPageSize->Width property in the KReportExample's property editor, it is interpreted as Height. And coversely, Height is interpreted as Width.

{F3858801}

Well ... that is maybe me trying to be too clever ... because the orientation is portrait, it tries to interpret the width/height appropriately. Thoughts about how this should work?

staniek added a comment.EditedAug 12 2017, 7:29 PM
In D1263#134956, @piggz wrote:

Well ... that is maybe me trying to be too clever ... because the orientation is portrait, it tries to interpret the width/height appropriately. Thoughts about how this should work?

I guessed that may be the case. Pre-3.1 is the last chance to reconsider the logic. I am browsing the QPageLayout docs and noticed the following:

QPageLayout::pageSize() const
Returns the page size of the page layout. Note that the QPageSize is always defined in a Portrait orientation. To obtain a size that takes the set orientation into account you must use fullRect().

So how about the customSize property be like that? Orientation could be taken into account just prior to drawing/event handling.
So comparing pageWidth with pageHeight in order to modify orientation would be not needed. If user changes orientation, effective page size changes too regardless the size being custom or A4 or whatever. Probably most of the logic is in place already since we're using QPageLayout already?

On loading from XML we're calling d->pageLayout.setPageSize(custom) which really assumes size is a logical size regardless orientation.

staniek added inline comments.Aug 12 2017, 7:32 PM
src/wrtembed/KReportDesigner.cpp
894

Do we need toSize() here? QSizeF is accepted in the ctor.

piggz updated this revision to Diff 18516.Aug 21 2017, 8:08 PM
piggz edited edge metadata.
  • Add size to printed page
  • Get custom size to work in kreportexample
  • Address comments
  • Remove unused code
  • Fix class enum
piggz added inline comments.Aug 21 2017, 8:21 PM
src/wrtembed/KReportDesigner.cpp
894

QSize is used to gice size in POints .. QSizeF is used when units are specified. The ctors are different.

staniek added inline comments.Aug 21 2017, 8:27 PM
src/wrtembed/KReportDesigner.cpp
894

I am looking at accuracy here. Do you think QPageSize(customSize,
QPageSize::Point, ...) is not more accurate?

piggz marked 3 inline comments as done.Aug 21 2017, 8:51 PM
piggz updated this revision to Diff 18527.Aug 22 2017, 7:55 AM
  • Is this the correct behaviour?
staniek accepted this revision.Aug 23 2017, 9:44 PM

Looks good!
One warning to remove

kreport/src/wrtembed/KReportDesigner.cpp:860: warning: variable ‘pageHeight’ set but not used [-Wunused-but-set-variable]
     int pageHeight;
         ^~~~~~~~~~
This revision is now accepted and ready to land.Aug 23 2017, 9:44 PM
This revision was automatically updated to reflect the committed changes.