~eliasnaur/gio

11 4

Improve function scoped layout

Details
Message ID
<BX3DBHEVQRUU.Z81PFN8PUX71@toolbox>
DKIM signature
missing
Download raw message
I just pushed commits

	gioui.org/commit/29639565
	gioui.org/commit/7ca1d7a3

that replaces the begin/end based layout APIs with function scoped
APIs. See the first commit for details.

I'm pretty happy with the result, except for the rather long function
signature for widgets. From the commit example:

	al := layout.Align{Alignment: layout.NE}
	return al.Layout(ops, cs, func(cs layout.Constraints) layout.Dimensions {
	       in := layout.Inset{Top: ui.Dp(16)}
	       return in.Layout(c, ops, cs, func(cs layout.Constraints) layout.Dimensions {
		       txt := fmt.Sprintf("m: %d %s", mallocs, u.profile.Timings)
		       return text.Label{Material: theme.text, Face: u.face(fonts.mono, 10), Text: txt}.Layout(ops, cs)
	       })
	})

typing `func(cs layout.Constraints) layout.Dimensions {` at every layout boundary
was so tedious I had a copy buffer with that string while writing new layout.

Can it be done better? One (old) idea is to shorten the type names:

	func(cs layout.Cons) layout.Dims

or even

	func(cs layout.C) layout.D

I've rejected shorter names before, and I even renamed Dimens to Dimensions recently.
However, because the types are now everywhere,

- the impact of the long names is larger.
- it will be easier to recognize the abbreviations.

Another more radical idea is to add, say, a layout.Stack that keeps track of the current
Constraints and Dimensions:

	type Stack struct{...}

	// Push start a layout operation.
	func (s *Stack) Push(cs Constraints)
	
	// Pop ends the most recent layout operation.
	func (s *Stack) Pop(d Dimensions)

	// Dimensions returns the dimensions from the most
	// recent completed layout.
	func (s *Stack) Dimensions() Dimensions
	
	// Constraints returns the constraints for the most
	// recent started layout.
	func (s *Stack) Dimensions() Dimensions

The example would become

	st := new(layout.Stack)

	al := layout.Align{Alignment: layout.NE}
	al.Layout(ops, st, func() {
	       in := layout.Inset{Top: ui.Dp(16)}
	       in.Layout(c, ops, st, func() {
		       txt := fmt.Sprintf("m: %d %s", mallocs, u.profile.Timings)
		       text.Label{Material: theme.text, Face: u.face(fonts.mono, 10), Text: txt}.Layout(ops, st)
	       })
	})

Note that the function signature is the shortest possible and the return
statements are gone. If we decide to add a layout.Context object
(gioui.org/issue/33), Stack would fit right in.

However, widgets will be easier to get wrong, because
instead of a return statement, you have to pass your dimensions to the stack.

So,

	func widget(..., cs layout.Constraints) layout.Dimensions {
		dims := layout.Dimensions{...}
		return dims
	}

becomes

	func widget(..., st *layout.Stack) {
		cs := st.Constraints()

		dims := layout.Dimensions{...}

		st.Pop(dims)
	}

Likewise, a layout must remember to pass in the modified constraints. So,

	func layout(..., cs layout.Constraints, w layout.Widget) layout.Dimensions {
		cs1 := ...
		dims := w(cs1)
		dims1 := ...
		return dims1
	}

becomes

	func layout(..., st *layout.Stack, w layout.Widget) {
		cs1 := ...
		st.Push(cs1)
		w()
		dims := st.Dimensions()
		dims1 := ...
		st.Pop(dims1)
	}

Yet another idea would be a way to collapse nested calls of layouts, but I
failed to find a way that doesn't generate garbage.

Ideas and comments very welcome!

