Refactor drawing subsystem

[wawuwo]

Hi!

This is a major refactoring of Qucs-S drawing subsystem. There are a lot of changes, both visible by for the end user and internal.

Visible changes:

• line widths are now subject to scaling too -- the more you zoom in the thicker lines appear, and vice verca
• texts are scaled smoothly, without "size jitter"

This gives natural and beautiful schematic appearance, especially when you're actually zooming in or out. Here some examples to compare between 24.1 and this patched version:

Export to image 24.1:

Export to image patched:

Print to file fitting to page:
X2_100_Bipolar-print-fit-to-page_Qucs-S-24.1.pdf
X2_100_Bipolar-print-fit-to-page_Qucs-S-patched.pdf

Zooming:
zooming-Qucs-S-24.1.webm
zooming-Qucs-S-patched.webm

Components don't turn into lumps when view is zoomed out, texts don't "shiver". And the same goes for printing and exporting. Schematic looks the same when exported or printed: natural scaling with no lumps

The most significant internal change is migrating all drawing from ViewPainter to a bare QPainter and relying heavily on its facilities like save()/restore() and transformations. This makes ViewPainter obsolete and it's deleted.

There are several reasons for this change, in short: ViewPainter is a hand-crafted analogue of QPainter+QTranform and it does its work bad. For example

• ViewPainter holds a pointer to an underlying QPainter, and there a lot of direct accesses to the QPainter. Why wrap it then?
• ViewPainter members are accessed from everywhere, you can find a lot of examples like p->DX + float(cx + pt->x) * p->Scale which are quite cryptic to be honest
• ViewPainter usage is not uniform, you draw lines like this:

    for (qucs::Line *pl: Lines) {
        p->Painter->setPen(pl->style);
        p->drawLine(cx + pl->x1, cy - pl->y1, cx + pl->x2, cy - pl->y2);
    }

and texts likes this:

    for (Text *pt: Texts) {
        p->Painter->setWorldTransform(
                QTransform(pt->mCos, -pt->mSin, pt->mSin, pt->mCos,
                           p->DX + float(cx + pt->x) * p->Scale,
                           p->DY + float(cy - pt->y) * p->Scale));

        p->Painter->setPen(pt->Color);
        p->Painter->drawText(0, 0, pt->s);
    }

or this:

            p->Painter->setWorldTransform(
                    QTransform(pt->mCos, -pt->mSin, pt->mSin, pt->mCos,
                               p->DX + float(cx + pt->x) * p->Scale,
                               p->DY + float(cy + pt->y) * p->Scale));
            newFont.setPointSizeF(p->Scale * pt->Size);
            newFont.setOverline(pt->over);
            newFont.setUnderline(pt->under);
            p->Painter->setFont(newFont);
            if ((Simulator & QucsSettings.DefaultSimulator) == QucsSettings.DefaultSimulator) {
                p->Painter->setPen(pt->Color);
            } else {
                p->Painter->setPen(WrongSimulatorPen);
            }
            int w, h;
            w = p->drawTextMapped(pt->s, 0, 0, &h);

— notice how wordy it is, notice the direct access to QPainter and repeating cryptic computations.

At the same time QPainter and QTransform are tested and provide convinient API for scaling, translating, etc. which hides all the gory details; QPainter and QTransform come from a lib used by a lot of people and one can search for help online, etc.

PR organization

There are a lot of changes in a lot of commits. I know it's against all the rules, but unfortunately there was no way to migrate gradually. And it didn't make sense also: having parts of the schematic drawn in different ways resulted in just ugly picture.

I made commits atomic and none of them breaks the app, you can checkout on any of them and the app would compile.

The general structure is the following:

1. At first a QPainter-based drawing subsystem is built alongside with the old ViewPainter-based one. Commit by commit a "QPainter" equivalents of paint function are added, laying the foundation, and when everything is ready the Schematic class is switched to a new drawing subsystem.
2. Printing and exporting to an image migrated to QPainter in a single commit
3. Then Component::paintIcon is migrated to a QPainter in a single commit
4. And finally all the ViewPainter-related code is removed as it becomes obsolete.

[ra3xdh]

Thanks for the contribution! It should imporve the schematic rendering quality. I will try to test and review this within next two weeks. I am planning to prepare release in September, so we have a large time window for refactoring things like this.

Wow, that's interesting, both checks failed after "Built target components"

The failed checks are related to Clang. It may not suppport some C++ extensions. Here is the link to the error message in the log: https://github.com/ra3xdh/qucs_s/pull/723/checks#step:6:1052 You may use the Search in log input field at the right top corner of the build task information window and and input the error string.

[ra3xdh]

@wawuwo Should #694 and #708 be merged before this PR?

[ra3xdh]

Here is a log portion with the error message from MacOS build. Unfortunately I am not able to debug this. I have no acces to Mac hardware and Clang fails on the CMake step using my host Linux machine.

