modelica / ModelicaSpecification

Specification of the Modelica Language
https://specification.modelica.org
Creative Commons Attribution Share Alike 4.0 International
98 stars 41 forks source link

Is Line thickness invariant to extents and zoom factors? #3525

Closed casella closed 3 months ago

casella commented 4 months ago

This issue was originally brought up by @ceraolo in ticket OpenModelica/OpenModelica#12519. Consider this MWE:

model L_ground
  Modelica.Electrical.Analog.Basic.Inductor inductor annotation(
    Placement(visible = true, transformation(origin = {6, 8}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
  Modelica.Electrical.Analog.Basic.Ground ground annotation(
    Placement(visible = true, transformation(origin = {-22, -10}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
equation
  connect(inductor.p, ground.p) annotation(
    Line(points = {{-4, 8}, {-22, 8}, {-22, 0}}, color = {0, 0, 255}));
annotation(
    Diagram(coordinateSystem(extent = {{-40, 20}, {20, -20}})));
end L_ground;

338139121-39d02618-09e0-4f8b-85da-9d61d205c0e7

There are several lines in the diagram of this model:

All these lines do not have a thickness attribute, so they all have thickness = 0.25 units (i.e. mm). Is the intention of the specification that this thickness is always used in an absolute sense, irrespective of the zoom factor?

338138921-65ae3c03-6592-40d5-8aed-77df48bdaf00

This definitely makes sense, but I guess it should be stated explicitly, as other uses of the DrawingUnit type (e.g. determining the coordinates of icon elements when drawing diagrams) are used in a relative sense. Maybe I missed it and it is already there?

d-hedberg commented 4 months ago

I'm not aware of any such exception in the Modelica specification and it has never been discussed as far as I know. To me the only sensible thing to do is to scale the line thickness, regardless of its value.

HansOlsson commented 4 months ago

I'm not aware of any such exception in the Modelica specification and it has never been discussed as far as I know. To me the only sensible thing to do is to scale the line thickness, regardless of its value.

I think I agree in general. However, there is a point where the line thickness will be smaller than a pixel on the screen, and in that case using a minimum thickness or anti-aliasing becomes necessary (alternatively just skipping the line).

d-hedberg commented 4 months ago

I think I agree in general. However, there is a point where the line thickness will be smaller than a pixel on the screen, and in that case using a minimum thickness or anti-aliasing becomes necessary (alternatively just skipping the line).

Indeed. This can be handled in various ways by the tools as you say. I guess it could be of interest to specify whether a line with a thickness > 0 should always be visible when rendered (regardless how thin it becomes with all transformations applied), but I think we can leave this to the tool as well. In the extreme cases it will not matter. Whatever icon the line it is part of is most likely too small to make any sense out of anyway.

eshmoylova commented 4 months ago

I am confused by this sentence from @casella's description:

All these lines do not have a lineThickness attribute, so they all have thickness = 0.25 units (i.e. mm).

Do lines ever have lineThickness? Which is also the question in #3523.

HansOlsson commented 4 months ago

I am confused by this sentence from @casella's description:

All these lines do not have a lineThickness attribute, so they all have thickness = 0.25 units (i.e. mm).

Do lines ever have lineThickness? Which is also the question in #3523.

Yes, it should be thickness.

casella commented 4 months ago

I am confused by this sentence from @casella's description:

All these lines do not have a lineThickness attribute, so they all have thickness = 0.25 units (i.e. mm).

Do lines ever have lineThickness? Which is also the question in #3523.

Apologies, Line objects have a thickness attribute, while FilledShape have a lineThickness attribute. The discussion in this ticket applies to both.

casella commented 4 months ago

Speaking now as the leader of MAP-LIB, this topic is actually quite sensitive, as it may have quite substantial consequences on how models using the Modelica Standard Library are displayed. Most icons and diagrams in the MSL have lines without an explicitly set thickness attribute, or shapes without an explicitly set lineThickness attribute; people developed them mostly with Dymola for 25 years, got some graphical rendering and were fine with it. We must make sure that whatever decisions we take on this issue complies with these requirements:

Please consider the following MWE:

package TestLineWidth
  model M
    annotation (
      Icon(graphics={
        Line(origin = {-22, 80}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}),
        Text(origin = {71, 84}, extent = {{-19, 10}, {19, -10}}, textString = "none"),
        Line(origin = {-21, 55}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0),
        Text(origin = {69, 58}, extent = {{-19, 10}, {19, -10}}, textString = "0.0"),
        Line(origin = {-21, 27}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0.25),
        Text(origin = {69, 30}, extent = {{-19, 10}, {19, -10}}, textString = "0.25"),
        Line(origin = {-21, 0}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0.5),
        Text(origin = {69, 2}, extent = {{-19, 10}, {19, -10}}, textString = "0.5"),
        Line(origin = {-21, -24}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 1),
        Text(origin = {69, -22}, extent = {{-19, 10}, {19, -10}}, textString = "1.0"),
        Line(origin = {-21, -50}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 2),
        Text(origin = {69, -48}, extent = {{-19, 10}, {19, -10}}, textString = "2.0"),
        Line(origin = {-21, -80}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 4),
        Text(origin = {69, -78}, extent = {{-19, 10}, {19, -10}}, textString = "4.0"),
        Rectangle(extent = {{-100, 100}, {100, -100}})}));
  end M;

  model S
    M m1 annotation (
      Placement(transformation(origin = {-10, 70}, extent = {{-10, -10}, {10, 10}})));
    M m2 annotation (
      Placement(transformation(origin = {-20, 32}, extent = {{-20, -20}, {20, 20}})));
    M m3 annotation (
      Placement(transformation(origin = {-40, -40}, extent = {{-40, -40}, {40, 40}})));

    annotation (
      Diagram(
        graphics={
          Line(origin = {77, 81}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}),
          Text(origin = {171, 84}, extent = {{-19, 10}, {19, -10}}, textString = "none"),
          Line(origin = {78, 55}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0),
          Text(origin = {169, 58}, extent = {{-19, 10}, {19, -10}}, textString = "0.0"),
          Line(origin = {78, 27}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0.25),
          Text(origin = {169, 30}, extent = {{-19, 10}, {19, -10}}, textString = "0.25"),
          Line(origin = {78, 0}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 0.5),
          Text(origin = {169, 2}, extent = {{-19, 10}, {19, -10}}, textString = "0.5"),
          Line(origin = {78, -24}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 1),
          Text(origin = {169, -22}, extent = {{-19, 10}, {19, -10}}, textString = "1.0"),
          Line(origin = {78, -50}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 2),
          Text(origin = {169, -48}, extent = {{-19, 10}, {19, -10}}, textString = "2.0"),
          Line(origin = {78, -80}, points = {{-58, 0}, {62, 0}, {62, 0}, {62, 0}}, thickness = 4),
          Text(origin = {169, -78}, extent = {{-19, 10}, {19, -10}}, textString = "4.0")},
        coordinateSystem(extent = {{-200, -100}, {200, 100}})));
  end S;
end TestLineWidth;

which shows all possible interesting combinations:

Some lines are drawn directly on the Diagram layer of model S, which is 400 units wide and 200 units high), others are drawn in the Icon layer of model M, which is 200 units wide and 200 units high, which is then instantiated three times in S, once with an extent of 20x20 units, once with an extent of 40x40 units, and once with an extent of 80x80 units.

This is how Dymola shows the diagram layer of model S on my laptop, which has a 1920x1024 resolution:

Zoom out

immagine

Full screen diagram

immagine

Zoom in of the top center area

immagine

Two things stand out very clearly:

(continued...)

casella commented 4 months ago

If we now decide, as @d-hedberg proposed, that the right interpretation is that we scale the line thickness, and we consider the default value thickness = 0.25, we possibly end up with unintended behaviour. Consider the L_ground model shown at the beginning of this ticket, as it it currently shown in OpenModelica. If you set the zoom to cover a large extent, it looks pretty reasonable:

immagine

however, if you zoom in, the 0.25 mm connection line will become very fat, and unnecessarily so: why should an electrical connection wire be so fat? On the other hand, the 0.25 mm line inside the inductor icon, whose extent is much smaller than the extent of the diagram, will look much thinner.

immagine

This looks quite odd, and of course caused @ceraolo's reaction with the OpenModelica developer's team: what the hell are you doing, I want the previous behaviour back! It turned out, we were just following the specification 😅.

I myself could live with such an outcome, but I agree it's ugly and need to be careful not to annoy the Modelica user base.

My point is: people have been using the MSL (where I recall S stands for "Standard") for 25 years and have always been expecting to see the connection lines of electrical circuits to have the same thickness as the lines inside the icons going from the connector to the component symbol. This is in particular true for electrical and electronic components. Everybody gave this for granted, so I would argue this has always been the intended behaviour, regardless what was actually written in the specification. BTW, this was also the behaviour in OpenModelica until the very last release of the tool, when we tried to stick to the specification more closely.

At the moment, I only see two viable options:

  1. We enforce the MLS as it is written, and we get ugly diagrams from the MSL (and other libraries developed with Dymola in the past) as soon as you zoom them in or use large, high-resolution screens
  2. We update the MLS to reliably get what has arguably always been the intended behaviour, i.e., lines with default thickness are represented on screen or on paper with some reasonable minimum thickness, regardless of scale. We already did something like that when we changed the look-up rules to make the MultiBody world object's gravity function legal in MLS 3.2 rev.1.

Option 2. can be achieved by amending the specification to set the default thickness/lineThickness value to zero instead of 0.25 and by explicitly requiring that lines are rendered with a minimum, scale-invariant value on screen and on print, e.g. 0.25 mm. This will guarantee a visible but thin line both in print and on screens of all sizes and resolutions on all tools. Tools could actually allow to change this 0.25 mm value to the user's preference, of course the best outcome is different if I am generating an image for a 2-column conference paper or for an A2-size large diagram print-out. This also addresses @HansOlsson's comment

I think I agree in general. However, there is a point where the line thickness will be smaller than a pixel on the screen, and in that case using a minimum thickness or anti-aliasing becomes necessary (alternatively just skipping the line).

I don't think skipping lines below 1 pixel is a good idea, because it will cause some icons to disappear from view when you zoom out, which will look like a tool bug to most people, I guess. For sure much worse than having cluttered icons because of too may lines which are not thin enough because of resolution limits.

Whatever the final decision will be, as the leader of MAP-LIB I would definitely recommend to test it on all the diagrams of the MSL at various zoom levels, to make sure we are not getting ugly results.

maltelenz commented 4 months ago

There are other tools than Dymola, and other libraries than the MSL. If we would change the specification, we would break other libraries that were developed in other tools, who actually followed the specification.

I am strongly against changing the specification to adapt to a broken implementation in one tool.

I don't see the slightly thicker lines in zoomed in electrical diagrams as a problem. It is how this model has always looked (in standard-compliant tools).

casella commented 4 months ago

Two tools :)

maltelenz commented 4 months ago

Two tools :)

OK, let me amend: I am strongly against changing the specification to adapt to a broken implementation in some tools.

casella commented 4 months ago

For the record, in general I am also against changing the specification to adapt to existing tool implementations. On the other hand, if changing the specification improves the situation for some tool users, without making it worse for other tool users, I would definitely consider that. I would try to see if such a solution exists.

HansOlsson commented 4 months ago

As I recall a similar issue was discussed in #2835 - and Dymola then adapted the behavior and added functionality to find the problematic cases.

The issue here isn't about scaling, but whether the default for thickness should be 0.25 or 0 (with the interpretation that thickness 0 gives smallest possible line). @DagBruck

ceraolo commented 4 months ago

I don't see the slightly thicker lines in zoomed in electrical diagrams as a problem. It is how this model has always looked (in standard-compliant tools).

If this is the intended behaviour, this is good.

What is ugly for us users is when zooming the ratio between wdths of connecting lines and lines internal to components change. This is what currently happens with OM. If I understand well, Casella says that this is MS compliant. If this is MS compliant, IMO MS is broken, see the three screens below.

1: Low zoom, (nearly) equal internal and external lines: image 2: medium zoom: connecting lines larger than internal's image 3: high zoom: internal lines smaller than in the previous screens and much smaller than conencting lines: image

maltelenz commented 4 months ago

To me, screenshots 1 and 2 look fine in the above comment, but it seems like a tool (OM) issue that the internal lines in the inductor and ground become thinner in 3 than in 2.

casella commented 4 months ago

Yes, there is certainly some remaining tool issue in OpenModelica. The question is, how should we fix it, according to the current standard or according some amended version of it?

Here is my proposal:

This would allow to experiment with all possible combinations discussed in this ticket (and more), so we can take an informed decision.

Pls. mark this comment with a thumbs-up if you agree.

maltelenz commented 4 months ago

I have yet to see a good reason to change the default thickness.

You as a tool vendor are of course free to have any switches or user settings you like.

This very basic feature has existed in the language since MLS 2.0 in 2002, so for 22 years. To break any library that has been developed since then relying on this feature, would require some extraordinarily strong arguments.

d-hedberg commented 4 months ago

Also consider other effects of changing the default thickness to 0. It's possible to specify that line ends should have arrow heads and arrow heads always scale. When zooming in, the proportions are lost for lines with thickness 0 and arrow heads.

casella commented 4 months ago

I guess we'll start implementing the specification in OMC and see what happens 😅

BTW, we've had MSL reference results and regression testing for 20+ years, but we never bothered to have any reference in terms of graphical rendering. This is potentially a problem, as library developers do not have access to all tools, so they have no idea how their diagrams and icons will look like. If a library developer uses tool X to develop his/her library and the tool implementation is not according to the standard the library developer has no idea, he/she will do whatever it takes to get a certain look. Then somebody else tries to use it in another tool and the result is ugly. I think it is in the interest of the whole Modelica community to try preventing these situations.

As we set up a CI for the development of the MSL, maybe we should also consider this aspect. It's probably not possible to have an automated verification (after all, implementations do not have to be identical), but having some automatically generated diagrams from different tools for inspection could definitely be useful.

maltelenz commented 4 months ago

BTW, we've had MSL reference results and regression testing for 20+ years, but we never bothered to have any reference in terms of graphical rendering. This is potentially a problem, as library developers do not have access to all tools, so they have no idea how their diagrams and icons will look like. If a library developer uses tool X to develop his/her library and the tool implementation is not according to the standard the library developer has no idea, he/she will do whatever it takes to get a certain look. Then somebody else tries to use it in another tool and the result is ugly. I think it is in the interest of the whole Modelica community to try preventing these situations.

As we set up a CI for the development of the MSL, maybe we should also consider this aspect. It's probably not possible to have an automated verification (after all, implementations do not have to be identical), but having some automatically generated diagrams from different tools for inspection could definitely be useful.

This is something I agree with :)

