KoDpi / KReportDpi

[Jaroslaw Staniek]

Hi,
Comments, please.

Background: We aim for autotestable KReports (roundtrip for 2.x-3.x)
so I've been working on some basic bits explained at [1]. I then
realized KoReportTest is disabled in KReport. So I fixed it and now
ctest fails in particular because on my system KReportDpi::dpiY()  ==
96 QScreen::logicalDotsPerInchY() == 96.19 :)

This leads to related observation and questions as follows.

When I look at calligralibs, KoDpi is used in context of screens. No
surprise, as it uses QApplication::desktop()->logicalDpiX*().
(it's correct that KoDpi is even part of kowidgets, not of some ODF libs)
I understand that proper approach is to not useing screen DPIs info
internally in objects that represent document elements.

Now, since kreport.git commit 145197c11e545e785fc, Wed Mar 18 (Port to
a standalone Qt 5 library) we have changes in KReport such as:

 void KRPos::setScenePos(const QPointF& pos, UpdatePropertyFlag update)
 {
-    const qreal x = INCH_TO_POINT(pos.x() / KoDpi::dpiX());
-    const qreal y = INCH_TO_POINT(pos.y() / KoDpi::dpiY());
+    QScreen *srn = QApplication::screens().at(0);
+
+    const qreal x = INCH_TO_POINT(pos.x() / srn->logicalDotsPerInchX());
+    const qreal y = INCH_TO_POINT(pos.y() / srn->logicalDotsPerInchY());

A number of questions here lead to my conclusion that any use of
screen DPI should be removed from kreport's code that's not tied to
screen rendering:

* we have KReportDpi already but still using the
srn->logicalDotsPerInch*(), this is what the
KoReportTest::testPageOptions() catched (good!)
* the above does not mean we have to use KReportDpi and store in
screen coordinates, right? calligralibs does not seem doing that
* two grand reasons why using (KReportDpi |
srn->logicalDotsPerInch*()) makes no sense:
  1. KReportDpi like KoDpi uses QDesktopWidget, and thus QWidget. We
don't want report processing that hooks into the QWidget or even the
screen world. Calligra libs dropped this assumption already IIRC
thanks to the Qt Quick ports. The needs for headless/batch processing
is of similar nature.
  2. For srn->logicalDotsPerInch*() there's similar issue as in 1.
QScreen is a Qt GUI thing not Qt Widgets but it still hardcodes usage
of the first the screen's DPI. Moreover first screen's DPI -- quite
strong assumption. At the level of document elements API we don't have
access to screen # information, it's available at the last step --
screen rendering, if it happens at all.

* finally, I guess the current code works for non-screen targets
(printing, pdf) probably only because we're scaling back the painter
by a proper factor

[1] https://community.kde.org/KReport/API_Changes

-- 
regards, Jaroslaw Staniek

KDE:
: A world-wide network of software engineers, artists, writers, translators
: and facilitators committed to Free Software development - http://kde.org
Calligra Suite:
: A graphic art and office suite - http://calligra.org
Kexi:
: A visual database apps builder - http://calligra.org/kexi
Qt Certified Specialist:
: http://www.linkedin.com/in/jstaniek