~eliasnaur/gio

4 3

RFC: [API] separate unit types, integer op.Offset/clip.RRect etc.

Details
Message ID
<CAMAFT9XLLdmRBfSz=7v9Hnr0NBcNFygABVfd3V7kt57wp=9pJA@mail.gmail.com>
DKIM signature
pass
Download raw message
Hi,

The units[0] branch contains a bunch of related API updates that will
affect most Gio programs. Please let me know if you have any
questions, improvements or objections to the changes.

The details are in the commit messages, here's a quick overview:

- unit.Value is replaced by separate unit.Sp and unit.Dp types.
- op.Offset, clip.RRect, clip.UniformRRect and other minor API is
changed to accept integer coordinates.
- f32.Rectangle is unexported, and layout.FRect is removed.
- layout.FPt is removed.

Elias

[0]: https://git.sr.ht/~eliasnaur/gio/refs/units
Details
Message ID
<CAFcc3FRNWGv_gx_DYT1pc+d6Gtgm3EBDQhtjOH_rWyHZEnB_Hg@mail.gmail.com>
In-Reply-To
<CAMAFT9XLLdmRBfSz=7v9Hnr0NBcNFygABVfd3V7kt57wp=9pJA@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
> The units[0] branch contains a bunch of related API updates that will
> affect most Gio programs. Please let me know if you have any
> questions, improvements or objections to the changes.
>
> The details are in the commit messages, here's a quick overview:
>
> - unit.Value is replaced by separate unit.Sp and unit.Dp types.

Having tried it, this is nice. However, I find myself wanting to be
very explicit whenever anything (like a field or parameter) is in
pixels. Could we also introduce:

type Pixel = int

This would mean it can be used as an int without a cast, but it lets
me label values as unit.Pixel. Maybe it should be plural? unit.Pixels?
Either way, I think this would help me write clearer code. This would
also mean that unit.Metric.{S,D}p could return Pixels, which would
make their signature easier to read.

> - op.Offset, clip.RRect, clip.UniformRRect and other minor API is
> changed to accept integer coordinates.

This is nice, and does save me some casts. However, the fact that
pointer.Event still returns f32.Points now creates a bunch of new
friction when working with offset transformations related to pointer
events. Do we actually have fractional-pixel precision on touch
events? If not, can we use image.Point for those fields?

> - f32.Rectangle is unexported, and layout.FRect is removed.
> - layout.FPt is removed.

I don't understand the rationale for removing layout.FPt while the
f32.Point type is still in the public API. As long as I need to be
able to compare between an image.Point and an f32.Point, I'll want
helpers for converting between the two.

On the whole, I think this is a step forward. It removes a bunch of
times that I previously needed to use floats purely to satisfy an API,
rather than because I needed fractional precision. That's an
ergonomics win, and also probably a tiny performance one. However, I
do think we need to make converting to/from floating representations
really easy so long as we keep them anywhere within the exported API.

Also, this change will mean a lot of tedious work for most Gio
programmers. I spent about half an hour trying to convert
gioui.org/x/component to this API (generating the above feedback) and
I'm nowhere near done. This changeset should not be merged lightly.

Nice work Elias!
Chris
Details
Message ID
<CAMAFT9U6uFLE0DMCgHNVaZqurXY-BwdHiz=pNKbowF+=vJqyGQ@mail.gmail.com>
In-Reply-To
<CAFcc3FRNWGv_gx_DYT1pc+d6Gtgm3EBDQhtjOH_rWyHZEnB_Hg@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On Sun, 1 May 2022 at 19:49, Chris Waldon
<christopher.waldon.dev@gmail.com> wrote:
>
> > The units[0] branch contains a bunch of related API updates that will
> > affect most Gio programs. Please let me know if you have any
> > questions, improvements or objections to the changes.
> >
> > The details are in the commit messages, here's a quick overview:
> >
> > - unit.Value is replaced by separate unit.Sp and unit.Dp types.
>
> Having tried it, this is nice. However, I find myself wanting to be
> very explicit whenever anything (like a field or parameter) is in
> pixels. Could we also introduce:
>
> type Pixel = int
>
> This would mean it can be used as an int without a cast, but it lets
> me label values as unit.Pixel. Maybe it should be plural? unit.Pixels?

What about just "Px" like Sp and Dp?

> Either way, I think this would help me write clearer code. This would
> also mean that unit.Metric.{S,D}p could return Pixels, which would
> make their signature easier to read.
>

This seems like a good idea to me. I have one objection, which is that
we're using both ints and float32s for pixel values. Blessing one of them
as Px seems off. WDYT?

In any case, I believe your unit.Px proposal is orthogonal to the other
changes, because unit.Px is a type alias.

> > - op.Offset, clip.RRect, clip.UniformRRect and other minor API is
> > changed to accept integer coordinates.
>
> This is nice, and does save me some casts. However, the fact that
> pointer.Event still returns f32.Points now creates a bunch of new
> friction when working with offset transformations related to pointer
> events. Do we actually have fractional-pixel precision on touch
> events? If not, can we use image.Point for those fields?
>

I struggled with this issue as well, and I can't see how we can get rid
of float32 coordinates in raw events, because AFAIK some high-quality
pointing devices have sub-pixel precision. I haven't checked, but I imagine
the fancy Apple pens are among them.

I landed on a middle-ground which is that gestures will report integer
coordinates
and hide the complexity in their implementation. Unfortunately, that
won't solve the
issue for custom gestures.

> > - f32.Rectangle is unexported, and layout.FRect is removed.
> > - layout.FPt is removed.
>
> I don't understand the rationale for removing layout.FPt while the
> f32.Point type is still in the public API. As long as I need to be
> able to compare between an image.Point and an f32.Point, I'll want
> helpers for converting between the two.
>

