Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor drawing of elements: hide logic behind common interface #694

Closed

Conversation

wawuwo
Copy link
Contributor

@wawuwo wawuwo commented Apr 18, 2024

Hi!

This is yet another bit of refactoring. The core idea is to create a common interface for such primitives like qucs::Line, qucs::Arc, qucs::Area and to hide drawing logic behind the interface.

Look at this, where client has to select a proper method of QPainter and deal with its arguments not its own:

        for (qucs::Area *pa: Rects) {
            p->setBrush(pa->Brush);
            p->drawRect(cx + pa->x, cy + pa->y, pa->w, pa->h);
        } 

— next time when rect is drawn everything has to be done again, and most likely will be just copied and pasted, making duplicate.

And compare with this:

        for (qucs::DrawingPrimitive* rect : Rects) {
            p->setBrush(rect->brushHint());
            rect-draw(p, cx, cy);
        } 

— where

  • drawing logic (drawRect(cx + pa->x, cy + pa->y, pa->w, pa->h)) is hidden and defined only once in member function body
  • it's cleaner
  • you can drawn different kinds of primitives all in one loop, because they have the same interface

To achieve this I did the following steps:

  1. Created a new type for ellipses. Previously ellipses and rectangles were defined by the same type Area and client had to decide somehow what this area is — an ellipse or a rectangle
  2. Created a common interface qucs::DrawingPrimitive for qucs::Line, qucs::Arc, qucs::Rect, qucs::Ellips
  3. Replaced all approach to drawing of primitives with invocations of new API.

Everything should look the same, I haven't found any issues using the changed build on my machine.

Both ellipses and rectangles are drawn from a single object named
"Area". Drawing logic looks from where it got the object: if it's
from "rectangles" container, then draw a rectangle, otherwise
draw an ellipse.

This commit creates new "Ellips" object type, separate from "Area".
It opens a way to:
- hide a dedicated ellipsis rectangle drawing logic in a method
  associated only with "Ellips" objects
- distinguish ellipses and rectangles not by their place of residence,
  but by their nature.

The name "Ellips" without "e" at the end is used because "Ellipse"
conflicts with a painting named that way, which is declared in the
same namespace. I didn't want to go into any additional trouble
and chose to go with this type of a "workaround"
Some classes have members of container types, storing "Ellips"
objects, i.e. ellipses. Just assigning a proper name.
After extracting ellipse from "Area" to a separate class, "Area"
means only a rectangle.
The goals of this are
- add a generic API for qucs::Line, qucs::Arc, qucs::Rect, qucs::Ellips
- hide drawing details from clients. If you take a look at code
  now you'll see that drawing of for example a line in Component::paint
  looks like "take this coordinate, add this nubmer to it, and call
  a method of QPainter". Drawing logic is outside, it must be written
  every time you want to draw a line.
  New API provides a way to just invoke DrawingPrimitive::draw giving
  it a QPainter and be done with drawing.
@zergud zergud added this to the 24.3.0 milestone Apr 18, 2024
@zergud
Copy link
Collaborator

zergud commented Apr 18, 2024

Looks good

@wawuwo wawuwo marked this pull request as draft May 7, 2024 13:26
@wawuwo
Copy link
Contributor Author

wawuwo commented May 7, 2024

I'm sorry I had to turn it into draft, it feels like it went in a wrong direction. I'll try to find a way to make it better

@ra3xdh ra3xdh removed this from the 24.3.0 milestone May 7, 2024
@wawuwo
Copy link
Contributor Author

wawuwo commented May 19, 2024

Closed in favour of #723

@wawuwo wawuwo closed this May 19, 2024
@wawuwo wawuwo deleted the drawing-primitive-common-interface branch May 23, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants