Infinite resize loop with multiline labels (possible bug)

[Dretch]

I have found some unexpected behaviour. It might be a bug or I might just be doing things wrong 😉

For context, I am looking at changing monomer-hagrid so it only creates widgets for visible (or nearly visible) rows, to improve performance. This is tricky because rows can have arbitrary height, so it is not trivial to know how many rows will be needed to cover the visible window.

Currently the solution I have implemented does a loop of 1. add some rows to the model; 2. trigger a resize (so the rows attain their correct height; 3. If the visible window is covered, finish, otherwise go to (1).

This mostly works ok, but I have discovered an issue with multiline labels where adding a label, then resizing, then adding another label and resizing again (all in the same frame) can trigger the labels to keep requesting a resize in an infinite loop.

To Reproduce

1. stack run this modified hello-world app: monomer-label-resize-loop.zip
2. Click the "Add Label" button.
3. Observe that the app has frozen: it is now in an infinite loop.

Possible Explanations

Firstly, I might just be doing something silly with custom widgets here. I don't understand the Monomer internals deeply, so might have missed something (or misread the docs, or something).

On the other hand, there is some code here that is related (according to my debugging):

monomer/src/Monomer/Widgets/Singles/Label.hs

Line 290 in d1ddcc2

needsSndResize = mode == MultiLine && (prevTs /= ts || not prevStep)

... at runtime, in the demo, there are two labels, and on each iteration of the infinite loop an alternating label requests a resize: the timestamps remain the same but needsSndResize flip-flops and rather than just doing a second resize, each label does an infinite number of resizes. It is not entirely clear how to fix this, at the moment, though.

[fjvallarino]

This is definitely an issue! I just pushed a branch with a potencial fix. The infinite loop in your example is gone, but I guess your use case is more complex.

You can test the fix by pointing your stack.yaml to:

- git: https://github.com/fjvallarino/monomer.git
  commit: d9308b794771ab81061bade19d7534adfcfd5e45

Let me know if it works for you and it does not cause unexpected issues. Thanks!

[Dretch]

Hi @fjvallarino , thanks for looking into this.

I have tried the branch, and it does fix the issue (both on my test case and the real app) -- thanks!

However, I have noticed that it changes the label resize behaviour. When a multiline label has been wrapped, and then gets resized to be wider (so the wrapping should change or disappear completely) then the text stays wrapped rather than unwrapping. Prior to this fix, the text would "unwrap" to fill out the newly available horizontal space.

When I get some time I will put together a test case for this...

[fjvallarino]

@Dretch curious: does this happen while resizing the window only? Maybe I can replicate it with a bit more detail.

[Dretch]

@fjvallarino , after some more investigation I now think this was a bug in my code, and this fix is fine -- sorry!

My code only gave labels the width they requested, even when more width was available. When multiline labels wrap they seem to start to request the wrapped text width (i.e. width with the extra newlines) rather than the unwrapped text width: so if they only get given the width they request then they get stuck at that width.

I changed my code to give all available width, since I think this is ok in the particular situation, and this seems to work fine.

[fjvallarino]

@Dretch sorry for the long delay. Just to be sure: do you think the PR is good to merge, then?

By the way, I've been working on some improvements to the development workflow. Maybe you find it useful: #239

[Dretch]

@Dretch sorry for the long delay. Just to be sure: do you think the PR is good to merge, then?

Yup 👍

By the way, I've been working on some improvements to the development workflow. Maybe you find it useful: #239

I'll have to try it out 😄

[fjvallarino]

The PR has been merged. I'll close the issue now. Please re-open or create a new one if needed. Thanks!