Closed Bug 1158120 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics, defect, trivial)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: nical, Assigned: amanda.sambath, Mentored)

Details

Attachments

(11 files, 2 obsolete files)

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.
Oh, and same treatment for nsIntSize which is also a typedef of gfx::IntSize.
Summary: Remove all occurrences of the gfxIntSize typedef in gfx code, replace it by gfx::IntSize → Remove all occurrences of the gfxIntSize and nsIntSize typedefs in gfx code, replace them by gfx::IntSize
I would like to fix this bug.
Assignee: nobody → amanda.sambath
Attached patch thebesIntSize.patch (obsolete) — Splinter Review
Build succeeded.
Attachment #8612178 - Flags: review?(nical.bugzilla)
Attached patch glIntSize.patchSplinter Review
Build successful.
Attachment #8612193 - Flags: review?(nical.bugzilla)
Attached patch layersIntSize.patch (obsolete) — Splinter Review
Build successful.
Attachment #8612212 - Flags: review?(nical.bugzilla)
Build successful.
Attachment #8612228 - Flags: review?(nical.bugzilla)
Attached patch ipcIntSize.patchSplinter Review
Build successful.
Attachment #8612244 - Flags: review?(nical.bugzilla)
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
Attachment #8612178 - Flags: review?(nical.bugzilla) → feedback+
Attachment #8612193 - Flags: review?(nical.bugzilla) → review+
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
Attachment #8612212 - Flags: review?(nical.bugzilla) → feedback+
Attachment #8612228 - Flags: review?(nical.bugzilla) → review+
Attachment #8612244 - Flags: review?(nical.bugzilla) → review+
Build successful.
Attachment #8612744 - Flags: review?(nical.bugzilla)
Attachment #8612744 - Flags: review?(nical.bugzilla) → review+
Build successful.
Attachment #8612812 - Flags: review?(nical.bugzilla)
Attachment #8612812 - Flags: review?(nical.bugzilla) → review+
Build successful.
Attachment #8612844 - Flags: review?(nical.bugzilla)
Build successful.
Attachment #8612845 - Flags: review?(nical.bugzilla)
Attachment #8612844 - Flags: review?(nical.bugzilla) → review+
Attachment #8612845 - Flags: review?(nical.bugzilla) → review+
Build successful.
Attachment #8612896 - Flags: review?(nical.bugzilla)
Attachment #8612896 - Flags: review?(nical.bugzilla) → review+
Attached patch rev1.patchSplinter Review
Attachment #8612178 - Attachment is obsolete: true
Attachment #8613421 - Flags: review?(nical.bugzilla)
Attached patch rev3.patchSplinter Review
Attachment #8612212 - Attachment is obsolete: true
Attachment #8613422 - Flags: review?(nical.bugzilla)
Attachment #8613421 - Flags: review?(nical.bugzilla) → review+
Attachment #8613422 - Flags: review?(nical.bugzilla) → review+
Whiteboard: [leave-open]
Attachment #8614633 - Flags: review?(nical.bugzilla)
Attachment #8614633 - Flags: review?(nical.bugzilla) → review+
That does it for nsIntSize and gfxIntSize in the gfx module \o/
Whiteboard: [leave-open]
Keywords: checkin-needed
I already landed all of the patches in mozilla-inbound.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ef8381db1873
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.