-- elias
Details
Message ID
<20190920150208.GA51850@larrymbp14.local>
In-Reply-To
<BX3DBHEVQRUU.Z81PFN8PUX71@toolbox> (view parent)
DKIM signature
missing
Download raw message
On Wed, Sep 18, 2019 at 09:17:04PM +0200, Elias Naur wrote:
> I just pushed commits
> 
> 	gioui.org/commit/29639565
> 	gioui.org/commit/7ca1d7a3
> 
> that replaces the begin/end based layout APIs with function scoped
> APIs. See the first commit for details.
> 
> I'm pretty happy with the result, except for the rather long function
> signature for widgets. From the commit example:
> 
> 	al := layout.Align{Alignment: layout.NE}
> 	return al.Layout(ops, cs, func(cs layout.Constraints) layout.Dimensions {
> 	       in := layout.Inset{Top: ui.Dp(16)}
> 	       return in.Layout(c, ops, cs, func(cs layout.Constraints) layout.Dimensions {
> 		       txt := fmt.Sprintf("m: %d %s", mallocs, u.profile.Timings)
> 		       return text.Label{Material: theme.text, Face: u.face(fonts.mono, 10), Text: txt}.Layout(ops, cs)
> 	       })
> 	})

I converted my fledgling app to this style and it seems kind neat.
Relatively freer of scaffolding.

The

    return ...
      ... return ...
	  ... return ...

idiom looks kind of weird, though.

> typing `func(cs layout.Constraints) layout.Dimensions {` at every layout boundary
> was so tedious I had a copy buffer with that string while writing new layout.

Yeah, I could see adding a Vim/Emacs macro for that.

> Can it be done better? One (old) idea is to shorten the type names:
> 
> 	func(cs layout.Cons) layout.Dims
> 
> or even
> 
> 	func(cs layout.C) layout.D

You might consider type aliases:

  package layout

  type C = Constraints
  type D = Dimensions

and then layout.Constraints and layout.C mean exactly the same thing.
(I think.  I haven't actually tried this.)

The rest of the proposal strikes me as interesting and attractive.  I
await further comments.

-- Larry
Gregory Pomerantz
Details
Message ID
<9e50a8c0-2f39-57c9-087f-f011a238f5c3@wow.st>
In-Reply-To
<20190920150208.GA51850@larrymbp14.local> (view parent)
DKIM signature
missing
Download raw message
I did some playing around with this. The Go memory model makes it 
difficult to avoid allocations and garbage, especially if you want to 
make use of some of the nicer golang features. Since Widget is a 
function, you cannot write a function that returns a Widget without 
creating garbage (functions are pointer types and closures will always 
allocate at least one pointer, which will probably escape to the heap). 
We can't fix this by making Widget an interface, because interfaces 
contain pointers, and passing a concrete type to an interface parameter 
will create a pointer and likely cause local data to escape to the heap.

In addition, lack of some functional programming convenience features, 
like type inference when declaring anonymous functions and currying, 
make it hard to code in this style without being explicit about all of 
your plumbing. Nesting functions are not really going to be very nice to 
read when they have to declare all of their parameter types over and 
over again, it obscures the important functionality that is going on in 
our core rendering loops. I don't think people are going to like 
reviewing code where the main layout functionality is buried under lots 
of redundant plumbing.

For example, the following approach works and doesn't leak any garbage:

mylab := Label{Face: face, Text: "txt"} // a struct with a "Layout" method
ins := layout.UniformInset(ui.Dp(10))
ins.Layout(ops, cs, mylab.Layout)

But Inset.Layout(...) returns Dimensions, not another Widget, and it 
can't return another Widget (whether it is a function or interface type) 
without a closure or some other approach that will leak garbage, so we 
don't have the ability to build larger structures through composition.

Ultimately I think something like the stack approach seems likely to be 
most convenient. I would prefer trading off complexity in defining new 
Widgets (which will be done less often and by more experienced users) 
vs. convenience of coding the core layout loops in apps.

Have you considered an approach where Stack is an Ops-like structure for 
layout operations? Perhaps it will be possible to have something like this:

     st := new(layout.Stack)

     for { select { ...         // main event loop
         layout.Flex{Axis: layout.Vertical}.Add(st) // add to stack, 
subsequent entries are added as flex children
             layout.UniformInset(ui.Dp(10)).Add(st) // added to the flex
     text.Label{Face: face, Text: "txt"}.Add(st) // inset only consumes 
one subsequent widget
             layout.Flexible(0.5)).Add(st)     // add instructions to 
control the flex
             layout.UniformInset(ui.Dp(10)).Add(st)     // a second 
FlexChild
                 text.Label{Face: face, Text: "txt"}.Add(st)
         st.End()            // done with the Flex
         st.Layout(c, q, ops, cs)    // also calls st.Reset() so wecan 
add more widgets later
         // can work directly with ops here or use our stack again
         w.Update(ops)
         }}
