mooman219 / fontdue

The fastest font renderer in the world, written in pure rust.
Apache License 2.0
1.44k stars 72 forks source link

Add support for inspecting line positions #64

Closed avl closed 2 years ago

avl commented 3 years ago

This PR makes it possible for fontdue clients to determine the position of lines of text.

I needed information about the base line Y-coordinate in order to position the caret in an editable textbox.

I'm not sure this fits with future plans for fontdue, so feel free to just junk this if it doesn't :-).

But it was useful for me, and possibly someone else, so I thought I'd create a PR just in case.

Also, I think there are many ways in which this information could be exposed. If the approach I've implemented here doesn't fit for fontdue, I'd be happy to make adjustments.

mooman219 commented 3 years ago

I think this is pretty reasonable, let me pull it locally and see how it feels

therocode commented 2 years ago

I encountered the exact same use-case. I need interactable text, not just the caret but rectangles for the blue regions of selected text as well as being able to mouse-click characters. All of this requires line awareness. Given a Layout, which chars ended up on which line, what are those lines' x/y/w/h/ etc. This PR looks reasonable and I'll try it out and report back how it worked for my use case.

mooman219 commented 2 years ago

I encountered the exact same use-case. I need interactable text, not just the caret but rectangles for the blue regions of selected text as well as being able to mouse-click characters. All of this requires line awareness. Given a Layout, which chars ended up on which line, what are those lines' x/y/w/h/ etc. This PR looks reasonable and I'll try it out and report back how it worked for my use case.

I'm not totally happy with this PR, and I started experimenting with exposing more metrics. I just pushed this, does that work with your usecases a bit better? I also added some docs on the internal bits.

therocode commented 2 years ago

I encountered the exact same use-case. I need interactable text, not just the caret but rectangles for the blue regions of selected text as well as being able to mouse-click characters. All of this requires line awareness. Given a Layout, which chars ended up on which line, what are those lines' x/y/w/h/ etc. This PR looks reasonable and I'll try it out and report back how it worked for my use case.

I'm not totally happy with this PR, and I started experimenting with exposing more metrics. I just pushed this, does that work with your usecases a bit better? I also added some docs on the internal bits.

For my usecase, your new push is great, thanks a lot for that! I already had it mostly working off this PR but you adding max/min stuff taking different styles into account is very helpful.

One thing that I see as missing is that there's x_start but no x_end which would be handy to implement go to line end and detect if mouse clicks are past a line.

mooman219 commented 2 years ago

@therocode So that x_start in there is private because it's identifies the x position of the first glyph that would start a new line when the text is laid out on a single line. So if you had a max width of 100, the x_start of the second like would be 100, third would be 200, etc.

I recommend using line_start / line_end to get the actual start/end glyph and use its position.

therocode commented 2 years ago

@mooman219 Gotcha.

Btw, I also saw that you got rid of the byte_index in GlyphPosition that this PR added. This makes it hard for me to map from glyph -> line. The use case here is that to work out my text interaction bounds on a per-glyph basis I need to use .glyphs() and during the iteration I need to know which LinePosition corresponds to each glyph. The way I did it with this PR was something like:

  1. collect .lines() into a Vec<LinePosition>.
  2. define a closure that takes a &GlyphPosition and compares its byte offset to the ranges of the LinePositions and returns a &LinePosition accordingly.
  3. Use the &GlyphPosition + &LinePosition pair to do logic.

Seems like now I have to manually increment a byte_index using += glyph.parent.utf8_width();? Maybe there is room for something better here... Random ideas:

Thoughts?

mooman219 commented 2 years ago

also saw that you got rid of the byte_index

@therocode Yeah that sounds like a pain. I added byte_offset back in the meantime.

I renamed line_start/line_end -> glyph_start/glyph_end and renamed the internal variable to clarify better. See the latest commit https://github.com/mooman219/fontdue/commit/8f55da0a367979739960f36798817acf2a8daaee. Also both glyphs() and lines() are non-mutable borrows as of 0.7 which should be less painful.

I'm not sure how I want to approach additional changes mainly because I don't have a good demo setup to feel out the ergonomics.

I assume right now you could something like (pseudocode)

let glpyhs = font.glyphs();
for line in font.lines() {
    for x in line.glyph_start..=line.glyph_end {
        let glyph= glyphs[x];
        // Use the line + glyph pair to do logic.
    }
}

Which doesn't feel too bad.

therocode commented 2 years ago

Cool, thanks for the byte_offset. I've got it working based off of that commit, except there might be a bug in the line layouting unless I'm misunderstanding something.

