Inconsistent spacing between arcs

[GoogleCodeExporter]

What steps will reproduce the problem?

Difference between the two MSC's below (Note the change in order of the arcs in 
the following MSC's).

msc {
    a, b;
    a => b [label="Single line"];
    a => b [label="Two\nline"];
}

msc {
    a, b;
    a => b [label="Two\nline"];
    a => b [label="Single line"];
}

What is the expected output? What do you see instead?

It is expected that spacing between arcs is consistent, but it is not. There is 
space between arcs in first msc and no space in second msc. Also single line of 
text is NOT centered vertically, whereas multi-line text is centered vertically.

Analysis:

Initially mscgen was designed to have constant arc height. But later when 
multi-line text issue is fixed, arc height became variable, subjected to a 
minimum height. This minimum height (arc spacing parameter in code) was 
probably a requirement to retain the legacy view and spacing of msc's. It is 
the source of the current issue.

Possible solution:

One way to fix the problem is to have a consistent/constant spacing between 
variable-height arcs. By default we can have a non-zero spacing between arcs. 
Remove the minimum height parameter in code and introduce the 
SpacingBetweenArcs parameter. This spacing between arcs parameter shall be 
controlled through as an msc attribute.

Using ||| is not a solution, because it is explicitly needed between every 
multi-line arc or needed whereever there is a box and also spacing is still 
inconsistent and NOT controllable.

With SpacingBetweenArcs parameter introduced, |||, ... can add more space 
between arcs i.e. they serve as additional space arc. This additional space 
amount introduced by ||| and ... can also be an attribute.

Original issue reported on code.google.com by v.arunmo...@gmail.com on 10 Sep 2010 at 9:38

• Blocked on: #50

[GoogleCodeExporter]

I've checked this, and the output I get is consistent with how it's always been 
and is by design.

For a single row of text output, the text is just a above the arc line to 
prevent the line type (dotted, dashed, double etc..) being obscured.  To keep 
multiple lines more compact and keep regular spacing of the lines (according to 
the minimum height), a second row of text is then under the arc line.

In the case that >2 lines of text are used, then the strategy changes as more 
space is needed between the arc lines to accommodate the text, so the line 
becomes vertically centred on text.

So basically the arc line spacing will always be regular if each arc has <=2 
rows of text.  If you go above that, the spacing will expand as required.

So I don't think this is really a bug as such - just that you'd like it to 
behave differently when using multi-row text?  If that's the case, please 
supply a an image of the problematic output (with markup if you like) and mark 
this bug an enhancement.

Original comment by Michael....@gmail.com on 14 Sep 2010 at 9:23

Attachments:

• b1.msc.png
• b2.msc.png

[GoogleCodeExporter]

The main problem according to this bug is that there is no vertical gap between 
the last line of text of previous arc and the first line of text of next arc. 
If the previous arc has one line of text, there is enough vertical gap to the 
next arc. If the previous arc has two lines of text, it looks as if the 
vertical gap to the next arc is much smaller than in the first case. If the 
previous arc has >2 lines of text, there seems to be no gap at all. Here the 
term gap implies vertical white space.

I would like to have a constant look and feel of arcs regardless of number of 
lines of text and also I would like to avoid introducing ||| unnecessarily.

Regarding alignment of text vertically, all the line arcs can have the text 
completely above the line regardless of single or multi-line text (As you told 
this will NOT obscure the line).  For a box arc, the text can be within the box 
as it is now.

Regarding white space between arcs, every arc should be followed by 
configurable amount of white space. As we can see from the attached pdf, every 
arc is followed by a 15pt vertical space. Attached pdf is the desired ouput of 
the following msc.

msc {
    A [label="Entity A\n(Entity Info)"], B [label="Entity B"];
    A => B [label="Single Line Text"];
    A << B [label="Two Line\nText"];
    A <<= B [label="Three\nLine\nText"];
    --- [label="Multi\nLine\nText\nin\nDivider"];
    A box B [label="Single-line inside box"];
    A abox B [label="This\nis\nmulti-line\ninside\nbox"];
    ... [label="Space generated by ..."];
    A -> B [label="Four\nLines\nof\nText"];
    ||| [label="Space generated by |||"];
}