Details
Message ID
<BX8EP7F7L74F.1TZKIPE67SI2T@toolbox>
In-Reply-To
<9e50a8c0-2f39-57c9-087f-f011a238f5c3@wow.st> (view parent)
DKIM signature
missing
Download raw message
On Tue Sep 24, 2019 at 9:30 AM Gregory Pomerantz wrote:
> I did some playing around with this. The Go memory model makes it 
> difficult to avoid allocations and garbage, especially if you want to 
> make use of some of the nicer golang features. Since Widget is a 
> function, you cannot write a function that returns a Widget without 
> creating garbage (functions are pointer types and closures will always 
> allocate at least one pointer, which will probably escape to the heap). 
> We can't fix this by making Widget an interface, because interfaces 
> contain pointers, and passing a concrete type to an interface parameter 
> will create a pointer and likely cause local data to escape to the heap.
> 

I had similar experience during my attempts to merge functional scopes.

> In addition, lack of some functional programming convenience features, 
> like type inference when declaring anonymous functions and currying, 
> make it hard to code in this style without being explicit about all of 
> your plumbing. Nesting functions are not really going to be very nice to 
> read when they have to declare all of their parameter types over and 
> over again, it obscures the important functionality that is going on in 
> our core rendering loops. I don't think people are going to like 
> reviewing code where the main layout functionality is buried under lots 
> of redundant plumbing.
> 

I agree.

> 
> Ultimately I think something like the stack approach seems likely to be 
> most convenient. I would prefer trading off complexity in defining new 
> Widgets (which will be done less often and by more experienced users) 
> vs. convenience of coding the core layout loops in apps.
> 

I just pushed

	https://gioui.org/commit/ec307008db58ec6f0d7e788978a930f61c6bd1bf
	https://gioui.org/commit/ce9bcee62baa42c29fe37ca011bdd5ee1d902e39

that implements layout.Context for tracking the current Constraints and
dimensions. It is unfortunate that we have to give up the direct and simpler
explicit functional approach, but I think the much shorter Context approach
is worth the complexity. Even moreso if we merge the Ops and Config into
Context.

> Have you considered an approach where Stack is an Ops-like structure for 
> layout operations? Perhaps it will be possible to have something like this:
> 
>      st := new(layout.Stack)
> 
>      for { select { ...         // main event loop
>          layout.Flex{Axis: layout.Vertical}.Add(st) // add to stack, 
> subsequent entries are added as flex children
>              layout.UniformInset(ui.Dp(10)).Add(st) // added to the flex
>      text.Label{Face: face, Text: "txt"}.Add(st) // inset only consumes 
> one subsequent widget
>              layout.Flexible(0.5)).Add(st)     // add instructions to 
> control the flex
>              layout.UniformInset(ui.Dp(10)).Add(st)     // a second 
> FlexChild
>                  text.Label{Face: face, Text: "txt"}.Add(st)
>          st.End()            // done with the Flex
>          st.Layout(c, q, ops, cs)    // also calls st.Reset() so wecan 
> add more widgets later
>          // can work directly with ops here or use our stack again
>          w.Update(ops)
>          }}
> 

I can't see how that can be done in a garbage free way? The set of
operation types are known in advance so they can be encoded to a
byte slice. Layouts are code and can't be encoded.