I reproed it in your Storm project too:

  1. Change Cargo.toml to use that specific commit
  2. Change layout width to 206.0 font size to 25.0 and the text string to:
    "a                                                      g                           "
  3. Add a loop after self.layout.append to get all lines and println!("{}..={}", glyph_start, glyph_end).
  4. Observe output: 0..=54 69..=82 i.e. there is a range of chars that are not considered part of any line between byte index 54 and 69.

As far as I understand, there should never be gaps like that? Maybe something with lots of ' ' in a row is tripping up the algo?

(also feel free to redirect me if this shouldn't go in this issue)

mooman219 commented 2 years ago

@therocode Ah yeah before when these variables were tracked privately, they didn't need to have tight constraints. It didn't really matter before if the starting glyph of a line was actually the starting glyph as long as it still looked the same.

I committed https://github.com/mooman219/fontdue/commit/b698ee6318f79e8df42aca50b548b91d882fc111 which probably fixes it.

therocode commented 2 years ago

Oh, yep it does! Thanks a bunch. 👍 All this is very useful, and I think it pretty much covers my use case in implementing text interations. I had a way harder time doing this with glyph_brush and it's the main reason why I'm switching over.

therocode commented 2 years ago

@mooman219 Hiya, actually encountered another issue with this. The byte_index on each glyph is a byte index into the current TextStyle. Meaning, if I have a text area with multiple styles, and I try to use the glyphs() and lines() info to implement text interactions, I run into issues when indexing since text interaction since I don't know which style each glyph belongs to, so I cannot index properly. I can work around this by traversing the glyphs and notice when byte_index resets to zero and increment a style_index and add as extra meta-data but maybe this should be provided by glyphs() since a byte_index without knowing the style is incomplete information?

therocode commented 2 years ago

Also, on another issue which might be a bug unless I'm misunderstanding. Seems like LinePosition baseline_y resets back to the top when a new style is being processed even though it's in the same layout operation. I would expect baseline_y to keep going downwards even as several style parts are being processed.

mooman219 commented 2 years ago

@therocode for the ping

byte_index on each glyph is a byte index into the current TextStyle

Yeahhh that was also the case with this PR. I don't think there's a correct solution here for me since the user may actually want it indexed into the style that generated it.

    /// The byte offset into the original string used in the append call which created
    /// this glyph.
    pub byte_offset: usize,

I did document this though! My recommended workaround is store metadata about the style in the user_data field on the style which gets attached to each glyph.

    /// Additional user data to associate with glyphs produced by this text style.
    pub user_data: U,

Layout unfortunately doesn't store references into the original text because that would require me to either clone it out which is real slow, or make things a bit messy with additional lifetimes.

Also, on another issue which might be a bug unless I'm misunderstanding. Seems like LinePosition baseline_y resets back to the top when a new style is being processed even though it's in the same layout operation. I would expect baseline_y to keep going downwards even as several style parts are being processed.

Sorry, could you elaborate? Are you triggering a case where baseline_y is returned as 0?

If you're asking about the direction baseline_y goes:

If your coordinate system is PositiveYUp, then each additional line will have a lower baseline_y than the previous one since that's lower on the screen. This is reversed for PositiveYDown. I include both because people seem to use fontdue's layout in both coordinate systems.

If you're asking about why does layout recalculate it every time:

VerticalAlign may be set to center or bottom, meaning each new line adjusts all previous line's baseline_y.

therocode commented 2 years ago

@mooman219 Thanks for the reply.

True @ it being documented 😅. Hmmm yeah. I have used the suggested workaround along with another one where I add meta-data to each glyph like:

let mut combined_byte_index = 0;

for glyph in glyphs {
    // Store combined_byte_index
    combined_byte_index += glyph.parent.len_utf8();
}

This all works for my case, thanks for the tip. From an API standpoint it's a little bit weird I think because the byte_offset isn't really useful without also knowing the style - what are you to use it for when you don't have the context of which string it came from? To not let every user make the style-metadata workaround I suppose the GlyphPosition could store a style_index such that the byte_offset,style_index pair would effectively work together as a reference. But I'll leave it up to you to judge if that's worth it or just extra bloat.

For the baseline_y issue - sorry about that, I thought I had isolated it down to the fontdue output but I see now it's actually a bug on my end. I was using byte_offset matched between the glyph_start and glyph_end of LinePosition to find the corresponding line, but in the light of your reply I realised that's incorrect. This bug vanished with your suggested work arounds.