Potential Reflow Optimization

[pvande]

As I'm working with the resize/reflow functionality here, I'm wondering if there isn't a way to make this both more performant and a bit simpler…

Presently, that reflow behavior is wrapped up in these methods:

module Input
  class Multiline
    def w=(val)
      @w = val
      reflow!
    end

    def size_enum=(val)
      @font_style = FontStyle.from(word_chars: @word_chars.keys, font: @font_style.font, size_enum: val)
      reflow!
      @font_height = @font_style.font_height
    end

    def size_px=(val)
      @font_style = FontStyle.from(word_chars: @word_chars.keys, font: @font_style.font, size_px: val)
      reflow!
      @font_height = @font_style.font_height
    end

    def reflow!
      @ensure_line_visible = @value.lines.length - ((@scroll_y + @h) / @font_height).floor
      @value = MultilineValue.new(@value.to_s, @word_wrap_chars, @crlf_chars, @w, font_style: @font_style)
    end
  end
end

This ensures that any time the width or font size changes, a new MultilineValue is created, [soft] wrapped at the appropriate places.

The first point of improvement I would suggest is that assignments that don't change value shouldn't reflow. Not only does reassigning the same width have negative performance implications, but it also introduces [what appear as] display bugs, with the content never "resolving" into the correct position. (See attached screenshot, where the multiline input occupies the expected space, but the first line renders at the bottom.)

The second thought is that a new MutilineValue is created on reflow, because the Multiline input is considered "responsible" for managing the line wrapping. This seems a little strange, since the reason the MultilineValue exists seems to be managing line wrapping… This leads me to wonder if a patterning like this might make sense:

module Input
  class Multiline
    # Original definitions of #w=, #size_enum=, #size_px=

    def tick
      @value.reflow(@w, @font_style)
      super
    end
  end

  class MultilineValue
    def reflow(w, font_style)
      return if @w == w && @font_style == font_style
      @w = w
      @font_style = font_style

      # Note that a similar refactoring could be made to the `LineParser`, `FontStyle`,
      # and/or to the `LineCollection` to make this do even fewer allocations in the future.
      @line_parser = LineParser.new(@word_wrap_chars, @crlf_chars, font_style: font_style)
      @lines = @line_parser.perform_word_wrap(@lines.text, w)
    end
  end
end

On the subject of "why does this matter", I think there are a couple of reasons. Firstly is that while writing input.w = 400 inside a tick method is unorthodox, the behavior shouldn't (read: doesn't need to) be pathological; there's a perfectly reasonable interpretation of that code which doesn't come with the performance drawbacks. Secondly, it makes using the library friendlier overall — while consumers can track previous values for w and font_size, then conditionally assign them on update, it's not a core part of their application's logic (read: it distracts from their goals) and they have to know that they need to take care about that detail of the library's implementation (counterpoint: the library authors know how different values will behave, and can remove that burden from consumers with minimal cost).

[marcheiligers]

Agreed in every way possible.

[marcheiligers]

I didn't follow your suggestions exactly, but this optimization has been released with #25 (v0.0.19)