1224482 - Use LayoutDevicePixel type even more in widget/

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Nicholas Nethercote [inactive]]

More patches to add coordinate types to widget/.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Tweak typed/untyped versions of Get{,Client,Screen}Bounds()

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Replace GetNaturalBoundsUntyped() with GetNaturalBounds()

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Replace GetClientOffsetUntyped() with GetClientOffset()

The patch also changes nsMenuPopupFrame::mLastClientOffset to
LayoutDeviceIntPoint.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Make GetClientSize() return a LayoutDeviceIntSize

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Avoid excessive mozilla:: prefixes in nsIWidget and its subclasses

A couple of typedefs make things a lot nicer.

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Change nsScreenGonk::m{Virtual,Natural}Bounds to LayoutDevcieIntRect

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: (part 7) - Make GetScaledScreenBounds() return a CSSIntRect

Also changes mOriginalBounds to a CSSIntRect*.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687061 [details] [diff] [review]
(part 1) - Tweak typed/untyped versions of Get{,Client,Screen}Bounds()

Review of attachment 8687061 [details] [diff] [review]:
-----------------------------------------------------------------

s/untyped versions/typed versions/ in the commit message. Also there's a lot of misplaced &s but maybe we can just wait until the Big Tree Style Reformatting happens.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687063 [details] [diff] [review]
(part 3) - Replace GetClientOffsetUntyped() with GetClientOffset()

Review of attachment 8687063 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsIWidget.h
@@ +891,4 @@
>       * Get the client offset from the window origin.
>       *
>       * The untyped version exists temporarily to ease conversion to typed
>       * coordinates.

Fix comment

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687064 [details] [diff] [review]
(part 4) - Make GetClientSize() return a LayoutDeviceIntSize

Review of attachment 8687064 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsIWidget.h
@@ +907,3 @@
>        mozilla::LayoutDeviceIntRect rect;
>        GetClientBounds(rect);
> +      return mozilla::LayoutDeviceIntSize(rect.width, rect.height);

You should be able to do |return rect.Size();| here

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687065 [details] [diff] [review]
(part 5) - Avoid excessive mozilla:: prefixes in nsIWidget and its subclasses

Review of attachment 8687065 [details] [diff] [review]:
-----------------------------------------------------------------

\o/ thanks!

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8687074 [details] [diff] [review]
(part 7) - Make GetScaledScreenBounds() return a CSSIntRect

Review of attachment 8687074 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsBaseWidget.cpp
@@ +1734,5 @@
>    CSSToLayoutDeviceScale scale = GetDefaultScale();
> +  return CSSIntRect(NSToIntRound(bounds.x / scale.scale),
> +                    NSToIntRound(bounds.y / scale.scale),
> +                    NSToIntRound(bounds.width / scale.scale),
> +                    NSToIntRound(bounds.height / scale.scale));

We would usually write this as:

return RoundedToInt(bounds / scale);

which uses [1] and [2]. I think that should be ok to do here, although the specific rounding behaviour might be slightly different for negative numbers. I'm not sure if that would cause problems or not.

[1] https://dxr.mozilla.org/mozilla-central/rev/0c648a1efbe06b5ec866ba058d18256b80808b46/layout/base/Units.h#449
[2] https://dxr.mozilla.org/mozilla-central/rev/0c648a1efbe06b5ec866ba058d18256b80808b46/gfx/2d/Rect.h#200

Comment 13

[Kartikaya Gupta (email:[e-mail])]

And thanks again for all this work! It's nice to see the strongly typed units propagating further into the codebase :)

Comment 14

[Nicholas Nethercote [inactive]]

> Also there's a lot of misplaced &s but maybe we can just wait until the
> Big Tree Style Reformatting happens.

I'm skeptical that'll happen in my lifetime so I'm happy to do it now :)

Comment 15

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c289aa0d6c842f66a6bc716c6f4322df0dcad085
Bug 1224482 (part 1) - Tweak typed/untyped versions of Get{,Client,Screen}Bounds(). r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d77e6ba0c79530b5ffb2211140fbf7cbb3d0614a
Bug 1224482 (part 2) - Replace GetNaturalBoundsUntyped() with GetNaturalBounds(). r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e49cd993ae2a456bdb2ad126dfccb7aa09d3d9ff
Bug 1224482 (part 3) - Replace GetClientOffsetUntyped() with GetClientOffset(). r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/af31b9ebda9bedbbef816053dc400bcbcd9b4f9f
Bug 1224482 (part 4) - Make GetClientSize() return a LayoutDeviceIntSize. r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ab1bb90e75cac6c9c62fc0ffbb3a3a267f91671
Bug 1224482 (part 5) - Avoid excessive mozilla:: prefixes in nsIWidget and its subclasses. r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/40219e92a71d7f69e7fcd7207e0a389d279e50e8
Bug 1224482 (part 6) - Change nsScreenGonk::m{Virtual,Natural}Bounds to LayoutDevcieIntRect. r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/25836f531c30e3119d41d69b71e8c4daccb85f85
Bug 1224482 (part 7) - Make GetScaledScreenBounds() return a CSSIntRect. r=kats.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c289aa0d6c84
https://hg.mozilla.org/mozilla-central/rev/d77e6ba0c795
https://hg.mozilla.org/mozilla-central/rev/e49cd993ae2a
https://hg.mozilla.org/mozilla-central/rev/af31b9ebda9b
https://hg.mozilla.org/mozilla-central/rev/5ab1bb90e75c
https://hg.mozilla.org/mozilla-central/rev/40219e92a71d
https://hg.mozilla.org/mozilla-central/rev/25836f531c30