Closed Bug 1223310 Opened 5 years ago Closed 5 years ago

Convert nsIWidget bounds to LayoutDevicePixel


(Core :: Widget, defect)

Not set



Tracking Status
firefox45 --- fixed


(Reporter: njn, Assigned: njn)



(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+
Bug 1223310 (part 1) - Pass a LayoutDeviceIntPoint instead of an nsIntPoint to InitEvent(). r=kats.
Bug 1223310 (part 2) - Use LayoutDeviceIntRect for bounds-related functions in nsIWidget. r=kats.
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.