Closed Bug 1223310 Opened 5 years ago Closed 5 years ago

Convert nsIWidget bounds to LayoutDevicePixel

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(2 files)

nsIWidget has various bounds functions (GetBounds() and friends) that are currently nsIntRect but which can be typed as LayoutDeviceIntRect.
Also use direct assignment for some LayoutDeviceIntPoint assignments, rather
than doing it field-by-field.
Attachment #8685339 - Flags: review?(bugmail.mozilla)
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+
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.
https://hg.mozilla.org/mozilla-central/rev/31531f850dec
https://hg.mozilla.org/mozilla-central/rev/49a5f1fe2128
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.