Closed Bug 1423570 Opened 2 years ago Closed 2 years ago

Use BaseRect methods instead of directly member variables in gfx/

Categories

(Core :: Graphics, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: milan, Assigned: milan)

References

(Blocks 3 open bugs)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

No description provided.
Assignee: nobody → milan
Blocks: 1386277
Priority: -- → P3
Whiteboard: [gfx-noted]
Comment on attachment 8934979 [details]
Bug 1423570: Use BaseRect access methods instead of member variables in gfx/ .schouten

https://reviewboard.mozilla.org/r/205920/#review211478


C/C++ static analysis found 8 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 1)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 1)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil

::: gfx/thebes/gfxBlur.cpp:780
(Diff revision 1)
>  static void
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:781
(Diff revision 1)
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> -               ceil(aDestRect.y + aDestRect.height / 2));
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
> +               ceil(aDestRect.Y() + aDestRect.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDestRect.Y() + aDestRect.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:841
(Diff revision 1)
>                           const Rect& aSkipRect, bool aMiddle = false)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:842
(Diff revision 1)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> -               ceil(aDstOuter.y + aDstOuter.height / 2));
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
> +               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 1)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                    ^
                    std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 1)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil
Comment on attachment 8934979 [details]
Bug 1423570: Use BaseRect access methods instead of member variables in gfx/ .schouten

https://reviewboard.mozilla.org/r/205920/#review211486


C/C++ static analysis found 8 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 2)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 2)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil

::: gfx/thebes/gfxBlur.cpp:780
(Diff revision 2)
>  static void
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:781
(Diff revision 2)
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> -               ceil(aDestRect.y + aDestRect.height / 2));
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
> +               ceil(aDestRect.Y() + aDestRect.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDestRect.Y() + aDestRect.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:841
(Diff revision 2)
>                           const Rect& aSkipRect, bool aMiddle = false)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:842
(Diff revision 2)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> -               ceil(aDstOuter.y + aDstOuter.height / 2));
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
> +               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 2)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                    ^
                    std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 2)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil
Comment on attachment 8934979 [details]
Bug 1423570: Use BaseRect access methods instead of member variables in gfx/ .schouten

https://reviewboard.mozilla.org/r/205920/#review211542


C/C++ static analysis found 8 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 3)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 3)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil

::: gfx/thebes/gfxBlur.cpp:780
(Diff revision 3)
>  static void
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:781
(Diff revision 3)
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> -               ceil(aDestRect.y + aDestRect.height / 2));
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
> +               ceil(aDestRect.Y() + aDestRect.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDestRect.Y() + aDestRect.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:841
(Diff revision 3)
>                           const Rect& aSkipRect, bool aMiddle = false)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:842
(Diff revision 3)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> -               ceil(aDstOuter.y + aDstOuter.height / 2));
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
> +               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 3)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                    ^
                    std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 3)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil
Blocks: 1423919
No longer blocks: 1386277
Comment on attachment 8934979 [details]
Bug 1423570: Use BaseRect access methods instead of member variables in gfx/ .schouten

https://reviewboard.mozilla.org/r/205920/#review211896

::: gfx/2d/BaseRect.h:63
(Diff revision 3)
>    }
>  
>    // Emptiness. An empty rect is one that has no area, i.e. its height or width
> -  // is <= 0
> +  // is <= 0.  Zero rect is the one with height and width set to zero.  Note
> +  // that SetEmpty() may change a rectangle that identified as IsEmpty().
> +  bool IsZero() const { return height == 0 || width == 0; }

If we're adding MOZ_ALWAYS_INLINE should these be marked as such as well?

::: gfx/2d/BaseRect.h:299
(Diff revision 3)
>    bool IsEqualEdges(const Sub& aRect) const
>    {
>      return x == aRect.x && y == aRect.y &&
>             width == aRect.width && height == aRect.height;
>    }
> +  bool IsEqualEdges(T aX, T aY, T aW, T aH)

I don't really like the name for this, the name sort of suggests the 3rd and 4th argument represent XMost() and YMost().

