Fixing some endian-ness bugs for color management - bug 439704. r=vlad
authorBobby Holley <bholley@mozilla.com>
Wed, 10 Sep 2008 19:21:03 -0700
changeset 19127 4fab4fa3d85c3ed4157f27698b9dbf5c464ad99c
parent 19126 fd1e113db2f489a539bad27655f0abcf6f13e76f
child 19128 f46156216b9d09084c9fb3b08bbb0c5fa980deb6
push idunknown
push userunknown
push dateunknown
reviewersvlad
bugs439704
milestone1.9.1b1pre
Fixing some endian-ness bugs for color management - bug 439704. r=vlad
gfx/thebes/public/gfxPlatform.h
gfx/thebes/src/gfxContext.cpp
gfx/thebes/src/gfxPattern.cpp
gfx/thebes/src/gfxPlatform.cpp
widget/src/cocoa/nsCocoaWindow.mm
--- a/gfx/thebes/public/gfxPlatform.h
+++ b/gfx/thebes/public/gfxPlatform.h
@@ -41,16 +41,17 @@
 
 #include "prtypes.h"
 #include "nsVoidArray.h"
 
 #include "nsIObserver.h"
 
 #include "gfxTypes.h"
 #include "gfxASurface.h"
+#include "gfxColor.h"
 
 #ifdef XP_OS2
 #undef OS2EMX_PLAIN_CHAR
 #endif
 
 typedef void* cmsHPROFILE;
 typedef void* cmsHTRANSFORM;
 
@@ -233,16 +234,23 @@ public:
      * value is returned. Otherwise, -1 is returned and the embedded intent
      * should be used.
      *
      * See bug 444014 for details.
      */
     static int GetRenderingIntent();
 
     /**
+     * Convert a pixel using a cms transform in an endian-aware manner.
+     *
+     * Sets 'out' to 'in' if transform is NULL.
+     */
+    static void TransformPixel(const gfxRGBA& in, gfxRGBA& out, cmsHTRANSFORM transform);
+
+    /**
      * Return the output device ICC profile.
      */
     static cmsHPROFILE GetCMSOutputProfile();
 
     /**
      * Return the sRGB ICC profile.
      */
     static cmsHPROFILE GetCMSsRGBProfile();
--- a/gfx/thebes/src/gfxContext.cpp
+++ b/gfx/thebes/src/gfxContext.cpp
@@ -617,39 +617,26 @@ gfxContext::GetClipExtents()
 }
 
 // rendering sources
 
 void
 gfxContext::SetColor(const gfxRGBA& c)
 {
     if (gfxPlatform::GetCMSMode() == eCMSMode_All) {
-        cmsHTRANSFORM transform = gfxPlatform::GetCMSRGBTransform();
-        if (transform) {
-#ifdef IS_LITTLE_ENDIAN
-            PRUint32 packed = c.Packed(gfxRGBA::PACKED_ABGR);
-            cmsDoTransform(transform,
-                           (PRUint8 *)&packed, (PRUint8 *)&packed,
-                           1);
-            gfxRGBA cms(packed, gfxRGBA::PACKED_ABGR);
-#else
-            PRUint32 packed = c.Packed(gfxRGBA::PACKED_ARGB);
-            cmsDoTransform(transform,
-                           (PRUint8 *)&packed + 1, (PRUint8 *)&packed + 1,
-                           1);
-            gfxRGBA cms(packed, gfxRGBA::PACKED_ARGB);
-#endif
-            // Use the original alpha to avoid unnecessary float->byte->float
-            // conversion errors
-            cairo_set_source_rgba(mCairo, cms.r, cms.g, cms.b, c.a);
-            return;
-        }
+
+        gfxRGBA cms;
+        gfxPlatform::TransformPixel(c, cms, gfxPlatform::GetCMSRGBTransform());
+
+        // Use the original alpha to avoid unnecessary float->byte->float
+        // conversion errors
+        cairo_set_source_rgba(mCairo, cms.r, cms.g, cms.b, c.a);
     }
-
-    cairo_set_source_rgba(mCairo, c.r, c.g, c.b, c.a);
+    else
+        cairo_set_source_rgba(mCairo, c.r, c.g, c.b, c.a);
 }
 
 void
 gfxContext::SetDeviceColor(const gfxRGBA& c)
 {
     cairo_set_source_rgba(mCairo, c.r, c.g, c.b, c.a);
 }
 
