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

RESOLVED FIXED in Firefox 41

Status

()

Core
Graphics
--
trivial
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: nical, Assigned: Amanda Sambath, Mentored)

Tracking

unspecified
mozilla41
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(11 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
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
(Assignee)

Comment 2

3 years ago
I would like to fix this bug.
(Reporter)

Updated

3 years ago
Assignee: nobody → amanda.sambath
(Assignee)

Comment 3

3 years ago
Created attachment 8612178 [details] [diff] [review]
thebesIntSize.patch

Build succeeded.
Attachment #8612178 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 4

3 years ago
Created attachment 8612193 [details] [diff] [review]
glIntSize.patch

Build successful.
Attachment #8612193 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 5

3 years ago
Created attachment 8612212 [details] [diff] [review]
layersIntSize.patch

Build successful.
Attachment #8612212 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 6

3 years ago
Created attachment 8612228 [details] [diff] [review]
testsIntSize.patch

Build successful.
Attachment #8612228 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 7

3 years ago
Created attachment 8612244 [details] [diff] [review]
ipcIntSize.patch

Build successful.
Attachment #8612244 - Flags: review?(nical.bugzilla)
(Reporter)

Comment 8

3 years ago
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+
(Reporter)

Updated

3 years ago
Attachment #8612193 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Comment 9

3 years ago
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+
(Reporter)

Updated

3 years ago
Attachment #8612228 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Updated

3 years ago
Attachment #8612244 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 10

3 years ago
Created attachment 8612744 [details] [diff] [review]
8_thebes_nsIntSize.patch

Build successful.
Attachment #8612744 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8612744 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 11

3 years ago
Created attachment 8612812 [details] [diff] [review]
9_gl_nsIntSize.patch

Build successful.
Attachment #8612812 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8612812 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 12

3 years ago
Created attachment 8612844 [details] [diff] [review]
10_ipc_nsIntSize.patch

Build successful.
Attachment #8612844 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 13

3 years ago
Created attachment 8612845 [details] [diff] [review]
11_srcrc_nsIntSize.patch

Build successful.
Attachment #8612845 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8612844 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Updated

3 years ago
Attachment #8612845 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 14

3 years ago
Created attachment 8612896 [details] [diff] [review]
12_include_comment.patch

Build successful.
(Assignee)

Updated

3 years ago
Attachment #8612896 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8612896 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 15

3 years ago
Created attachment 8613421 [details] [diff] [review]
rev1.patch
Attachment #8612178 - Attachment is obsolete: true
Attachment #8613421 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 16

3 years ago
Created attachment 8613422 [details] [diff] [review]
rev3.patch
Attachment #8612212 - Attachment is obsolete: true
Attachment #8613422 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8613421 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Updated

3 years ago
Attachment #8613422 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Updated

3 years ago
Whiteboard: [leave-open]
(Assignee)

Comment 20

3 years ago
Created attachment 8614633 [details] [diff] [review]
layers_nsIntSize.patch
Attachment #8614633 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

3 years ago
Attachment #8614633 - Flags: review?(nical.bugzilla) → review+
(Reporter)

Comment 22

3 years ago
That does it for nsIntSize and gfxIntSize in the gfx module \o/
Whiteboard: [leave-open]
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Reporter)

Comment 23

3 years ago
I already landed all of the patches in mozilla-inbound.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ef8381db1873
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.