We do have some testing for this for our tool. In my experience, it is really hard to automate these kinds of tests. It is really hard to get the "fuzzyness" of the checking correct. The line between acceptable changes (different platform, font rendering differences, anti-aliasing, ...) and actually incorrect implementation effects, is in my experience very narrow, if it is possible to define at all. So to catch any actual errors, one has to be perhaps to strict in the check, and inspect any failures manually to decide if it is an actual problem or not. This of course leads to a very heavy maintenance burden.

casella commented 3 months ago

@maltelenz I agree that automatic verification of graphical rendering is probably not a feasible option. But I would like the future MAP-LIB CI not only to run tests on all the MSL at each commit with all tools (so the authors can compare the outcome), but also to show the graphical appearance to the library author. This way, one gets a feedback on potential portability/compatibility issues up front, rather than months or years afterwards.

@gwr69 could tell an interesting story about this 😄

HansOlsson commented 3 months ago

This is something I agree with :)

We do have some testing for this for our tool. In my experience, it is really hard to automate these kinds of tests. It is really hard to get the "fuzzyness" of the checking correct. The line between acceptable changes (different platform, font rendering differences, anti-aliasing, ...) and actually incorrect implementation effects, is in my experience very narrow, if it is possible to define at all. So to catch any actual errors, one has to be perhaps to strict in the check, and inspect any failures manually to decide if it is an actual problem or not. This of course leads to a very heavy maintenance burden.

Agreed - and we also have the same issue even for one implementation.

HansOlsson commented 3 months ago

For normal models at normal zoom I don't see a major change with using the standardized thickness - and for some cases we need a non-zero thickness, so a default of zero would be problematic.

Looking at the specific diagrams I see that the internals of components in some libraries (Modelica.Blocks, Modelica.Electrical) as well as sensors (and sometimes sources) in some other libraries are designed with lines that sort of look like connection lines - whereas some other libraries don't have that.

When there is a change:

On one hand it seems a bit odd that the "connection" line grows thinner when it enters a component.

But on the other hand one experimental feature we have had in Dymola for some time is to allow users to see the diagram instead of the icon - and obviously connection lines need to be thinner in that case (since the entire diagram is shrunk to the size of a component) - and the "connection" lines drawn in icons correspond to those lines in thickness, which gives some form of consistency (basically when we scale down a diagram to component scale things shrink - including line thickness).

HansOlsson commented 3 months ago

Phone meeting: No change.