Closed Bug 542670 Opened 10 years ago Closed 10 years ago

Add equality operator for nsSMILValue

Categories

(Core :: SVG, defect)

x86
All
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files, 6 obsolete files)

In bug 533291 comment 9, I note that we'll need to...
> check the base value from last sample's compositor against the base value from
> this samples compositor

To do that, we need to be able to compare two nsSMILValues for equality.  Filing this bug on that.
Comment on attachment 423891 [details] [diff] [review]
Patch A: Add method to interface, w/ conservative implementation

This first patch adds operator== and operator!= to nsSMILValue. These just check the object's address for equality, and if that doesn't match, they fall back on a nsISMILType helper method "TestForEquality".

So far, that helper method is extremely conservative -- it only has one implementation, inherited by all subclasses, which unconditionally returns PR_FALSE. (See its header comment in nsISMILType. This is the correct fallback behavior, so that bug 533291's optimization will only take effect if we're *sure* the base value hasn't changed.)

I'm adding smarter, per-subclass specializations of this method in a separate patch.
Attachment #423891 - Attachment description: patch A: Add dummy equality method → Patch A: Add method to interface, w/ conservative implementation
This version just tweaks the header comment for nsISMILType::TestForEquality
Attachment #423891 - Attachment is obsolete: true
Attachment #423924 - Flags: review?(roc)
Attachment #423925 - Flags: review?(roc)
Comment on attachment 423925 [details] [diff] [review]
Patch B: Add specializations of TestForEquality in nsISMILType subclasses

Note that Patch B removes the dummy "return PR_FALSE" implementation that Patch A provides in nsISMILType.h.  Patch A only includes this dummy implementation to allow you to compile (and get working behavior) with *just* Patch A applied.
(this version just removes an accidentally-added comment that's unrelated to this patch, & removes a period at end of an added warning message.)
Attachment #423925 - Attachment is obsolete: true
Attachment #423928 - Flags: review?(roc)
Attachment #423925 - Flags: review?(roc)
Attachment #423924 - Attachment description: Patch A v2: Patch A: Add method to interface, w/ conservative implementation → Patch A v2: Add method to interface, w/ conservative implementation
Comment on attachment 423924 [details] [diff] [review]
Patch A v2: Add method to interface, w/ conservative implementation

+  return mType == aVal.mType ?
+    mType->TestForEquality(*this, aVal) : PR_FALSE;

Use && instead of ?:
Attachment #423924 - Flags: review?(roc) → review+
Oh, I think TestForEquality would be better named IsEqual.
(In reply to comment #7)
> Use && instead of ?:

Fixed.

(In reply to comment #8)
> Oh, I think TestForEquality would be better named IsEqual.

Fixed.
Attachment #423924 - Attachment is obsolete: true
Attachment #423935 - Flags: review+
Fixed patch B to use "IsEqual" method name.

Also fixed a recursive death-spiral in "nsSVGSMILTransform::operator!=" -- previous version had that method calling itself recursively instead of calling !operator==().  (I caught that with a run through mochitests with this plus the not-yet-posted base-value-caching patch for bug 533291.)

Carrying forward r=roc
Attachment #423928 - Attachment is obsolete: true
Attachment #423936 - Flags: review+
Rebased Patch A to apply on top of Bug 542731 Patches A-D.  (Just fixing bitrot in context)
Attachment #423935 - Attachment is obsolete: true
Attachment #424348 - Flags: review+
Rebased Patch B to apply on top of Bug 542731 Patches A-D.

In particular, I've shifted some code-cleanup in nsSVGTransformSMILAttr from this patch into Patch C on Bug 542731 -- so, this version no longer applies that cleanup.
Attachment #423936 - Attachment is obsolete: true
Attachment #424349 - Flags: review+
Landed:
http://hg.mozilla.org/mozilla-central/rev/da48199647d0
http://hg.mozilla.org/mozilla-central/rev/a3cd5f10742b

The latter patch included some (trivial) new IsEqual implementations for a few nsISMILType subclasses that have recently appeared on trunk: SMILIntegerType, SVGOrientSMILType, and SVGViewBoxSMILType (along with nsSVGViewBoxRect::operator= to support SVGViewBoxSMILType)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.