This fix prevents a segmentation violation when a text.Cache doesn't
contain a font that is requested during layout. It also makes the
Cache fallback to returning any font from the Cache if a particular
font is not found.
Signed-off-by: Jeff Williams <kanobe@gmail.com>
---
text/shaper.go | 12 ++++++++++++widget/editor.go | 3 +++
2 files changed, 15 insertions(+)
diff --git a/text/shaper.go b/text/shaper.go
index 59020e07..d5d7ecdb 100644
--- a/text/shaper.go+++ b/text/shaper.go
@@ -3,6 +3,7 @@
package text
import (
+ "errors" "io"
"strings"
@@ -11,6 +12,8 @@ import (
"gioui.org/op/clip"
)
+var noSuchFont = errors.New("cannot find matching font")+// Shaper implements layout and shaping of text.
type Shaper interface {
// Layout a text according to a set of options.
@@ -63,6 +66,9 @@ func (c *Cache) faceForStyle(font Font) *faceCache {
if closest, ok := c.closestFont(font); ok {
return c.faces[closest]
}
+ for _, v := range c.faces {+ return v+ } return nil
}
@@ -110,12 +116,18 @@ func NewCache(collection []FontFace) *Cache {
// Layout implements the Shaper interface.
func (c *Cache) Layout(font Font, size fixed.Int26_6, maxWidth int, txt io.Reader) ([]Line, error) {
cache := c.lookup(font)
+ if cache == nil {+ return nil, noSuchFont+ } return cache.face.Layout(size, maxWidth, txt)
}
// LayoutString is a caching implementation of the Shaper interface.
func (c *Cache) LayoutString(font Font, size fixed.Int26_6, maxWidth int, str string) []Line {
cache := c.lookup(font)
+ if cache == nil {+ return nil+ } return cache.layout(size, maxWidth, str)
}
diff --git a/widget/editor.go b/widget/editor.go
index a9785575..b3064213 100644
--- a/widget/editor.go+++ b/widget/editor.go
@@ -1218,6 +1218,9 @@ func (e *Editor) SetCaret(start, end int) {
func (e *Editor) makeValidCaret(positions ...*combinedPos) {
// Jump through some hoops to order the offsets given to offsetToScreenPos,
// but still be able to update them correctly with the results thereof.
+ if e.lines == nil {+ return+ } positions = append(positions, &e.caret.start, &e.caret.end)
sort.Slice(positions, func(i, j int) bool {
return positions[i].ofs < positions[j].ofs
--
2.11.0
On Tue Jan 18, 2022 at 01:54 CET, Jeff Williams wrote:
> This fix prevents a segmentation violation when a text.Cache doesn't> contain a font that is requested during layout. It also makes the> Cache fallback to returning any font from the Cache if a particular> font is not found.>> Signed-off-by: Jeff Williams <kanobe@gmail.com>> ---> text/shaper.go | 12 ++++++++++++> widget/editor.go | 3 +++> 2 files changed, 15 insertions(+)>> diff --git a/text/shaper.go b/text/shaper.go> index 59020e07..d5d7ecdb 100644> --- a/text/shaper.go> +++ b/text/shaper.go> @@ -3,6 +3,7 @@> package text> > import (> + "errors"> "io"> "strings"> > @@ -11,6 +12,8 @@ import (> "gioui.org/op/clip"> )> > +var noSuchFont = errors.New("cannot find matching font")
to follow Go guidelines, I think this should be named
var errNoSuchFont
or
var errNoMatchingFont
-s
On Tue Jan 18, 2022 at 01:54 CET, Jeff Williams wrote:
> This fix prevents a segmentation violation when a text.Cache doesn't> contain a font that is requested during layout. It also makes the> Cache fallback to returning any font from the Cache if a particular> font is not found.>
Thank you. One comment below. Tests for the fixed panics would be nice.
> Signed-off-by: Jeff Williams <kanobe@gmail.com>> ---> text/shaper.go | 12 ++++++++++++> widget/editor.go | 3 +++> 2 files changed, 15 insertions(+)>> diff --git a/text/shaper.go b/text/shaper.go> index 59020e07..d5d7ecdb 100644> --- a/text/shaper.go> +++ b/text/shaper.go> @@ -11,6 +12,8 @@ import (> "gioui.org/op/clip"> )> > +var noSuchFont = errors.New("cannot find matching font")> +
Same comment as Sebastien's.
> // Shaper implements layout and shaping of text.> type Shaper interface {> // Layout a text according to a set of options.> @@ -63,6 +66,9 @@ func (c *Cache) faceForStyle(font Font) *faceCache {> if closest, ok := c.closestFont(font); ok {> return c.faces[closest]> }> + for _, v := range c.faces {> + return v> + }> return nil> }> > @@ -110,12 +116,18 @@ func NewCache(collection []FontFace) *Cache {> // Layout implements the Shaper interface.> func (c *Cache) Layout(font Font, size fixed.Int26_6, maxWidth int, txt io.Reader) ([]Line, error) {> cache := c.lookup(font)> + if cache == nil {> + return nil, noSuchFont> + }> return cache.face.Layout(size, maxWidth, txt)> }> > // LayoutString is a caching implementation of the Shaper interface.> func (c *Cache) LayoutString(font Font, size fixed.Int26_6, maxWidth int, str string) []Line {> cache := c.lookup(font)> + if cache == nil {> + return nil> + }> return cache.layout(size, maxWidth, str)> }> > diff --git a/widget/editor.go b/widget/editor.go> index a9785575..b3064213 100644> --- a/widget/editor.go> +++ b/widget/editor.go> @@ -1218,6 +1218,9 @@ func (e *Editor) SetCaret(start, end int) {> func (e *Editor) makeValidCaret(positions ...*combinedPos) {> // Jump through some hoops to order the offsets given to offsetToScreenPos,> // but still be able to update them correctly with the results thereof.> + if e.lines == nil {> + return> + }
Is this an optimization? If so, I don't think it's worth its weight.
> positions = append(positions, &e.caret.start, &e.caret.end)> sort.Slice(positions, func(i, j int) bool {> return positions[i].ofs < positions[j].ofs> -- > 2.11.0
>> Thank you. One comment below. Tests for the fixed panics would be nice.>
Sure, I'll try and write tests for it.
> > @@ -1218,6 +1218,9 @@ func (e *Editor) SetCaret(start, end int) {> > func (e *Editor) makeValidCaret(positions ...*combinedPos) {> > // Jump through some hoops to order the offsets given to offsetToScreenPos,> > // but still be able to update them correctly with the results thereof.> > + if e.lines == nil {> > + return> > + }>> Is this an optimization? If so, I don't think it's worth its weight.>
Oh, it's another part of the fix. It seems that in an otherwise
functioning program,
as a consequence of not finding the requested font the editor ends up
with lines set to nil,
and later in this method it calls offsetToScreenPos which tries to read e.lines
unconditionally:
func (e *Editor) offsetToScreenPos(offset int) (combinedPos,
func(int) combinedPos) {
var col, line, idx int
var x fixed.Int26_6
l := e.lines[line]
...
It seemed easier to return with that sanity check from SetCaret that
has no return
value than pick a return value that offsetToScreenPos should return on failure.
> > positions = append(positions, &e.caret.start, &e.caret.end)> > sort.Slice(positions, func(i, j int) bool {> > return positions[i].ofs < positions[j].ofs> > --> > 2.11.0>