445269 - Change nsRect to nsIntRect in nsSVGFilters.cpp

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This is just cleanup to change nsRect to nsIntRect in nsSVGFilters.cpp to make it clear these are pixel rects, not appunit rects. Since nsIntRect is currently typedefed to nsRect this won't change any behaviour.

Comment 1

[Robert Longson [:longsonr]]

Comment on attachment 329607 [details] [diff] [review]
fix

> static void
> ClipComputationRectToSurface(nsSVGFilterInstance* aInstance,
>                              nsIntRect* aDataRect)
> {
>   aDataRect->IntersectRect(*aDataRect,
>-          nsRect(nsPoint(0, 0), aInstance->GetSurfaceRect().Size()));
>+          nsIntRect(nsPoint(0, 0), aInstance->GetSurfaceRect().Size()));

nsIntRect only has this constructor by dint of it being a typedef of nsRect. If NS_COORD_IS_FLOAT is ever defined then this won't compile. Seems easy to use GetSufraceWidth and GetSurfaceHeight to avoid.

>   NS_IMETHOD FrameChanged(imgIContainer *aContainer, gfxIImageFrame *newframe,
>-                          nsRect * dirtyRect);
>+                          nsIntRect * dirtyRect);

Nit: Even though we're all over the place regarding whether its type* x or type *x or type * x which is not your fault. We might at least try to be consistent within a single function.

> NS_IMETHODIMP
> nsSVGFEImageElement::FrameChanged(imgIContainer *aContainer,
>                                   gfxIImageFrame *newframe,
>-                                  nsRect * dirtyRect)
>+                                  nsIntRect * dirtyRect)

ditto.

...

One other nit which I didn't notice before, you could use some spaces after the commas in this assertion.
 NS_ASSERTION(nsIntRect(0,0,aDest->mImage->Width(),aDest->mImage->Height()).Contains(aDataRect),
               "aDataRect out of bounds");

Note there are two occurrances of this line in nsSVGFilters.cpp

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 329607 [details] [diff] [review]
fix

sr=mats

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #1)
> (From update of attachment 329607 [details] [diff] [review])
> > static void
> > ClipComputationRectToSurface(nsSVGFilterInstance* aInstance,
> >                              nsIntRect* aDataRect)
> > {
> >   aDataRect->IntersectRect(*aDataRect,
> >-          nsRect(nsPoint(0, 0), aInstance->GetSurfaceRect().Size()));
> >+          nsIntRect(nsPoint(0, 0), aInstance->GetSurfaceRect().Size()));
> 
> nsIntRect only has this constructor by dint of it being a typedef of nsRect. If
> NS_COORD_IS_FLOAT is ever defined then this won't compile. Seems easy to use
> GetSufraceWidth and GetSurfaceHeight to avoid.

Actually all I need there is to use nsIntPoint instead of nsPoint...

> >   NS_IMETHOD FrameChanged(imgIContainer *aContainer, gfxIImageFrame *newframe,
> >-                          nsRect * dirtyRect);
> >+                          nsIntRect * dirtyRect);
> 
> Nit: Even though we're all over the place regarding whether its type* x or type
> *x or type * x which is not your fault. We might at least try to be consistent
> within a single function.

:-)

> One other nit which I didn't notice before, you could use some spaces after the
> commas in this assertion.
> 
> NS_ASSERTION(nsIntRect(0,0,aDest->mImage->Width(),aDest->mImage->Height()).Contains(aDataRect),
>                "aDataRect out of bounds");
> 
> Note there are two occurrances of this line in nsSVGFilters.cpp

Fixed.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed 23f7948191ec.