Closed Bug 950677 Opened 6 years ago Closed 6 years ago

Use IntSize instead of gfxIntSize in layers

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: torarvid, Assigned: torarvid)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(10 files, 10 obsolete files)

29.62 KB, patch
nical
: review+
Details | Diff | Splinter Review
12.77 KB, patch
nical
: review+
Details | Diff | Splinter Review
19.38 KB, patch
nical
: review+
Details | Diff | Splinter Review
18.10 KB, patch
nical
: review+
Details | Diff | Splinter Review
21.12 KB, patch
nical
: review+
Details | Diff | Splinter Review
16.98 KB, patch
nical
: review+
Details | Diff | Splinter Review
8.59 KB, patch
nical
: review+
Details | Diff | Splinter Review
7.20 KB, patch
nical
: review+
Details | Diff | Splinter Review
14.44 KB, patch
nical
: review+
Details | Diff | Splinter Review
7.75 KB, patch
nical
: review+
Details | Diff | Splinter Review
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.
(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)
Attachment #8348041 - Flags: review?(nical.bugzilla)
Attachment #8348042 - Flags: review?(nical.bugzilla)
Attachment #8348043 - Flags: review?(nical.bugzilla)
Attachment #8348044 - Flags: review?(nical.bugzilla)
Attachment #8348045 - Flags: review?(nical.bugzilla)
Attachment #8348046 - Flags: review?(nical.bugzilla)
Attachment #8348047 - Flags: review?(nical.bugzilla)
Attachment #8348048 - Flags: review?(nical.bugzilla)
Attachment #8348049 - Flags: review?(nical.bugzilla)
(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.
Assignee: nobody → torarvid
Attachment #8348041 - Flags: review?(nical.bugzilla) → review+
Attachment #8348042 - Flags: review?(nical.bugzilla) → review+
Attachment #8348043 - Flags: review?(nical.bugzilla) → review+
Attachment #8348044 - Flags: review?(nical.bugzilla) → review+
Attachment #8348045 - Flags: review?(nical.bugzilla) → review+
Attachment #8348046 - Flags: review?(nical.bugzilla) → review+
Attachment #8348047 - Flags: review?(nical.bugzilla) → review+
Attachment #8348048 - Flags: review?(nical.bugzilla) → review+
Attachment #8348049 - Flags: review?(nical.bugzilla) → review+
Blocks: 882113
Attachment #8348041 - Attachment is obsolete: true
Attachment #8349423 - Flags: review?(nical.bugzilla)
Attachment #8348042 - Attachment is obsolete: true
Attachment #8349424 - Flags: review?(nical.bugzilla)
Attachment #8348043 - Attachment is obsolete: true
Attachment #8349425 - Flags: review?(nical.bugzilla)
Attachment #8348044 - Attachment is obsolete: true
Attachment #8349426 - Flags: review?(nical.bugzilla)
(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)
Attachment #8348045 - Attachment is obsolete: true
Attachment #8349428 - Flags: review?(nical.bugzilla)
Attachment #8348046 - Attachment is obsolete: true
Attachment #8349429 - Flags: review?(nical.bugzilla)
Attachment #8348047 - Attachment is obsolete: true
Attachment #8349430 - Flags: review?(nical.bugzilla)
Attachment #8348048 - Attachment is obsolete: true
Attachment #8349431 - Flags: review?(nical.bugzilla)
Attachment #8348049 - Attachment is obsolete: true
Attachment #8349432 - Flags: review?(nical.bugzilla)
The errors were discovered when running on the try server. Chose #include
in GrallocImages because there were problems with fwd declarations again.
Attachment #8349433 - Flags: review?(nical.bugzilla)
These patches have been run on the try server: https://tbpl.mozilla.org/?tree=Try&rev=7967ae9bc110
(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?
(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.
Attachment #8349423 - Flags: review?(nical.bugzilla) → review+
Attachment #8349424 - Flags: review?(nical.bugzilla) → review+
Attachment #8349425 - Flags: review?(nical.bugzilla) → review+
Attachment #8349426 - Flags: review?(nical.bugzilla) → review+
Attachment #8349428 - Flags: review?(nical.bugzilla) → review+
Attachment #8349429 - Flags: review?(nical.bugzilla) → review+
Attachment #8349430 - Flags: review?(nical.bugzilla) → review+
Attachment #8349431 - Flags: review?(nical.bugzilla) → review+
Attachment #8349432 - Flags: review?(nical.bugzilla) → review+
Attachment #8349433 - Flags: review?(nical.bugzilla) → review+
No changes from last iteration except I added "r=nical" to the subject line...
Attachment #8349433 - Attachment is obsolete: true
Attachment #8350534 - Flags: review?(nical.bugzilla)
(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!
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 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
Attachment #8350534 - Flags: review?(nical.bugzilla) → review+
(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.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.