I agree. I've restored layout.FPt.

> On the whole, I think this is a step forward. It removes a bunch of
> times that I previously needed to use floats purely to satisfy an API,
> rather than because I needed fractional precision. That's an
> ergonomics win, and also probably a tiny performance one. However, I
> do think we need to make converting to/from floating representations
> really easy so long as we keep them anywhere within the exported API.
>
> Also, this change will mean a lot of tedious work for most Gio
> programmers. I spent about half an hour trying to convert
> gioui.org/x/component to this API (generating the above feedback) and
> I'm nowhere near done. This changeset should not be merged lightly.
>

Do you have any suggestions that would ease conversion? The changes are
split in several commits, so it is possible to ratchet a code base one
commit at a time,
but I'm not sure it's worth it compared to doing it all at once.

Thanks for the feedback,
Elias
Details
Message ID
<3dbf32b1-40fc-4836-81dd-4858aa8a22bc@www.fastmail.com>
In-Reply-To
<CAMAFT9U6uFLE0DMCgHNVaZqurXY-BwdHiz=pNKbowF+=vJqyGQ@mail.gmail.com> (view parent)
DKIM signature
missing
Download raw message
"f32.Rectangle is unexported,". Does it also applies for f32.Point? Because, for custom paths, f32.Point is required, and I think f32.Rectangle can be also useful for custom paths, since it's just 2x f32.Point.

-- 
  Lucas Rodrigues
  inkeliz@inkeliz.com

On Mon, May 2, 2022, at 9:02 AM, Elias Naur wrote:
> On Sun, 1 May 2022 at 19:49, Chris Waldon
> <christopher.waldon.dev@gmail.com> wrote:
>>
>> > The units[0] branch contains a bunch of related API updates that will
>> > affect most Gio programs. Please let me know if you have any
>> > questions, improvements or objections to the changes.
>> >
>> > The details are in the commit messages, here's a quick overview:
>> >
>> > - unit.Value is replaced by separate unit.Sp and unit.Dp types.
>>
>> Having tried it, this is nice. However, I find myself wanting to be
>> very explicit whenever anything (like a field or parameter) is in
>> pixels. Could we also introduce:
>>
>> type Pixel = int
>>
>> This would mean it can be used as an int without a cast, but it lets
>> me label values as unit.Pixel. Maybe it should be plural? unit.Pixels?
>
> What about just "Px" like Sp and Dp?
>
>> Either way, I think this would help me write clearer code. This would
>> also mean that unit.Metric.{S,D}p could return Pixels, which would
>> make their signature easier to read.
>>
>
> This seems like a good idea to me. I have one objection, which is that
> we're using both ints and float32s for pixel values. Blessing one of them
> as Px seems off. WDYT?
>
> In any case, I believe your unit.Px proposal is orthogonal to the other
> changes, because unit.Px is a type alias.
>
>> > - op.Offset, clip.RRect, clip.UniformRRect and other minor API is
>> > changed to accept integer coordinates.
>>
>> This is nice, and does save me some casts. However, the fact that
>> pointer.Event still returns f32.Points now creates a bunch of new
>> friction when working with offset transformations related to pointer
>> events. Do we actually have fractional-pixel precision on touch
>> events? If not, can we use image.Point for those fields?
>>
>
> I struggled with this issue as well, and I can't see how we can get rid
> of float32 coordinates in raw events, because AFAIK some high-quality
> pointing devices have sub-pixel precision. I haven't checked, but I imagine
> the fancy Apple pens are among them.
>
> I landed on a middle-ground which is that gestures will report integer
> coordinates
> and hide the complexity in their implementation. Unfortunately, that
> won't solve the
> issue for custom gestures.
>
>> > - f32.Rectangle is unexported, and layout.FRect is removed.
>> > - layout.FPt is removed.
>>
>> I don't understand the rationale for removing layout.FPt while the
>> f32.Point type is still in the public API. As long as I need to be
>> able to compare between an image.Point and an f32.Point, I'll want
>> helpers for converting between the two.
>>
>
> I agree. I've restored layout.FPt.
>
>> On the whole, I think this is a step forward. It removes a bunch of
>> times that I previously needed to use floats purely to satisfy an API,
>> rather than because I needed fractional precision. That's an
>> ergonomics win, and also probably a tiny performance one. However, I
>> do think we need to make converting to/from floating representations
>> really easy so long as we keep them anywhere within the exported API.
>>
>> Also, this change will mean a lot of tedious work for most Gio
>> programmers. I spent about half an hour trying to convert
>> gioui.org/x/component to this API (generating the above feedback) and
>> I'm nowhere near done. This changeset should not be merged lightly.
>>
>
> Do you have any suggestions that would ease conversion? The changes are
> split in several commits, so it is possible to ratchet a code base one
> commit at a time,
> but I'm not sure it's worth it compared to doing it all at once.
>
> Thanks for the feedback,
> Elias
Details
Message ID
<CAMAFT9VG-onTLbfWFcO6NEWkY3t5jh-SrQgchinJ8=jr=Wck_g@mail.gmail.com>
In-Reply-To
<3dbf32b1-40fc-4836-81dd-4858aa8a22bc@www.fastmail.com> (view parent)
DKIM signature
pass
Download raw message
On Mon, 2 May 2022 at 11:11, Lucas Rodrigues <inkeliz@inkeliz.com> wrote:
>
> "f32.Rectangle is unexported,". Does it also applies for f32.Point? Because, for custom paths, f32.Point is required, and I think f32.Rectangle can be also useful for custom paths, since it's just 2x f32.Point.
>

f32.Point is kept.

Elias
Reply to thread Export thread (mbox)