Closed Bug 542670 Opened 10 years ago Closed 10 years ago
Add equality operator for ns
3.21 KB, patch
|Details | Diff | Splinter Review|
20.35 KB, patch
|Details | Diff | Splinter Review|
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
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 #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.
Attachment #423928 - Flags: review?(roc) → review+
(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
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
Rebased Patch A to apply on top of Bug 542731 Patches A-D. (Just fixing bitrot in context)
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
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.