alexheretic / glyph-brush

Fast GPU cached text rendering
Apache License 2.0
688 stars 52 forks source link

Do not use word-wrapping in single-line mode #116

Closed dhardy closed 4 years ago

dhardy commented 4 years ago

Currently, when word-wrapping is disabled, glyph-brush-layout will omit any word after the first which can only partially appear on the line (as is usually the case with line-wrapping). This PR changes that to include the next full word. It could maybe be better (omit letters which are fully outside the bounds, perhaps a nicer way of drawing cut-off text).

Rationale: when writing text into a single-line edit box, normally one does not expect a word to be hidden because it doesn't fully fit on the line. This doesn't fix everything (e.g. horizontal scrolling within the edit box), but is an improvement. (One can test with KAS 0.4 using the gallery example.)

Two tests fail; I wasn't sure how best to fix these:

---- builtin::layout_test::single_line_limited_horizontal_room stdout ----
thread 'builtin::layout_test::single_line_limited_horizontal_room' panicked at 'assertion failed: `(left == right)`
  left: `"hell"`,
 right: `"hello"`', layout/src/builtin.rs:664:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- builtin::layout_test::wrap_word_left stdout ----
thread 'builtin::layout_test::wrap_word_left' panicked at 'assertion failed: `(left == right)`
  left: `"hello "`,
 right: `"hello what\'s _happening_☐"`', layout/src/builtin.rs:615:9
alexheretic commented 4 years ago

I would have thought this the job of the LineBreaker. If you want all characters to remain on one line always, then never return any soft or hard breaks.

#[derive(Clone, Copy, Debug, Hash)]
struct NeverLineBreaker;
impl LineBreaker for NeverLineBreaker {
    fn line_breaks<'a>(&self, text: &'a str) -> Box<dyn Iterator<Item = LineBreak> + 'a> {
        Box::new(std::iter::empty())
    }
}
let custom_layout = Layout::default().line_breaker(NeverLineBreaker);
dhardy commented 4 years ago

If you say so. Then it might make sense to add NeverLineBreaker and to de-unionise the BuiltInLineBreaker, or even to move the wrap-mode into a method on the LineBreaker trait.

Do you still want the first commit here?

alexheretic commented 4 years ago

We could add that impl to the BuiltInLineBreaker enum if it's useful.

Do you still want the first commit here?

Yes please.

dhardy commented 4 years ago

The design seems over-complicated to me. Anyway, take that first commit...

alexheretic commented 4 years ago

Yeah fair enough. I guess there are times when you want a single line with no overlapping out-of-bounds. Really the line breaker has one job. The single-line/wrap is just about the special case of wanting a single line, you may still want to break normally, or on every char, or perhaps not at all.

The API is quite generic meaning you can change stuff outside the crate, but it also makes it more complicated. Ultimately I don't understand many use cases that well, so the ability to change bits was meant to help there.

If you'd like to improve the API raise an issue with how you'd like it to work. I'm sure there are many ways we can improve it.

dhardy commented 4 years ago

Hmm, I don't have any specific recommendations now, though I've been playing with the code in kas-text. A couple of things do strike me though:

Anyway, these are just my thoughts; I'm probably going to bypass glyph-brush-layout for kas-text.