Hi,
I've pushed a proposal for replacing the op.Save/Load state stack with
explicit per-state stacks to the op-stacks branch[0]. Please review the
proposed API changes for any improvements and issues, and let me know if the
changes will pose difficulties to your Gio programs.
The detailed description is in the commit message for "all: [API] split
operation stack into per-state stacks"[0].
Thanks,
Elias
[0] https://git.sr.ht/~eliasnaur/gio/refs/op-stacks
[1] https://git.sr.ht/~eliasnaur/gio/commit/6e9574245074656fe272928ec0523be4749a80a9
I've converted the gioui.org/x/component package and example program
to use this new API as a means of trying it out. You can find those
changes on the op-stacks branches in their respective repositories.
I think this API makes sense, but it would be nice to ease the pain of
migration. Updating all of those components was very tedious, and
there isn't a good way to automate some of the transformations.
I'm not sure how to do it, but it'd be really good to ease the
migration pain to this API somehow. Perhaps we still provide
op.{Load,Save}, just implemented on top of the new API? Similarly,
could all of the stack-based ops retain their Add() methods, but those
add methods just don't provide a pop facility? We're already doing
that with TransformOp now. I'm not sure what the tradeoff of leaving
the Add() method is though.
I do think that this API makes the need (or lack of) for the stack
very clear, so it achieves the stated goal. Nicely done there!
Cheers,
Chris
My only concern is with multiple `defer`, that is one example:
```
stack := op.Stack(gtx.Ops)
gtx.Constraints.Max.X = size
gtx.Constraints.Max.Y = size
for _, elm := range elements {
elm.Layout(gtx)
op.Offset(f32.Point(float32(size), 0).Add(gtx.Ops)
}
stack.Load()
```
So, the goal is to move the next element by constant `size` and any
element will have the same `size`. In that case, I want to "leak" the
`op.Offset` across multiple layouts.
The alternative seems to replace by:
```
gtx.Constraints.Max.X = size
for _, elm := range elements {
elm.Layout(gtx)
defer op.Offset(f32.Point(float32(size), 0).Push(gtx.Ops).Pop()
}
```
However, the IDE warning that it's a "resource leak", since it's creating
multiples `defer` inside a `for`. _I don't know if `go vet` also report it._
Another alternative is to use a single `offset` for each one, but
that needs to keep a "external state":
```
gtx.Constraints.Max.X = size
usedSize := 0 // Needs to keep a external state of the offset, instead of increase.
for _, elm := range elements {
offset := p.Offset(f32.Point(float32(usedSize), 0).Push(gtx.Ops)
elm.Layout(gtx)
offset.Pop()
usedSize += size
}
```
That is the only thing that I don't like about the new API. However, the new
API might help to avoid the "Stack Hell", where everything is a new stack/load,
without actually need it.
--
Lucas Rodrigues
inkeliz@inkeliz.com
On Thu, Oct 7, 2021, at 9:03 PM, Chris Waldon wrote:
> I've converted the gioui.org/x/component package and example program> to use this new API as a means of trying it out. You can find those> changes on the op-stacks branches in their respective repositories.>> I think this API makes sense, but it would be nice to ease the pain of> migration. Updating all of those components was very tedious, and> there isn't a good way to automate some of the transformations.>> I'm not sure how to do it, but it'd be really good to ease the> migration pain to this API somehow. Perhaps we still provide> op.{Load,Save}, just implemented on top of the new API? Similarly,> could all of the stack-based ops retain their Add() methods, but those> add methods just don't provide a pop facility? We're already doing> that with TransformOp now. I'm not sure what the tradeoff of leaving> the Add() method is though.>> I do think that this API makes the need (or lack of) for the stack> very clear, so it achieves the stated goal. Nicely done there!>> Cheers,> Chris
On Thu Oct 7, 2021 at 23:35 CEST, Lucas Rodrigues wrote:
> My only concern is with multiple `defer`, that is one example:>> ```> stack := op.Stack(gtx.Ops)> gtx.Constraints.Max.X = size> gtx.Constraints.Max.Y = size>> for _, elm := range elements {> elm.Layout(gtx)> op.Offset(f32.Point(float32(size), 0).Add(gtx.Ops)> }>> stack.Load()> ```>> So, the goal is to move the next element by constant `size` and any > element will have the same `size`. In that case, I want to "leak" the > `op.Offset` across multiple layouts. >
I belive that's solved by the addition (re-introducing) of TransformOp.Add.
Doesn't that method solve this problem? Note that the initial op.Save
can be replaced by a dummy `op.Offset(image.Point{}).Push`.
Elias
On Thu Oct 7, 2021 at 22:03 CEST, Chris Waldon wrote:
> I've converted the gioui.org/x/component package and example program> to use this new API as a means of trying it out. You can find those> changes on the op-stacks branches in their respective repositories.>> I think this API makes sense, but it would be nice to ease the pain of> migration. Updating all of those components was very tedious, and> there isn't a good way to automate some of the transformations.>> I'm not sure how to do it, but it'd be really good to ease the> migration pain to this API somehow. Perhaps we still provide> op.{Load,Save}, just implemented on top of the new API? Similarly,> could all of the stack-based ops retain their Add() methods, but those> add methods just don't provide a pop facility? We're already doing> that with TransformOp now. I'm not sure what the tradeoff of leaving> the Add() method is though.>
Good idea. I've pushed the API change with a op.Save/Load that works for
the transformation and clip stack. I didn't figure out a nice way to
preserve the pointer.AreaOp stack.
Elias
On Fri, Oct 8, 2021 at 11:36 AM Elias Naur <mail@eliasnaur.com> wrote:
>> On Thu Oct 7, 2021 at 22:03 CEST, Chris Waldon wrote:> > I've converted the gioui.org/x/component package and example program> > to use this new API as a means of trying it out. You can find those> > changes on the op-stacks branches in their respective repositories.> >> > I think this API makes sense, but it would be nice to ease the pain of> > migration. Updating all of those components was very tedious, and> > there isn't a good way to automate some of the transformations.> >> > I'm not sure how to do it, but it'd be really good to ease the> > migration pain to this API somehow. Perhaps we still provide> > op.{Load,Save}, just implemented on top of the new API? Similarly,> > could all of the stack-based ops retain their Add() methods, but those> > add methods just don't provide a pop facility? We're already doing> > that with TransformOp now. I'm not sure what the tradeoff of leaving> > the Add() method is though.> >>> Good idea. I've pushed the API change with a op.Save/Load that works for> the transformation and clip stack. I didn't figure out a nice way to> preserve the pointer.AreaOp stack.
Hmm. So it'll be a breaking change, but only for pointer hit areas.
That limits the blast area. I think those are rarer than clips and
transforms in my applications. Seems like about as good of a tradeoff
as we can make. Nice!
Now that the changes have been made, here is a quick report on
updating giocanvas with the new API. Originally, giocanvas has
several functions that perform transforms like this:
// AbsTranslate moves current location by (x,y)
func (c *Canvas) AbsTranslate(x, y float32) op.StateOp {
ops := c.Context.Ops
op.InvalidateOp{}.Add(ops)
stack := op.Save(ops)
tr := f32.Affine2D{}
tr = tr.Offset(f32.Pt(x, y))
op.Affine(tr).Add(ops)
return stack
}
// Translate moves current location by (x,y) using percentage-based measures
func (c *Canvas) Translate(x, y float32) op.StateOp {
x, y = dimen(x, y, c.Width, c.Height)
return c.AbsTranslate(x, y)
}
// EndTransform ends a transformation
func EndTransform(stack op.StateOp) {
stack.Load()
}
With the new stack API:
// AbsTranslate moves current location by (x,y)
func (c *Canvas) AbsTranslate(x, y float32) op.TransformStack {
ops := c.Context.Ops
op.InvalidateOp{}.Add(ops)
stack := op.Offset(f32.Pt(0, 0)).Push(ops)
tr := f32.Affine2D{}
tr = tr.Offset(f32.Pt(x, y))
op.Affine(tr).Add(ops)
return stack
}
// Translate moves current location by (x,y) using percentage-based measures
func (c *Canvas) Translate(x, y float32) op.TransformStack {
x, y = dimen(x, y, c.Width, c.Height)
return c.AbsTranslate(x, y)
}
// EndTransform ends a transformation
func EndTransform(stack op.TransformStack) {
stack.Pop()
}
On Mon, Oct 4, 2021 at 11:53 AM Elias Naur <mail@eliasnaur.com> wrote:
>> Hi,>> I've pushed a proposal for replacing the op.Save/Load state stack with> explicit per-state stacks to the op-stacks branch[0]. Please review the> proposed API changes for any improvements and issues, and let me know if the> changes will pose difficulties to your Gio programs.>> The detailed description is in the commit message for "all: [API] split> operation stack into per-state stacks"[0].>> Thanks,> Elias>> [0] https://git.sr.ht/~eliasnaur/gio/refs/op-stacks> [1] https://git.sr.ht/~eliasnaur/gio/commit/6e9574245074656fe272928ec0523be4749a80a9
> I think this API makes sense, but it would be nice to ease the pain of> migration. Updating all of those components was very tedious, and> there isn't a good way to automate some of the transformations.
Is go fix still a thing? How easy would it be to create something for this?
On Tue Oct 12, 2021 at 17:53 CEST, Tanguy ⧓ Herrmann wrote:
> > I think this API makes sense, but it would be nice to ease the pain of> > migration. Updating all of those components was very tedious, and> > there isn't a good way to automate some of the transformations.>> Is go fix still a thing? How easy would it be to create something for this?
I try to provide `go fix` scripts when possible. For this change
however, there wasn't enough information to split op.Saves into per-op
Push operations.
Elias