Network: nodes.widthConstraint: similar min/max values cause infinite loading loop

[cityy]

Creating a network with similar minimum/maximum values for nodes.widthConstraint causes an infinite loading loop. I stumbled across the issue trying to set circle shape nodes to all have the same size.

The following fails to load:

var options = {
nodes:{
  widthConstraint:
     { minimum: 150, maximum: 170,}
   }
};

network = new vis.Network(container, data, options);

I've been able to reproduce the issue by simply modifying the Network Node Shapes example: http://visjs.org/examples/network/nodeStyles/shapes.html

It's really weirdly breaking if min/max values get too close to each other:

{ minimum: 150, maximum: 150,} // fails
{ minimum: 150, maximum: 175,} // fails
{ minimum: 150, maximum: 176,} // works
{ minimum: 150, maximum: 190,} // works 

Update: Just a note. The exact values that cause it to fail differ between my current project and the node shapes example.

[wimrijnders]

Got a fix on the infinite loop. It's in Label._processLabel() roundabout line 910.

In a nutshell:

            while (w < words.length) {
...
              if ((measure.width > this.fontOptions.maxWdt) && (lastMeasure.width != 0)) {
                // Do things here, variables in if() don't change
                // w not incremented!
              } else {
                // Do other things here
                w++;
              }
            }

A situation occurs that the if-condition always holds and thus is always entered. Variable w is not incremented and infinity ensues. It seems to happen at the end of a label text where the loop processes an empty string.

@cityy Great catch, and thanks for supplying instructions to reproduce. Here, have a virtual beer on me 🍺.

[cityy]

@wimrijnders My pleasure!

[satyajitsinh]

I also fond similar behavior.
Label._processLabel() also causes an infinite loading loop in below condition.
I had a network with node shape 'box' and applied widthConstraint.

1. If I apply large label with only one word box will expand breaking widthConstraint.maximum.
2. If I apply large label with two words where first word is smaller and second is large then network will go in infinite loading loop

same thing is happening w++ is never getting executed for second word.

[wimrijnders]

@satyajitsinh:

1. confirmed, although I really don't know what to do about this. Seems to me that this behavior is the best option. Do you have a better idea perhaps?

2. Confirmed. This only happens when widthContraint is enabled as an object. Also I see no direct relation with the first word being smaller than the second. It appears to me that the length of any of the words is the cause of the infinite loop.

[wimrijnders]

Letting you know that I'm working on this.

It's a really tough issue to solve, even though the problem appears simple when asked. The thing is, it's pretty convoluted code, and I find myself cleaning it up and refactoring just to understand it. I also intend to add some unit tests to be sure that all is working as expected.

So please bear with me; this is one that I can not fix trivially.

[wimrijnders]

In the original test case, the problem is with the text 'big triangleDown'.
Let's see if I can explain what's happening:

In Label._processLabel():

1. The label words are split into lines
2. Per line, an accumulated string variable is used to check if max width has been crossed
3. The text 'big triangleDown' is too big, so the first line becomes just 'big'.
4. The processing continues with 'triangleDown', the accumulation variable is made empty.
5. 'triangleDown' is already bigger than the max width.
6. So, 3. is done again, the accumulation variable, an empty string, is put on a new line,
7. ...and processing continues with the next word, which is again 'triangleDown'. In effect, execution jumps back to 4. Hence, infinite loop.

---

What to do about this:

I think the best strategy here is, if a single-word line is already too big, add it anyway. This is in effect @satyajitsinh's observation 1., but for multiple-word labels. It does mean that the max width can be broken.

Feel free to add feedback for this!

[cityy]

Just thinking about alternatives, maybe one would be this:

Trimming words like 'triangleD..' and overgo max width when the node is selected so it displays the full word only when the node is selected.
Alternatively, maybe label width and node width should just be different things (like size for the circularImage shape for instance). Or it should be an option to enable size for the circle shape compared to inheriting width from label. I recognize this would be quite a heavy intervention but I'm not sure if I wouldn't prefer that for the sake of consistency.

For example: I'm rendering a circularImage graph where I'm changing shape to circle on low zoom values (when a lot of the graph is drawn) for the sake of performance. That currently requires me to set widthConstraint for circle as well as size for circularImage in order to keep the node widths consistent.

[wimrijnders]

It sounds very sensible, although the example is not fully clear to me. Would you care to make it more specific?

I can image alternative approaches to label handling to be options. So, the current label drawing is the default, but what you describe can be selected if required.

[wimrijnders]

This is to notify you I have a fix ready for this issue.

However, due to its extensive changes, I am going to publish it in a PR after the upcoming next release, which is planned for june 24th '17. This is to not unnecessarily burden the maintainers making the release. I hope you have understanding for this.

Update 2017-07-03: The new version vis.js 4.20.1 has just been released. Hence, I have submitted the PR to fix this issue.

[satyajitsinh]

@wimrijnders thanks for your great support.
I agree with @cityy 'triangleD..' will be a good option. And with @wimrijnders's suggestion 'label handling to be options' completely make sense.

[wimrijnders]

@satyajitsinh thank you for your support!

I should mention that, after giving it thought, I came to the conclusion that words larger than max width should be broken into multiple lines. Please see the discussion in #3185. Breaking up the big words is going to be part of the PR that fixes this issue.

Options for label handling will be something for the future; I haven't changed my mind about that, I just want to get this and other essential stuff done first.

[cityy]

@wimrijnders:

It sounds very sensible, although the example is not fully clear to me. Would you care to make it more specific?

Sorry for the late reply, I was a bit occupied with other things. Maybe the particular example is not so important. I generally find it inconsistent that the way to control the size of nodes depends on what shape the node has. IMO; it would be more elegant to have size management standardized and offer options like "inherit from label size".
Maybe this matter should be put into a new issue?!

[wimrijnders]

@cityy OK, so let's enumerate the options here. This is what I can think of now and which looks useful:

• Adjust nodes to label size - how it works now
• Break on max width - labels are broken to next lines, max width enforced
• Crop on max width - lines in label are trimmed to max width
• Crop on max height - max height enforced, lines over height not displayed. Can be combined with one of the two previous options.
• Fixed size of nodes - width and height the same for all nodes, labels are cropped to fit into this

Feel free to make your own suggestions.

I generally find it inconsistent that the way to control the size of nodes depends on what shape the node has.

I think you mean that e.g. option node.size works only for shape objects and not, for example, image nodes and database. Correct? I have some sympathy for this.

[cityy]

size:

The size is used to determine the size of node shapes that do not have the label inside of them. These shapes are: image, circularImage, diamond, dot, star, triangle, triangleDown, square and icon
http://visjs.org/docs/network/nodes.html

That's the behaviour I am referring to. Size should set the size, no matter what shape, for the sake of consistency. That way widthConstraint would become obsolete (there could still be something like min/max-size if required). Inheriting size from Label should be an option that can be true by default and considers min- / max-size.

[wimrijnders]

@cityy I find that a very reasonable question; would you mind making an issue for that? It's a bit out of the scope of current issue. Please refer to this issue.

[AndrewMut]

Is there any way to know when will be released new version with this fix?
Thank you

[wimrijnders]

@AndrewMut Next release is in about two weeks time; but please don't take that literally, the dates are flexible and depend on the availability of the releasers.