[education/rkward] rkward/rbackend/rkwarddevice: Fix / clarify DPI handling

Thomas Friedrichsmeier null at kde.org
Sat Sep 7 10:22:47 BST 2024


Git commit 9b54f2ae6d5638367fb4186fdb4b01782118ba8e by Thomas Friedrichsmeier.
Committed on 06/09/2024 at 17:56.
Pushed by tfry into branch 'master'.

Fix / clarify DPI handling

M  +0    -13   rkward/rbackend/rkwarddevice/rkgraphicsdevice.cpp
M  +14   -7    rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.cpp
M  +1    -0    rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.h
M  +8    -8    rkward/rbackend/rkwarddevice/rkgraphicsdevice_setup.cpp
M  +2    -2    rkward/rbackend/rkwarddevice/rkgraphicsdevice_stubs.cpp

https://invent.kde.org/education/rkward/-/commit/9b54f2ae6d5638367fb4186fdb4b01782118ba8e

diff --git a/rkward/rbackend/rkwarddevice/rkgraphicsdevice.cpp b/rkward/rbackend/rkwarddevice/rkgraphicsdevice.cpp
index 9c9e7cc4e..146502acf 100644
--- a/rkward/rbackend/rkwarddevice/rkgraphicsdevice.cpp
+++ b/rkward/rbackend/rkwarddevice/rkgraphicsdevice.cpp
@@ -505,13 +505,6 @@ void RKGraphicsDevice::glyph(const QString &font, quint8 index, const QString &f
 	RK_TRACE(GRAPHICS_DEVICE);
 	RK_ASSERT(points.size() == glyphs.size());
 
-	// TODO Do we need to adjust size?
-	// R has this is Cairo_Glyph:
-	/* Text size (in "points") MUST match the scale of the glyph
-	* location (in "bigpts").  The latter depends on device dpi.
-	*/
-	//cairo_set_font_size(xd->cc, size / (72*dd->ipr[0]));
-	size = size / 72 * 96;
 	QRawFont rfnt;
 	if (index == 0) {
 		rfnt = QRawFont(font, size);
@@ -523,12 +516,6 @@ qDebug("invalid font %s, %d", qPrintable(font), index);
 		fnt.setStyle(style);
 		rfnt = QRawFont::fromFont(fnt);
 	}
-	// TODO Do we need to adjust size?
-	// R has this is Cairo_Glyph:
-	/* Text size (in "points") MUST match the scale of the glyph
-	* location (in "bigpts").  The latter depends on device dpi.
-	*/
-	//cairo_set_font_size(xd->cc, size / (72*dd->ipr[0]));
 
 	// Qt API (6.7) is a little inconsistent in this niche. There is a QGlyphRun, for painting several glyphs at once,
 	// but this cannot procude a QPainterPath. Further, QGlyphRun does not support R's requirement that glyph can be
diff --git a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.cpp b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.cpp
index 33cff7328..2aac1b19a 100644
--- a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.cpp
+++ b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.cpp
@@ -25,8 +25,8 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "../../debug.h"
 
-double RKGraphicsDeviceFrontendTransmitter::lwdscale = 72.0/96;
-RKGraphicsDeviceFrontendTransmitter::RKGraphicsDeviceFrontendTransmitter () : QObject () {
+double RKGraphicsDeviceFrontendTransmitter::lwdscale = 72.0/96; // NOTE: reinitialized more appropriately, later
+RKGraphicsDeviceFrontendTransmitter::RKGraphicsDeviceFrontendTransmitter() : QObject(), dpix(0), dpiy(0) {
 	RK_TRACE (GRAPHICS_DEVICE);
 
 	connection = nullptr;
@@ -254,12 +254,17 @@ void RKGraphicsDeviceFrontendTransmitter::newData () {
 					RK_DEBUG (GRAPHICS_DEVICE, DL_WARNING, "Graphics operation canceled");
 					Q_EMIT stopInteraction();
 				} else if (opcode == RKDQueryResolution) {
-					auto screen = QGuiApplication::primaryScreen();
-					streamer.outstream << (qint32) screen->logicalDotsPerInchX() << (qint32) screen->logicalDotsPerInchY();
-					RK_DEBUG (GRAPHICS_DEVICE, DL_INFO, "DPI for device %d: %d by %d", devnum+1, screen->logicalDotsPerInchX(), screen->logicalDotsPerInchY());
+					if (dpix < 1) {
+						auto screen = QGuiApplication::primaryScreen();
+						dpix = screen->logicalDotsPerInchX();
+						dpiy = screen->logicalDotsPerInchY();
+					}
+					streamer.outstream << (qreal) dpix << (qreal) dpiy;
+					RK_DEBUG (GRAPHICS_DEVICE, DL_INFO, "DPI for device %d: %d by %d", devnum+1, (int) dpix, (int) dpiy);
 					streamer.writeOutBuffer ();
 					// Actually, this is only needed once, but where to put it...
-					RKGraphicsDeviceFrontendTransmitter::lwdscale = ((double) screen->logicalDotsPerInchX()) / 96;   // taken from devX11.c
+					// The 96 is taken from devX11.c, where it is hardcoded (historical reasons?)
+					RKGraphicsDeviceFrontendTransmitter::lwdscale = dpix / 96;
 				} else {
 					if (devnum) RK_DEBUG (GRAPHICS_DEVICE, DL_ERROR, "Received transmission of type %d for unknown device number %d. Skipping.", opcode, devnum+1);
 					sendDummyReply (opcode);
@@ -466,12 +471,14 @@ void RKGraphicsDeviceFrontendTransmitter::newData () {
 			QVector<quint32> glyphs;
 			points.reserve(n);
 			glyphs.reserve(n);
-			for (int i = 0; i < n; ++i) {
+			for (decltype(n) i = 0; i < n; ++i) {
 				qint32 glyphi;
 				points.append(readPoint(streamer.instream));
 				streamer.instream >> glyphi;
 				glyphs.append(glyphi);
 			}
+			// NOTE: contrary to other font sizes, size here is given in device independent "bigpts" (1 inch / 72), which need to be scaled to resolution
+			size = size * (dpix / 72.0);
 			device->glyph(font, index, family, weight, static_cast<QFont::Style>(style), size, col, rot, points, glyphs);
 		} else if (opcode == RKDCapture) {
 			QImage image = device->capture ();
diff --git a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.h b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.h
index 0e062bcfe..727994122 100644
--- a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.h
+++ b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_frontendtransmitter.h
@@ -39,6 +39,7 @@ private:
 	QIODevice *connection;
 	QLocalServer *local_server;
 	RKAsyncDataStreamHelper<RKGraphicsDeviceTransmittionLengthType> streamer;
+	double dpix, dpiy;
 };
 
 #endif
diff --git a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_setup.cpp b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_setup.cpp
index 38098481d..62b723c49 100644
--- a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_setup.cpp
+++ b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_setup.cpp
@@ -23,7 +23,7 @@ struct RKGraphicsDeviceDesc {
 	int devnum;
 	quint32 id;
 	double width, height;
-	int dpix, dpiy;
+	double dpix, dpiy;
 	QString getFontFamily (bool symbolfont) const {
 		if (symbolfont) return default_symbol_family;
 		return default_family;
@@ -36,9 +36,9 @@ struct RKGraphicsDeviceDesc {
 #include "rkgraphicsdevice_stubs.cpp"
 static SEXP RKD_capabilities(SEXP capabilities);
 
-// No, I do not really understand what this is for.
-// Mostly trying to mimick the X11 device's behavior, here.
-#define RKGD_DPI 72.0
+// Fallback resolution for onscreen devices (as used by R itself)
+// Several functions pass sizes assuming 72DPI resolution, too.
+#define RKGD_DEFAULT_DPI 72.0
 
 void RKStartGraphicsDevice (double width, double height, double pointsize, const QStringList &family, rcolor bg, const char* title, bool antialias) {
 	static quint32 id = 0;
@@ -101,8 +101,8 @@ bool RKGraphicsDeviceDesc::init (pDevDesc dev, double pointsize, const QStringLi
 	default_family = family.value (0, "Helvetica");
 	default_symbol_family = family.value (0, "Symbol");
 	RKD_QueryResolution (&dpix, &dpiy);
-	if (dpix <= 1) dpix = RKGD_DPI;
-	if (dpiy <= 1) dpiy = RKGD_DPI;
+	if (dpix <= 1) dpix = RKGD_DEFAULT_DPI;
+	if (dpiy <= 1) dpiy = RKGD_DEFAULT_DPI;
 	width *= dpix;
 	height *= dpiy;
 //	Rprintf ("dpi: %d * %d, dims: %f * %f\n", dpix, dpiy, width, height);
@@ -127,8 +127,8 @@ bool RKGraphicsDeviceDesc::init (pDevDesc dev, double pointsize, const QStringLi
 	dev->right  = dev->clipRight  = width;
 	dev->bottom = dev->clipBottom = height;
 	dev->top    = dev->clipTop    = 0;
-	dev->cra[0] = 0.9 * pointsize * (dpix / RKGD_DPI);
-	dev->cra[1] = 1.2 * pointsize * (dpiy / RKGD_DPI);
+	dev->cra[0] = 0.9 * pointsize * (dpix / RKGD_DEFAULT_DPI);
+	dev->cra[1] = 1.2 * pointsize * (dpiy / RKGD_DEFAULT_DPI);
 	dev->xCharOffset = 0.4900;
 	dev->yCharOffset = 0.3333;
 	dev->yLineBias = 0.2;
diff --git a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_stubs.cpp b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_stubs.cpp
index f4e081796..19890d3aa 100644
--- a/rkward/rbackend/rkwarddevice/rkgraphicsdevice_stubs.cpp
+++ b/rkward/rbackend/rkwarddevice/rkgraphicsdevice_stubs.cpp
@@ -180,7 +180,7 @@ public:
 #define WRITE_FONT(dev) \
 	RKD_OUT_STREAM << gc->cex << gc->ps << gc->lineheight << (quint8) gc->fontface << (gc->fontfamily[0] ? QString (gc->fontfamily) : (static_cast<RKGraphicsDeviceDesc*> (dev->deviceSpecific)->getFontFamily (gc->fontface == 5)))
 
-static void RKD_QueryResolution (int *dpix, int *dpiy) {
+static void RKD_QueryResolution (double *dpix, double *dpiy) {
 	RK_TRACE(GRAPHICS_DEVICE);
 	{
 		RKGraphicsDataStreamWriteGuard wguard;
@@ -188,7 +188,7 @@ static void RKD_QueryResolution (int *dpix, int *dpiy) {
 	}
 	{
 		RKGraphicsDataStreamReadGuard rguard;
-		qint32 _dpix, _dpiy;
+		qreal _dpix, _dpiy;
 		RKD_IN_STREAM >> _dpix >> _dpiy;
 		*dpix = _dpix; *dpiy = _dpiy;
 	}



More information about the rkward-tracker mailing list