I have yet more patches to increase the usage of LayoutDevicePixel coordinates in widget/.

(Fixes some OS X compile errors.)

(Fixes some OS X compile errors.)

Comment on attachment 8693961 [details] [diff] [review] (part 1) - Make nsIWidget::{Create,CreateChildren}() take a LayoutDeviceIntRect Review of attachment 8693961 [details] [diff] [review]: ----------------------------------------------------------------- General note: |LayoutDeviceIntRect()| does the same thing as |LayoutDeviceIntRect(0, 0, 0, 0)|. I'll leave it up to you which you prefer as a stylistic choice. ::: widget/PuppetWidget.cpp @@ -146,5 @@ > > already_AddRefed<nsIWidget> > -PuppetWidget::CreateChild(const nsIntRect &aRect, > - nsWidgetInitData *aInitData, > - bool aForceUseIWidgetParent) I liked the alignment, but I guess strictly speaking it's not provided for in the style guide. ::: widget/nsBaseWidget.cpp @@ +308,5 @@ > // Basic create. > // > //------------------------------------------------------------------------- > void nsBaseWidget::BaseCreate(nsIWidget *aParent, > + const LayoutDeviceIntRect &aRect, Might as well move the & next to the type here too.

Comment on attachment 8693963 [details] [diff] [review] (part 2) - Make nsIWidget::DrawWindowUnderlay() take a LayoutDeviceIntRect Review of attachment 8693963 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/composite/LayerManagerComposite.cpp @@ +809,5 @@ > > // Allow widget to render a custom background. > + mCompositor->GetWidget()->DrawWindowUnderlay( > + this, LayoutDeviceIntRect(actualBounds.x, actualBounds.y, > + actualBounds.width, actualBounds.height)); Consider |LayoutDeviceIntRect::FromUnknownRect(TruncatedToInt(actualBounds))|. (Truncating from floats to ints is what we're doing here. In addition to being less verbose, this form makes that explicit.)

(In reply to Botond Ballo [:botond] from comment #9) > Comment on attachment 8693963 [details] [diff] [review] > (part 2) - Make nsIWidget::DrawWindowUnderlay() take a LayoutDeviceIntRect > > Review of attachment 8693963 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/layers/composite/LayerManagerComposite.cpp > @@ +809,5 @@ > > > > // Allow widget to render a custom background. > > + mCompositor->GetWidget()->DrawWindowUnderlay( > > + this, LayoutDeviceIntRect(actualBounds.x, actualBounds.y, > > + actualBounds.width, actualBounds.height)); > > Consider > |LayoutDeviceIntRect::FromUnknownRect(TruncatedToInt(actualBounds))|. > > (Truncating from floats to ints is what we're doing here. In addition to > being less verbose, this form makes that explicit.) Oh, I guess we don't actually have a TruncatedToInt() for rects, only for points [1]. We do have RoundedToInt() for both points [2] and rects [3], so this is probably a good opportunity to add a TruncatedToInt() for rects. [1] https://dxr.mozilla.org/mozilla-central/rev/470f4f8c2b2d6f82e56e161a4b05262c85f55b59/gfx/2d/Point.h#107 [2] https://dxr.mozilla.org/mozilla-central/rev/470f4f8c2b2d6f82e56e161a4b05262c85f55b59/gfx/2d/Point.h#101 [3] https://dxr.mozilla.org/mozilla-central/rev/470f4f8c2b2d6f82e56e161a4b05262c85f55b59/gfx/2d/Rect.h#213

Comment on attachment 8694483 [details] [diff] [review] (part 3) - Make nsIWidget::Invalidate() take a LayoutDeviceIntRect Review of attachment 8694483 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gtk/nsWindow.cpp @@ +2451,5 @@ > // of toplevels.) > if (mBounds.width < size.width) { > GdkRectangle rect = DevicePixelsToGdkRectRoundOut( > + LayoutDeviceIntRect(mBounds.width, 0, > + size.width - mBounds.width, size.height)); (You should still be able to use the { ... } syntax. If you're choosing not to for stylistic reasons, that's fine.)

https://hg.mozilla.org/integration/mozilla-inbound/rev/dccb1af2b609e003e6dcc9726ea73070eadb30a3 Bug 1229237 (part 1) - Make nsIWidget::{Create,CreateChildren}() take a LayoutDeviceIntRect. r=botond. https://hg.mozilla.org/integration/mozilla-inbound/rev/81cb37b5d061b7e51ab465482a219315867b2535 Bug 1229237 (part 2) - Make nsIWidget::DrawWindowUnderlay() take a LayoutDeviceIntRect. r=botond. https://hg.mozilla.org/integration/mozilla-inbound/rev/e091d14c936c948a490457a6420261abe8c16cc6 Bug 1229237 (part 3) - Make nsIWidget::Invalidate() take a LayoutDeviceIntRect. r=botond. https://hg.mozilla.org/integration/mozilla-inbound/rev/de467557ee9ef4d31e1674bc5595dbe9c9cc8834 Bug 1229237 (part 4) - Make ThemeGeometry::mRect a LayoutDeviceIntRect. r=botond. https://hg.mozilla.org/integration/mozilla-inbound/rev/d1167d19ab51196d6e075017c949dea459e21abf Bug 1229237 (part 5) - Make Update{Opaque,WindowDragging}Region() take a LayoutDeviceIntRegion. r=botond.

https://hg.mozilla.org/mozilla-central/rev/dccb1af2b609 https://hg.mozilla.org/mozilla-central/rev/81cb37b5d061 https://hg.mozilla.org/mozilla-central/rev/e091d14c936c https://hg.mozilla.org/mozilla-central/rev/de467557ee9e https://hg.mozilla.org/mozilla-central/rev/d1167d19ab51

Comment on attachment 8693961 [details] [diff] [review] (part 1) - Make nsIWidget::{Create,CreateChildren}() take a LayoutDeviceIntRect Review of attachment 8693961 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/cocoa/nsCocoaWindow.mm @@ +254,2 @@ > nsNativeWidget aNativeParent, > + const LayoutDeviceIntRect& aRect, FWIW, this isn't correct; as the comment above says, the rect here is in "global display pixels", which are not the same as "layout device pixels". This applies to several of the changes here (e.g. in the static functions above)... not every use of nsIntRect was in fact for "device pixels". And that means the caller of this function must be wrong, too. (I happened to notice this because it conflicts with patches I have in progress for bug 890156, where I introduce a separate DesktopPixel type to address this. I'll try to disentangle things there...)

> (I happened to notice this because it conflicts with patches I have in > progress for bug 890156, where I introduce a separate DesktopPixel type to > address this. I'll try to disentangle things there...) Sorry for the errors, and thank you for the future distentangling.