Closed
Bug 940226
Opened 11 years ago
Closed 11 years ago
Build image/src in unified mode
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file, 1 obsolete file)
4.06 KB,
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•11 years ago
|
||
Comment 2•11 years ago
|
||
Comment on attachment 8334392 [details] [diff] [review]
unify-image-src.patch
Review of attachment 8334392 [details] [diff] [review]:
-----------------------------------------------------------------
::: image/src/moz.build
@@ +33,5 @@
> 'SVGDocumentWrapper.cpp',
> 'VectorImage.cpp',
> ]
>
> +SOURCES += [
Please add a comment saying why these files are not unified.
Attachment #8334392 -
Flags: review+
Assignee | ||
Comment 3•11 years ago
|
||
patch v2: I needed to #undef GetCurrentTime to fix some SVG link errors because winbase.h #defines GetCurrentTime GetTickCount, stomping on calls to SVG member function GetCurrentTime().
https://tbpl.mozilla.org/?tree=Try&rev=f3796a617b93
Attachment #8334392 -
Attachment is obsolete: true
Attachment #8335828 -
Flags: review?(seth)
Comment 4•11 years ago
|
||
Comment on attachment 8335828 [details] [diff] [review]
unify-image-src-v2.patch
Review of attachment 8335828 [details] [diff] [review]:
-----------------------------------------------------------------
Nice. Thanks, Chris!
::: image/src/SVGDocumentWrapper.cpp
@@ +28,5 @@
> #include "mozilla/dom/SVGAnimatedLength.h"
> #include "nsMimeTypes.h"
>
> +// undef the GetCurrentTime macro defined in WinBase.h from the MS Platform SDK
> +#undef GetCurrentTime
If you know or can easily get the information, it'd be great if this comment noted which header file was responsible for pulling in WinBase.h.
::: image/src/SVGDocumentWrapper.h
@@ +25,5 @@
>
> #define OBSERVER_SVC_CID "@mozilla.org/observer-service;1"
>
> +// undef the GetCurrentTime macro defined in WinBase.h from the MS Platform SDK
> +#undef GetCurrentTime
(Same here.)
::: image/src/VectorImage.cpp
@@ +26,5 @@
> #include "nsIDOMEventListener.h"
> #include "SurfaceCache.h"
>
> +// undef the GetCurrentTime macro defined in WinBase.h from the MS Platform SDK
> +#undef GetCurrentTime
(Same here.)
::: image/src/moz.build
@@ +33,5 @@
> 'SVGDocumentWrapper.cpp',
> 'VectorImage.cpp',
> ]
>
> +# These files can't be unified because of prlog.h #include order issues.
It'd be good to file a followup about fixing this issue.
Attachment #8335828 -
Flags: review?(seth) → review+
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Seth Fowler [:seth] from comment #4)
> > +// undef the GetCurrentTime macro defined in WinBase.h from the MS Platform SDK
> > +#undef GetCurrentTime
>
> If you know or can easily get the information, it'd be great if this comment
> noted which header file was responsible for pulling in WinBase.h.
Unfortunately, I don't know which header files is indirectly including WinBase.h because I don't have a Windows dev environment. I'm just testing my Windows fix on the try server. I just copy/pasted that WinBase.h comment from another header file that was #undef'ing GetCurrentTime.
> > +# These files can't be unified because of prlog.h #include order issues.
>
> It'd be good to file a followup about fixing this issue.
This prlog.h issue is not really bug. image/public/ImageLogging.h must #define FORCE_PR_LOG before prlog.h is included to enable logging in release builds. See bug 481962. In the unified .cpp file, prlog.h is indirectly included before ImageLogging.h. I'll change this comment to say ImageLogging.h instead of prlog.h.
Assignee | ||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•