Reset Label's cached size hint on Resize

[cceckman]

#19 introduced caching of the SizeHint for the Label type. This is good for performance (thanks @yml!)

As I was writing a custom Widget, I ran into an issue around this - namely, that the SizeHint remains cached until a SetText, even when there's a Resize. This is an issue when the label is set to word-wrap - given a specified width, the hint changes its height.

At least, it's an issue in the widget I implemented, which is included (in the test) in its entirety - I can totally believe a response of "you're doing it wrong," especially since I couldn't really find a smaller example. But in principle, it seems like the cache should be invalidated on Resize, since that's one of the inputs.

There are two hard problems in computer science: cache invalidation, naming, and off-by-one errors.

[marcusolsson]

Interesting! I believe that would effectively disable caching for the current implementation since I believe Resize is called every repaint. Like you've discovered, Label needs to calculate the size hint on every resize. I did some reading on https://www.finseth.com/craft/ when I implemented the RuneBuffer and I think it would be wise to have a better separation of the text view and text content. Right now we draw everything and then mask it away.

Also, separating the view and the content would make it feasable to load large files and maybe even remotely(?).

[cceckman]

Hm - you're right, Resize is called on every Repaint. Some alternatives:

• Calling l.SetText(l.Text()) to force a reeval would work... but that's a hack, not a fix; it relies on the implementation details of caching.

• The logic on when the cache invalidation happens could be more precise: "If word wrap is set, and the width changes, invalidate the cache." I'm wary of making cache invalidation logic more clever, though - the cleverer it is, the more likely it is to be wrong. :-)

• Other suggestions?

#14 / #19 mentioned profiling pointing to the resize operation as expensive- are there any benchmarks that would measure the impact of this / related / alternative changes?

---

"Separation of text view and text content" --> A traditional web principle, and a desirable ends.

But in any marginally responsive (#73) layout, there's some amount of interplay. The size hint of a text display is always, necessarily, based on its content: "How much space would it take to display the entire contents?" If the text display has word wrap, the Y hint is also based on a constrained / current X. I don't see how to decouple these entirely.

---

I have a bunch more thoughts on "rendering large files" / "rendering lots of contents" - I copied them up to a separate discussion in #86.

[cceckman]

OK; 028217b has the more restrictive version, of "only reset cache if width changed + wordwrap enabled."

[marcusolsson]

There's definitely more we can do here but I think your solution is the cleanest one right now. I'm actually thinking whether it can be moved entirely to WidgetBase so that all widgets benefit from this?

[cceckman]

Agree that having a good cacheing story could probably improve performance; not sure it's worth it to move to WidgetBase up on its own, without more refactoring. Cache invalidation logic would be different for each Widget, as would computation of the hint- the only thing that would be shared would be the field itself.