GFX has two essentially-identical "IntSize" classes: IntSize and nsIntSize

NEW
Unassigned

Status

()

Core
Graphics
6 years ago
4 years ago

People

(Reporter: dholbert, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
Bug 651858 added[1] a new struct "IntSize", which (now at least) appears near-identical to the existing nsIntSize structs.

Both structs live in GFX. There's no reason to have two of them.  We should just standardize on one of them and drop the other one.

Comparison of the two classes:
33 struct IntSize :
34   public BaseSize<int32_t, IntSize> {
35   typedef BaseSize<int32_t, IntSize> Super;
36 
37   IntSize() : Super() {}
38   IntSize(int32_t aWidth, int32_t aHeight) : Super(aWidth, aHeight) {}
39 };

31 struct nsIntSize : public mozilla::gfx::BaseSize<int32_t, nsIntSize> {
32   typedef mozilla::gfx::BaseSize<int32_t, nsIntSize> Super;
33 
34   nsIntSize() : Super() {}
35   nsIntSize(int32_t aWidth, int32_t aHeight) : Super(aWidth, aHeight) {}
36 
37   inline nsSize ToAppUnits(nscoord aAppUnitsPerPixel) const;
38 };

Aside from the presence of the ToAppUnits() method, I think they're identical.

Filing this bug on dropping IntSize (leaving behind a typedef for convenience, perhaps) and then converting  all IntSize clients to use nsIntSize instead.

(BONUS POINTS: We should probably also do s/gfxIntSize/nsIntSize/, too -- gfxIntSize used to be a distinct type but is now just a typedef for nsIntSize.)

[1] IntSize struct was added here: http://hg.mozilla.org/mozilla-central/rev/4ede291d2e4c#l21.74
(Reporter)

Updated

6 years ago
OS: Linux → All
Hardware: x86_64 → All
Summary: GFX has two identical "IntSize" classes: IntSize and nsIntSize → GFX has two essentially-identical "IntSize" classes: IntSize and nsIntSize
IntSize should stay and nsIntSize should go away. The ns prefix has no meaning in Graphics land :)
(In reply to Daniel Holbert [:dholbert] from comment #0)
> Aside from the presence of the ToAppUnits() method, I think they're
> identical.

That is exactly the important difference. I guess you'll need to make a global function or something to replace it. Which is OK, I suppose, but it's going to make some code a little uglier, which is why this wasn't done before.

Comment 4

4 years ago
Ping for remove of IntSize or nsIntSize, that's really messed me up.

Comment 5

4 years ago
(In reply to Yonggang Luo from comment #4)
> Ping for remove of IntSize or nsIntSize, that's really messed me up.
For the simplicity, we can do a typedef nsIntSize IntSize or vice versa
You need to log in before you can comment on or make changes to this bug.