[ 37%] Building CXX object qucs/components/CMakeFiles/components.dir/rectline.cpp.o
/Users/runner/work/qucs_s/qucs_s/qucs/element.h:186:16: note: overridden virtual function is here
  virtual void getCenter(int&, int&);
               ^
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:120:43: error: non-constant-expression cannot be narrowed from type 'int' to 'qreal' (aka 'double') in initializer list [-Wc++11-narrowing]
    misc::draw_resize_handle(painter, {0, -y2});
                                          ^~~
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:120:43: note: insert an explicit cast to silence this issue
    misc::draw_resize_handle(painter, {0, -y2});
                                          ^~~
                                          static_cast<qreal>( )
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:122:40: error: non-constant-expression cannot be narrowed from type 'int' to 'qreal' (aka 'double') in initializer list [-Wc++11-narrowing]
    misc::draw_resize_handle(painter, {x2, -y2});
                                       ^~
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:122:40: note: insert an explicit cast to silence this issue
    misc::draw_resize_handle(painter, {x2, -y2});
                                       ^~
                                       static_cast<qreal>( )
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:122:44: error: non-constant-expression cannot be narrowed from type 'int' to 'qreal' (aka 'double') in initializer list [-Wc++11-narrowing]
    misc::draw_resize_handle(painter, {x2, -y2});
                                           ^~~
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:122:44: note: insert an explicit cast to silence this issue
    misc::draw_resize_handle(painter, {x2, -y2});
                                           ^~~
                                           static_cast<qreal>( )
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:123:40: error: non-constant-expression cannot be narrowed from type 'int' to 'qreal' (aka 'double') in initializer list [-Wc++11-narrowing]
    misc::draw_resize_handle(painter, {x2, 0});
                                       ^~
/Users/runner/work/qucs_s/qucs_s/qucs/diagrams/tabdiagram.cpp:123:40: note: insert an explicit cast to silence this issue
    misc::draw_resize_handle(painter, {x2, 0});
                                       ^~
                                       static_cast<qreal>( )
6 warnings and 4 errors generated.

[wawuwo]

@ra3xdh

Should #694 and #708 be merged before this PR?

#694 must not be merged, it's just bad, but at least it helped me figure out a way to prepare this PR. I'll close it.
#708 is independent from this PR, no matter before or after.

Here is a log portion with the error message from MacOS build

Hm-m, I'll look into it, I think I know what's wrong

[ra3xdh]

I have tested this PR and found a bug. The net names are shown after every simulation. Press F2 and you will see something similar to the attached screenshot. The DC bias mode F8 works as expected. Other schematic editing features work as expected. I am considering to merge this after fixing the bug with net names display.

[wawuwo]

@ra3xdh I think the last commit should fix this (I accidentally lost the if (showBias > 0) condition while porting Schematic::drawContents to QPainter, which made net names to be drawn unconditionally)

[ra3xdh]

Yes, the problem with net names show after the simulation has gone, but I have found another bug. The parameter sweep plots are rendered wrong. The plotting engine connects the end and start of every curve. See the attache screenshot:

[ra3xdh]

You may use the following schematic to test parameter sweep:
bjt_qucs.sch.gz

[wawuwo]

@ra3xdh that was hard, but I believe I've grasped the logic (I left the comment) and the fix will help! I've tested it with the provided schematic and it worked perfect.

By the way is there an option to do something like rebase -i --autosquash when merging to get rid of fixup! commits?

[ra3xdh]

Yes, the plots are rendered correctly now.

is there an option to do something like rebase -i --autosquash when merging to get rid of fixup! commits?

I think, it's better to keep the history as is.

[ra3xdh]

One remaining note. The text size in equations headers like .PARAM seems to be too small. Is this expected?

• New
  
• Old
  

[wawuwo]

@ra3xdh yes, this is expected. I am going to prepare a PR with adjustments to individual components like this one, or resistor, 4bit pattern, etc.

All components having texts as part of their symbol are affected in some way, because font size in Text structs are now treated as set in pixels rather than in points as before. I changed this to make text sizes be measured in same units as other parts of a symbol and to always scale them equally.

"Points" font size has different size in pixels on different mediums, for example 12 points may be 120 pixels on one screen, 96 on another and 1600 pixels on a printer, but a 10x50 pixels rectangle is 10x50 pixels everywhere.

[wawuwo]

I do hope I haven't confused or missed something, it would be great to see how it renders on screens with different PPIs. If everything is right then there should be no difference in relative sizes of component symbol's lines, circles, etc. and texts on any device.

[ra3xdh]

@wawuwo I have tested this PR only using FullHD hardware with scaling. Everything seems to work correctly. I didn't observe scaling issues using Qt6 build. The #469 seems to be self-fixed. Unfortunatelly I have no access to 2k and 4k displays and cannot test these systems.

[ra3xdh]

Thanks for the contribution! Merged. The schematic rendering is improved with this PR.