Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[shaping] Add an input segmenter #110

Merged
merged 7 commits into from
Dec 2, 2023
Merged

[shaping] Add an input segmenter #110

merged 7 commits into from
Dec 2, 2023

Conversation

benoitkugler
Copy link
Contributor

This is prototype for #109

I propose the following API

// Split segments the given [text] according to :
//   - text direction
//   - script
//   - face, as defined by [faces]
//
// As a consequence, it sets the following fields of the returned runs :
//   - Text, RunStart, RunEnd
//   - Direction
//   - Script
//   - Face
//
// [defaultDirection] is used during bidi ordering, and should refer to the general
// context [text] is used in (typically the user system preference for GUI apps.)
//
// The returned sliced is owned by the [Segmenter] and is only valid until
// the next call to [Split].
func (seg *Segmenter) Split(text []rune, faces Fontmap, defaultDirection di.Direction) []Input

I'm happy to read your thoughts !

@benoitkugler benoitkugler linked an issue Nov 24, 2023 that may be closed by this pull request
@whereswaldon
Copy link
Member

The implementation looks good, and is definitely an improvement over what we have in Gio. However, if we're taking the time to overhaul this, there's an important feature gap: to implement richtext functionality, the user needs to already have the capability to segment the text on arbitrary internal boundaries to change things like the text size, the requested font family, etc... This API does not permit that because the construction of the shaping.Inputs is completely internalized.

What would you think about changing the API to:

// Split segments the given pre-configured input according to:
//   - text direction
//   - script
//   - face, as defined by [faces]
//
// Only the input runes in the range text.RunStart to text.RunEnd will be split.
//
// As a consequence, it sets the following fields of the returned runs:
//   - Text, RunStart, RunEnd
//   - Direction
//   - Script
//   - Face
//
// [defaultDirection] is used during bidi ordering, and should refer to the general
// context [text] is used in (typically the user system preference for GUI apps.)
//
// The returned sliced is owned by the [Segmenter] and is only valid until
// the next call to [Split].
func (seg *Segmenter) Split(text Input, faces Fontmap, defaultDirection di.Direction) []Input

We could additionally offer a:

// SplitText offers a simple segmentation API for unstyled text. It is identical to calling
// [Split] with a single run containing all text. See the documentation of [Split] for important
// memory ownership info.
func (seg *Segmenter) SplitText(text []rune, faces Fontmap, defaultDirection di.Direction) []Input {
    return seg.Split(Input{Text:text, RunEnd: len(text)}, faces, defaultDirection)
}

@benoitkugler
Copy link
Contributor Author

What would you think about changing the API to:

Why not, it seems more versatile without too much complication ! Nice point.

@benoitkugler
Copy link
Contributor Author

What would you think about changing the API to:

Done in 2f2ea8d. I've remove the defaultDirection parameter, since we can now use the Direction field of the Input.

Copy link
Member

@whereswaldon whereswaldon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks excellent! Thank you very much for the work on this.

Copy link
Contributor

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I don't have time to test the functionality in Fyne yet, but it looks good and tests are in anyway :)

@andydotxyz andydotxyz merged commit 5949287 into main Dec 2, 2023
20 checks passed
@whereswaldon whereswaldon deleted the input-segmenter branch December 4, 2023 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

About input segmentation
3 participants