Working on my preferences, missing in LibreCad

Well, I have started to study the code several days ago, so don't expect too much.

These changes are made for my own comfort, but, of course, can be included in the official code.

At first, something easy (sand1024, thanks for the code navigation):

librecad.diff

Optional "rendering speed optimizations". I have added two checkboxes for fine-grained rendering setup:


Both are not checked by default, because the rendering performance on my slow old machine is still excellent without such "optimizations".

BTW, I have discovered (at least grep thinks this way) that from these 6 parameters, only "Height of text line" is really used in the code. All others are ignored. Why they exists at all?

Best regards.

Hi John found,

The performance trouble underneath should be taking care of, as those workarounds were created due to the performance issue.

Ideally, skipping rendering is only necessary for huge models with ancient computers, but currently, moderate models have problems on modern machines.

It's okay to merge the change, but I suggest to keep the default for the time being.

johnfound wrote

Well, I have started to study the code several days ago, so don't expect too much.

These changes are made for my own comfort, but, of course, can be included in the official code.

At first, something easy (sand1024, thanks for the code navigation):

librecad.diff

Optional "rendering speed optimizations". I have added two checkboxes for fine-grained rendering setup:


Both are not checked by default, because the rendering performance on my slow old machine is still excellent without such "optimizations".

BTW, I have discovered (at least grep thinks this way) that from these 6 parameters, only "Height of text line" is really used in the code. All others are ignored. Why they exists at all?

Best regards.

dxli wrote

It's okay to merge the change, but I suggest to keep the default for the time being.

No problem. Here is the .diff with these options enabled by default:

optional-render-optimizations.diff

well, I suppose current implementation is misleading.

Actually, as far as I can quickly see by diff, these two settings added are related only to text/mtexts.

I suppose that should be explicitly stated in UI.

Actually, other settings in that group also define rendering minimal values for different entities (like lines, circles etc) - and if the label is just "Simplify the small entities", the user may decide that this is applied to all rendering.

But in more general sense - I'm not sure that that "Simplify the small entities" setting is really necessary at all.  

From the performance reason - such optimization of rendering is more than reasonable, as the user cant's see, say, a circle of 2 px - but such a small entity still requires lot's of computation.  

Of course, on small drawings it's not important, yet for really large drawings - it's more than important.

If someone would like to disable such behavior (as far as I understand, your intention was disabling this for texts) - I can simply reduce allowed minimal value for spin box. So if 0 will be entered - the simplified rendering of texts will not be invoked.

sand1024 wrote

Actually, as far as I can quickly see by diff, these two settings added are related only to text/mtexts.

This is only because I missed to find where these limits are used in the code.  But was intended to make all these simplifications optional. At least in order to see the difference.
 

sand1024 wrote

 as the user cant's see, say, a circle of 2 px - but such a small entity still requires lot's of computation.  

Well, while you are right that rendering very small entities is a waste of performance, I can't agree that the user can't see a particular circle with 2px radius. It is perfectly visible and identifiable. So, simplifying it will definitely lower the rendering quality.

I have made some experiments with the text and the text, even with height of 2..3 pixels is clearly identified as a text. While replacing it with straight line is definitely recognized as a deterioration of rendering quality.

So, IMHO, such optimizations should be applied absolutely silently and only when they are surely not recognizable at all.

The draft mode on spans and drags is also very controversial approach and should be definitely optional and switched off by default.

well, as for the optimizations for small entities - well, I'll check that the user is able to disable them.

However, I can say that they delivers quite a significant impact on generic performance.

You can roughly check this on large files (say, the test set I've used by my tests, it's available as attachments to the following pull request: https://github.com/LibreCAD/LibreCAD/pull/1922

Btw, please check the discussion regarding that pull request and mtext performance rendering in comments for that pull request).

And if you'd like to check how some your changes affects the rendering pipeline - well, just rename define
#define DEBUG_RENDERING_ to #define DEBUG_RENDERING in rs_graphicview.h (and rebuild the project).

That define will enable collection of rendering time within paint event. Of course, metrics collected there are quite rough, yet are still informative. Of course, in order to see the difference, the dxf file with really large amount of entities should be used - and that's why tests from that pull requests should be fine.

We have performance issue with mtext.

I will do a profiling to with your files to get some ideas of the critical methods.

From there, we will tweak the algorithms first.

sand1024 wrote

well, as for the optimizations for small entities - well, I'll check that the user is able to disable them.

However, I can say that they delivers quite a significant impact on generic performance.

You can roughly check this on large files (say, the test set I've used by my tests, it's available as attachments to the following pull request: https://github.com/LibreCAD/LibreCAD/pull/1922

Btw, please check the discussion regarding that pull request and mtext performance rendering in comments for that pull request).

And if you'd like to check how some your changes affects the rendering pipeline - well, just rename define
#define DEBUG_RENDERING_ to #define DEBUG_RENDERING in rs_graphicview.h (and rebuild the project).

That define will enable collection of rendering time within paint event. Of course, metrics collected there are quite rough, yet are still informative. Of course, in order to see the difference, the dxf file with really large amount of entities should be used - and that's why tests from that pull requests should be fine.

well, I suppose the most promising direction there - just simplify implementation of letters. Throw away all that thing for inserts, polyline etc... But that might be quite a significant change - yet still quite isolated, I suppose.

@sand1024

I did a profiling with callgrind on a simple mtext,

The results appear to be normal to me, most time spent on rendering.

Just wondering, whether it makes sense to use one painterpath for each letter. To directly call drawLine/DrawArc is considered slower than combined qpainterpath.



mtext_profiling.txt

Well, yes, that' was my point - instead of individual drawing of letter elements, change the implementation to draw the entire letter (where possible) by single path.

Previously, there was quite a significant loss of performance for setting pen for each individual line/arc that is part of polyline that defines each single letter. I've removed that (so style is calculated only once for all mtext - and set to painter once).  

I'll take a deeper look on that implementation, yet probably a bit later. If will be great if you could merge my last merge request, and I suppose soon next week I'll put another one - probably after that it will be possible to try to do something with mtext.

Btw - did you perform profiling on 2.2.1 version (as I've seen Qt5 in logs) or for master one?