Bug 1216843 - Part 3: Use nsCSSValue instead of nscolor to store over 1.0 color components as an accumulated value so that we can correctly calculate intermediate values in interpolation. r?dholbert draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 12 Sep 2016 07:05:02 +0900
changeset 412497 6870d9c9e45dfa1a10164545ec8820259bc42e89
parent 412496 bb2c59ede587c40f6948f9ec5d2ef00781189edf
child 412498 1d0e0187a91e7d48af506b205c9b0a2bd0d0fc3b
push id29190
push userbmo:hiikezoe@mozilla-japan.org
push dateMon, 12 Sep 2016 05:22:33 +0000
reviewersdholbert
bugs1216843
milestone51.0a1
Bug 1216843 - Part 3: Use nsCSSValue instead of nscolor to store over 1.0 color components as an accumulated value so that we can correctly calculate intermediate values in interpolation. r?dholbert MozReview-Commit-ID: GYdqYZA4xXf
layout/style/StyleAnimationValue.cpp
layout/style/StyleAnimationValue.h
layout/style/nsStyleContext.cpp
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -561,18 +561,18 @@ StyleAnimationValue::ComputeDistance(nsC
       // that we should use Euclidean RGB cube distance.  However, we
       // have to extend that to RGBA.  For now, we'll just use the
       // Euclidean distance in the (part of the) 4-cube of premultiplied
       // colors.
       // FIXME (spec): The CSS transitions spec doesn't say whether
       // colors are premultiplied, but things work better when they are,
       // so use premultiplication.  Spec issue is still open per
       // http://lists.w3.org/Archives/Public/www-style/2009Jul/0050.html
-      nscolor startColor = aStartValue.GetColorValue();
-      nscolor endColor = aEndValue.GetColorValue();
+      nscolor startColor = aStartValue.GetCSSValueValue()->GetColorValue();
+      nscolor endColor = aEndValue.GetCSSValueValue()->GetColorValue();
 
       // Get a color component on a 0-1 scale, which is much easier to
       // deal with when working with alpha.
       #define GET_COMPONENT(component_, color_) \
         (NS_GET_##component_(color_) * (1.0 / 255.0))
 
       double startA = GET_COMPONENT(A, startColor);
       double startR = GET_COMPONENT(R, startColor) * startA;
@@ -1263,17 +1263,18 @@ AddShadowItems(double aCoeff1, const nsC
       (color2.GetColorValue(), StyleAnimationValue::ColorConstructor);
     StyleAnimationValue resultColorValue;
     DebugOnly<bool> ok =
       StyleAnimationValue::AddWeighted(eCSSProperty_color,
                                        aCoeff1, color1Value,
                                        aCoeff2, color2Value,
                                        resultColorValue);
     MOZ_ASSERT(ok, "should not fail");
-    resultArray->Item(4).SetColorValue(resultColorValue.GetColorValue());
+    resultArray->Item(4).SetColorValue(
+      resultColorValue.GetCSSValueValue()->GetColorValue());
   }
 
   MOZ_ASSERT(inset1 == inset2, "should match");
   resultArray->Item(5) = inset1;
 
   nsCSSValueList *resultItem = new nsCSSValueList;
   resultItem->mValue.SetArrayValue(resultArray, eCSSUnit_Array);
   *aResultTail = resultItem;
@@ -2403,18 +2404,18 @@ StyleAnimationValue::AddWeighted(nsCSSPr
     }
     case eUnit_Float: {
       aResultValue.SetFloatValue(RestrictValue(aProperty,
         aCoeff1 * aValue1.GetFloatValue() +
         aCoeff2 * aValue2.GetFloatValue()));
       return true;
     }
     case eUnit_Color: {
-      nscolor color1 = aValue1.GetColorValue();
-      nscolor color2 = aValue2.GetColorValue();
+      nscolor color1 = aValue1.GetCSSValueValue()->GetColorValue();
+      nscolor color2 = aValue2.GetCSSValueValue()->GetColorValue();
       nscolor resultColor;
 
       // We are using AddWeighted() with a zero aCoeff2 for colors to
       // pretend AddWeighted() against transparent color, i.e. rgba(0, 0, 0, 0).
       // But unpremultiplication in AddWeightedColors() does not work well
       // for such cases, so we use another function named DiluteColor() which
       // has a similar logic to AddWeightedColors().
       if (aCoeff2 == 0.0) {
@@ -3113,31 +3114,30 @@ StyleAnimationValue::UncomputeValue(nsCS
       break;
     case eUnit_Percent:
       aSpecifiedValue.SetPercentValue(aComputedValue.GetPercentValue());
       break;
     case eUnit_Float:
       aSpecifiedValue.
         SetFloatValue(aComputedValue.GetFloatValue(), eCSSUnit_Number);
       break;
-    case eUnit_Color:
-      // colors can be alone, or part of a paint server
-      aSpecifiedValue.SetColorValue(aComputedValue.GetColorValue());
-      break;
     case eUnit_CurrentColor:
       aSpecifiedValue.SetIntValue(NS_COLOR_CURRENTCOLOR, eCSSUnit_EnumColor);
       break;
     case eUnit_Calc:
+    case eUnit_Color:
     case eUnit_ObjectPosition:
     case eUnit_URL:
     case eUnit_DiscreteCSSValue: {
       nsCSSValue* val = aComputedValue.GetCSSValueValue();
       // Sanity-check that the underlying unit in the nsCSSValue is what we
       // expect for our StyleAnimationValue::Unit:
       MOZ_ASSERT((unit == eUnit_Calc && val->GetUnit() == eCSSUnit_Calc) ||
+                 (unit == eUnit_Color &&
+                  nsCSSValue::IsNumericColorUnit(val->GetUnit())) ||
                  (unit == eUnit_ObjectPosition &&
                   val->GetUnit() == eCSSUnit_Array) ||
                  (unit == eUnit_URL && val->GetUnit() == eCSSUnit_URL) ||
                  unit == eUnit_DiscreteCSSValue,
                  "unexpected unit");
       aSpecifiedValue = *val;
       break;
     }
@@ -4351,17 +4351,18 @@ StyleAnimationValue::StyleAnimationValue
   mUnit = eUnit_Float;
   mValue.mFloat = aFloat;
   MOZ_ASSERT(!mozilla::IsNaN(mValue.mFloat));
 }
 
 StyleAnimationValue::StyleAnimationValue(nscolor aColor, ColorConstructorType)
 {
   mUnit = eUnit_Color;
-  mValue.mColor = aColor;
+  mValue.mCSSValue = new nsCSSValue();
+  mValue.mCSSValue->SetColorValue(aColor);
 }
 
 StyleAnimationValue&
 StyleAnimationValue::operator=(const StyleAnimationValue& aOther)
 {
   if (this == &aOther) {
     return *this;
   }
@@ -4384,20 +4385,18 @@ StyleAnimationValue::operator=(const Sty
     case eUnit_Coord:
       mValue.mCoord = aOther.mValue.mCoord;
       break;
     case eUnit_Percent:
     case eUnit_Float:
       mValue.mFloat = aOther.mValue.mFloat;
       MOZ_ASSERT(!mozilla::IsNaN(mValue.mFloat));
       break;
+    case eUnit_Calc:
     case eUnit_Color:
-      mValue.mColor = aOther.mValue.mColor;
-      break;
-    case eUnit_Calc:
     case eUnit_ObjectPosition:
     case eUnit_URL:
     case eUnit_DiscreteCSSValue:
       MOZ_ASSERT(IsCSSValueUnit(mUnit),
                  "This clause is for handling nsCSSValue-backed units");
       MOZ_ASSERT(aOther.mValue.mCSSValue, "values may not be null");
       mValue.mCSSValue = new nsCSSValue(*aOther.mValue.mCSSValue);
       break;
@@ -4509,17 +4508,18 @@ StyleAnimationValue::SetFloatValue(float
   MOZ_ASSERT(!mozilla::IsNaN(mValue.mFloat));
 }
 
 void
 StyleAnimationValue::SetColorValue(nscolor aColor)
 {
   FreeValue();
   mUnit = eUnit_Color;
-  mValue.mColor = aColor;
+  mValue.mCSSValue = new nsCSSValue();
+  mValue.mCSSValue->SetColorValue(aColor);
 }
 
 void
 StyleAnimationValue::SetCurrentColorValue()
 {
   FreeValue();
   mUnit = eUnit_CurrentColor;
 }
@@ -4663,19 +4663,18 @@ StyleAnimationValue::operator==(const St
     case eUnit_Visibility:
     case eUnit_Integer:
       return mValue.mInt == aOther.mValue.mInt;
     case eUnit_Coord:
       return mValue.mCoord == aOther.mValue.mCoord;
     case eUnit_Percent:
     case eUnit_Float:
       return mValue.mFloat == aOther.mValue.mFloat;
+    case eUnit_Calc:
     case eUnit_Color:
-      return mValue.mColor == aOther.mValue.mColor;
-    case eUnit_Calc:
     case eUnit_ObjectPosition:
     case eUnit_URL:
     case eUnit_DiscreteCSSValue:
       MOZ_ASSERT(IsCSSValueUnit(mUnit),
                  "This clause is for handling nsCSSValue-backed units");
       return *mValue.mCSSValue == *aOther.mValue.mCSSValue;
     case eUnit_CSSValuePair:
       return *mValue.mCSSValuePair == *aOther.mValue.mCSSValuePair;
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -300,17 +300,18 @@ public:
     eUnit_None,
     eUnit_Enumerated,
     eUnit_Visibility, // special case for transitions (which converts
                       // Enumerated to Visibility as needed)
     eUnit_Integer,
     eUnit_Coord,
     eUnit_Percent,
     eUnit_Float,
-    eUnit_Color,
+    eUnit_Color, // nsCSSValue* (never null), always with an nscolor or
+                 // an nsCSSValueFloatColor
     eUnit_CurrentColor,
     eUnit_Calc, // nsCSSValue* (never null), always with a single
                 // calc() expression that's either length or length+percent
     eUnit_ObjectPosition, // nsCSSValue* (never null), always with a
                           // 4-entry nsCSSValue::Array
     eUnit_URL, // nsCSSValue* (never null), always with a css::URLValue
     eUnit_DiscreteCSSValue, // nsCSSValue* (never null)
     eUnit_CSSValuePair, // nsCSSValuePair* (never null)
@@ -327,17 +328,16 @@ public:
   };
 
 private:
   Unit mUnit;
   union {
     int32_t mInt;
     nscoord mCoord;
     float mFloat;
-    nscolor mColor;
     nsCSSValue* mCSSValue;
     nsCSSValuePair* mCSSValuePair;
     nsCSSValueTriplet* mCSSValueTriplet;
     nsCSSRect* mCSSRect;
     nsCSSValue::Array* mCSSValueArray;
     nsCSSValueList* mCSSValueList;
     nsCSSValueSharedList* mCSSValueSharedList;
     nsCSSValuePairList* mCSSValuePairList;
@@ -367,20 +367,16 @@ public:
   float GetPercentValue() const {
     NS_ASSERTION(mUnit == eUnit_Percent, "unit mismatch");
     return mValue.mFloat;
   }
   float GetFloatValue() const {
     NS_ASSERTION(mUnit == eUnit_Float, "unit mismatch");
     return mValue.mFloat;
   }
-  nscolor GetColorValue() const {
-    NS_ASSERTION(mUnit == eUnit_Color, "unit mismatch");
-    return mValue.mColor;
-  }
   nsCSSValue* GetCSSValueValue() const {
     NS_ASSERTION(IsCSSValueUnit(mUnit), "unit mismatch");
     return mValue.mCSSValue;
   }
   nsCSSValuePair* GetCSSValuePairValue() const {
     NS_ASSERTION(IsCSSValuePairUnit(mUnit), "unit mismatch");
     return mValue.mCSSValuePair;
   }
@@ -506,17 +502,18 @@ private:
     return static_cast<char16_t*>(aBuffer->Data());
   }
 
   static bool IsIntUnit(Unit aUnit) {
     return aUnit == eUnit_Enumerated || aUnit == eUnit_Visibility ||
            aUnit == eUnit_Integer;
   }
   static bool IsCSSValueUnit(Unit aUnit) {
-    return aUnit == eUnit_Calc ||
+    return aUnit == eUnit_Color ||
+           aUnit == eUnit_Calc ||
            aUnit == eUnit_ObjectPosition ||
            aUnit == eUnit_URL ||
            aUnit == eUnit_DiscreteCSSValue;
   }
   static bool IsCSSValuePairUnit(Unit aUnit) {
     return aUnit == eUnit_CSSValuePair;
   }
   static bool IsCSSValueTripletUnit(Unit aUnit) {
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -1479,27 +1479,28 @@ ExtractAnimationValue(nsCSSPropertyID aP
 
 static nscolor
 ExtractColor(nsCSSPropertyID aProperty,
              nsStyleContext *aStyleContext)
 {
   StyleAnimationValue val;
   ExtractAnimationValue(aProperty, aStyleContext, val);
   return val.GetUnit() == StyleAnimationValue::eUnit_CurrentColor
-    ? aStyleContext->StyleColor()->mColor : val.GetColorValue();
+    ? aStyleContext->StyleColor()->mColor
+    : val.GetCSSValueValue()->GetColorValue();
 }
 
 static nscolor
 ExtractColorLenient(nsCSSPropertyID aProperty,
                     nsStyleContext *aStyleContext)
 {
   StyleAnimationValue val;
   ExtractAnimationValue(aProperty, aStyleContext, val);
   if (val.GetUnit() == StyleAnimationValue::eUnit_Color) {
-    return val.GetColorValue();
+    return val.GetCSSValueValue()->GetColorValue();
   } else if (val.GetUnit() == StyleAnimationValue::eUnit_CurrentColor) {
     return aStyleContext->StyleColor()->mColor;
   }
   return NS_RGBA(0, 0, 0, 0);
 }
 
 struct ColorIndexSet {
   uint8_t colorIndex, alphaIndex;