Closed Bug 980603 Opened 6 years ago Closed 6 years ago

Move the implementation of SurfaceToPackedBGRA to a source file to avoid linking errors

Categories

(Core :: Graphics, defect)

29 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file, 1 obsolete file)

I can't use SurfaceToPackedBGRA in different parts of the code because the linker on Mac gets upset about duplicate symbols. To avoid that we should move the implementation to a separate source file (it's rather too much code to be wanting to inline anyway).
Attached patch patch (obsolete) — Splinter Review
Attachment #8387155 - Flags: review?(bas)
Attached patch patchSplinter Review
Attachment #8387155 - Attachment is obsolete: true
Attachment #8387155 - Flags: review?(bas)
Attachment #8387179 - Flags: review?(bas)
Comment on attachment 8387155 [details] [diff] [review]
patch

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

::: gfx/2d/DataSurfaceHelpers.cpp
@@ +8,5 @@
> +
> +namespace mozilla {
> +namespace gfx {
> +
> +static inline void

inline static in a cpp file?

@@ +25,5 @@
> +    pixel += (aStride/4);
> +  }
> +}
> +
> +inline uint8_t *

dito

::: gfx/2d/DataSurfaceHelpers.h
@@ +7,5 @@
>  
>  #include "2D.h"
>  
>  namespace mozilla {
>  namespace gfx {

do you need to define ConvertBGRXToBGRA here? extern?

@@ +13,5 @@
>  /**
>   * Convert aSurface to a packed buffer in BGRA format. The pixel data is
>   * returned in a buffer allocated with new uint8_t[].
>   */
>  inline uint8_t *

you mean extern here right?
Attachment #8387155 - Attachment is obsolete: false
Attachment #8387155 - Attachment is obsolete: true
Comment on attachment 8387179 [details] [diff] [review]
patch

Stealing. Just the right level of complexity for me to review.
Attachment #8387179 - Flags: review?(bas) → review+
Comment on attachment 8387179 [details] [diff] [review]
patch

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

Unsure why this gave linker errors? That seems like a bug. But I have no issue with this patch!
https://hg.mozilla.org/mozilla-central/rev/1d057a053e6b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.