950677 - Use IntSize instead of gfxIntSize in layers

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Tor Arvid Lund [:torarvid] (Telenor)]

This is a 'continuation' of 929513, which was set to resolved because of its size.

It is desired to change gfxIntSize references to mozilla::gfx::IntSize as a part of Moz2Difying the code base. This bug tracks this progress in the layers directory.

Comment 1

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize -> gfx::IntSize some places in layers/ipc

Comment 2

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize to gfx::IntSize in DirectX files

Comment 3

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize to gfx::IntSize in ImageContainer.h

Comment 4

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: More gfxIntSize -> IntSize changes in Layer* classes

Comment 5

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ipc/ directory

(Detail: Chose to put an #include for Point.h in a header instead of a
forward declaration because a fwd decl caused an "Already defined" error
and having neither fwd decl or #include caused another compile failure
saying IntSize was undefined)

Comment 6

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ipc/Shadow* files

Comment 7

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize for Texture* classes

Comment 8

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ThebesLayer* classes

Comment 9

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change all remaining gfxIntSize -> IntSize refs

Comment 10

[Nicolas Silva [:nical]]

(In reply to Tor Arvid Lund [:torarvid] from comment #5)
> (Detail: Chose to put an #include for Point.h in a header instead of a
> forward declaration because a fwd decl caused an "Already defined" error
> and having neither fwd decl or #include caused another compile failure
> saying IntSize was undefined)

Yeah, gfx::Point flavors are a pain to forward declare, and Point.h is okay to include in headers because it has very few dependencies.

Comment 11

[Nicolas Silva [:nical]]

try push https://tbpl.mozilla.org/?tree=Try&rev=1f451f3716f6

Comment 12

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize -> gfx::IntSize some places in layers/ipc r=nical

Comment 13

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize to gfx::IntSize in DirectX files r=nical

Comment 14

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change gfxIntSize to gfx::IntSize in ImageContainer.h r=nical

Comment 15

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: More gfxIntSize -> IntSize changes in Layer* classes r=nical

Comment 16

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ipc/ directory r=nical

(Detail: Chose to put an #include for Point.h in a header instead of a
forward declaration because a fwd decl caused an "Already defined" error
and having neither fwd decl or #include caused another compile failure
saying IntSize was undefined)

Comment 17

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ipc/Shadow* files r=nical

Comment 18

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize for Texture* classes r=nical

Comment 19

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: gfxIntSize -> IntSize in ThebesLayer* classes r=nical

Comment 20

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Change all remaining gfxIntSize -> IntSize refs r=nical

Comment 21

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Fix compile errors on Windows/FFOS builds

The errors were discovered when running on the try server. Chose #include
in GrallocImages because there were problems with fwd declarations again.

Comment 22

[Tor Arvid Lund [:torarvid] (Telenor)]

These patches have been run on the try server: https://tbpl.mozilla.org/?tree=Try&rev=7967ae9bc110

Comment 23

[Tor Arvid Lund [:torarvid] (Telenor)]

(In reply to Tor Arvid Lund [:torarvid] from comment #22)
> These patches have been run on the try server:
> https://tbpl.mozilla.org/?tree=Try&rev=7967ae9bc110

... though I rebased them on top of master afterwards. Is there a policy then that I redo the push to try?

Comment 24

[Nicolas Silva [:nical]]

(In reply to Tor Arvid Lund [:torarvid] from comment #23)
> (In reply to Tor Arvid Lund [:torarvid] from comment #22)
> > These patches have been run on the try server:
> > https://tbpl.mozilla.org/?tree=Try&rev=7967ae9bc110
> 
> ... though I rebased them on top of master afterwards. Is there a policy
> then that I redo the push to try?

Depends on how painful the rebase was, and if you had to insert new logic that may be risky. There isn't a strict policy there, it's up to you.

Comment 25

[Tor Arvid Lund [:torarvid] (Telenor)]

Attachment: Fix compile errors on Windows/FFOS builds r=nical

No changes from last iteration except I added "r=nical" to the subject line...

Comment 26

[Tor Arvid Lund [:torarvid] (Telenor)]

(In reply to Nicolas Silva [:nical] from comment #24)
> (In reply to Tor Arvid Lund [:torarvid] from comment #23)
> > ... though I rebased them on top of master afterwards. Is there a policy
> > then that I redo the push to try?
> 
> Depends on how painful the rebase was, and if you had to insert new logic
> that may be risky. There isn't a strict policy there, it's up to you.

Ok. The rebase applied cleanly, with no changes needed. So I won't re-send to the try server. Thanks for reviewing!

Comment 27

[Tor Arvid Lund [:torarvid] (Telenor)]

nical: btw, does this bug need to have another status than "UNCONFIRMED" for the patches to be picked up by a sheriff? (I only have permission to set it to "Resolved as INVALID/WONTFIX/DUPLICATE/WORKSFORME/INCOMPLETE"...)

Comment 28

[Nicolas Silva [:nical]]

Comment on attachment 8350534 [details] [diff] [review]
Fix compile errors on Windows/FFOS builds r=nical

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

::: gfx/layers/ipc/ShadowLayerUtilsX11.cpp
@@ +15,5 @@
>  #include "GLDefs.h"                     // for GLenum
>  #include "gfxASurface.h"                // for gfxASurface, etc
>  #include "gfxPlatform.h"                // for gfxPlatform
>  #include "gfxXlibSurface.h"             // for gfxXlibSurface
> +#include "gfx2DGlue.h"                  // for Moz2D transistion helpers

note: you don't need to add "// for ..." comments when including stuff, unless it's a bit controversial (like when you are forced to pull in big dependencies in a header). Most of these comments were automatically added by a tool that helps cleaning up dependencies, but often it has a lot of garbage.
In this case you don't have to remove this comment either, but just so you know, this is not a style convention of Gecko code.
It's especially useless for things like #include <stdint.h> // for uint32_t

Comment 29

[Nicolas Silva [:nical]]

(In reply to Tor Arvid Lund [:torarvid] from comment #27)
> nical: btw, does this bug need to have another status than "UNCONFIRMED" for
> the patches to be picked up by a sheriff? (I only have permission to set it
> to "Resolved as INVALID/WONTFIX/DUPLICATE/WORKSFORME/INCOMPLETE"...)

I don't think anyone pays much attention to UNCONFIRMED. Once the patches are landed and green on inbound the sheriff will land them into central and mark the bug as resolved regardless of the previous status (unless the [leave open] whiteboard tag is present.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7f2772694b7
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c6b33e402f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7bbc523532f
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bf036817d83
https://hg.mozilla.org/integration/mozilla-inbound/rev/61a05fd01e08
https://hg.mozilla.org/integration/mozilla-inbound/rev/42e0c7dfcba2
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b6bd3cd3ac4
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc09c38cb5b3
https://hg.mozilla.org/integration/mozilla-inbound/rev/aef7ff86117d
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b8adcc27782

Comment 31

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b7f2772694b7
https://hg.mozilla.org/mozilla-central/rev/0c6b33e402f1
https://hg.mozilla.org/mozilla-central/rev/b7bbc523532f
https://hg.mozilla.org/mozilla-central/rev/0bf036817d83
https://hg.mozilla.org/mozilla-central/rev/61a05fd01e08
https://hg.mozilla.org/mozilla-central/rev/42e0c7dfcba2
https://hg.mozilla.org/mozilla-central/rev/8b6bd3cd3ac4
https://hg.mozilla.org/mozilla-central/rev/cc09c38cb5b3
https://hg.mozilla.org/mozilla-central/rev/aef7ff86117d
https://hg.mozilla.org/mozilla-central/rev/8b8adcc27782