Convert nsIWidget bounds to LayoutDevicePixel

RESOLVED FIXED in Firefox 45

Status

()

Core
Widget
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
nsIWidget has various bounds functions (GetBounds() and friends) that are currently nsIntRect but which can be typed as LayoutDeviceIntRect.
(Assignee)

Comment 1

2 years ago
Created attachment 8685339 [details] [diff] [review]
(part 1) - Pass a LayoutDeviceIntPoint instead of an nsIntPoint to InitEvent()

Also use direct assignment for some LayoutDeviceIntPoint assignments, rather
than doing it field-by-field.
Attachment #8685339 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 2

2 years ago
Created attachment 8685351 [details] [diff] [review]
(part 2) - Use LayoutDeviceIntRect for bounds-related functions in nsIWidget

The patch renames the existing functions (GetBounds(), GetClientBounds(), etc)
by adding an |Untyped| suffix. It then adds typed equivalents, and uses those
typed equivalents in all the call sites where it's easy to do so. The trickier
remaining call sites are converted to use the Untyped-suffix version.
Attachment #8685351 - Flags: review?(bugmail.mozilla)
Comment on attachment 8685339 [details] [diff] [review]
(part 1) - Pass a LayoutDeviceIntPoint instead of an nsIntPoint to InitEvent()

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

Nice, thanks!
Attachment #8685339 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8685351 [details] [diff] [review]
(part 2) - Use LayoutDeviceIntRect for bounds-related functions in nsIWidget

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

r+ with widget/gonk fixed up as well. Worth doing a build-only try push before landing to make sure nothing got missed. Thanks!

::: widget/PuppetWidget.cpp
@@ +240,5 @@
>      NS_ASSERTION(w->GetParent() == this,
>                   "Configured widget is not a child");
>      w->SetWindowClipRegion(configuration.mClipRegion, true);
>      nsIntRect bounds;
> +    w->GetBoundsUntyped(bounds);    // njn: make configuration typed?

Not sure if you meant to leave this comment in

::: widget/nsIWidget.h
@@ +1976,5 @@
>       */
> +    virtual mozilla::LayoutDeviceIntRect GetNaturalBounds() {
> +        mozilla::LayoutDeviceIntRect bounds;
> +        GetBounds(bounds);
> +        return bounds;

I'd prefer if this followed the same pattern - calling GetNaturalBoundsUntyped rather than the typed version of GetBounds. The reason is that if somebody wanted to override this function they would only need to override the GetNaturalBoundsUntyped rather than having to override both.

(And in fact it looks like widget/gonk/nsWindow does override it, and will need to be fixed up as well).
Attachment #8685351 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/31531f850dec8eb0131fd05c8b9a926cf9163b02
Bug 1223310 (part 1) - Pass a LayoutDeviceIntPoint instead of an nsIntPoint to InitEvent(). r=kats.

https://hg.mozilla.org/integration/mozilla-inbound/rev/49a5f1fe2128ff16b17eccdd77017f751c8efb11
Bug 1223310 (part 2) - Use LayoutDeviceIntRect for bounds-related functions in nsIWidget. r=kats.

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/31531f850dec
https://hg.mozilla.org/mozilla-central/rev/49a5f1fe2128
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.