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

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

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).

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 3

[Andreas Gal :gal]

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?

Comment 4

[Andreas Gal :gal]

Comment on attachment 8387179 [details] [diff] [review]
patch

Stealing. Just the right level of complexity for me to review.

Comment 5

[Bas Schouten (:bas.schouten)]

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!

Comment 6

[Jonathan Watt [:jwatt]]

Thank, both!

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d057a053e6b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d057a053e6b