Closed Bug 571734 Opened 14 years ago Closed 4 years ago

Replace nsSVGLength2::DOMAnimVal and nsSVGLength2::DOMBaseVal with DOMSVGLength

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: jwatt, Assigned: longsonr)

References

Details

Attachments

(1 file)

We should try and avoid some duplication of code by making nsSVGLength use DOMSVGLength as its DOM wrapper class, and kill nsSVGLength2::DOMAnimVal and nsSVGLength2::DOMBaseVal. Note that we probably don't want to get rid of nsSVGLength2 itself (in favor of SVGLength). nsSVGLength2 manages to pack the value and unit for both its baseVal and animVal into 3*32 bits because it can order the float values first, followed by both PRUint8 units. If we were to use separate SVGLength objects for baseVal and animVal then we have no control over the ordering, so we end up using 4*32 bits. Multiplying the extra 32 bits per attribute by the number of attributes of type <length> on the element (e.g. 6 for rect, including rx, ry) and we start using a bit too much extra memory to want to make this change. Notes from roc: -- In DOMSVGLength, split off a bit to indicate whether this value is a non-list value (an nsSVGLength2 underneath) -- change mList to mOwner and make it nsRefPtr<nsISupports>, let it point to an nsSVGElement when this refers to a non-list value -- if it's a non-list value, use mListIndex to encode the offset of the nsSVGLength2* inside the nsSVGElement -- Then all nsIDOMSVGLengths would be DOMSVGLengths and we could potentially speed up setting and getting values by avoiding a virtual call. It would be less code overall, probably. -- Rename nsSVGLength2 to SVGAnimatedLength
Depends on: 515116
Blocks: 816778
Attached file Bug 571734 - rewrite DOMSVGLength (deleted) —
  • Remove various redundant fields to shrink size by two pointers per instance
Assignee: nobody → longsonr
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: