Closed Bug 1224482 Opened 4 years ago Closed 4 years ago

Use LayoutDevicePixel type even more in widget/

Categories

(Core :: Widget, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(7 files)

More patches to add coordinate types to widget/.
The patch also changes nsMenuPopupFrame::mLastClientOffset to
LayoutDeviceIntPoint.
Attachment #8687063 - Flags: review?(bugmail.mozilla)
A couple of typedefs make things a lot nicer.
Attachment #8687065 - Flags: review?(bugmail.mozilla)
Also changes mOriginalBounds to a CSSIntRect*.
Attachment #8687074 - Flags: review?(bugmail.mozilla)
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.
Attachment #8687061 - Flags: review?(bugmail.mozilla) → review+
Attachment #8687062 - Flags: review?(bugmail.mozilla) → review+
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
Attachment #8687063 - Flags: review?(bugmail.mozilla) → review+
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
Attachment #8687064 - Flags: review?(bugmail.mozilla) → review+
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!
Attachment #8687065 - Flags: review?(bugmail.mozilla) → review+
Attachment #8687066 - Flags: review?(bugmail.mozilla) → review+
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
Attachment #8687074 - Flags: review?(bugmail.mozilla) → review+
And thanks again for all this work! It's nice to see the strongly typed units propagating further into the codebase :)
> 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 :)
Blocks: 1225007
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.
Depends on: 1229237
No longer depends on: 1229237
You need to log in before you can comment on or make changes to this bug.