From 3c03bc8da72998dfc4cd77713026441c03acb4e4 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 13 Jan 2021 03:21:26 -0500 Subject: [PATCH 01/12] create entryCursorAnimation object to handle cursor animation, use this object instead of a raw fyne.Animation in entry.go --- widget/entry.go | 35 ++++----- widget/entry_cursor_anim.go | 144 ++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 19 deletions(-) create mode 100644 widget/entry_cursor_anim.go diff --git a/widget/entry.go b/widget/entry.go index ebcff463d3..c8328f269d 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -57,6 +57,8 @@ type Entry struct { CursorRow, CursorColumn int OnCursorChanged func() `json:"-"` + cursorAnim *entryCursorAnimation + focused bool text *textProvider placeholder *textProvider @@ -160,7 +162,10 @@ func (e *Entry) CreateRenderer() fyne.WidgetRenderer { box := canvas.NewRectangle(theme.FocusColor()) line := canvas.NewRectangle(theme.ShadowColor()) + cursor := canvas.NewRectangle(theme.PrimaryColor()) + cursor.Hide() + e.cursorAnim = newEntryCursorAnimation(cursor) e.content = &entryContent{entry: e} scroll := widget.NewScroll(e.content) objects := []fyne.CanvasObject{box, line, scroll} @@ -467,7 +472,9 @@ func (e *Entry) TypedKey(key *fyne.KeyEvent) { if e.Disabled() { return } - + if e.cursorAnim != nil { + e.cursorAnim.TemporaryPause() + } e.propertyLock.RLock() provider := e.textProvider() onSubmitted := e.OnSubmitted @@ -1184,9 +1191,6 @@ type entryContent struct { func (e *entryContent) CreateRenderer() fyne.WidgetRenderer { e.ExtendBaseWidget(e) - cursor := canvas.NewRectangle(theme.PrimaryColor()) - cursor.Hide() - e.entry.propertyLock.Lock() defer e.entry.propertyLock.Unlock() provider := e.entry.textProvider() @@ -1194,9 +1198,9 @@ func (e *entryContent) CreateRenderer() fyne.WidgetRenderer { if provider.len() != 0 { placeholder.Hide() } - objects := []fyne.CanvasObject{placeholder, provider, cursor} + objects := []fyne.CanvasObject{placeholder, provider, e.entry.cursorAnim.cursor} - r := &entryContentRenderer{cursor, []fyne.CanvasObject{}, nil, objects, + r := &entryContentRenderer{e.entry.cursorAnim.cursor, []fyne.CanvasObject{}, objects, provider, placeholder, e} r.Layout(e.size) return r @@ -1312,10 +1316,9 @@ func (e *entryContent) TappedSecondary(pe *fyne.PointEvent) { var _ fyne.WidgetRenderer = (*entryContentRenderer)(nil) type entryContentRenderer struct { - cursor *canvas.Rectangle - selection []fyne.CanvasObject - cursorAnim *fyne.Animation - objects []fyne.CanvasObject + cursor *canvas.Rectangle + selection []fyne.CanvasObject + objects []fyne.CanvasObject provider, placeholder *textProvider content *entryContent @@ -1327,7 +1330,7 @@ func (r *entryContentRenderer) BackgroundColor() color.Color { } func (r *entryContentRenderer) Destroy() { - r.cursorAnim.Stop() + r.content.entry.cursorAnim.Stop() } func (r *entryContentRenderer) Layout(size fyne.Size) { @@ -1377,15 +1380,9 @@ func (r *entryContentRenderer) Refresh() { r.cursor.FillColor = theme.PrimaryColor() if focused { r.cursor.Show() - if r.cursorAnim == nil { - r.cursorAnim = makeCursorAnimation(r.cursor) - r.cursorAnim.Start() - } + r.content.entry.cursorAnim.Start() } else { - if r.cursorAnim != nil { - r.cursorAnim.Stop() - r.cursorAnim = nil - } + r.content.entry.cursorAnim.Stop() r.cursor.Hide() } r.moveCursor() diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go new file mode 100644 index 0000000000..b639b32e20 --- /dev/null +++ b/widget/entry_cursor_anim.go @@ -0,0 +1,144 @@ +package widget + +import ( + "fmt" + "image/color" + "sync" + "time" + + "fyne.io/fyne" + "fyne.io/fyne/canvas" + "fyne.io/fyne/theme" +) + +type safeCounter struct { + mu sync.RWMutex + cnt int +} + +func (c *safeCounter) Inc(n int) { + c.mu.Lock() + defer c.mu.Unlock() + c.cnt += n +} + +func (c *safeCounter) Value() int { + c.mu.RLock() + defer c.mu.RUnlock() + return c.cnt +} + +func (c *safeCounter) Reset() { + c.mu.Lock() + defer c.mu.Unlock() + c.cnt = 0 +} + +type entryCursorAnimation struct { + mu *sync.RWMutex + // timeout *time.Ticker + counter *safeCounter + paused bool + stopped bool + cursor *canvas.Rectangle + anim *fyne.Animation +} + +func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { + a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a.stopped = true + return a +} + +func (a *entryCursorAnimation) createAnim() { + cursorOpaque := theme.PrimaryColor() + r, g, b, _ := theme.PrimaryColor().RGBA() + cursorDim := color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 0x16} + anim := canvas.NewColorRGBAAnimation(cursorDim, cursorOpaque, time.Second/2, func(c color.Color) { + a.cursor.FillColor = c + a.cursor.Refresh() + }) + anim.RepeatCount = fyne.AnimationRepeatForever + anim.AutoReverse = true + a.anim = anim +} + +// Start must only be called once. +func (a *entryCursorAnimation) Start() { + a.mu.Lock() + defer a.mu.Unlock() + // anim should be nil and current state should be stopped + if a.anim != nil || !a.stopped { + return + } + fmt.Printf("anim: %p\n", a) + a.createAnim() + a.stopped = false + // a.timeout = time.NewTicker(10 * time.Millisecond) + go func() { + fmt.Println("start goroutine") + defer func() { + fmt.Println("===> stop goroutine") + a.mu.Lock() + a.stopped = true + a.mu.Unlock() + }() + for { + // <-a.timeout.C + time.Sleep(10 * time.Millisecond) + // >>>> lock + a.mu.RLock() + paused := a.paused + cancel := a.anim == nil + a.mu.RUnlock() + // <<<< unlock + if cancel { + return + } + if !paused { + continue + } + a.counter.Inc(1) + if a.counter.Value() == 50 { + // >>>> lock + a.mu.Lock() + if a.anim != nil { + a.anim.Start() + } + a.paused = false + a.mu.Unlock() + // <<<< unlock + } + } + }() + a.anim.Start() +} + +func (a *entryCursorAnimation) TemporaryPause() { + a.counter.Reset() + a.mu.Lock() + defer a.mu.Unlock() + if a.anim == nil { + return + } + a.anim.Stop() + if a.paused { + return + } + a.paused = true + a.cursor.FillColor = theme.PrimaryColor() + a.cursor.Refresh() +} + +// Stop must only be called once when cursor is destroyed. +func (a *entryCursorAnimation) Stop() { + a.mu.Lock() + defer a.mu.Unlock() + // if a.timeout != nil { + // a.timeout.Stop() + // } + if a.anim != nil { + a.anim.Stop() + a.anim = nil + } +} From 359f4dd206ad5bee379a8f7982bea39524ded33d Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 13 Jan 2021 13:50:26 -0500 Subject: [PATCH 02/12] delete extra comments, remove unused code --- widget/entry.go | 15 --------------- widget/entry_cursor_anim.go | 16 ++++------------ 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/widget/entry.go b/widget/entry.go index a204aa8b5d..fd39571f46 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -4,7 +4,6 @@ import ( "image/color" "math" "strings" - "time" "unicode" "fyne.io/fyne" @@ -1534,17 +1533,3 @@ func getTextWhitespaceRegion(row []rune, col int) (int, int) { } return start, end } - -func makeCursorAnimation(cursor *canvas.Rectangle) *fyne.Animation { - cursorOpaque := theme.PrimaryColor() - r, g, b, _ := theme.PrimaryColor().RGBA() - cursorDim := color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 0x16} - anim := canvas.NewColorRGBAAnimation(cursorDim, cursorOpaque, time.Second/2, func(c color.Color) { - cursor.FillColor = c - cursor.Refresh() - }) - anim.RepeatCount = fyne.AnimationRepeatForever - anim.AutoReverse = true - - return anim -} diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index b639b32e20..c6c7f76123 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -1,7 +1,6 @@ package widget import ( - "fmt" "image/color" "sync" "time" @@ -35,8 +34,7 @@ func (c *safeCounter) Reset() { } type entryCursorAnimation struct { - mu *sync.RWMutex - // timeout *time.Ticker + mu *sync.RWMutex counter *safeCounter paused bool stopped bool @@ -63,7 +61,7 @@ func (a *entryCursorAnimation) createAnim() { a.anim = anim } -// Start must only be called once. +// Start starts cursor animation. func (a *entryCursorAnimation) Start() { a.mu.Lock() defer a.mu.Unlock() @@ -71,14 +69,10 @@ func (a *entryCursorAnimation) Start() { if a.anim != nil || !a.stopped { return } - fmt.Printf("anim: %p\n", a) a.createAnim() a.stopped = false - // a.timeout = time.NewTicker(10 * time.Millisecond) go func() { - fmt.Println("start goroutine") defer func() { - fmt.Println("===> stop goroutine") a.mu.Lock() a.stopped = true a.mu.Unlock() @@ -114,6 +108,7 @@ func (a *entryCursorAnimation) Start() { a.anim.Start() } +// TemporaryPause pauses temporarily the cursor by 500 ms. func (a *entryCursorAnimation) TemporaryPause() { a.counter.Reset() a.mu.Lock() @@ -130,13 +125,10 @@ func (a *entryCursorAnimation) TemporaryPause() { a.cursor.Refresh() } -// Stop must only be called once when cursor is destroyed. +// Stop stops cursor animation. func (a *entryCursorAnimation) Stop() { a.mu.Lock() defer a.mu.Unlock() - // if a.timeout != nil { - // a.timeout.Stop() - // } if a.anim != nil { a.anim.Stop() a.anim = nil From dc4f394ec292d0848cd2dcd10d395cd6d85a6d7e Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 15 Jan 2021 02:46:34 -0500 Subject: [PATCH 03/12] decrease temporary pause time, invert cursor animation, added test --- widget/entry_cursor_anim.go | 14 +-- widget/entry_cursor_anim_test.go | 143 +++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 widget/entry_cursor_anim_test.go diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index c6c7f76123..c023496ee6 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -33,6 +33,8 @@ func (c *safeCounter) Reset() { c.cnt = 0 } +const cursorPauseTimex10ms = 35 // pause time in multiple of 10 ms + type entryCursorAnimation struct { mu *sync.RWMutex counter *safeCounter @@ -40,11 +42,13 @@ type entryCursorAnimation struct { stopped bool cursor *canvas.Rectangle anim *fyne.Animation + sleepFn func(time.Duration) // added for tests, do not change it outside of tests!! } func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} a.stopped = true + a.sleepFn = time.Sleep return a } @@ -52,7 +56,7 @@ func (a *entryCursorAnimation) createAnim() { cursorOpaque := theme.PrimaryColor() r, g, b, _ := theme.PrimaryColor().RGBA() cursorDim := color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 0x16} - anim := canvas.NewColorRGBAAnimation(cursorDim, cursorOpaque, time.Second/2, func(c color.Color) { + anim := canvas.NewColorRGBAAnimation(cursorOpaque, cursorDim, time.Second/2, func(c color.Color) { a.cursor.FillColor = c a.cursor.Refresh() }) @@ -78,8 +82,7 @@ func (a *entryCursorAnimation) Start() { a.mu.Unlock() }() for { - // <-a.timeout.C - time.Sleep(10 * time.Millisecond) + a.sleepFn(10 * time.Millisecond) // >>>> lock a.mu.RLock() paused := a.paused @@ -93,7 +96,7 @@ func (a *entryCursorAnimation) Start() { continue } a.counter.Inc(1) - if a.counter.Value() == 50 { + if a.counter.Value() == cursorPauseTimex10ms { // >>>> lock a.mu.Lock() if a.anim != nil { @@ -108,7 +111,7 @@ func (a *entryCursorAnimation) Start() { a.anim.Start() } -// TemporaryPause pauses temporarily the cursor by 500 ms. +// TemporaryPause pauses temporarily the cursor by `cursorPauseTimex10ms`. func (a *entryCursorAnimation) TemporaryPause() { a.counter.Reset() a.mu.Lock() @@ -131,6 +134,7 @@ func (a *entryCursorAnimation) Stop() { defer a.mu.Unlock() if a.anim != nil { a.anim.Stop() + a.paused = false a.anim = nil } } diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go new file mode 100644 index 0000000000..195dab9757 --- /dev/null +++ b/widget/entry_cursor_anim_test.go @@ -0,0 +1,143 @@ +package widget + +import ( + "image/color" + "runtime" + "sync" + "testing" + "time" + + "fyne.io/fyne/canvas" + "fyne.io/fyne/theme" + "github.com/stretchr/testify/assert" +) + +func TestEntryCursorAnim(t *testing.T) { + cursor := canvas.NewRectangle(color.Black) + a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a.stopped = true + sleeper := make(chan int) + cycle := make(chan int) + a.sleepFn = func(d time.Duration) { + cycle <- 1 + <-sleeper + } + + // start animation + a.Start() + <-cycle + assert.False(t, a.stopped) + assert.False(t, a.paused) + assert.NotNil(t, a.anim) + assert.Zero(t, a.counter.Value()) + + // pass some time + for i := 0; i < 10; i++ { + sleeper <- 1 + <-cycle + } + + // entry cursor animation must have the same values as before + assert.False(t, a.stopped) + assert.False(t, a.paused) + assert.NotNil(t, a.anim) + assert.Zero(t, a.counter.Value()) + + // now run a TemporaryPause() + a.TemporaryPause() + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Zero(t, a.counter.Value()) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + + // make some steps in time less than cursorPauseTimex10ms + // cursor.FillColor should be PrimaryColor always + for i := 0; i < (cursorPauseTimex10ms - 1); i++ { + sleeper <- 1 + <-cycle + assert.Equal(t, i+1, a.counter.Value()) + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + } + + // advance one step more (equals to cursorPauseTimex10ms) and the animation + // should start again + sleeper <- 1 + <-cycle + assert.False(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + + // make some steps + sleeper <- 1 + <-cycle + sleeper <- 1 + <-cycle + // animation should continue (not paused) + assert.False(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + // counter value should be cursorPauseTimex10ms (it shouldn't increase) + assert.Equal(t, cursorPauseTimex10ms, a.counter.Value()) + + // temporary pause again + a.TemporaryPause() + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Zero(t, a.counter.Value()) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + + for i := 0; i < 10; i++ { + sleeper <- 1 + <-cycle + assert.Equal(t, i+1, a.counter.Value()) + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + } + + // temporary pause again (counter should be resetted) + a.TemporaryPause() + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Zero(t, a.counter.Value()) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + + for i := 0; i < 10; i++ { + sleeper <- 1 + <-cycle + assert.Equal(t, i+1, a.counter.Value()) + assert.True(t, a.paused) + assert.False(t, a.stopped) + assert.NotNil(t, a.anim) + assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) + } + + // stop the animation + a.Stop() + sleeper <- 1 + time.Sleep(1 * time.Millisecond) + runtime.Gosched() + time.Sleep(5 * time.Millisecond) + assert.False(t, a.paused) + assert.True(t, a.stopped) + assert.Nil(t, a.anim) + assert.Equal(t, 10, a.counter.Value()) + + // calling a.TemporaryPause() on stopped animation, does not do anything (just reset the counter) + a.TemporaryPause() + assert.False(t, a.paused) + assert.True(t, a.stopped) + assert.Nil(t, a.anim) + assert.Zero(t, a.counter.Value()) + + assert.NotPanics(t, func() { a.TemporaryPause() }) + assert.NotPanics(t, func() { a.Start() }) + assert.NotPanics(t, func() { a.Stop() }) +} From db19a086baabe4348e0d21dd324b328b4a8ba610 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Sun, 17 Jan 2021 15:06:00 -0500 Subject: [PATCH 04/12] keep previous enter cursor animation and replace it when pause is requested to make current tests succeed --- widget/entry_cursor_anim.go | 32 +++++++++++++++++++++----------- widget/entry_cursor_anim_test.go | 13 +++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index c023496ee6..4f653867bb 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -36,33 +36,40 @@ func (c *safeCounter) Reset() { const cursorPauseTimex10ms = 35 // pause time in multiple of 10 ms type entryCursorAnimation struct { - mu *sync.RWMutex - counter *safeCounter - paused bool - stopped bool - cursor *canvas.Rectangle - anim *fyne.Animation - sleepFn func(time.Duration) // added for tests, do not change it outside of tests!! + mu *sync.RWMutex + counter *safeCounter + inverted bool + paused bool + stopped bool + cursor *canvas.Rectangle + anim *fyne.Animation + sleepFn func(time.Duration) // added for tests, do not change it outside of tests!! } func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a.inverted = false a.stopped = true a.sleepFn = time.Sleep return a } -func (a *entryCursorAnimation) createAnim() { +func (a *entryCursorAnimation) createAnim(inverted bool) *fyne.Animation { cursorOpaque := theme.PrimaryColor() r, g, b, _ := theme.PrimaryColor().RGBA() cursorDim := color.NRGBA{R: uint8(r), G: uint8(g), B: uint8(b), A: 0x16} - anim := canvas.NewColorRGBAAnimation(cursorOpaque, cursorDim, time.Second/2, func(c color.Color) { + a.inverted = inverted + start, end := color.Color(cursorDim), color.Color(cursorOpaque) + if inverted { + start, end = end, start + } + anim := canvas.NewColorRGBAAnimation(start, end, time.Second/2, func(c color.Color) { a.cursor.FillColor = c a.cursor.Refresh() }) anim.RepeatCount = fyne.AnimationRepeatForever anim.AutoReverse = true - a.anim = anim + return anim } // Start starts cursor animation. @@ -73,7 +80,7 @@ func (a *entryCursorAnimation) Start() { if a.anim != nil || !a.stopped { return } - a.createAnim() + a.anim = a.createAnim(false) a.stopped = false go func() { defer func() { @@ -120,6 +127,9 @@ func (a *entryCursorAnimation) TemporaryPause() { return } a.anim.Stop() + if !a.inverted { + a.anim = a.createAnim(true) + } if a.paused { return } diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index 195dab9757..d5a3405372 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -15,6 +15,7 @@ import ( func TestEntryCursorAnim(t *testing.T) { cursor := canvas.NewRectangle(color.Black) a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a.inverted = false a.stopped = true sleeper := make(chan int) cycle := make(chan int) @@ -26,6 +27,7 @@ func TestEntryCursorAnim(t *testing.T) { // start animation a.Start() <-cycle + assert.False(t, a.inverted) assert.False(t, a.stopped) assert.False(t, a.paused) assert.NotNil(t, a.anim) @@ -38,6 +40,7 @@ func TestEntryCursorAnim(t *testing.T) { } // entry cursor animation must have the same values as before + assert.False(t, a.inverted) assert.False(t, a.stopped) assert.False(t, a.paused) assert.NotNil(t, a.anim) @@ -45,6 +48,7 @@ func TestEntryCursorAnim(t *testing.T) { // now run a TemporaryPause() a.TemporaryPause() + assert.True(t, a.inverted) assert.True(t, a.paused) assert.False(t, a.stopped) assert.NotNil(t, a.anim) @@ -56,6 +60,7 @@ func TestEntryCursorAnim(t *testing.T) { for i := 0; i < (cursorPauseTimex10ms - 1); i++ { sleeper <- 1 <-cycle + assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) assert.True(t, a.paused) assert.False(t, a.stopped) @@ -67,6 +72,7 @@ func TestEntryCursorAnim(t *testing.T) { // should start again sleeper <- 1 <-cycle + assert.True(t, a.inverted) assert.False(t, a.paused) assert.False(t, a.stopped) assert.NotNil(t, a.anim) @@ -77,6 +83,7 @@ func TestEntryCursorAnim(t *testing.T) { sleeper <- 1 <-cycle // animation should continue (not paused) + assert.True(t, a.inverted) assert.False(t, a.paused) assert.False(t, a.stopped) assert.NotNil(t, a.anim) @@ -85,6 +92,7 @@ func TestEntryCursorAnim(t *testing.T) { // temporary pause again a.TemporaryPause() + assert.True(t, a.inverted) assert.True(t, a.paused) assert.False(t, a.stopped) assert.NotNil(t, a.anim) @@ -94,6 +102,7 @@ func TestEntryCursorAnim(t *testing.T) { for i := 0; i < 10; i++ { sleeper <- 1 <-cycle + assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) assert.True(t, a.paused) assert.False(t, a.stopped) @@ -103,6 +112,7 @@ func TestEntryCursorAnim(t *testing.T) { // temporary pause again (counter should be resetted) a.TemporaryPause() + assert.True(t, a.inverted) assert.True(t, a.paused) assert.False(t, a.stopped) assert.NotNil(t, a.anim) @@ -112,6 +122,7 @@ func TestEntryCursorAnim(t *testing.T) { for i := 0; i < 10; i++ { sleeper <- 1 <-cycle + assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) assert.True(t, a.paused) assert.False(t, a.stopped) @@ -125,6 +136,7 @@ func TestEntryCursorAnim(t *testing.T) { time.Sleep(1 * time.Millisecond) runtime.Gosched() time.Sleep(5 * time.Millisecond) + assert.True(t, a.inverted) assert.False(t, a.paused) assert.True(t, a.stopped) assert.Nil(t, a.anim) @@ -132,6 +144,7 @@ func TestEntryCursorAnim(t *testing.T) { // calling a.TemporaryPause() on stopped animation, does not do anything (just reset the counter) a.TemporaryPause() + assert.True(t, a.inverted) assert.False(t, a.paused) assert.True(t, a.stopped) assert.Nil(t, a.anim) From 365b868a9b46c2d9c5ec52cf91db67f7046dc709 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Sat, 23 Jan 2021 20:51:46 -0500 Subject: [PATCH 05/12] update import paths --- widget/entry_cursor_anim.go | 6 +++--- widget/entry_cursor_anim_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 4f653867bb..46e985b203 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -5,9 +5,9 @@ import ( "sync" "time" - "fyne.io/fyne" - "fyne.io/fyne/canvas" - "fyne.io/fyne/theme" + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/theme" ) type safeCounter struct { diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index d5a3405372..e310efa66b 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -7,8 +7,8 @@ import ( "testing" "time" - "fyne.io/fyne/canvas" - "fyne.io/fyne/theme" + "fyne.io/fyne/v2/canvas" + "fyne.io/fyne/v2/theme" "github.com/stretchr/testify/assert" ) From 4c56395db0aa84a8da5789c8e68b93141b049d2e Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 26 Jan 2021 03:18:50 -0500 Subject: [PATCH 06/12] rename entryCursorAnimation.TemporaryPause to TemporaryStop, remove stopped and paused entryCursorAnimation boolean properties in favor of a state property of type cursorState, update tests --- widget/entry.go | 2 +- widget/entry_cursor_anim.go | 38 ++++++++++-------- widget/entry_cursor_anim_test.go | 66 +++++++++++++------------------- 3 files changed, 50 insertions(+), 56 deletions(-) diff --git a/widget/entry.go b/widget/entry.go index 9802c77996..c364eec45e 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -499,7 +499,7 @@ func (e *Entry) TypedKey(key *fyne.KeyEvent) { return } if e.cursorAnim != nil { - e.cursorAnim.TemporaryPause() + e.cursorAnim.TemporaryStop() } e.propertyLock.RLock() provider := e.textProvider() diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 46e985b203..addda90c4d 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -33,14 +33,21 @@ func (c *safeCounter) Reset() { c.cnt = 0 } -const cursorPauseTimex10ms = 35 // pause time in multiple of 10 ms +const cursorStopTimex10ms = 35 // stop time in multiple of 10 ms + +type cursorState int + +const ( + cursorStateRunning cursorState = iota + cursorStateTemporarilyStopped + cursorStateStopped +) type entryCursorAnimation struct { mu *sync.RWMutex counter *safeCounter inverted bool - paused bool - stopped bool + state cursorState cursor *canvas.Rectangle anim *fyne.Animation sleepFn func(time.Duration) // added for tests, do not change it outside of tests!! @@ -49,7 +56,7 @@ type entryCursorAnimation struct { func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} a.inverted = false - a.stopped = true + a.state = cursorStateStopped a.sleepFn = time.Sleep return a } @@ -77,39 +84,39 @@ func (a *entryCursorAnimation) Start() { a.mu.Lock() defer a.mu.Unlock() // anim should be nil and current state should be stopped - if a.anim != nil || !a.stopped { + if a.anim != nil || a.state != cursorStateStopped { return } a.anim = a.createAnim(false) - a.stopped = false + a.state = cursorStateRunning go func() { defer func() { a.mu.Lock() - a.stopped = true + a.state = cursorStateStopped a.mu.Unlock() }() for { a.sleepFn(10 * time.Millisecond) // >>>> lock a.mu.RLock() - paused := a.paused + tempStop := a.state == cursorStateTemporarilyStopped cancel := a.anim == nil a.mu.RUnlock() // <<<< unlock if cancel { return } - if !paused { + if !tempStop { continue } a.counter.Inc(1) - if a.counter.Value() == cursorPauseTimex10ms { + if a.counter.Value() == cursorStopTimex10ms { // >>>> lock a.mu.Lock() if a.anim != nil { a.anim.Start() } - a.paused = false + a.state = cursorStateRunning a.mu.Unlock() // <<<< unlock } @@ -118,8 +125,8 @@ func (a *entryCursorAnimation) Start() { a.anim.Start() } -// TemporaryPause pauses temporarily the cursor by `cursorPauseTimex10ms`. -func (a *entryCursorAnimation) TemporaryPause() { +// TemporaryStop temporarily stops the cursor by "cursorStopTimex10ms". +func (a *entryCursorAnimation) TemporaryStop() { a.counter.Reset() a.mu.Lock() defer a.mu.Unlock() @@ -130,10 +137,10 @@ func (a *entryCursorAnimation) TemporaryPause() { if !a.inverted { a.anim = a.createAnim(true) } - if a.paused { + if a.state == cursorStateTemporarilyStopped { return } - a.paused = true + a.state = cursorStateTemporarilyStopped a.cursor.FillColor = theme.PrimaryColor() a.cursor.Refresh() } @@ -144,7 +151,6 @@ func (a *entryCursorAnimation) Stop() { defer a.mu.Unlock() if a.anim != nil { a.anim.Stop() - a.paused = false a.anim = nil } } diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index e310efa66b..f881ce65a0 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -16,7 +16,7 @@ func TestEntryCursorAnim(t *testing.T) { cursor := canvas.NewRectangle(color.Black) a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} a.inverted = false - a.stopped = true + a.state = cursorStateStopped sleeper := make(chan int) cycle := make(chan int) a.sleepFn = func(d time.Duration) { @@ -28,8 +28,7 @@ func TestEntryCursorAnim(t *testing.T) { a.Start() <-cycle assert.False(t, a.inverted) - assert.False(t, a.stopped) - assert.False(t, a.paused) + assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) @@ -41,29 +40,26 @@ func TestEntryCursorAnim(t *testing.T) { // entry cursor animation must have the same values as before assert.False(t, a.inverted) - assert.False(t, a.stopped) - assert.False(t, a.paused) + assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) - // now run a TemporaryPause() - a.TemporaryPause() + // now call a TemporaryStop() + a.TemporaryStop() assert.True(t, a.inverted) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - // make some steps in time less than cursorPauseTimex10ms + // make some steps in time less than cursorStopTimex10ms // cursor.FillColor should be PrimaryColor always - for i := 0; i < (cursorPauseTimex10ms - 1); i++ { + for i := 0; i < (cursorStopTimex10ms - 1); i++ { sleeper <- 1 <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } @@ -73,8 +69,7 @@ func TestEntryCursorAnim(t *testing.T) { sleeper <- 1 <-cycle assert.True(t, a.inverted) - assert.False(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) // make some steps @@ -82,19 +77,17 @@ func TestEntryCursorAnim(t *testing.T) { <-cycle sleeper <- 1 <-cycle - // animation should continue (not paused) + // animation should continue (not temp. stopped) assert.True(t, a.inverted) - assert.False(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - // counter value should be cursorPauseTimex10ms (it shouldn't increase) - assert.Equal(t, cursorPauseTimex10ms, a.counter.Value()) + // counter value should be cursorStopTimex10ms (it shouldn't increase) + assert.Equal(t, cursorStopTimex10ms, a.counter.Value()) - // temporary pause again - a.TemporaryPause() + // temporary stop again + a.TemporaryStop() assert.True(t, a.inverted) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) @@ -104,17 +97,15 @@ func TestEntryCursorAnim(t *testing.T) { <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } - // temporary pause again (counter should be resetted) - a.TemporaryPause() + // temporary stop again (counter should be resetted) + a.TemporaryStop() assert.True(t, a.inverted) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) @@ -124,8 +115,7 @@ func TestEntryCursorAnim(t *testing.T) { <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.True(t, a.paused) - assert.False(t, a.stopped) + assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } @@ -137,20 +127,18 @@ func TestEntryCursorAnim(t *testing.T) { runtime.Gosched() time.Sleep(5 * time.Millisecond) assert.True(t, a.inverted) - assert.False(t, a.paused) - assert.True(t, a.stopped) + assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) assert.Equal(t, 10, a.counter.Value()) - // calling a.TemporaryPause() on stopped animation, does not do anything (just reset the counter) - a.TemporaryPause() + // calling a.TemporaryStop() on stopped animation, does not do anything (just reset the counter) + a.TemporaryStop() assert.True(t, a.inverted) - assert.False(t, a.paused) - assert.True(t, a.stopped) + assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) assert.Zero(t, a.counter.Value()) - assert.NotPanics(t, func() { a.TemporaryPause() }) + assert.NotPanics(t, func() { a.TemporaryStop() }) assert.NotPanics(t, func() { a.Start() }) assert.NotPanics(t, func() { a.Stop() }) } From 3fd96e15804669dc98f2727cec5ae35578ac19a4 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Tue, 26 Jan 2021 17:28:41 -0500 Subject: [PATCH 07/12] rename safeCounter.cnt to safeCounter.val, remove useless comments, make all entryCursorAnimation functions privates, reduce complexity in entryCursorAnimation.start method --- widget/entry.go | 8 ++++---- widget/entry_cursor_anim.go | 34 ++++++++++++++------------------ widget/entry_cursor_anim_test.go | 18 ++++++++--------- 3 files changed, 28 insertions(+), 32 deletions(-) diff --git a/widget/entry.go b/widget/entry.go index c364eec45e..be7a07e0c4 100644 --- a/widget/entry.go +++ b/widget/entry.go @@ -499,7 +499,7 @@ func (e *Entry) TypedKey(key *fyne.KeyEvent) { return } if e.cursorAnim != nil { - e.cursorAnim.TemporaryStop() + e.cursorAnim.temporaryStop() } e.propertyLock.RLock() provider := e.textProvider() @@ -1268,7 +1268,7 @@ type entryContentRenderer struct { } func (r *entryContentRenderer) Destroy() { - r.content.entry.cursorAnim.Stop() + r.content.entry.cursorAnim.stop() } func (r *entryContentRenderer) Layout(size fyne.Size) { @@ -1318,9 +1318,9 @@ func (r *entryContentRenderer) Refresh() { if focused { r.cursor.Show() - r.content.entry.cursorAnim.Start() + r.content.entry.cursorAnim.start() } else { - r.content.entry.cursorAnim.Stop() + r.content.entry.cursorAnim.stop() r.cursor.Hide() } r.moveCursor() diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index addda90c4d..7d7949e2d9 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -12,25 +12,25 @@ import ( type safeCounter struct { mu sync.RWMutex - cnt int + val int } func (c *safeCounter) Inc(n int) { c.mu.Lock() defer c.mu.Unlock() - c.cnt += n + c.val += n } func (c *safeCounter) Value() int { c.mu.RLock() defer c.mu.RUnlock() - return c.cnt + return c.val } func (c *safeCounter) Reset() { c.mu.Lock() defer c.mu.Unlock() - c.cnt = 0 + c.val = 0 } const cursorStopTimex10ms = 35 // stop time in multiple of 10 ms @@ -80,10 +80,9 @@ func (a *entryCursorAnimation) createAnim(inverted bool) *fyne.Animation { } // Start starts cursor animation. -func (a *entryCursorAnimation) Start() { +func (a *entryCursorAnimation) start() { a.mu.Lock() defer a.mu.Unlock() - // anim should be nil and current state should be stopped if a.anim != nil || a.state != cursorStateStopped { return } @@ -97,12 +96,10 @@ func (a *entryCursorAnimation) Start() { }() for { a.sleepFn(10 * time.Millisecond) - // >>>> lock a.mu.RLock() tempStop := a.state == cursorStateTemporarilyStopped cancel := a.anim == nil a.mu.RUnlock() - // <<<< unlock if cancel { return } @@ -110,23 +107,22 @@ func (a *entryCursorAnimation) Start() { continue } a.counter.Inc(1) - if a.counter.Value() == cursorStopTimex10ms { - // >>>> lock - a.mu.Lock() - if a.anim != nil { - a.anim.Start() - } - a.state = cursorStateRunning - a.mu.Unlock() - // <<<< unlock + if a.counter.Value() != cursorStopTimex10ms { + continue + } + a.mu.Lock() + if a.anim != nil { + a.anim.Start() } + a.state = cursorStateRunning + a.mu.Unlock() } }() a.anim.Start() } // TemporaryStop temporarily stops the cursor by "cursorStopTimex10ms". -func (a *entryCursorAnimation) TemporaryStop() { +func (a *entryCursorAnimation) temporaryStop() { a.counter.Reset() a.mu.Lock() defer a.mu.Unlock() @@ -146,7 +142,7 @@ func (a *entryCursorAnimation) TemporaryStop() { } // Stop stops cursor animation. -func (a *entryCursorAnimation) Stop() { +func (a *entryCursorAnimation) stop() { a.mu.Lock() defer a.mu.Unlock() if a.anim != nil { diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index f881ce65a0..435a11177c 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -25,7 +25,7 @@ func TestEntryCursorAnim(t *testing.T) { } // start animation - a.Start() + a.start() <-cycle assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) @@ -45,7 +45,7 @@ func TestEntryCursorAnim(t *testing.T) { assert.Zero(t, a.counter.Value()) // now call a TemporaryStop() - a.TemporaryStop() + a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) @@ -85,7 +85,7 @@ func TestEntryCursorAnim(t *testing.T) { assert.Equal(t, cursorStopTimex10ms, a.counter.Value()) // temporary stop again - a.TemporaryStop() + a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) @@ -103,7 +103,7 @@ func TestEntryCursorAnim(t *testing.T) { } // temporary stop again (counter should be resetted) - a.TemporaryStop() + a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateTemporarilyStopped, a.state) assert.NotNil(t, a.anim) @@ -121,7 +121,7 @@ func TestEntryCursorAnim(t *testing.T) { } // stop the animation - a.Stop() + a.stop() sleeper <- 1 time.Sleep(1 * time.Millisecond) runtime.Gosched() @@ -132,13 +132,13 @@ func TestEntryCursorAnim(t *testing.T) { assert.Equal(t, 10, a.counter.Value()) // calling a.TemporaryStop() on stopped animation, does not do anything (just reset the counter) - a.TemporaryStop() + a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) assert.Zero(t, a.counter.Value()) - assert.NotPanics(t, func() { a.TemporaryStop() }) - assert.NotPanics(t, func() { a.Start() }) - assert.NotPanics(t, func() { a.Stop() }) + assert.NotPanics(t, func() { a.temporaryStop() }) + assert.NotPanics(t, func() { a.start() }) + assert.NotPanics(t, func() { a.stop() }) } From 8095c782520ba06f0fc360c5ee76b71099d312e7 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 27 Jan 2021 12:41:00 -0500 Subject: [PATCH 08/12] in entry_cursor_anim.go rename cursorStateTemporarilyStopped to cursorStateInterrupted and rename cursorStopTimex10ms to cursorInterruptTimex10ms --- widget/entry_cursor_anim.go | 14 +++++++------- widget/entry_cursor_anim_test.go | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 7d7949e2d9..32bf854656 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -33,13 +33,13 @@ func (c *safeCounter) Reset() { c.val = 0 } -const cursorStopTimex10ms = 35 // stop time in multiple of 10 ms +const cursorInterruptTimex10ms = 35 // interrupt time in multiple of 10 ms type cursorState int const ( cursorStateRunning cursorState = iota - cursorStateTemporarilyStopped + cursorStateInterrupted cursorStateStopped ) @@ -97,17 +97,17 @@ func (a *entryCursorAnimation) start() { for { a.sleepFn(10 * time.Millisecond) a.mu.RLock() - tempStop := a.state == cursorStateTemporarilyStopped + interrupted := a.state == cursorStateInterrupted cancel := a.anim == nil a.mu.RUnlock() if cancel { return } - if !tempStop { + if !interrupted { continue } a.counter.Inc(1) - if a.counter.Value() != cursorStopTimex10ms { + if a.counter.Value() != cursorInterruptTimex10ms { continue } a.mu.Lock() @@ -133,10 +133,10 @@ func (a *entryCursorAnimation) temporaryStop() { if !a.inverted { a.anim = a.createAnim(true) } - if a.state == cursorStateTemporarilyStopped { + if a.state == cursorStateInterrupted { return } - a.state = cursorStateTemporarilyStopped + a.state = cursorStateInterrupted a.cursor.FillColor = theme.PrimaryColor() a.cursor.Refresh() } diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index 435a11177c..f9920684c6 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -47,19 +47,19 @@ func TestEntryCursorAnim(t *testing.T) { // now call a TemporaryStop() a.temporaryStop() assert.True(t, a.inverted) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - // make some steps in time less than cursorStopTimex10ms + // make some steps in time less than cursorInterruptTimex10ms // cursor.FillColor should be PrimaryColor always - for i := 0; i < (cursorStopTimex10ms - 1); i++ { + for i := 0; i < (cursorInterruptTimex10ms - 1); i++ { sleeper <- 1 <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } @@ -81,13 +81,13 @@ func TestEntryCursorAnim(t *testing.T) { assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - // counter value should be cursorStopTimex10ms (it shouldn't increase) - assert.Equal(t, cursorStopTimex10ms, a.counter.Value()) + // counter value should be cursorInterruptTimex10ms (it shouldn't increase) + assert.Equal(t, cursorInterruptTimex10ms, a.counter.Value()) // temporary stop again a.temporaryStop() assert.True(t, a.inverted) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) @@ -97,7 +97,7 @@ func TestEntryCursorAnim(t *testing.T) { <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } @@ -105,7 +105,7 @@ func TestEntryCursorAnim(t *testing.T) { // temporary stop again (counter should be resetted) a.temporaryStop() assert.True(t, a.inverted) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Zero(t, a.counter.Value()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) @@ -115,7 +115,7 @@ func TestEntryCursorAnim(t *testing.T) { <-cycle assert.True(t, a.inverted) assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateTemporarilyStopped, a.state) + assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) } From cab80a966fe80cc2c65b4a8dbdde6586d9555d1f Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Wed, 27 Jan 2021 16:32:08 -0500 Subject: [PATCH 09/12] remove entryCursorAnim.safeCounter in favor of time.Ticker to manage cursor animation interruption --- widget/entry_cursor_anim.go | 85 ++++++++++++++-------- widget/entry_cursor_anim_test.go | 118 ++++++++++++++----------------- 2 files changed, 110 insertions(+), 93 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 32bf854656..3aeb9e837a 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -10,30 +10,60 @@ import ( "fyne.io/fyne/v2/theme" ) -type safeCounter struct { - mu sync.RWMutex - val int +// =============================================================== +// Cursor ticker +// =============================================================== + +type cursorTicker interface { + WaitTick() + Stop() + Reset() + Start(d time.Duration) + Started() bool } -func (c *safeCounter) Inc(n int) { - c.mu.Lock() - defer c.mu.Unlock() - c.val += n +type realTicker struct { + tk *time.Ticker + duration time.Duration } -func (c *safeCounter) Value() int { - c.mu.RLock() - defer c.mu.RUnlock() - return c.val +func (t *realTicker) Start(d time.Duration) { + if t.Started() { + return + } + t.duration = d + t.tk = time.NewTicker(t.duration) } -func (c *safeCounter) Reset() { - c.mu.Lock() - defer c.mu.Unlock() - c.val = 0 +func (t *realTicker) Reset() { + if !t.Started() { + return + } + t.tk.Reset(t.duration) } -const cursorInterruptTimex10ms = 35 // interrupt time in multiple of 10 ms +func (t *realTicker) Stop() { + if !t.Started() { + return + } + t.tk.Stop() + t.tk = nil // TODO is it safe? +} + +func (t *realTicker) WaitTick() { + if !t.Started() { + return + } + <-t.tk.C +} + +func (t *realTicker) Started() bool { return t.tk != nil } + +// =============================================================== +// Implementation +// =============================================================== + +const cursorInterruptTime = 300 * time.Millisecond type cursorState int @@ -45,19 +75,18 @@ const ( type entryCursorAnimation struct { mu *sync.RWMutex - counter *safeCounter inverted bool state cursorState + ticker cursorTicker cursor *canvas.Rectangle anim *fyne.Animation - sleepFn func(time.Duration) // added for tests, do not change it outside of tests!! } func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { - a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor} + a.ticker = &realTicker{} a.inverted = false a.state = cursorStateStopped - a.sleepFn = time.Sleep return a } @@ -83,19 +112,21 @@ func (a *entryCursorAnimation) createAnim(inverted bool) *fyne.Animation { func (a *entryCursorAnimation) start() { a.mu.Lock() defer a.mu.Unlock() - if a.anim != nil || a.state != cursorStateStopped { + if a.anim != nil || a.ticker.Started() || a.state != cursorStateStopped { return } a.anim = a.createAnim(false) a.state = cursorStateRunning + a.ticker.Start(cursorInterruptTime) go func() { defer func() { a.mu.Lock() a.state = cursorStateStopped + a.ticker.Stop() a.mu.Unlock() }() for { - a.sleepFn(10 * time.Millisecond) + a.ticker.WaitTick() a.mu.RLock() interrupted := a.state == cursorStateInterrupted cancel := a.anim == nil @@ -106,10 +137,6 @@ func (a *entryCursorAnimation) start() { if !interrupted { continue } - a.counter.Inc(1) - if a.counter.Value() != cursorInterruptTimex10ms { - continue - } a.mu.Lock() if a.anim != nil { a.anim.Start() @@ -121,14 +148,14 @@ func (a *entryCursorAnimation) start() { a.anim.Start() } -// TemporaryStop temporarily stops the cursor by "cursorStopTimex10ms". +// TemporaryStop temporarily stops the cursor by "cursorInterruptTime". func (a *entryCursorAnimation) temporaryStop() { - a.counter.Reset() a.mu.Lock() defer a.mu.Unlock() - if a.anim == nil { + if a.anim == nil || !a.ticker.Started() { return } + a.ticker.Reset() a.anim.Stop() if !a.inverted { a.anim = a.createAnim(true) diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index f9920684c6..15af83c3ca 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -12,131 +12,121 @@ import ( "github.com/stretchr/testify/assert" ) +type mockTicker struct { + started bool + ticks int + cycle chan int + tickCh chan int +} + +func (m *mockTicker) sendTick() { + m.tickCh <- 1 + <-m.cycle + m.ticks++ +} +func (m *mockTicker) WaitTick() { + m.cycle <- 1 + <-m.tickCh +} +func (m *mockTicker) Stop() { + m.started = false +} +func (m *mockTicker) Reset() { m.ticks = 0 } +func (m *mockTicker) Start(d time.Duration) { + m.started = true + m.tickCh = make(chan int) + m.cycle = make(chan int) + go func() { <-m.cycle }() + runtime.Gosched() +} +func (m *mockTicker) Started() bool { return m.started } + func TestEntryCursorAnim(t *testing.T) { cursor := canvas.NewRectangle(color.Black) - a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor, counter: &safeCounter{}} + a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor} a.inverted = false a.state = cursorStateStopped - sleeper := make(chan int) - cycle := make(chan int) - a.sleepFn = func(d time.Duration) { - cycle <- 1 - <-sleeper - } + mticker := &mockTicker{} + a.ticker = mticker // start animation a.start() - <-cycle assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.True(t, mticker.Started()) // pass some time for i := 0; i < 10; i++ { - sleeper <- 1 - <-cycle + assert.Equal(t, i, mticker.ticks) + mticker.sendTick() } // entry cursor animation must have the same values as before assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.True(t, mticker.Started()) // now call a TemporaryStop() a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.True(t, mticker.Started()) + assert.Zero(t, mticker.ticks) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - // make some steps in time less than cursorInterruptTimex10ms - // cursor.FillColor should be PrimaryColor always - for i := 0; i < (cursorInterruptTimex10ms - 1); i++ { - sleeper <- 1 - <-cycle - assert.True(t, a.inverted) - assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateInterrupted, a.state) - assert.NotNil(t, a.anim) - assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - } - - // advance one step more (equals to cursorPauseTimex10ms) and the animation - // should start again - sleeper <- 1 - <-cycle + // when ticks, the animation should start again + mticker.sendTick() assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) + assert.True(t, mticker.Started()) + assert.Equal(t, 1, mticker.ticks) assert.NotNil(t, a.anim) // make some steps - sleeper <- 1 - <-cycle - sleeper <- 1 - <-cycle - // animation should continue (not temp. stopped) + mticker.sendTick() + mticker.sendTick() + // animation should continue (not interrupted) assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) + assert.True(t, mticker.Started()) + assert.Equal(t, 3, mticker.ticks) assert.NotNil(t, a.anim) - // counter value should be cursorInterruptTimex10ms (it shouldn't increase) - assert.Equal(t, cursorInterruptTimex10ms, a.counter.Value()) // temporary stop again a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.True(t, mticker.Started()) + assert.Zero(t, mticker.ticks) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - for i := 0; i < 10; i++ { - sleeper <- 1 - <-cycle - assert.True(t, a.inverted) - assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateInterrupted, a.state) - assert.NotNil(t, a.anim) - assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - } - // temporary stop again (counter should be resetted) + mticker.ticks = 100 // just to ensure it is resetted below a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.Zero(t, mticker.ticks) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - for i := 0; i < 10; i++ { - sleeper <- 1 - <-cycle - assert.True(t, a.inverted) - assert.Equal(t, i+1, a.counter.Value()) - assert.Equal(t, cursorStateInterrupted, a.state) - assert.NotNil(t, a.anim) - assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - } - // stop the animation a.stop() - sleeper <- 1 - time.Sleep(1 * time.Millisecond) - runtime.Gosched() + mticker.tickCh <- 1 time.Sleep(5 * time.Millisecond) assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) - assert.Equal(t, 10, a.counter.Value()) - // calling a.TemporaryStop() on stopped animation, does not do anything (just reset the counter) + // calling a.TemporaryStop() on stopped animation, does not do anything a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) - assert.Zero(t, a.counter.Value()) + assert.False(t, mticker.Started()) assert.NotPanics(t, func() { a.temporaryStop() }) assert.NotPanics(t, func() { a.start() }) From 5743eb461bb7179dc74e1608f6d01bd4a63ab233 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Fri, 29 Jan 2021 03:09:53 -0500 Subject: [PATCH 10/12] remove time.Ticker usage in entry_cursor_anim.realTicker in favor of time.Timer to guarantee compatibility with older go versions --- widget/entry_cursor_anim.go | 43 ++++++++++++++++++++++++-------- widget/entry_cursor_anim_test.go | 4 ++- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 3aeb9e837a..b541807d68 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -15,7 +15,7 @@ import ( // =============================================================== type cursorTicker interface { - WaitTick() + WaitTick() (reset bool) Stop() Reset() Start(d time.Duration) @@ -23,7 +23,8 @@ type cursorTicker interface { } type realTicker struct { - tk *time.Ticker + tim *time.Timer + rstCh chan struct{} duration time.Duration } @@ -32,32 +33,52 @@ func (t *realTicker) Start(d time.Duration) { return } t.duration = d - t.tk = time.NewTicker(t.duration) + t.rstCh = make(chan struct{}, 1) + t.tim = time.NewTimer(t.duration) } func (t *realTicker) Reset() { if !t.Started() { return } - t.tk.Reset(t.duration) + select { + case t.rstCh <- struct{}{}: + default: + } } +// Stop must be called in the same go routine where WaitTick is used. func (t *realTicker) Stop() { if !t.Started() { return } - t.tk.Stop() - t.tk = nil // TODO is it safe? + if !t.tim.Stop() { + <-t.tim.C + } + t.tim = nil // TODO is it safe? + t.rstCh = nil } -func (t *realTicker) WaitTick() { +func (t *realTicker) WaitTick() (reset bool) { if !t.Started() { + reset = true // TODO what to do here? return } - <-t.tk.C + select { + case <-t.tim.C: + reset = false + t.tim.Stop() + case <-t.rstCh: + reset = true + if !t.tim.Stop() { + <-t.tim.C + } + } + t.tim.Reset(t.duration) + return } -func (t *realTicker) Started() bool { return t.tk != nil } +func (t *realTicker) Started() bool { return t.tim != nil } // =============================================================== // Implementation @@ -126,7 +147,9 @@ func (a *entryCursorAnimation) start() { a.mu.Unlock() }() for { - a.ticker.WaitTick() + if reset := a.ticker.WaitTick(); reset { + continue + } a.mu.RLock() interrupted := a.state == cursorStateInterrupted cancel := a.anim == nil diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index 15af83c3ca..eeb8adc07c 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -24,9 +24,11 @@ func (m *mockTicker) sendTick() { <-m.cycle m.ticks++ } -func (m *mockTicker) WaitTick() { +func (m *mockTicker) WaitTick() (reset bool) { m.cycle <- 1 <-m.tickCh + reset = false + return } func (m *mockTicker) Stop() { m.started = false From 5da8029a5a6d0afdf6c278c83d5e0346e5ba88e3 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Thu, 4 Feb 2021 12:37:37 -0500 Subject: [PATCH 11/12] remove cursorTicker interface, rename realTicker to cursorTicker, remove mockTicker type from entry_cursor_anim_test.go --- widget/entry_cursor_anim.go | 31 +++++----- widget/entry_cursor_anim_test.go | 97 ++++++++++---------------------- 2 files changed, 43 insertions(+), 85 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index b541807d68..22882f2e12 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -14,30 +14,22 @@ import ( // Cursor ticker // =============================================================== -type cursorTicker interface { - WaitTick() (reset bool) - Stop() - Reset() - Start(d time.Duration) - Started() bool -} - -type realTicker struct { +type cursorTicker struct { tim *time.Timer rstCh chan struct{} duration time.Duration + mockWait func() (reset bool) } -func (t *realTicker) Start(d time.Duration) { +func (t *cursorTicker) Start() { if t.Started() { return } - t.duration = d t.rstCh = make(chan struct{}, 1) t.tim = time.NewTimer(t.duration) } -func (t *realTicker) Reset() { +func (t *cursorTicker) Reset() { if !t.Started() { return } @@ -48,7 +40,7 @@ func (t *realTicker) Reset() { } // Stop must be called in the same go routine where WaitTick is used. -func (t *realTicker) Stop() { +func (t *cursorTicker) Stop() { if !t.Started() { return } @@ -59,11 +51,14 @@ func (t *realTicker) Stop() { t.rstCh = nil } -func (t *realTicker) WaitTick() (reset bool) { +func (t *cursorTicker) WaitTick() (reset bool) { if !t.Started() { reset = true // TODO what to do here? return } + if t.mockWait != nil { + return t.mockWait() + } select { case <-t.tim.C: reset = false @@ -78,7 +73,7 @@ func (t *realTicker) WaitTick() (reset bool) { return } -func (t *realTicker) Started() bool { return t.tim != nil } +func (t *cursorTicker) Started() bool { return t.tim != nil } // =============================================================== // Implementation @@ -98,14 +93,14 @@ type entryCursorAnimation struct { mu *sync.RWMutex inverted bool state cursorState - ticker cursorTicker + ticker *cursorTicker cursor *canvas.Rectangle anim *fyne.Animation } func newEntryCursorAnimation(cursor *canvas.Rectangle) *entryCursorAnimation { a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor} - a.ticker = &realTicker{} + a.ticker = &cursorTicker{duration: cursorInterruptTime} a.inverted = false a.state = cursorStateStopped return a @@ -138,7 +133,7 @@ func (a *entryCursorAnimation) start() { } a.anim = a.createAnim(false) a.state = cursorStateRunning - a.ticker.Start(cursorInterruptTime) + a.ticker.Start() go func() { defer func() { a.mu.Lock() diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index eeb8adc07c..c04c8e3953 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -2,7 +2,6 @@ package widget import ( "image/color" - "runtime" "sync" "testing" "time" @@ -12,113 +11,77 @@ import ( "github.com/stretchr/testify/assert" ) -type mockTicker struct { - started bool - ticks int - cycle chan int - tickCh chan int -} - -func (m *mockTicker) sendTick() { - m.tickCh <- 1 - <-m.cycle - m.ticks++ -} -func (m *mockTicker) WaitTick() (reset bool) { - m.cycle <- 1 - <-m.tickCh - reset = false - return -} -func (m *mockTicker) Stop() { - m.started = false -} -func (m *mockTicker) Reset() { m.ticks = 0 } -func (m *mockTicker) Start(d time.Duration) { - m.started = true - m.tickCh = make(chan int) - m.cycle = make(chan int) - go func() { <-m.cycle }() - runtime.Gosched() -} -func (m *mockTicker) Started() bool { return m.started } - func TestEntryCursorAnim(t *testing.T) { cursor := canvas.NewRectangle(color.Black) a := &entryCursorAnimation{mu: &sync.RWMutex{}, cursor: cursor} a.inverted = false a.state = cursorStateStopped - mticker := &mockTicker{} - a.ticker = mticker + a.ticker = &cursorTicker{} + cycle := make(chan int) + sleep := make(chan int) + a.ticker.mockWait = func() (reset bool) { + cycle <- 1 + <-sleep + return + } + flushReset := func() { <-a.ticker.rstCh } // start animation a.start() + <-cycle assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.True(t, mticker.Started()) + assert.True(t, a.ticker.Started()) - // pass some time - for i := 0; i < 10; i++ { - assert.Equal(t, i, mticker.ticks) - mticker.sendTick() - } + // pass 1 cursorTicker cycle + sleep <- 1 + <-cycle // entry cursor animation must have the same values as before assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.True(t, mticker.Started()) + assert.True(t, a.ticker.Started()) // now call a TemporaryStop() + go flushReset() a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.True(t, mticker.Started()) - assert.Zero(t, mticker.ticks) + assert.True(t, a.ticker.Started()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - // when ticks, the animation should start again - mticker.sendTick() + // after 1 cursorTicker cycle, the animation should start again + sleep <- 1 + <-cycle assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) - assert.True(t, mticker.Started()) - assert.Equal(t, 1, mticker.ticks) + assert.True(t, a.ticker.Started()) assert.NotNil(t, a.anim) - // make some steps - mticker.sendTick() - mticker.sendTick() - // animation should continue (not interrupted) + // after 1 cursorTicker cycle, the animation should continue (not interrupted) + sleep <- 1 + <-cycle assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) - assert.True(t, mticker.Started()) - assert.Equal(t, 3, mticker.ticks) + assert.True(t, a.ticker.Started()) assert.NotNil(t, a.anim) // temporary stop again + go flushReset() a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.True(t, mticker.Started()) - assert.Zero(t, mticker.ticks) - assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - - // temporary stop again (counter should be resetted) - mticker.ticks = 100 // just to ensure it is resetted below - a.temporaryStop() - assert.True(t, a.inverted) - assert.Equal(t, cursorStateInterrupted, a.state) - assert.NotNil(t, a.anim) - assert.Zero(t, mticker.ticks) + assert.True(t, a.ticker.Started()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) // stop the animation a.stop() - mticker.tickCh <- 1 - time.Sleep(5 * time.Millisecond) + sleep <- 1 + time.Sleep(2 * time.Millisecond) assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) @@ -128,7 +91,7 @@ func TestEntryCursorAnim(t *testing.T) { assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) - assert.False(t, mticker.Started()) + assert.False(t, a.ticker.Started()) assert.NotPanics(t, func() { a.temporaryStop() }) assert.NotPanics(t, func() { a.start() }) From ff0ba74722ef4043764b8efd33f1957660c95185 Mon Sep 17 00:00:00 2001 From: FPabl0 Date: Mon, 8 Feb 2021 18:45:08 -0500 Subject: [PATCH 12/12] rename cursorTicker.tim and cursorTicker.rstCh to cursorTicker.timer and cursorTicker.resetChan respectively, make the cursorTicker methods non-exported, rename channels used in entry_cursor_anim_test.go --- widget/entry_cursor_anim.go | 89 +++++++++++++++----------------- widget/entry_cursor_anim_test.go | 50 +++++++++--------- 2 files changed, 70 insertions(+), 69 deletions(-) diff --git a/widget/entry_cursor_anim.go b/widget/entry_cursor_anim.go index 22882f2e12..f963c2bbc5 100644 --- a/widget/entry_cursor_anim.go +++ b/widget/entry_cursor_anim.go @@ -15,66 +15,65 @@ import ( // =============================================================== type cursorTicker struct { - tim *time.Timer - rstCh chan struct{} - duration time.Duration - mockWait func() (reset bool) + timer *time.Timer + resetChan chan struct{} + duration time.Duration + mockWait func() (reset bool) } -func (t *cursorTicker) Start() { - if t.Started() { +func (t *cursorTicker) reset() { + if !t.started() { return } - t.rstCh = make(chan struct{}, 1) - t.tim = time.NewTimer(t.duration) + select { + case t.resetChan <- struct{}{}: + default: + } } -func (t *cursorTicker) Reset() { - if !t.Started() { +func (t *cursorTicker) start() { + if t.started() { return } - select { - case t.rstCh <- struct{}{}: - default: - } + t.resetChan = make(chan struct{}, 1) + t.timer = time.NewTimer(t.duration) } -// Stop must be called in the same go routine where WaitTick is used. -func (t *cursorTicker) Stop() { - if !t.Started() { +func (t *cursorTicker) started() bool { return t.timer != nil } + +// stop must be called in the same go routine where WaitTick is used. +func (t *cursorTicker) stop() { + if !t.started() { return } - if !t.tim.Stop() { - <-t.tim.C + if !t.timer.Stop() { + <-t.timer.C } - t.tim = nil // TODO is it safe? - t.rstCh = nil + t.timer = nil + t.resetChan = nil } -func (t *cursorTicker) WaitTick() (reset bool) { - if !t.Started() { - reset = true // TODO what to do here? +func (t *cursorTicker) waitTick() (reset bool) { + if !t.started() { return } if t.mockWait != nil { return t.mockWait() } select { - case <-t.tim.C: + case <-t.timer.C: reset = false - t.tim.Stop() - case <-t.rstCh: + t.timer.Stop() + case <-t.resetChan: reset = true - if !t.tim.Stop() { - <-t.tim.C + if !t.timer.Stop() { + <-t.timer.C } } - t.tim.Reset(t.duration) + t.timer.Reset(t.duration) return } -func (t *cursorTicker) Started() bool { return t.tim != nil } - // =============================================================== // Implementation // =============================================================== @@ -128,21 +127,15 @@ func (a *entryCursorAnimation) createAnim(inverted bool) *fyne.Animation { func (a *entryCursorAnimation) start() { a.mu.Lock() defer a.mu.Unlock() - if a.anim != nil || a.ticker.Started() || a.state != cursorStateStopped { + if a.anim != nil || a.ticker.started() || a.state != cursorStateStopped { return } a.anim = a.createAnim(false) a.state = cursorStateRunning - a.ticker.Start() + a.ticker.start() go func() { - defer func() { - a.mu.Lock() - a.state = cursorStateStopped - a.ticker.Stop() - a.mu.Unlock() - }() for { - if reset := a.ticker.WaitTick(); reset { + if reset := a.ticker.waitTick(); reset { continue } a.mu.RLock() @@ -150,7 +143,7 @@ func (a *entryCursorAnimation) start() { cancel := a.anim == nil a.mu.RUnlock() if cancel { - return + break } if !interrupted { continue @@ -162,18 +155,22 @@ func (a *entryCursorAnimation) start() { a.state = cursorStateRunning a.mu.Unlock() } + a.mu.Lock() + a.state = cursorStateStopped + a.ticker.stop() + a.mu.Unlock() }() a.anim.Start() } -// TemporaryStop temporarily stops the cursor by "cursorInterruptTime". +// temporaryStop temporarily stops the cursor by "cursorInterruptTime". func (a *entryCursorAnimation) temporaryStop() { a.mu.Lock() defer a.mu.Unlock() - if a.anim == nil || !a.ticker.Started() { + if a.anim == nil || !a.ticker.started() { return } - a.ticker.Reset() + a.ticker.reset() a.anim.Stop() if !a.inverted { a.anim = a.createAnim(true) @@ -186,7 +183,7 @@ func (a *entryCursorAnimation) temporaryStop() { a.cursor.Refresh() } -// Stop stops cursor animation. +// stop stops cursor animation. func (a *entryCursorAnimation) stop() { a.mu.Lock() defer a.mu.Unlock() diff --git a/widget/entry_cursor_anim_test.go b/widget/entry_cursor_anim_test.go index c04c8e3953..237b1a1d39 100644 --- a/widget/entry_cursor_anim_test.go +++ b/widget/entry_cursor_anim_test.go @@ -17,32 +17,33 @@ func TestEntryCursorAnim(t *testing.T) { a.inverted = false a.state = cursorStateStopped a.ticker = &cursorTicker{} - cycle := make(chan int) - sleep := make(chan int) + beforeNextTick := make(chan int) + afterTick := make(chan int) a.ticker.mockWait = func() (reset bool) { - cycle <- 1 - <-sleep + beforeNextTick <- 1 + <-afterTick return } - flushReset := func() { <-a.ticker.rstCh } + flushReset := func() { <-a.ticker.resetChan } // start animation a.start() - <-cycle + // unblock waitTick() to start testing + <-beforeNextTick assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) - // pass 1 cursorTicker cycle - sleep <- 1 - <-cycle + // pass 1 cursorTicker tick + afterTick <- 1 // unblock to execute code below waitTick() + <-beforeNextTick // wait until the code below waitTick was effectively executed // entry cursor animation must have the same values as before assert.False(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) assert.NotNil(t, a.anim) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) // now call a TemporaryStop() go flushReset() @@ -50,23 +51,23 @@ func TestEntryCursorAnim(t *testing.T) { assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) - // after 1 cursorTicker cycle, the animation should start again - sleep <- 1 - <-cycle + // after 1 cursorTicker tick, the animation should start again + afterTick <- 1 // unblock to execute code below waitTick() + <-beforeNextTick // wait until the code below waitTick was effectively executed assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) assert.NotNil(t, a.anim) - // after 1 cursorTicker cycle, the animation should continue (not interrupted) - sleep <- 1 - <-cycle + // after 1 cursorTicker tick, the animation should continue (not interrupted) + afterTick <- 1 // unblock to execute code below waitTick() + <-beforeNextTick // wait until the code below waitTick was effectively executed assert.True(t, a.inverted) assert.Equal(t, cursorStateRunning, a.state) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) assert.NotNil(t, a.anim) // temporary stop again @@ -75,23 +76,26 @@ func TestEntryCursorAnim(t *testing.T) { assert.True(t, a.inverted) assert.Equal(t, cursorStateInterrupted, a.state) assert.NotNil(t, a.anim) - assert.True(t, a.ticker.Started()) + assert.True(t, a.ticker.started()) assert.Equal(t, theme.PrimaryColor(), a.cursor.FillColor) // stop the animation a.stop() - sleep <- 1 + // unblock to execute code below waitTick() and so it should break the for-loop, and stop + // ticker and animation + afterTick <- 1 time.Sleep(2 * time.Millisecond) assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) + assert.False(t, a.ticker.started()) // calling a.TemporaryStop() on stopped animation, does not do anything a.temporaryStop() assert.True(t, a.inverted) assert.Equal(t, cursorStateStopped, a.state) assert.Nil(t, a.anim) - assert.False(t, a.ticker.Started()) + assert.False(t, a.ticker.started()) assert.NotPanics(t, func() { a.temporaryStop() }) assert.NotPanics(t, func() { a.start() })