--
elias
Gregory Pomerantz
Details
Message ID
<3ea7258c-4447-92e3-ec44-792633496bc2@wow.st>
In-Reply-To
<BX8EP7F7L74F.1TZKIPE67SI2T@toolbox> (view parent)
DKIM signature
missing
Download raw message
>>       st := new(layout.Stack)
>>
>>       for { select { ...         // main event loop
>>           layout.Flex{Axis: layout.Vertical}.Add(st) // add to stack,
>> subsequent entries are added as flex children
>>               layout.UniformInset(ui.Dp(10)).Add(st) // added to the flex
>>       text.Label{Face: face, Text: "txt"}.Add(st) // inset only consumes
>> one subsequent widget
>>               layout.Flexible(0.5)).Add(st)     // add instructions to
>> control the flex
>>               layout.UniformInset(ui.Dp(10)).Add(st)     // a second
>> FlexChild
>>                   text.Label{Face: face, Text: "txt"}.Add(st)
>>           st.End()            // done with the Flex
>>           st.Layout(c, q, ops, cs)    // also calls st.Reset() so wecan
>> add more widgets later
>>           // can work directly with ops here or use our stack again
>>           w.Update(ops)
>>           }}
> I can't see how that can be done in a garbage free way? The set of
> operation types are known in advance so they can be encoded to a
> byte slice. Layouts are code and can't be encoded.
It would require a definition of a fixed set of primitive layout 
operations, which can be combined for more complex widgets, and possibly 
the ability to pre-register additional primitives via function pointers 
(these would be "garbage" but only created once at app startup).
Details
Message ID
<20190924181132.GA3497@larrymbp14.local>
In-Reply-To
<BX8EP7F7L74F.1TZKIPE67SI2T@toolbox> (view parent)
DKIM signature
missing
Download raw message
On Tue, Sep 24, 2019 at 07:25:21PM +0200, Elias Naur wrote:
> I just pushed
> 
> 	https://gioui.org/commit/ce9bcee62baa42c29fe37ca011bdd5ee1d902e39
> 
> that implements layout.Context for tracking the current Constraints
> and dimensions. It is unfortunate that we have to give up the direct
> and simpler explicit functional approach, but I think the much
> shorter Context approach is worth the complexity. Even moreso if we
> merge the Ops and Config into Context.

> - 	var cs layout.Constraints = ...
> + 	ctx := new(layout.Context)
> + 	ctx.Constraints = ...

FWIW, I still think starting a convention of calling layout.Context
"ctx" invites confusion with respect to context.Context, which is also
usually called "ctx".  I don't have a solid "experience report", as
such, but it's just a hunch.  I'd suggest gctx or even gtx.

Just my two cents.

-- Larry
Details
Message ID
<BX8GCBELV02X.8MA86NJQXUB1@toolbox>
In-Reply-To
<20190924181132.GA3497@larrymbp14.local> (view parent)
DKIM signature
missing
Download raw message
On Tue Sep 24, 2019 at 2:11 PM Larry Clapp wrote:
> On Tue, Sep 24, 2019 at 07:25:21PM +0200, Elias Naur wrote:
> > I just pushed
> > 
> > 	https://gioui.org/commit/ce9bcee62baa42c29fe37ca011bdd5ee1d902e39
> > 
> > that implements layout.Context for tracking the current Constraints
> > and dimensions. It is unfortunate that we have to give up the direct
> > and simpler explicit functional approach, but I think the much
> > shorter Context approach is worth the complexity. Even moreso if we
> > merge the Ops and Config into Context.
> 
> > - 	var cs layout.Constraints = ...
> > + 	ctx := new(layout.Context)
> > + 	ctx.Constraints = ...
> 
> FWIW, I still think starting a convention of calling layout.Context
> "ctx" invites confusion with respect to context.Context, which is also
> usually called "ctx".  I don't have a solid "experience report", as
> such, but it's just a hunch.  I'd suggest gctx or even gtx.
> 

`gtx` is nice, but how about just `c`, after merging Ops, Config and
Queue into Context?

-- elias
Details
Message ID
<BX8JE1KV9JOF.3CV3YGE5N9IH6@toolbox>
In-Reply-To
<BX8GCBELV02X.8MA86NJQXUB1@toolbox> (view parent)
DKIM signature
missing
Download raw message
On Tue Sep 24, 2019 at 8:42 PM Elias Naur wrote:
> On Tue Sep 24, 2019 at 2:11 PM Larry Clapp wrote:
> > On Tue, Sep 24, 2019 at 07:25:21PM +0200, Elias Naur wrote:
> > > I just pushed
> > > 
> > > 	https://gioui.org/commit/ce9bcee62baa42c29fe37ca011bdd5ee1d902e39
> > > 
> > > that implements layout.Context for tracking the current Constraints
> > > and dimensions. It is unfortunate that we have to give up the direct
> > > and simpler explicit functional approach, but I think the much
> > > shorter Context approach is worth the complexity. Even moreso if we
> > > merge the Ops and Config into Context.
> > 
> > > - 	var cs layout.Constraints = ...
> > > + 	ctx := new(layout.Context)
> > > + 	ctx.Constraints = ...
> > 
> > FWIW, I still think starting a convention of calling layout.Context
> > "ctx" invites confusion with respect to context.Context, which is also
> > usually called "ctx".  I don't have a solid "experience report", as
> > such, but it's just a hunch.  I'd suggest gctx or even gtx.
> > 
> 
> `gtx` is nice, but how about just `c`, after merging Ops, Config and
> Queue into Context?
> 

