From bdd6b32711df95e279d497d617b3ad083bc99957 Mon Sep 17 00:00:00 2001
From: Elias Naur <mail@eliasnaur.com>
Date: Thu, 26 Sep 2019 18:58:25 +0200
Subject: [PATCH] ui/app/internal/input: schedule an extra frame after event
 delivery

A program might choose to process events that affect UI state already laid out.
For example, a button click might switch to a completely different UI page, but
the click might be processed during the drawing of the current page.

Avoiding that require either processing events very early during layout or
adding InvalidateOps whenever events are handled late.

Early event processing is awkward and InvalidateOps are easy to forget and
their absence masked by any other cause of redraw.

This change adds an implicit InvalidateOp for each frame where events have been
delivered to the program, allowing late event handling without use of
InvalidateOps.

In the worst case we waste a frame, increasing power use. I hope that future
optimizations will detect and discard the duplicate frame before it reaches
the GPU.

A similar situation applies to the delayed delivery of Editor events, but
since Editor.Layout flushes remaining events, extra InvalidateOps are not
required. Add a comment.

Signed-off-by: Elias Naur <mail@eliasnaur.com>
---
 ui/app/internal/input/router.go | 32 ++++++++++++++++++++++----------
 ui/text/editor.go               |  1 +
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/ui/app/internal/input/router.go b/ui/app/internal/input/router.go
index a663a66b..fba8ac2f 100644
--- a/ui/app/internal/input/router.go
+++ b/ui/app/internal/input/router.go
@@ -24,6 +24,15 @@ type Router struct {
 
 	reader ops.Reader
 
+	// deliveredEvents tracks whether events has been returned to the
+	// user from Events. If so, another frame is scheduled to flush
+	// half-updated state. This is important when a an event changes
+	// UI state that has already been laid out. In the worst case, we
+	// waste a frame, increasing power usage.
+	// Gio is expected to grow the ability to construct frame-to-frame
+	// differences and only render to changed areas. In that case, the
+	// waste of a spurious frame should be minimal.
+	deliveredEvents bool
 	// InvalidateOp summary.
 	wakeup     bool
 	wakeupTime time.Time
@@ -33,12 +42,14 @@ type Router struct {
 }
 
 type handlerEvents struct {
-	handlers map[ui.Key][]ui.Event
-	updated  bool
+	handlers  map[ui.Key][]ui.Event
+	hadEvents bool
 }
 
 func (q *Router) Events(k ui.Key) []ui.Event {
-	return q.handlers.Events(k)
+	events := q.handlers.Events(k)
+	q.deliveredEvents = q.deliveredEvents || len(events) > 0
+	return events
 }
 
 func (q *Router) Frame(ops *ui.Ops) {
@@ -50,7 +61,8 @@ func (q *Router) Frame(ops *ui.Ops) {
 
 	q.pqueue.Frame(ops, &q.handlers)
 	q.kqueue.Frame(ops, &q.handlers)
-	if q.handlers.Updated() {
+	if q.deliveredEvents || q.handlers.HadEvents() {
+		q.deliveredEvents = false
 		q.wakeup = true
 		q.wakeupTime = time.Time{}
 	}
@@ -63,7 +75,7 @@ func (q *Router) Add(e ui.Event) bool {
 	case key.EditEvent, key.Event, key.FocusEvent:
 		q.kqueue.Push(e, &q.handlers)
 	}
-	return q.handlers.Updated()
+	return q.handlers.HadEvents()
 }
 
 func (q *Router) TextInputState() TextInputState {
@@ -109,18 +121,18 @@ func (h *handlerEvents) init() {
 func (h *handlerEvents) Set(k ui.Key, evts []ui.Event) {
 	h.init()
 	h.handlers[k] = evts
-	h.updated = true
+	h.hadEvents = true
 }
 
 func (h *handlerEvents) Add(k ui.Key, e ui.Event) {
 	h.init()
 	h.handlers[k] = append(h.handlers[k], e)
-	h.updated = true
+	h.hadEvents = true
 }
 
-func (h *handlerEvents) Updated() bool {
-	u := h.updated
-	h.updated = false
+func (h *handlerEvents) HadEvents() bool {
+	u := h.hadEvents
+	h.hadEvents = false
 	return u
 }
 
diff --git a/ui/text/editor.go b/ui/text/editor.go
index cb68d9c9..dae1a9ec 100644
--- a/ui/text/editor.go
+++ b/ui/text/editor.go
@@ -175,6 +175,7 @@ func (e *Editor) Focus() {
 	e.requestFocus = true
 }
 
+// Layout flushes any remaining events and lays out the editor.
 func (e *Editor) Layout(gtx *layout.Context) {
 	cs := gtx.Constraints
 	for _, ok := e.Event(gtx); ok; _, ok = e.Event(gtx) {