Closed Bug 905135 Opened 7 years ago Closed 7 years ago

move zero values to their point of use in nsSMILCSSValueType.cpp

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
This patch avoids static constructors at startup.
Attachment #790164 - Flags: review?(dholbert)
Comment on attachment 790164 [details] [diff] [review]
move zero values to their point of use in nsSMILCSSValueType.cpp

Bleh, this doesn't work, because some of the zero values are used elsewhere in the file.
Attachment #790164 - Flags: review?(dholbert)
This patch, however, does compile and still eliminates static constructors.
Attachment #790164 - Attachment is obsolete: true
Attachment #790168 - Flags: review?(dholbert)
Version: unspecified → Trunk
Comment on attachment 790168 [details] [diff] [review]
move zero values to their point of use in nsSMILCSSValueType.cpp

>-  if (*aValue1 == sZeroCoord &&
>+  if (*aValue1 == *GetZeroValueForUnit(nsStyleAnimation::eUnit_Coord) &&
>       aValue2->GetUnit() == nsStyleAnimation::eUnit_Float) {
>-    aValue1 = &sZeroFloat;
>-  } else if (*aValue2 == sZeroCoord &&
>+    aValue1 = GetZeroValueForUnit(nsStyleAnimation::eUnit_Float);
>+  } else if (*aValue2 == *GetZeroValueForUnit(nsStyleAnimation::eUnit_Coord) &&
>              aValue1->GetUnit() == nsStyleAnimation::eUnit_Float) {
>-    aValue2 = &sZeroFloat;
>+    aValue2 = GetZeroValueForUnit(nsStyleAnimation::eUnit_Float);
>   }

So if we make the "else if" check, we'll be invoking GetZeroValueForUnit(nsStyleAnimation::eUnit_Coord) twice in a row, redundantly, which is wasteful.

Instead, could you do something like:
  const nsStyleAnimation::Value& zeroCoord =
    GetZeroValueForUnit(nsStyleAnimation::eUnit_Coord);
  if (*aValue1 == zeroCoord && ...

(I'd only suggest doing that for eUnit_Coord -- not for eUnit_Float -- since we don't always need the eUnit_Float zero-value here, and when we do need it, we already only ask for it once.)

r=me with that.
Attachment #790168 - Flags: review?(dholbert) → review+
Assignee: nobody → nfroyd
Status: NEW → ASSIGNED
Component: General → SVG
https://hg.mozilla.org/mozilla-central/rev/f67ef09058e2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.