--- a/gfx/thebes/src/gfxPattern.cpp
+++ b/gfx/thebes/src/gfxPattern.cpp
@@ -83,39 +83,26 @@ gfxPattern::CairoPattern()
 {
     return mPattern;
 }
 
 void
 gfxPattern::AddColorStop(gfxFloat offset, const gfxRGBA& c)
 {
     if (gfxPlatform::GetCMSMode() == eCMSMode_All) {
-        cmsHTRANSFORM transform = gfxPlatform::GetCMSRGBTransform();
-        if (transform) {
-#ifdef IS_LITTLE_ENDIAN
-            PRUint32 packed = c.Packed(gfxRGBA::PACKED_ABGR);
-            cmsDoTransform(transform,
-                           (PRUint8 *)&packed, (PRUint8 *)&packed,
-                           1);
-            gfxRGBA cms(packed, gfxRGBA::PACKED_ABGR);
-#else
-            PRUint32 packed = c.Packed(gfxRGBA::PACKED_ARGB);
-            cmsDoTransform(transform,
-                           (PRUint8 *)&packed + 1, (PRUint8 *)&packed + 1,
-                           1);
-            gfxRGBA cms(packed, gfxRGBA::PACKED_ARGB);
-#endif
-            // Use the original alpha to avoid unnecessary float->byte->float
-            // conversion errors
-            cairo_pattern_add_color_stop_rgba(mPattern, offset,
-                                              cms.r, cms.g, cms.b, c.a);
-            return;
-        }
+        gfxRGBA cms;
+        gfxPlatform::TransformPixel(c, cms, gfxPlatform::GetCMSRGBTransform());
+
+        // Use the original alpha to avoid unnecessary float->byte->float
+        // conversion errors
+        cairo_pattern_add_color_stop_rgba(mPattern, offset,
+                                          cms.r, cms.g, cms.b, c.a);
     }
-    cairo_pattern_add_color_stop_rgba(mPattern, offset, c.r, c.g, c.b, c.a);
+    else
+        cairo_pattern_add_color_stop_rgba(mPattern, offset, c.r, c.g, c.b, c.a);
 }
 
 void
 gfxPattern::SetMatrix(const gfxMatrix& matrix)
 {
     cairo_matrix_t mat = *reinterpret_cast<const cairo_matrix_t*>(&matrix);
     cairo_pattern_set_matrix(mPattern, &mat);
 }
--- a/gfx/thebes/src/gfxPlatform.cpp
+++ b/gfx/thebes/src/gfxPlatform.cpp
@@ -533,16 +533,41 @@ gfxPlatform::GetRenderingIntent()
 
         /* If we didn't get a valid intent from prefs, use the default. */
         if (gCMSIntent == -2) 
             gCMSIntent = INTENT_DEFAULT;
     }
     return gCMSIntent;
 }
 
+void 
+gfxPlatform::TransformPixel(const gfxRGBA& in, gfxRGBA& out, cmsHTRANSFORM transform)
+{
+
+    if (transform) {
+#ifdef IS_LITTLE_ENDIAN
+        PRUint32 packed = in.Packed(gfxRGBA::PACKED_ABGR);
+        cmsDoTransform(transform,
+                       (PRUint8 *)&packed, (PRUint8 *)&packed,
+                       1);
+        out.~gfxRGBA();
+        new (&out) gfxRGBA(packed, gfxRGBA::PACKED_ABGR);
+#else
+        PRUint32 packed = in.Packed(gfxRGBA::PACKED_ARGB);
+        cmsDoTransform(transform,
+                       (PRUint8 *)&packed + 1, (PRUint8 *)&packed + 1,
+                       1);
+        out.~gfxRGBA();
+        new (&out) gfxRGBA(packed, gfxRGBA::PACKED_ARGB);
+#endif
+    }
+
+    else if (&out != &in)
+        out = in;
+}
 
 cmsHPROFILE
 gfxPlatform::GetPlatformCMSOutputProfile()
 {
     return nsnull;
 }
 
 cmsHPROFILE
--- a/widget/src/cocoa/nsCocoaWindow.mm
+++ b/widget/src/cocoa/nsCocoaWindow.mm
@@ -1332,19 +1332,27 @@ NS_IMETHODIMP nsCocoaWindow::SetWindowTi
   // If they pass a color with a complete transparent alpha component, use the
   // native titlebar appearance.
   if (NS_GET_A(aColor) == 0) {
     [(ToolbarWindow*)mWindow setTitlebarColor:nil forActiveWindow:(BOOL)aActive]; 
   } else {
     // Transform from sRGBA to monitor RGBA. This seems like it would make trying
     // to match the system appearance lame, so probably we just shouldn't color 
     // correct chrome.
-    cmsHTRANSFORM transform = NULL;
-    if ((gfxPlatform::GetCMSMode() == eCMSMode_All) && (transform = gfxPlatform::GetCMSRGBATransform()))
-      cmsDoTransform(transform, &aColor, &aColor, 1);
+    if (gfxPlatform::GetCMSMode() == eCMSMode_All) {
+      cmsHTRANSFORM transform = gfxPlatform::GetCMSRGBATransform();
+      if (transform) {
+        PRUint8 color[3];
+        color[0] = NS_GET_R(aColor);
+        color[1] = NS_GET_G(aColor);
+        color[2] = NS_GET_B(aColor);
+        cmsDoTransform(transform, color, color, 1);
+        aColor = NS_RGB(color[0], color[1], color[2]);
+      }
+    }
 
     [(ToolbarWindow*)mWindow setTitlebarColor:[NSColor colorWithDeviceRed:NS_GET_R(aColor)/255.0
                                                                     green:NS_GET_G(aColor)/255.0
                                                                      blue:NS_GET_B(aColor)/255.0
                                                                     alpha:NS_GET_A(aColor)/255.0]
                               forActiveWindow:(BOOL)aActive];
   }
   return NS_OK;