Remove all occurrences of the gfxIntRect and nsIntRect typedefs in gfx code, replace them by gfx::IntRect

RESOLVED FIXED in Firefox 40

Status

()

defect
--
trivial
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nical, Assigned: u538282, Mentored)

Tracking

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(9 attachments)

(Reporter)

Description

4 years ago
Now that gfxIntRect and nsIntRect are typedefs of gfx::IntRect and not separate classes, we can remove all of their 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 submitting contributions before tackling a real bug or feature.

grep -r nsIntRect gfx/
grep -r gfxIntRect gfx/

and manually replace them by gfx::IntRect in code that is under the mozilla namespace, IntRect for code under mozilla::gfx or .cpp files with "using namespace mozilla::gfx;" or mozilla::gfx::IntRect 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 rect classes/typedefs.
(Assignee)

Comment 1

4 years ago
What would be the best for this bug: a patch for the entire correction or several patches, one for each subdirectory of gfx/ for example?
(Reporter)

Comment 2

4 years ago
(In reply to thibaud.backenstrass from comment #1)
> What would be the best for this bug: a patch for the entire correction or
> several patches, one for each subdirectory of gfx/ for example?

Several patches would be best.
(Assignee)

Comment 3

4 years ago
This patch replaces around 45 of the 386 occurences of nsIntRect, all located in subdirectory gfx/src/. Rebuild completed with no errors.
Attachment #8602117 - Flags: review?(nical.bugzilla)
(Reporter)

Updated

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

Updated

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

Comment 5

4 years ago
Removes all occurences of nsIntRect in gfx/ipc/
Build successful
Complete patch 8602117
Attachment #8602252 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 6

4 years ago
Removes all occurences of nsIntRect in gfx/gl/
Build successful
Complete patches 8602117 & 8602252
Attachment #8602256 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 7

4 years ago
Removes all occurences of nsIntRect in gfx/thebes/
Build successful
Complete patches 8602117, 8602252 & 8602256
Attachment #8602258 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 8

4 years ago
Removes all occurences of nsIntRect in gfx/layers/basic/, gfx/layers/d3d9/, gfx/layers/d3d11 and related files :
 - gfx/layers/D3D11ShareHandleImage.h,
 - gfx/layers/D3D9SurfaceImage.cpp,
 - gfx/layers/D3D9SurfaceImage.h.
Build successful
Complete patches 8602117, 8602252, 8602256 & 8602258
Attachment #8602268 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 9

4 years ago
Removes all occurences of nsIntRect in gfx/layers/composite/
Build successful
Complete patches 8602117, 8602252, 8602256, 8602258 & 8602268
Attachment #8602273 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 10

4 years ago
Removes all occurences of nsIntRect in gfx/layers/client/
Build successful, but after editing the patch file manually (please verify it carefully, I can do another one if there is a problem!)
Complete patches 8602117, 8602252, 8602256, 8602258, 8602268 & 8602273
Attachment #8602520 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 11

4 years ago
Removes all remaining occurences of nsIntRect in gfx/layers/ (mostly gfx/layers/*.h and gfx/layers/*.cpp files)
Build successful
Complete patches 8602117, 8602252, 8602256, 8602258, 8602268, 8602273 & 8602520
Attachment #8602522 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 12

4 years ago
Removes most occurences of nsIntRect in gfx/tests/
Build successful, but after editing the patch file manually (please verify it carefully, I can do another one if there is a problem!)
Complete patches 8602117, 8602252, 8602256, 8602258, 8602268, 8602273, 8602520 & 8602522

It seems that I cannot change the lines 432 to 439 in https://hg.mozilla.org/integration/mozilla-inbound/file/27bd818ba469/gfx/tests/gtest/TestRect.cpp#l432 without having a build failure, so I didn't modified them.
Attachment #8602526 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 13

4 years ago
My 9 patches above removed nearly all occurences of nsIntRect in gfx/ :
  - the typedef in https://hg.mozilla.org/integration/mozilla-inbound/file/27bd818ba469/gfx/src/nsRect.h#l26 has not been removed since it is used in other directories than gfx/,
  - the lines https://hg.mozilla.org/integration/mozilla-inbound/file/27bd818ba469/gfx/tests/gtest/TestRect.cpp#l432 remained unchanged because it seems to cause trouble.
(Reporter)

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

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

Updated

4 years ago
Whiteboard: [leave-open]
(Reporter)

Updated

4 years ago
Assignee: nobody → thibaud.backenstrass
https://hg.mozilla.org/mozilla-central/rev/76becfa04f43
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
In the future, if you have more of these renames coming, please advise stability@m.o to be on the lookout, because these patches dramatically change crash signatures.
You need to log in before you can comment on or make changes to this bug.