Original comment by arunmozh...@gmail.com on 17 Sep 2010 at 5:31

Attachments:

• ConstantWhiteSpace.pdf
• ConstantWhiteSpace.png

[GoogleCodeExporter]

Attached png is what is currently generated by mscgen

Original comment by arunmozh...@gmail.com on 17 Sep 2010 at 5:36

[GoogleCodeExporter]

Is there a way to change this bug to enhancement. I am using firefox and I do 
not find an option to do the same :-(. Please change this bug to enhancement.

Original comment by arunmozh...@gmail.com on 17 Sep 2010 at 5:55

[GoogleCodeExporter]

Original comment by NThykier@gmail.com on 17 Sep 2010 at 11:13

• Added labels: OpSys-All, Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

I can point to some location in code which might be the source of the issue. In 
the codebase of version 0.18, lines 596 and 1469 has the following code

ymax = ymin + gOpts.arcSpacing;

if(arcLabelLines > 1)
{
    ymax += (arcLabelLines - 2) * textHeight;
}

I am doubting that "arcLabelLines - 2" is probably some hack to center the text 
vertically. Probably this should be "arcLabelLines - 1", because for the first 
line of text gOpts.arcSpacing has been added and for each further line 
textHeight should be added. "arcLabelLines - 2" implies we are removing 
vertical white space of one textHeight provided by gOpts.arcSpacing.

I do not have Cygwin or linux to verify if this is the case. May be someone can 
try this out.

Original comment by arunmozh...@gmail.com on 17 Sep 2010 at 3:35

[GoogleCodeExporter]

Here is the output from changing arcLabelLines - 2 into arcLabelLines -1.

Note that this also affects test7; I have attached those as well.

Original comment by NThykier@gmail.com on 17 Sep 2010 at 4:15

Attachments:

• test19.png
• test19-patched.png
• testinput7.msc.png
• test7-patched.png

[GoogleCodeExporter]

Hmm... at a first glance it looks like -2 for boxes and -1 for others might 
make sense. Anyhow this appears very related to issue 50, so marking this 
dependent on 50.

Original comment by NThykier@gmail.com on 17 Sep 2010 at 4:37

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Hi NThykier, Thanks for the images. With these images, we can conclude that 
"arcLabelLines - 2" is incorrect.

Assuming we change it to "arcLabelLines - 1",

    Total Vertical Space allocated for an arc = gOpts.arcSpacing + ((Number Of Lines of Text - 1) * textHeight)

    Out of this vertical space, White Space available = gOpts.arcSpacing - textHeight - any line thickness of line arc or box arc

    The following are the observations

    (1) Every line arc is centered within the vertical space allocated for the arc.

    (2) Every box arc is drawn occupying the complete vertical space

    (3) Text is always drawn top aligned except for few cases where it is shifted down a little to give an impression of center alignment. This hack is at line 803 in main.c (y += textHeight / 2)

    (4) Divider text looks vertically centered because the divider line is drawn only when rendering middle line of the text.

The following changes are needed to keep the spacing and alignment consistent

    (1) Meaning of gOpts.arcSpacing configuration is not consistent in code currently. Instead a more appropriate configuration gOpts.verticalGap can be used. With this change,

    Total Vertical Space allocated for an arc = gOpts.verticalGap + (Number Of Lines of Text * textHeight) + any line thickness of line arc or box arc

    Out of this vertical space, White Space available = gOpts.verticalGap

    (2) For line arcs, text can be above the line. So the vertical components of line arc are Single/Multi-line Text followed by line followed by white space (gOpts.verticalGap).

    (3) For divider, text can be vertically centered. So the vertical components of divider are Single/Multi-line Text followed by white space (gOpts.verticalGap). Divider line is drawn in the vertical middle of text as it is now and so it does not occupy any additional vertical space.

    (4) Space provided by ... & ||| has the text vertically centered. In this case the total vertical space occupied will be MAX(Total Text Height, gOpts.minimumSpace) + gOpts.verticalGap.

    (5) For box arcs, text will be within box. So the vertical components of box arc are top line of box followed by Single/Multi-line Text followed by bottom line of box followed by white space (gOpts.verticalGap).

    (6) In case of curved line arc, text is left/right aligned with the  top of the text aligned to the top of the arc. In this case the total vertical space occupied will be MAX(Total Text Height, ellipse minor axis length) + gOpts.verticalGap.

Original comment by arunmozh...@gmail.com on 18 Sep 2010 at 9:35

[GoogleCodeExporter]

Original comment by Michael....@gmail.com on 4 Oct 2010 at 8:59

• Changed state: Accepted

[GoogleCodeExporter]

I can give patch for the problems 44, 46, 48 and 50 in one or two days for you 
to verify.

Original comment by arunmozh...@gmail.com on 7 Oct 2010 at 3:01

[GoogleCodeExporter]

SVN patch containing the fix for the issues 44, 46, 48, 50 attached.

Changes Summary

(1) A bottom-up level iterator for MSC has been added. This is required for the 
calculation of size of levels bottom-up. This approach takes care of arc skips.

(2) Function to calculate the arc span along Y direction is added. There are 
mainly two output values of the function: span above mid and span below mid. 
These two variables allows to align parallel arcs along their middles.

(3) Computation of canvas size modified and information related to each level 
(size of each level) is stored during the computation. This is required for arc 
skip computation.

(4) Line/Box/Tex drawing functions do not change much. Only the parameters with 
which they are invoked changes in main().

Original comment by arunmozh...@gmail.com on 8 Oct 2010 at 1:41

Attachments:

• Fix44_46_48_50.patch

[GoogleCodeExporter]

Same patch merged with the current SVN head revision as there were some 
conflicts.

Original comment by v.arunmo...@gmail.com on 12 Oct 2010 at 2:28

Attachments:

• Fix44_46_48_50.patch

[GoogleCodeExporter]

Thanks for the patch.

I took a look at it, and took some of the ideas for issue #50 which is now 
fixed.  The code produces a lookup of row heights (or 'shelves' in your 
parlance) which simplifies the layout one the 2nd pass.

These issues are still open however!

Original comment by Michael....@gmail.com on 17 Oct 2010 at 10:32

[GoogleCodeExporter]

I do not understand what you mean by issues are still open. Did you apply this 
patch correctly? I have attached the pictures taken after the attached patch 
got applied.

If you find any issues, please let me know.

Original comment by v.arunmo...@gmail.com on 18 Oct 2010 at 2:28

Attachments:

• issue44.msc.png
• issue46.msc.png
• issue50.msc.png

[GoogleCodeExporter]

Sorry - to clarify, I've not taken the patch.  Instead I fixed issue 50 in a 
different way, and will subsequently work on the remaining open issues.

Original comment by Michael....@gmail.com on 19 Oct 2010 at 8:03

[GoogleCodeExporter]

Not a problem. Hope u will come to the same logic used in the patch ;-). Just 
that I wanted to make sure that my expectations are met.

