FlexLayout indeterminate behavior

[agg23]

Describe the bug
In attempting to determine the source of nodegui/react-nodegui#162 (comment), I rebuilt roughly the same widget structure that nodegui-react should have produced, seen below. This structure displays indeterminate sizing when modifying bits of the layout, just as in the React example.

To Reproduce

import { QMainWindow, QWidget } from '.';
import { QPushButton } from './lib/QtWidgets/QPushButton';
import { FlexLayout } from './lib/core/FlexLayout';

const win = new QMainWindow();
win.resize(500, 500);

const outer = new QWidget();
const outerLayout = new FlexLayout();
outerLayout.setFlexNode(outer.getFlexNode());
outer.setLayout(outerLayout);

const inner = new QWidget();
outerLayout.addWidget(inner);
const innerLayout = new FlexLayout();
innerLayout.setFlexNode(inner.getFlexNode());
inner.setLayout(innerLayout);

const button = new QPushButton();
button.setText('Toggle On');
button.addEventListener('clicked', () => {
    button.setText(button.text() === 'Toggle On' ? 'Toggle Off' : 'Toggle On');
});
innerLayout.addWidget(button);

const centralWidget = new QWidget();
const centralLayout = new FlexLayout();

centralLayout.setFlexNode(centralWidget.getFlexNode());
centralLayout.addWidget(outer);
centralWidget.setLayout(centralLayout);

win.setCentralWidget(centralWidget);
win.show();
(global as any).win = win;

Expected behavior
The layout should not vary when modifying non-layout properties of widgets.

Screenshots
Initial Render:

First Click:

After Resizing:

Desktop (please complete the following information):

• OS: macOS 10.15.5
• NodeGUI version: Latest master (0.21.0+)

[agg23]

I may be naive, but at least part of the issue appears to be due to geometry() always returning (0, 0, 0x0). This prevents the short circuit functionality when the size controlled nodes do not change size (therefore preventing recalculation of the layout children). Saving the rect elsewhere resolves this particular issue, but does not resolve some other indeterminism.

On a somewhat separate note, something needs to be done about the current layout timer. While it seems well intentioned, it can result in some jarring interactions as elements procedurally resize themselves (this is trivially viewed by expanding the timer to something large, say 1 second).

[wkirby]

@agg23 has this ever been resolved? This seems to be a pretty serious showstopper for all but the simplest layouts.

[agg23]

@wkirby Not to my knowledge. I have not worked with NodeGUI for a while though.

[wkirby]

@agg23 I’d love to work on this. Any pointers on where to start?

[wkirby]

@a7ul I think I've got an understanding of the issue being reported here, but I want to be sure I understand the intent of the code as it exists now.

My question is around this bit of code you wrote where we restore the prevStyleMinWidth and prevStyleMinHeight in the Yoga node tree.

Where I think this is causing problems is that, in the provided example, the initial render works as expected:

1. The centralLayout is size controlled, and sets its min width and height according to the provided rect
2. The centralLayout calculates its layout
3. The centralLayout walks its children and calls setGeometry
4. The outer calculates its layout, sets its rect, and walks its children
5. The inner calculates its layout, sets its rect, and walks its children
6. The whole tree has rendered, and centralLayout resets the min-width and min-height in the yoga tree by calling restoreNodeMinStyle

When the button is clicked, though, the re-render starts from the bottom up.

1. The button changes size, which triggers a resizeEvent, which triggers a setGeometry on inner
2. inner recalculates the Yoga layout; because it is not size controlled, it does not change any minimum sizes. Because after the render loop finished previously, centralLayout also has no minimum size, and the layout is calculated incorrectly.
3. When inner resizes, it triggers a resizeEvent which triggers a setGeometry on outer
4. outer recalculates the Yoga layout, also incorrectly for the same reason as inner, and walks its children
5. When outer resizes, it triggers a resizeEvent which triggers a setGeometry on centralLayout
6. centralLayout recalculates the Yoga layout — correctly, this time — but because its size is unchanged, this setGeometry does not propogate down the node tree.

The end result is that centralLayout is the right size, but inner and outer are both too narrow.

This behavior is fixed if we remove the call to restoreNodeMinStyle, but I am hesitant to do that because I don't know the original reason it was added.

Any insight you can provide would be helpful. I'd really like to knock out this problem and I feel like a solution is very close.

[a7ul]

Amazing work digging this @wkirby
There were many edge cases when I was implementing this.

And this was one of the reasons why I added that line of code (see the video attached)
Once you expand the size of the window, you can now longer reduce the size here.

Screen.Recording.2021-02-25.at.18.51.47.mov

[a7ul]

But again if we can get around this, then lets merge this change as you suggested. Thanks for the hard work!

[wkirby]

@a7ul Thanks! That's a great clarifying edge case. I think there's a way around this. I'll add your example to my test file. I'll be sure to tag you on a PR when I get one up.

[wkirby]

@a7ul I have opened PR #814 with what I believe will be a fix. It addresses both the reported issue here, and your edge case from above.

If there are other edge-cases from your prior work on this, please let me know. I'll be happy to test my changes against the PR.

[a7ul]

@wkirby
there was another edge case. This time with scrollArea

import { QMainWindow, QWidget } from '.';
import { QLabel } from './lib/QtWidgets/QLabel';
import { QScrollArea } from './lib/QtWidgets/QScrollArea';
import { FlexLayout } from './lib/core/FlexLayout';

const win = new QMainWindow();
win.resize(500, 500);

const outer = new QWidget();
outer.setInlineStyle(`border: 2px solid pink;`);
const scrollArea = new QScrollArea();
outer.setLayout(new FlexLayout());

scrollArea.setInlineStyle(`flex: 1; border: 2px solid green;`);
const sview = new QWidget();
sview.setLayout(new FlexLayout());
sview.setInlineStyle(`border:2px solid yellow;`);

const textView = new QLabel();
textView.setText(`
a
a
a
a
a
a
a
a
a
a
a
a
a
a
a
a
a
a
`);

sview.layout?.addWidget(textView);
scrollArea.setWidgetResizable(true);
scrollArea.setWidget(sview);
outer.layout?.addWidget(scrollArea);
win.setCentralWidget(outer);
win.show();
(global as any).win = win;

// <Window windowTitle="Hello 👋🏽" minSize={minSize} styleSheet={styleSheet}>
// <View>
//   <ScrollArea style="flex: 1; border: 1px solid green;">
//     <View style="border:2px solid yellow;">
//       <Text>
//            ... long text ...
//       </Text>
//     </View>
//   </ScrollArea>
// </View>
// </Window>

The scrollArea doesnt allow scrolling since the inner view takes the same size as that of scrollArea.
See reference issue: nodegui/react-nodegui#341 (comment)

[KRTirtho]

Will this issue get fixed? It's the only reason why I can't bump into newer versions of nodgui & react-nodegui...