CSSTUDIO-3325 Bugfix: Make labels of Byte Monitor widget visible when placed alongside a Label widget inside of a tab of a Navigation Tabs widget

[abrahamwolk]

This pull request fixes a bug that can be reproduced as follows:

1. Create an OPI ("OPI A") containing: a Label widget and a Byte Monitor widget with labels. It is important that the Label widget is placed into the OPI before the Byte Monitor widget is placed into the OPI.
2. Create a second OPI ("OPI B") containing a Navigation Tabs widget, one tab of which embeds OPI A.

The bug is: when running OPI B, the labels of the Byte Array widget are not visible when opening the tab that contains OPI A.

I don't have an explanation for this bug. Through testing, I have determined that this bug was introduced in #3354. In particular, it is the call jfx_node.setMinSize(Control.USE_PREF_SIZE, Control.USE_PREF_SIZE) in LabelRepresentation.updateChanges() that causes the issue: the bug doesn't occur when commenting out this call.

Since the goal of the pull request #3354 was to prevent the Label widget from increasing its size when the font was larger than the widget, the call to jfx_node.setMinSize() is actually unnecessary to implement the goal: the call to jfx_node.setMaxSize() already implements this goal. Therefore, I propose to work around this bug by simply removing the call to jfx_node.setMinSize(). Do you think this is acceptable, @rjwills28? (Unfortunately, I cannot assign you as a reviewer to this pull request for some reason, @rjwills28.)

[kasemir]

I don't have an explanation for this bug.
Through testing, I have determined .. setMinSize() .. setMaxSize()

I don't have a perfect explanation and solution, but here's the history:

Way back, profiling showed a lot of CPU activity in Javafx layout computations. Changing our widgets to setManaged(false) and then directly setting their x/y position, width and height dramatically reduced the CPU load.

Good news is that we know exactly where the widgets should be positioned. Our Widget has a fixed X, Y, Width, Height.

But many JavaFX widgets don't have an API to set the size. You can call setMinSize() and/or setMaxSize(), there is no setSize, setWidth, setHeight. So in a somewhat experimental mode we did just what you did now, checking which "...Min...", "...Max...", "...Preferred..." call gets us what we need.
In some cases, we had to add calls to javafxwidget.layout() to force JavaFX to figure it out that setMinWidth(100); setMaxWidth(100);layout() might mean: We simply want a widths of 100!!
In other cases, we gave up and went back to the default "Managed" mode.

[rjwills28]

Happy with this if removing that line fixes the issue!

I'm just interested into what's actually going on but I'm having trouble reproducing the issue myself. Could you take a quick look over my set up to see what I'm doing wrong?

I have opia with label and byte monitor with labels. The label was added first and is first in the navigation tree:

Then opib has a navigation tab like so which does show the labels?

[rjwills28]

Ah, I've been able to reproduce it now - needed the Byte Monitor not to be square.

Interestingly the Control.USE_PREF_SIZE that is used to set the min and max size is coming back as -Infinity which might explain why it isn't showing. Perhaps that means this variable isn't actually getting set? Alternative would be to check the value for this property before setting the min/max width but given that we aren't actually concerned about setting the minWidth I'm happy with this fix.
Thanks for catching this!

[abrahamwolk]

Interestingly the Control.USE_PREF_SIZE that is used to set the min and max size is coming back as -Infinity which might explain why it isn't showing. Perhaps that means this variable isn't actually getting set? Alternative would be to check the value for this property before setting the min/max width but given that we aren't actually concerned about setting the minWidth I'm happy with this fix.

I think that the use of -Infinity is an (ad-hoc) solution in the JavaFX library to the fact that the function setMinSize() accepts either a number of type double or a setting such as, e.g., Control.USE_PREF_SIZE, and the choice was made to represent settings such as Control.USE_PREF_SIZE (that are not numbers) as values of type double that are not reasonably interpreted as sizes such as, e.g., -Infinity. Therefore, I don't think it's the fact that the size is set to -Infinity that is the underlying problem, since -Infinity in this case does not represent negative infinity, but instead represents the fact that the preferred size should be used.

[rjwills28]

Oh dear - this has actually undone the fix for #3344...

Before the PR the label widget would remain the size you set even if this cropped the text

But now the label height property is getting ignored and the label is being resize to the text fontsize:

It appears we need both setMaxSize() and setMinSize() to be set to make the specified size mandatory.

Shall I open a new issue to discuss how we can solve this contention between issues?

[abrahamwolk]

@rjwills28 That's unfortunate! (And, I think, unexpected!) Yes, please open an issue. It should definitely be possible to both have fixed label heights as well as visible labels in the Byte Monitor widget.