::: gfx/2d/Blur.cpp:235
(Diff revision 3)
>          int32_t aStride,
>          IntRect aSkipRect)
>  {
>    if (aTranspose) {
>      swap(aWidth, aRows);
> -    swap(aSkipRect.x, aSkipRect.y);
> +    aSkipRect.Swap();

Eww.. we really call this 'Swap'? I guess you didn't add that but I wish we'd called it Transpose.

::: gfx/layers/DirectionUtils.h:20
(Diff revision 3)
>  
> -template <typename PointOrRect>
> -CoordOf<PointOrRect> GetAxisStart(ScrollDirection aDir, const PointOrRect& aValue) {
> +template <typename Point>
> +CoordOf<Point> GetAxisStartPoint(ScrollDirection aDir, const Point& aValue) {
> +
> -  if (aDir == ScrollDirection::eHorizontal) {
> + if (aDir == ScrollDirection::eHorizontal) {
>      return aValue.x;

nit: Would it be easier to just add X() and Y() to our BasePoint class? That seems like a minimal change, we don't need to convert the tree yet :).

::: gfx/src/nsRect.h:76
(Diff revision 3)
>    MOZ_MUST_USE nsRect SaturatingUnionEdges(const nsRect& aRect) const
>    {
>  #ifdef NS_COORD_IS_FLOAT
>      return UnionEdges(aRect);
>  #else
> -    nsRect result;
> +    auto resultX = std::min(aRect.X(), x);

nit: I'm not a huge fan of using auto when the type is known and not very hard to write. So I'd prefer having nscoord here. Especially since if someone changed something weird and this ends up doing float/int int/float conversion by accident that's expensive.

::: gfx/src/nsRect.h:258
(Diff revision 3)
>                              nscoord aAppUnitsPerPixel) const
>  {
> -  mozilla::gfx::IntRect rect;
> -  rect.x = NSToIntCeil(NSAppUnitsToFloatPixels(x, float(aAppUnitsPerPixel)) * aXScale);
> -  rect.y = NSToIntCeil(NSAppUnitsToFloatPixels(y, float(aAppUnitsPerPixel)) * aYScale);
>    // Avoid negative widths and heights due to overflow

nit: I don't think this comment still applies! This should be done inside SetBox now.
Attachment #8934979 - Flags: review?(bas) → review+
I'll make the BaseRect changes and extract just that (and the additional gtests) into bug 1423919 and carry r+ from here.
I'll make the other suggested changes and leave it in this bug.
(In reply to Bas Schouten (:bas.schouten) from comment #7)
> 
> > +  bool IsEqualEdges(T aX, T aY, T aW, T aH)
> 
> I don't really like the name for this, the name sort of suggests the 3rd and
> 4th argument represent XMost() and YMost().

Good point.  I'll go with "IsEqualRect" instead.
...
> 
> nit: Would it be easier to just add X() and Y() to our BasePoint class? That
> seems like a minimal change, we don't need to convert the tree yet :).

Will do that and deal with the nits as well.
Comment on attachment 8934979 [details]
Bug 1423570: Use BaseRect access methods instead of member variables in gfx/ .schouten

https://reviewboard.mozilla.org/r/205920/#review212562


C/C++ static analysis found 8 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 4)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

::: gfx/thebes/gfxBlur.cpp:530
(Diff revision 4)
>    Rect blurRect(minRect);
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil

::: gfx/thebes/gfxBlur.cpp:780
(Diff revision 4)
>  static void
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:781
(Diff revision 4)
>  DrawMirroredBoxShadow(DrawTarget* aDT,
>                        SourceSurface* aSurface,
>                        const Rect& aDestRect)
>  {
> -  Point center(ceil(aDestRect.x + aDestRect.width / 2),
> -               ceil(aDestRect.y + aDestRect.height / 2));
> +  Point center(ceil(aDestRect.X() + aDestRect.Width() / 2),
> +               ceil(aDestRect.Y() + aDestRect.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDestRect.Y() + aDestRect.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:841
(Diff revision 4)
>                           const Rect& aSkipRect, bool aMiddle = false)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:842
(Diff revision 4)
>  {
>    // Corners: top left, top right, bottom left, bottom right
>    // Compute quadrant bounds and then clip them to corners along
>    // dimensions where we need to stretch from min size.
> -  Point center(ceil(aDstOuter.x + aDstOuter.width / 2),
> -               ceil(aDstOuter.y + aDstOuter.height / 2));
> +  Point center(ceil(aDstOuter.X() + aDstOuter.Width() / 2),
> +               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

               ceil(aDstOuter.Y() + aDstOuter.Height() / 2));
               ^
               std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 4)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                    ^
                    std::ceil

::: gfx/thebes/gfxBlur.cpp:1130
(Diff revision 4)
>    Rect blurRect = aIsDestRect ? aOuterRect : aWhitespaceRect;
>    // If mirroring corners, we only need to draw the top-left quadrant.
>    // Use ceil to preserve the remaining 1x1 middle area for minimized box
>    // shadows.
>    if (aMirrorCorners) {
> -    blurRect.SizeTo(ceil(blurRect.width * 0.5f), ceil(blurRect.height * 0.5f));
> +    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));

Warning: Call to 'ceil' promotes float to double [clang-tidy: performance-type-promotion-in-math-fn]

    blurRect.SizeTo(ceil(blurRect.Width() * 0.5f), ceil(blurRect.Height() * 0.5f));
                                                   ^
                                                   std::ceil
Pushed by msreckovic@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0649658ddeb2
Use BaseRect access methods instead of member variables in gfx/ r=bas.schouten
https://hg.mozilla.org/mozilla-central/rev/0649658ddeb2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.