Original comment by arunmozh...@gmail.com on 20 Oct 2010 at 1:31

[GoogleCodeExporter]

This issue was closed by revision r161.

Original comment by Michael....@gmail.com on 26 Oct 2010 at 8:17

• Changed state: Fixed

[GoogleCodeExporter]

Thanks Michael for fixing the issue. Some minor issues still remain.

(1) The problem mentioned in the first msc's of the bug still remain (See the 
initially attached b1.msc.png  & b2.msc.png). That is, Space is more following 
a line arc with single line of text.

    Don't you think the following code is a hack to make sure enough space is available above a line arc with a single line of text? (See the 2 in the code below). It introduces more space below the line arc with single line of text.

    main.c:870: ymax = ymin + gOpts.arcSpacing;
    main.c:871: ymax += (M_Max(rowHeight[row].maxTextLines, 2) * textHeight);

    The problem will be much more obvious if a font of higher size is used.

(2) What is the necessity to vertically center the text of line arcs having 
text more than 2 lines? You had already commented that this will obscure the 
arc itself. Hope you did not think that there is a compatibility issue. Nobody 
cares where label is positioned, as there is no explicit control/documentation 
given in previous revisions.

The basic issue is that you have aligned the parallel arcs assuming that the 
span is equal above and below the mid line. It is required to consider these 
two spans different as mentioned in point 2 of the patch above. Not only it 
solves the above problems, it will help later when labels of arc skipped line 
arcs is positioned and when introducing new arcs.