The "fat" Context is implemented in

	https://gioui.org/commit/2782436ffc1348249baa41f54229b1826fed4e80
	https://gioui.org/commit/4d84f46edbb1cd8575c4e13d688eac47ea98b8fc

Another opportunity with Context is replacing Layout(*Context) with two
calls, Init(*Context) and Layout(), allowing us to skip the last function
scope. For example:

	diff --git a/apps/gophers/ui.go b/apps/gophers/ui.go
	index 1211afb..e93e378 100644
	--- a/apps/gophers/ui.go
	+++ b/apps/gophers/ui.go
	@@ -185,11 +185,10 @@ func (u *UI) layoutTimings(c *layout.Context) {
		runtime.ReadMemStats(&mstats)
		mallocs := mstats.Mallocs - u.lastMallocs
		u.lastMallocs = mstats.Mallocs
	+       txt := fmt.Sprintf("m: %d %s", mallocs, u.profile.Timings)
	+       lbl := (&text.Label{Material: theme.text, Face: u.face(fonts.mono, 10), Text: txt})
		layout.Align(layout.NE).Layout(c, func() {
	-               layout.Inset{Top: ui.Dp(16)}.Layout(c, func() {
	-                       txt := fmt.Sprintf("m: %d %s", mallocs, u.profile.Timings)
	-                       text.Label{Material: theme.text, Face: u.face(fonts.mono, 10), Text: txt}.Layout(c)
	-               })
	+               layout.Inset{Top: ui.Dp(16)}.Layout(c, lbl.Init(c).Layout2)
		})
	 }
	 
	diff --git a/ui/text/label.go b/ui/text/label.go
	index ebbd57d..31d8ad9 100644
	--- a/ui/text/label.go
	+++ b/ui/text/label.go
	@@ -31,7 +31,8 @@ type Label struct {
		// may fill.
		MaxLines int
	 
	-       it lineIterator
	+       ctx *layout.Context
	+       it  lineIterator
	 }
	 
	 type lineIterator struct {
	@@ -92,7 +93,18 @@ func (l *lineIterator) Next() (String, f32.Point, bool) {
		return String{}, f32.Point{}, false
	 }
	 
	+func (l *Label) Init(c *layout.Context) *Label {
	+       l.ctx = c
	+       return l
	+}
	+
	 func (l Label) Layout(c *layout.Context) {
	+       l.Init(c)
	+       l.Layout2()
	+}
	+
	+func (l Label) Layout2() {
	+       c := l.ctx
		cs := c.Constraints
		textLayout := l.Face.Layout(l.Text, LayoutOptions{MaxWidth: cs.Width.Max})
		lines := textLayout.Lines

Instead of an Init method, we could export a Context field in Label (and other widgets).

However, we're then opening up for the programmer missing an Init call (or Context field), leading to
a runtime panic. Can we do better?

-- elias
Gregory Pomerantz
Details
Message ID
<75bd1233-e49e-e3c4-d9a2-dcecd973d067@wow.st>
In-Reply-To
<BX8JE1KV9JOF.3CV3YGE5N9IH6@toolbox> (view parent)
DKIM signature
missing
Download raw message
This is starting to look really good!
> Instead of an Init method, we could export a Context field in Label (and other widgets).
>
> However, we're then opening up for the programmer missing an Init call (or Context field), leading to
> a runtime panic. Can we do better?
Does defining Widget as "func(*Context)" work better? If Label.Layout 
accepts a pointer to Context we won't need to call Init().
Details
Message ID
<BX8KCIGOWJZV.9UKGPPTRHVRD@toolbox>
In-Reply-To
<75bd1233-e49e-e3c4-d9a2-dcecd973d067@wow.st> (view parent)
DKIM signature
missing
Download raw message
On Tue Sep 24, 2019 at 5:46 PM Gregory Pomerantz wrote:
> This is starting to look really good!
> > Instead of an Init method, we could export a Context field in Label (and other widgets).
> >
> > However, we're then opening up for the programmer missing an Init call (or Context field), leading to
> > a runtime panic. Can we do better?
> Does defining Widget as "func(*Context)" work better? If Label.Layout 
> accepts a pointer to Context we won't need to call Init().

It works, but then we'll have to add *Context parameters to all intermediary layouts, such as from

	layout.Align(layout.NE).Layout(c, func() {

to

	layout.Align(layout.NE).Layout(c, func(c *Context) {

from my example above.


-- elias
Details
Message ID
<20190925024518.GA9771@larrymbp14.local>
In-Reply-To
<BX8GCBELV02X.8MA86NJQXUB1@toolbox> (view parent)
DKIM signature
missing
Download raw message
On Tue, Sep 24, 2019 at 08:42:33PM +0200, Elias Naur wrote:
> On Tue Sep 24, 2019 at 2:11 PM Larry Clapp wrote:
> > FWIW, I still think starting a convention of calling
> > layout.Context "ctx" invites confusion with respect to
> > context.Context, which is also usually called "ctx".  I don't have
> > a solid "experience report", as such, but it's just a hunch.  I'd
> > suggest gctx or even gtx.
> 
> `gtx` is nice, but how about just `c`, after merging Ops, Config and
> Queue into Context?

I'm using gtx in my own code so far, and think I prefer it.  YMMV.

With this new style ... Maybe I misunderstand Constraints.  I thought
Constraints were so you could create a child widget and say "no
smaller than this, no larger than that".  First of all: is that right?
(If not, what *are* they for?)

If so: If you set Constraints in a context, and then pass the context
to the child's Layout, what allows later children to use the previous,
unaltered constraints?  It seems like it should be stack-like (like it
was before), but I don't see how that could be right now.

I see some example code in apps/gophers/ui.go setting c.Constraints,
e.g. in layoutUsers(), but I don't see anything restoring the previous
value.

Thanks for your help.

-- Larry
Details
Message ID
<BX8WTRDY8A6R.20ERUT01UF0IF@toolbox>
In-Reply-To
<20190925024518.GA9771@larrymbp14.local> (view parent)
DKIM signature
missing
Download raw message
On Tue Sep 24, 2019 at 10:45 PM Larry Clapp wrote:
> On Tue, Sep 24, 2019 at 08:42:33PM +0200, Elias Naur wrote:
> > On Tue Sep 24, 2019 at 2:11 PM Larry Clapp wrote:
> > > FWIW, I still think starting a convention of calling
> > > layout.Context "ctx" invites confusion with respect to
> > > context.Context, which is also usually called "ctx".  I don't have
> > > a solid "experience report", as such, but it's just a hunch.  I'd
> > > suggest gctx or even gtx.
> > 
> > `gtx` is nice, but how about just `c`, after merging Ops, Config and
> > Queue into Context?
> 
> I'm using gtx in my own code so far, and think I prefer it.  YMMV.
> 

I warmed up to `gtx`. Changed in https://gioui.org/commit/a89c6d1, thanks
for persisting.

> With this new style ... Maybe I misunderstand Constraints.  I thought
> Constraints were so you could create a child widget and say "no
> smaller than this, no larger than that".  First of all: is that right?
> (If not, what *are* they for?)
>

That's what they're for.
 
> If so: If you set Constraints in a context, and then pass the context
> to the child's Layout, what allows later children to use the previous,
> unaltered constraints?  It seems like it should be stack-like (like it
> was before), but I don't see how that could be right now.
> 
> I see some example code in apps/gophers/ui.go setting c.Constraints,
> e.g. in layoutUsers(), but I don't see anything restoring the previous
> value.
>

Layout functions save and restore the constraints through the Context.Layout
method (https://git.sr.ht/~eliasnaur/gio/tree/master/ui/layout/layout.go#L84).
 
-- elias