151
edits
SVG:Cleanup: Difference between revisions
m
no edit summary
mNo edit summary |
|||
| Line 8: | Line 8: | ||
Figure out what the observer structure actually is, and hardcode it with notification methods. It seems to me that most notifications flow from coordinate objects to their frames. My suggestion is to have the SVG content objects notify SVG frames directly on relevant attribute changes and DOM calls. Style changes already reach the frames. When irregular notifications are required (e.g., gradients notifying their users), we can have an explicit observer list for just those objects that need it. As long as observers are limited to frames, don't use XPCOM weak references for the observer pattern. Frames are explicitly created and destroyed, and you can remove the observer relationship whenever the observer or the observee is destroyed. | Figure out what the observer structure actually is, and hardcode it with notification methods. It seems to me that most notifications flow from coordinate objects to their frames. My suggestion is to have the SVG content objects notify SVG frames directly on relevant attribute changes and DOM calls. Style changes already reach the frames. When irregular notifications are required (e.g., gradients notifying their users), we can have an explicit observer list for just those objects that need it. As long as observers are limited to frames, don't use XPCOM weak references for the observer pattern. Frames are explicitly created and destroyed, and you can remove the observer relationship whenever the observer or the observee is destroyed. | ||
[tor] The patch in bug 301628 took a first step down this route by removing most explicit use of SVG observers in the SVG frames, replacing them where needed by the AttributeChanged mechanism. How do you intend that the content objects notify the SVG frames? Via GetPrimaryFrameFor(), which I've been warned away from in the past due to it being a relatively heavyweight operation? | [tor] The patch in {{bug|301628}} took a first step down this route by removing most explicit use of SVG observers in the SVG frames, replacing them where needed by the AttributeChanged mechanism. How do you intend that the content objects notify the SVG frames? Via GetPrimaryFrameFor(), which I've been warned away from in the past due to it being a relatively heavyweight operation? | ||
[roc] That patch is very good stuff. I would use GetPrimaryFrameFor for whatever's left. The first use on a given element-frame pair is slow, but after that I believe it's just a hashtable lookup. I would want to see a detailed scenario where GetPrimaryFrameFor is too slow before trying someting bloatier/more complex. | [roc] That patch is very good stuff. I would use GetPrimaryFrameFor for whatever's left. The first use on a given element-frame pair is slow, but after that I believe it's just a hashtable lookup. I would want to see a detailed scenario where GetPrimaryFrameFor is too slow before trying someting bloatier/more complex. | ||
[tor] | [tor] {{bug|301628}} and follow-up patches for gradients and patterns have been checked in, removing much of the use of observers in the frame code. | ||
==== Fix (animated) lengths ==== | ==== Fix (animated) lengths ==== | ||
| Line 19: | Line 19: | ||
I have a proposal for this on the wiki at [[SVGDev:Notification_Mechanisms]] which I won't repeat here. The basic idea is to store coordinate data efficiently as fields of SVG content objects. DOM access is provided by creating tearoffs holding a reference to the content object holding the coordinate and the index of the coordinate within the content object. | I have a proposal for this on the wiki at [[SVGDev:Notification_Mechanisms]] which I won't repeat here. The basic idea is to store coordinate data efficiently as fields of SVG content objects. DOM access is provided by creating tearoffs holding a reference to the content object holding the coordinate and the index of the coordinate within the content object. | ||
As part of this, instead of having frames hold references to relevant coordinates, they would get the data through content every time it is needed, by casting their content element to the appropriate | As part of this, instead of having frames hold references to relevant coordinates, they would get the data through content every time it is needed, by casting their content element to the appropriate '''concrete''' SVG element class. | ||
==== Don't create unnecessary CSS rules ==== | ==== [DONE]Don't create unnecessary CSS rules ==== | ||
Only create a CSS rule for an SVG element if the SVG element actually has | Only create a CSS rule for an SVG element if the SVG element actually has | ||
| Line 30: | Line 30: | ||
[roc] I could be wrong but I think it's really simple. See [[http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsSVGElement.cpp#611]] | [roc] I could be wrong but I think it's really simple. See [[http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsSVGElement.cpp#611]] | ||
[tor] Done - bug 331481. | [tor] Done - {{bug|331481}}. | ||
==== Eliminate PRBool fields ==== | ==== Eliminate PRBool fields ==== | ||
| Line 36: | Line 36: | ||
PRPackedBool should be used for fields, and fields should be ordered to minimize padding. | PRPackedBool should be used for fields, and fields should be ordered to minimize padding. | ||
[jwatt] The PRBool to PRPackedBool changes are done (see | [jwatt] The PRBool to PRPackedBool changes are done (see {{bug|328571}}). More work could be done to audit the code for better ordering of fields. | ||
==== Fix nsAutoVoidArray field ==== | ==== Fix nsAutoVoidArray field ==== | ||
| Line 55: | Line 55: | ||
superclass. | superclass. | ||
[tor] Paint() cleaned up as part of the SVG effects (filter, mask, clipPath, opacity) unification bug 330498. | [tor] Paint() cleaned up as part of the SVG effects (filter, mask, clipPath, opacity) unification {{bug|330498}}. | ||
==== deCOMtaminate core data structures ==== | ==== deCOMtaminate core data structures ==== | ||
| Line 82: | Line 82: | ||
Elsewhere we use slots structs for this sort of thing. That may be the way to go here if it's rare for anything at all to be used. If it's common for _something_ to be there, just not all the things at once, then the property system may be better. --[[User:Bzbarsky|Bzbarsky]] 16:44, 15 Feb 2006 (PST) | Elsewhere we use slots structs for this sort of thing. That may be the way to go here if it's rare for anything at all to be used. If it's common for _something_ to be there, just not all the things at once, then the property system may be better. --[[User:Bzbarsky|Bzbarsky]] 16:44, 15 Feb 2006 (PST) | ||
[tor] Filter field moved to a property as part of bug 330498. | [tor] Filter field moved to a property as part of {{bug|330498}}. | ||
==== DeCOMtaminate non-DOM interfaces ==== | ==== DeCOMtaminate non-DOM interfaces ==== | ||
| Line 94: | Line 94: | ||
direct usage of the cairo renderer classes, followed by ... | direct usage of the cairo renderer classes, followed by ... | ||
[tor] GDI+ and libart renderers removed, bug 330516. | [tor] GDI+ and libart renderers removed, {{bug|330516}}. | ||
==== DeCOMtaminate/devirtualize cairo/Thebes renderer ==== | ==== DeCOMtaminate/devirtualize cairo/Thebes renderer ==== | ||