issue: text layout doesn't layout lines correctly

[rksm]

https://lively-next.org/worlds/issue:%20text%20layout%20doesn't%20layout%20lines%20correctly

[merryman]

The reason for this, is that the faster line-width estimation overestimates the actual width of the text row and instructs the layout to take into account the line break (if that is turned on). Since we currently do not enforce our estimated line heights on the rendered line divs, we end up with a layout that is usually of by one line to the actual rendered html text.

A quick and dirty fix (I tried) would be to just force the line height on the rendered text by default, which will cause unusual breaks in some lines, where the line width is estimated incorrectly, yet this does not propagate and lead to much more sever issues such as off-by-one-line cursors and such.

Another idea would be to use actual measurements from the DOM as a ground truth, and "correct" the line height in the layout, once we can observe rendered elements. However that would again intertwine rendering and text logic. I do not know how acceptable that is.

[rksm]

It's actually a little more nuanced than that. We already use direct DOM measuring for small texts (<= 10 lines and < 3000 chars), for larger texts we have a mixed approach: 1. estimate line heights to "roughly" figure out the bounding boxes of lines, 2. when part of the text is rendered on the screen, use this rendering to update the estimated bounds from what is in the DOM.

A failure in the latter part is the bug here, the line isn't correctly updated from the view once it is rendered (and the estimate gives the "double" line, but this is OK, it's not meant to be 100% correct, especially with proportional texts).

[rksm]

Investigating it further. The place where we set the line width and height from the DOM is text/renderer.js AfterTextRenderHook>>updateLineHeightOfNode. This detects a a difference between the estimated and actual line height, modifies the line extent and then marks the text as dirty which causes it to re-render... and re-estimate the lines, argh. Endless loop that also hogs the CPU. That only happens with proportional fonts, word-wise line wrapping, and fixedWidth = false. I played around with not doing the line height estimation when the line already has been DOM measured but this breaks texts when they are created, in those cases the DOM returns the wrong results and we actually need the line estimation to get the text "out there". So it looks like we need to introduce more render state so we can track what line has already been reliably rendered...

[merryman]

Please take a look at my fix in 6f4634fa9067ea228fbe56b0f052ad7dceb91772. It seems the problem was actually the following: When the updateLineHeightOfNode marks the text morph as dirty, this should not cause a forced but just a soft estimation of line heights in the next cycle, where the text layout will just consider those lines that have not yet gone through a updateLineHeightOfNode call. However that line estimation in the cycle that followed turned out to always be forced, which was the actual problem, since this will make the layout "start from scratch" for all lines, since it assumes there is no recourse it can rely on. The reason why it was forced, is that a call to fitIfNeeded() (which is called in each render cycle) always triggered a change in extent, which in turn caused a forced line estimation in the layout. I added a check in onChange now, which will only trigger a forced line estimation if the extent change actually changes the width of the text morph, since that is the only thing that will influence the layout after all.