Regression in the pdf export in commit from 17.10.2024

In this commit from 17.10.2024 was discovered a regression related to the pdf export with 0-width lines.

The previous commit works fine. All the following are broken.

The problem is  that this commit is huge and for me is pretty hard to navigate in the code.

Any help is welcome. Where to look? What modules might be related to this bug?

could you please create issue for this on GitHub?

It will be also ideal if you could attach sample .dxf file with such issue to the ticker.

As for the code - it's hard to say right now, there were lot's of changes related to rendering and I need to check this deeper.

Unfortunately I can't create an issue on GitHub because they want 2FA and I don't want to setup it. Too sad, MS ruined it as usual.

Any .dxf file currently renders in pdf with 0-width lines.

The test is simple - open .dxf with lines with some width <>0. Export it to pdf. Check the lines in the pdf - they are 0-width.

Here are two pdf files, generated during bisecting this issue:

test_good.pdf - this is from the last good version (link in the first post).

test_bad.pdf - this is from the first bad version (link in the first post).

hm.. I see. btw, how the same file (with non-renderable lines in pdf) looks on print preview? Are there correct line widths?

Also, could you please check whether "Apply Print scale to line width" setting on PrintPreview has any effect on generation to pdf?

sand1024 wrote

hm.. I see. btw, how the same file (with non-renderable lines in pdf) looks on print preview? Are there correct line widths?

The print preview is fine. All lines are displayed as they are in the drawing.

sand1024 wrote

Also, could you please check whether "Apply Print scale to line width" setting on PrintPreview has any effect on generation to pdf?

No. This button has no effect at all. Neither on the preview, nor on the generated file. What it is intended to do?

OK. I have some fix now, but not sure this is the right fix.

The problem is that during the printing (and exporting to PDF), the class RS_StaticGraphicView ignores the existing of the fields "unitFactor" and "unitFactor100" and never initialize them.

My fix is simply to set them to 1.0 and 0.01 in the constructor, but I am not very sure this is the right approach. (QG_GraphicView has more complex way for initializing these fields).

Index: librecad/src/lib/gui/rs_staticgraphicview.cpp
==================================================================
--- librecad/src/lib/gui/rs_staticgraphicview.cpp
+++ librecad/src/lib/gui/rs_staticgraphicview.cpp
@@ -44,10 +44,12 @@
 {
 	setBackground({255,255,255});
 	QSize b{5, 5};
 	if (pb) b = *pb;
 	setBorders(b.width(), b.height(), b.width(), b.height());
+    unitFactor = 1.0;
+    unitFactor100 = 0.01;
 }
 
 /**
  * @return width of widget.
  */

Nevertheless, this gives the right result in the PDF files. And during my research I have never see these fields with different values. Maybe with inch dimensions they are different, but well, I am not using inches.

Comments and directions are welcome. :)

oh, indeed - that might be right direction. Both unitFactor and unitFactor100 are cached values to avoid calculations for each entity, and yes, they are related to mapping of various drawing units.

Thanks for pointing on this, I'll check initialization for PDF generation once more and add uniform fix.

"Apply Print scale to line width" - What is it intended to do?

Let's say you have a drawing intended for a scale of 1:100 on A2 format and you want to print it scaled down to 1:200 on an A4 sheet for a handy oversight. With "Apply Print scale to line width" you can scale down the line widths too accordingly. Otherwise the line widths would appear much too thick.

LibreCAD reads the intended original scale from Drawing Preferences - Dimensions - General Scale and compares this to the scale setting in Print Preview and calculates the according line widths for printing.

I see. Thanks for the explanation.

I have tested it and this option is actually not working. It always scales the line width in the pdf export, regardless of the state of the button. But the preview is working correctly. Maybe the problem is the same as discussed in this theme.

hi sand1024,

is this an issue due to uninitialized primitives?

since reading from uninitialized variables is considered undefined behavior, I suggest we initialize all primitive members in class definitions.

I have pushed a commit https://github.com/LibreCAD/LibreCAD/commit/e9dbf9415043c68b1eb5ad6fccbe9a53222f6328

In this commit, I also enabled scaling by page center for printing preview.

Another thing not actually related, I found hungarian notations help code reading. Since we only work part timely on this project, clear code is very important here.

m_ to indicate member variables;
g_ to indicate global, etc.

https://en.wikipedia.org/wiki/Hungarian_notation

https://github.com/LibreCAD/LibreCAD/commit/e9dbf9415043c68b1eb5ad6fccbe9a53222f6328

johnfound wrote

OK. I have some fix now, but not sure this is the right fix.

The problem is that during the printing (and exporting to PDF), the class RS_StaticGraphicView ignores the existing of the fields "unitFactor" and "unitFactor100" and never initialize them.

My fix is simply to set them to 1.0 and 0.01 in the constructor, but I am not very sure this is the right approach. (QG_GraphicView has more complex way for initializing these fields).

Index: librecad/src/lib/gui/rs_staticgraphicview.cpp
==================================================================
--- librecad/src/lib/gui/rs_staticgraphicview.cpp
+++ librecad/src/lib/gui/rs_staticgraphicview.cpp
@@ -44,10 +44,12 @@
 {
 	setBackground({255,255,255});
 	QSize b{5, 5};
 	if (pb) b = *pb;
 	setBorders(b.width(), b.height(), b.width(), b.height());
+    unitFactor = 1.0;
+    unitFactor100 = 0.01;
 }
 
 /**
  * @return width of widget.
  */

Nevertheless, this gives the right result in the PDF files. And during my research I have never see these fields with different values. Maybe with inch dimensions they are different, but well, I am not using inches.

Comments and directions are welcome. :)

Hi Dxli,

>is this an issue due to uninitialized primitives?

Well, yes and no - I simply missed that for PDF generation a bit different logic is used. Yet yes - explicit initialization by reasonable defaults is definitely a good approach.

> I found hungarian notations help code reading.

Well, I suppose the notation is rather a question of preferences.

I feel that hungarian notation may make the code more heavyweight.  Nowadays, I think syntax highlighting in IDE handles separation of fields/variables/globals/parameters better.

Yet I have no issues with using any notation. The major thing, that it should be uniform over the entire codebase for consistency. Probably it is also worth to think about some coding style for the project.  

hi sand1024,

To avoid undefined behaviors is more important, so even unreasonable initialization values are better, if the default value is not actually known.

For example, if unsure about an enum A, we can simply:

A a{};

Of course, make sure we don't introduce another undefined behavior here: division by zero, so if unsure for a numerical value, and it's better to initialize to 1 for a potential denominator.

For Hungarian notation, it has been long in codebase, but since you have removed them. I suggest we should choose one convention coding style, instead of relying on IDE. Again, we want the code to be as clear as possible, without assumptions on IDE.

For example, when reviewing github Pull Requests, it is not clear to see whether a variable is a member, class static, or global without reading the complete files.

Even without Hungarian notation, other examples could be followed,

Google C++ coding style: class data members with a tailing underscore; struct data members as ordinary variables;
Microsoft Win32 coding style: uses m_ prefix for data members, and g_ prefix for global;

This matter is more than just personal preferences, if leading coding styles are addressing the issue.

In addition, following the official C++ core guidelines helps.