989337 - Convert Canvas2D to typed enums and EnumeratedArray

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: 1/3 - Initial manual changes

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: 2/3 - Automatic changes, commands in the commit msg

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: 3/3 - final manual changes

Comment 4

[Bas Schouten (:bas.schouten)]

Comment on attachment 8398515 [details] [diff] [review]
1/3 - Initial manual changes

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

::: content/canvas/src/CanvasGradient.h
@@ +75,5 @@
>    Type mType;
>    virtual ~CanvasGradient() {}
>  };
>  
> +MOZ_FINISH_NESTED_ENUM_CLASS(CanvasGradient::Type)

This Macro seems a little oddly named to me :) But it looks correct.

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 8398518 [details] [diff] [review]
3/3 - final manual changes

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

::: content/canvas/src/CanvasRenderingContext2D.h
@@ +916,5 @@
>  
>      nsRefPtr<gfxFontGroup> fontGroup;
> +    EnumeratedArray<Style, Style::MAX, nsRefPtr<CanvasGradient>> gradientStyles;
> +    EnumeratedArray<Style, Style::MAX, nsRefPtr<CanvasPattern>> patternStyles;
> +    EnumeratedArray<Style, Style::MAX, nscolor> colorStyles;

I wonder whether all compilers generate optimal code for this EnumeratedArray class, but I guess it doesn't matter too much.

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

I wouldn't worry about that at all. I've worked on a array-like container library (matrices really) and went 1000x more crazy than this and compilers these days are good at evaporating this kind of abstraction. Look at EnumeratedArray.h, it's tiny.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aea9916128c6
https://hg.mozilla.org/integration/mozilla-inbound/rev/f05cc8c8c16d
https://hg.mozilla.org/integration/mozilla-inbound/rev/b00d3a5f4b6f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aea9916128c6
https://hg.mozilla.org/mozilla-central/rev/f05cc8c8c16d
https://hg.mozilla.org/mozilla-central/rev/b00d3a5f4b6f