gfx: Add an implicit Insets to InsetsF conversion operator.

gfx: Add an implicit Insets to InsetsF conversion operator.

BUG=none
R=danakj@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243131

[sadrul, 2014-01-06 17:30:22]

There are similar operators for Point, Rect etc. and it makes sense to have this
for Insets too. WDYT?

[danakj, 2014-01-06 17:43:38]

On 2014/01/06 17:30:22, sadrul wrote:
> There are similar operators for Point, Rect etc. and it makes sense to have
this
> for Insets too. WDYT?

I've been starting to regret the implicit rect->rectf conversions that we have.
Going int->float->int all over the place has actually pretty severe performance
costs in the compositor.  I'd like to find more ways to keep ints as ints and
floats as floats.

[sadrul, 2014-01-06 17:47:17]

On 2014/01/06 17:43:38, danakj wrote:
> On 2014/01/06 17:30:22, sadrul wrote:
> > There are similar operators for Point, Rect etc. and it makes sense to have
> this
> > for Insets too. WDYT?
> 
> I've been starting to regret the implicit rect->rectf conversions that we
have.
> Going int->float->int all over the place has actually pretty severe
performance
> costs in the compositor.  I'd like to find more ways to keep ints as ints and
> floats as floats.

There is no implicit float -> int conversion. So I expect at least we do the
explicit conversion somewhere, and I would expect such explicit conversion to be
there because it is so desired.

Explicitly creating an InsetsF out of an Insets isn't a lot of work, and I am
happy to use the InsetsF() constructor instead, but having implicit conversions
for Rect->RectF etc. without having a corresponding Insets->InsetsF is ... a bit
jarring.

[danakj, 2014-01-06 17:51:39]

On Mon, Jan 6, 2014 at 12:47 PM, <sadrul@chromium.org> wrote:

> On 2014/01/06 17:43:38, danakj wrote:
>
>> On 2014/01/06 17:30:22, sadrul wrote:
>> > There are similar operators for Point, Rect etc. and it makes sense to
>> have
>> this
>> > for Insets too. WDYT?
>>
>
>  I've been starting to regret the implicit rect->rectf conversions that we
>>
> have.
>
>> Going int->float->int all over the place has actually pretty severe
>>
> performance
>
>> costs in the compositor.  I'd like to find more ways to keep ints as ints
>> and
>> floats as floats.
>>
>
> There is no implicit float -> int conversion. So I expect at least we do
> the
> explicit conversion somewhere, and I would expect such explicit conversion
> to be
> there because it is so desired.
>
> Explicitly creating an InsetsF out of an Insets isn't a lot of work, and I
> am
> happy to use the InsetsF() constructor instead, but having implicit
> conversions
> for Rect->RectF etc. without having a corresponding Insets->InsetsF is ...
> a bit
> jarring.
>

True, so I'm wondering what's your use case for this? Are you scaling the
insets or something?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2014-01-06 17:53:32]

On 2014/01/06 17:51:39, danakj wrote:
> On Mon, Jan 6, 2014 at 12:47 PM, <mailto:sadrul@chromium.org> wrote:
> 
> > On 2014/01/06 17:43:38, danakj wrote:
> >
> >> On 2014/01/06 17:30:22, sadrul wrote:
> >> > There are similar operators for Point, Rect etc. and it makes sense to
> >> have
> >> this
> >> > for Insets too. WDYT?
> >>
> >
> >  I've been starting to regret the implicit rect->rectf conversions that we
> >>
> > have.
> >
> >> Going int->float->int all over the place has actually pretty severe
> >>
> > performance
> >
> >> costs in the compositor.  I'd like to find more ways to keep ints as ints
> >> and
> >> floats as floats.
> >>
> >
> > There is no implicit float -> int conversion. So I expect at least we do
> > the
> > explicit conversion somewhere, and I would expect such explicit conversion
> > to be
> > there because it is so desired.
> >
> > Explicitly creating an InsetsF out of an Insets isn't a lot of work, and I
> > am
> > happy to use the InsetsF() constructor instead, but having implicit
> > conversions
> > for Rect->RectF etc. without having a corresponding Insets->InsetsF is ...
> > a bit
> > jarring.
> >
> 
> True, so I'm wondering what's your use case for this? Are you scaling the
> insets or something?

The use case is using RectF::Inset() with a gfx::Insets
(https://codereview.chromium.org/118553004/diff/320001/ui/wm/core/easy_resize_...)

[danakj, 2014-01-06 18:03:22]

On Mon, Jan 6, 2014 at 12:53 PM, <sadrul@chromium.org> wrote:

> On 2014/01/06 17:51:39, danakj wrote:
>
>  On Mon, Jan 6, 2014 at 12:47 PM, <mailto:sadrul@chromium.org> wrote:
>>
>
>  > On 2014/01/06 17:43:38, danakj wrote:
>> >
>> >> On 2014/01/06 17:30:22, sadrul wrote:
>> >> > There are similar operators for Point, Rect etc. and it makes sense
>> to
>> >> have
>> >> this
>> >> > for Insets too. WDYT?
>> >>
>> >
>> >  I've been starting to regret the implicit rect->rectf conversions that
>> we
>> >>
>> > have.
>> >
>> >> Going int->float->int all over the place has actually pretty severe
>> >>
>> > performance
>> >
>> >> costs in the compositor.  I'd like to find more ways to keep ints as
>> ints
>> >> and
>> >> floats as floats.
>> >>
>> >
>> > There is no implicit float -> int conversion. So I expect at least we do
>> > the
>> > explicit conversion somewhere, and I would expect such explicit
>> conversion
>> > to be
>> > there because it is so desired.
>> >
>> > Explicitly creating an InsetsF out of an Insets isn't a lot of work,
>> and I
>> > am
>> > happy to use the InsetsF() constructor instead, but having implicit
>> > conversions
>> > for Rect->RectF etc. without having a corresponding Insets->InsetsF is
>> ...
>> > a bit
>> > jarring.
>> >
>>
>
>  True, so I'm wondering what's your use case for this? Are you scaling the
>> insets or something?
>>
>
> The use case is using RectF::Inset() with a gfx::Insets
> (https://codereview.chromium.org/118553004/diff/320001/ui/
> wm/core/easy_resize_window_targeter.cc)
>

I see. You start with a Rect. Convert to a RectF to do transform, then also
have to convert the insets. I'd like us to never do that Rect-to-RectF
conversion in the first place if we could. But given the current state of
affairs, I guess this LGTM.


>
> https://codereview.chromium.org/102253009/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2014-01-06 18:58:21]

Committed patchset #1 manually as r243131 (presubmit successful).