You already seemed to have introduced the required variables, ymin, ymid and 
ymax. Just that the ymid need not be (ymin + ymax)/2.

Original comment by arunmozh...@gmail.com on 27 Oct 2010 at 2:30

[GoogleCodeExporter]

Hi arunmozhi.v,

There's no hacks ;)

The spacing is as per the original design whereby each arc always has space for 
1 line of text above the arc line and 1 line below it.  This keeps the spacing 
regular in the case that each arc has less than 3 lines of text.  If an arc has 
more than 2 lines of text, those lines get vertically centered on the line.

This harks back to the original mscgen which only allowed 1 line of text per 
arc.

The change now is that there is also a small gap left between each row to 
prevent lines of text on the bottom of one arc rolling into the first line of 
the next arc.  This is present very consistently, whether using arcs or boxes.

> (2) What is the necessity to vertically center the text of line arcs having 
text more than 2 lines?

Given an even number of lines, you get half above and half below the arc.  I 
think that's neater and uses less space than putting them all above the arc.

> Nobody cares where label is positioned,

So did you check with everybody ;-D

Original comment by Michael....@gmail.com on 27 Oct 2010 at 7:07

[GoogleCodeExporter]

Hi Michael, I do not agree with the explanation you have given. There is 
clearly a problem that you do not want to appreciate. The problem is that, the 
text obscures the arc, if vertically centered. If arc line is not visible, what 
is the use of text (See the output of testinput17.msc)? Does the Mscgen has 
responsibility to make sure there is no obscuring or overlapping? By default 
Mscgen should make sure that the arcs are not obscured and if required, there 
should be a valign=<top or center or bottom>, halign=<left or center or right> 
and textpos=<0 to 1> attribute which the user can use to position the text at 
his will. It is the user who should decide which positioning will make the MSC 
neater, and the tool should provide options. It is also the responsibility of 
tool to make it as much compact as possible, but not at the cost of something 
else.

Secondly, there is no reason to make sure that the spacing is regular when the 
the number of lines of text is below 2 and irregular when above 2 lines of 
text, just because it was like this before. You see it as a regular spacing, 
but I see it as irregular vertical gaps. If required, this spacing regularity 
might be provided through a separate user configurable variable e.g. Minimum 
arc height and by default it can be configured to have the existing behavior. 

> So did you check with everybody ;-D

I checked with your documentation which everybody reads i.e. I meant the change 
did not conflict with the documentation in any way.

Only by making sure that there are two variables (above span and below span), 
it is possible to provide any kind of vertical alignment and that is the 
difference between the patch and you design. I do not want to interfere in your 
design decisions, but just wanted to rationalize what I have done. It should be 
accepted that it is required for solving other problems and future enhancements 
(e.g. inline expression/frames, arc skips/gradients etc.).

Original comment by arunmozh...@gmail.com on 28 Oct 2010 at 4:47

[GoogleCodeExporter]

Thanks for your comments, and your contributions.

I think we have a difference in vision here as I'm happy with the changes I've 
made and don't feel the need to make further alterations in this area at 
present.  Most productive now is probably to mop up any regressions and corner 
cases and then push for the next release.

Original comment by Michael....@gmail.com on 2 Nov 2010 at 10:11