1158120 - Remove all occurrences of the gfxIntSize and nsIntSize typedefs in gfx code, replace them by gfx::IntSize

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Nicolas Silva [:nical]]

Now that gfxIntSize is a typedef of gfx::IntSize and not a separate classe, we can remove all of its occurrences in graphics code (anything under the gfx/ directory).
This is purely a cosmetic change, perfect for someone who would like to understand the workflow of code submitting contributions before tackling a real bug or feature.

grep -r gfxIntSize gfx/

and manually replace gfxIntSize by gfx::IntSize in code that is under the mozilla namespace, IntSize for code under mozilla::gfx or .cpp files with "using namespace mozilla::gfx;" or mozilla::gfx::IntSize for the few headers with no namespaces.

Let's leave out the code outside of gfx/ for now to let each module owner decide if, how and/or when to deal with updating the size classes/typedefs.

Comment 1

[Nicolas Silva [:nical]]

Oh, and same treatment for nsIntSize which is also a typedef of gfx::IntSize.

Comment 2

[Amanda Sambath]

I would like to fix this bug.

Comment 3

[Amanda Sambath]

Attachment: thebesIntSize.patch

Build succeeded.

Comment 4

[Amanda Sambath]

Attachment: glIntSize.patch

Build successful.

Comment 5

[Amanda Sambath]

Attachment: layersIntSize.patch

Build successful.

Comment 6

[Amanda Sambath]

Attachment: testsIntSize.patch

Build successful.

Comment 7

[Amanda Sambath]

Attachment: ipcIntSize.patch

Build successful.

Comment 8

[Nicolas Silva [:nical]]

Comment on attachment 8612178 [details] [diff] [review]
thebesIntSize.patch

Review of attachment 8612178 [details] [diff] [review]:
-----------------------------------------------------------------

The patch is good, but I'd like that you address the comments below. They are not problems with your changes, but rather leftovers from before and this patch is a good occasion to fix them.

::: gfx/thebes/gfxQtPlatform.cpp
@@ +94,5 @@
>  {
>      gfxImageFormat imageFormat = OptimalFormatForContent(contentType);
>  
>      nsRefPtr<gfxASurface> newSurface =
> +        new gfxImageSurface(IntSize(size.width, size.height), imageFormat);

Please just pass size directly (you have an IntSize in parameter and the constructor takes an IntSize). The unneeded conversion here is a leftover from the past but lets use patch as an occasion to fix it.

::: gfx/thebes/gfxQuartzImageSurface.cpp
@@ +38,1 @@
>                          cairo_image_surface_get_height(isurf));

Please align the parameters on the extra lines.

::: gfx/thebes/gfxQuartzImageSurface.h
@@ +19,5 @@
>      virtual ~gfxQuartzImageSurface();
>  
>      already_AddRefed<gfxImageSurface> GetAsImageSurface();
>      virtual int32_t KnownMemoryUsed();
> +    virtual const mozilla::gfx::IntSize GetSize() const { return mozilla::gfx::IntSize(mSize.width, mSize.height); }

Just "return mSize;" here.

::: gfx/thebes/gfxQuartzSurface.cpp
@@ +11,5 @@
>  
>  void
>  gfxQuartzSurface::MakeInvalid()
>  {
> +    mSize = mozilla::gfx::IntSize(-1, -1);    

please remove the trailing spaces (they were already there, but let's use this occasion to remove them)

::: gfx/thebes/gfxSharedImageSurface.h
@@ +13,5 @@
>  {
>    typedef gfxBaseSharedMemorySurface<gfxImageSurface, gfxSharedImageSurface> Super;
>    friend class gfxBaseSharedMemorySurface<gfxImageSurface, gfxSharedImageSurface>;
>  private:
> +    gfxSharedImageSurface(const mozilla::gfx::IntSize& aSize, long aStride, 

Please remove the trailing space on this line and the line below.

::: gfx/thebes/gfxSharedQuartzSurface.h
@@ +14,5 @@
>  {
>    typedef gfxBaseSharedMemorySurface<gfxQuartzSurface, gfxSharedQuartzSurface> Super;
>    friend class gfxBaseSharedMemorySurface<gfxQuartzSurface, gfxSharedQuartzSurface>;
>  private:
> +    gfxSharedQuartzSurface(const mozilla::gfx::IntSize& aSize, long aStride, 

please remove the trailing space.

::: gfx/thebes/gfxWindowsSurface.cpp
@@ +282,5 @@
>      return NS_ERROR_FAILURE;
>  #endif
>  }
>  
> +const mozilla::gfx::IntSize 

trailing space

Comment 9

[Nicolas Silva [:nical]]

Comment on attachment 8612212 [details] [diff] [review]
layersIntSize.patch

Review of attachment 8612212 [details] [diff] [review]:
-----------------------------------------------------------------

Two little comments to address like the first patch.

::: gfx/layers/basic/BasicLayerManager.cpp
@@ +777,1 @@
>                                                                         aDestRect.height),

Please align the paramaters

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +130,1 @@
>                                                              mSurfaceSize.height),

Please replace

> IntSize(mSurfaceSize.witdh, mSurfaceSize.height)

by

> mSurfaceSize

directly

Comment 10

[Amanda Sambath]

Attachment: 8_thebes_nsIntSize.patch

Build successful.

Comment 11

[Amanda Sambath]

Attachment: 9_gl_nsIntSize.patch

Build successful.

Comment 12

[Amanda Sambath]

Attachment: 10_ipc_nsIntSize.patch

Build successful.

Comment 13

[Amanda Sambath]

Attachment: 11_srcrc_nsIntSize.patch

Build successful.

Comment 14

[Amanda Sambath]

Attachment: 12_include_comment.patch

Build successful.

Comment 15

[Amanda Sambath]

Attachment: rev1.patch

Comment 16

[Amanda Sambath]

Attachment: rev3.patch

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3eae6ca6d822

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7654212d5a47
https://hg.mozilla.org/integration/mozilla-inbound/rev/4863d5946d1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/77bd155ffdc6
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e9a8b700143
https://hg.mozilla.org/integration/mozilla-inbound/rev/49cceccb1a45
https://hg.mozilla.org/integration/mozilla-inbound/rev/a75fce794713
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f7379cd9ddd
https://hg.mozilla.org/integration/mozilla-inbound/rev/b66e5680a70d
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff36a419222a

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3eae6ca6d822
https://hg.mozilla.org/mozilla-central/rev/7654212d5a47
https://hg.mozilla.org/mozilla-central/rev/4863d5946d1b
https://hg.mozilla.org/mozilla-central/rev/77bd155ffdc6
https://hg.mozilla.org/mozilla-central/rev/0e9a8b700143
https://hg.mozilla.org/mozilla-central/rev/49cceccb1a45
https://hg.mozilla.org/mozilla-central/rev/a75fce794713
https://hg.mozilla.org/mozilla-central/rev/4f7379cd9ddd
https://hg.mozilla.org/mozilla-central/rev/b66e5680a70d
https://hg.mozilla.org/mozilla-central/rev/ff36a419222a

Comment 20

[Amanda Sambath]

Attachment: layers_nsIntSize.patch

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef8381db1873

Comment 22

[Nicolas Silva [:nical]]

That does it for nsIntSize and gfxIntSize in the gfx module \o/

Comment 23

[Nicolas Silva [:nical]]

I already landed all of the patches in mozilla-inbound.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef8381db1873