Closed GoogleCodeExporter closed 8 years ago
Notes from Rick via email on this issue:
Regarding the multiple Sprites issue, for reference, Issue 128 [1] is the issue
where
this design trade off was last evaluated and documented.
Originally, the code was designed to consolidate the Sprites and the 4 sprites
would
just be aliases for a single sprite. (A benefit of the massive effort to
centralize
the design, which was Issue 64 [3] and r375 [4].) The revision where we
switched to
"always on" multiple sprites was r636 [2] . As you can see, the amount of code
which
changed the design was minimal.
However, as Issue 128 describes, the problem with the consolidated sprite
design is
that dynamic scripting can change the properties and introduce a quirk scenario
and
then you do need to have multiple sprites and so you'd have to deal with those
scenarios on the fly. I was not too excited about the complexity and performance
issues of detecting those corner cases dynamically but that is what would need
to be
done to make this design work. Its not a terrible situation. Its just
complexity I'd
rather not have to deal with. The performance concern is that we would need to
check
a few attributes for a quirk scenario on every relevant setAttribute call. I'm
hopeful, though, that the performance issues will not be significant.
If you think the performance benefits are worth it, though, I'm willing to work
on
getting this functional again. Would you mind opening an Issue and assigning it
to
me? If you could include the scenario where you see lag, that would be helpful.
I
know its the pan and zoom stuff, but if you could say exactly which files and
sequence of operations I should play with to know if progress is made that
would be
helpful. Also, if you could give some hints as to priority, that would be
great, as I
am juggling a few things right now.
Thanks,
Rick
1. http://code.google.com/p/svgweb/issues/detail?id=128
2. http://code.google.com/p/svgweb/source/detail?r=636
3. http://code.google.com/p/svgweb/issues/detail?id=64
4. http://code.google.com/p/svgweb/source/detail?r=375
Original comment by bradneub...@gmail.com
on 19 Sep 2009 at 11:57
Started in r894:
Change SVGNode inheritance from Sprite to a non-display class to account for
non-display nodes such as SVGDOMTextNodes. Also, create extra sprites only when
certain features are used instead of creating them just in case (which is a
reversal
of Issue 128 / r636). Certain quirks may reappear in scripting scenarios until
further works is done on this.
The % reduction in sprites was significant. See spreadsheet:
http://spreadsheets.google.com/pub?key=tflExO9S_Nsm8uIfFIbPfpg&output=html
In contrast with the provided test case, the extra sprites used in SVG Web are
generally not doing much so I was not sure that removing them would make a
significant difference. However, once the extra feature sprites and once all
sprites
for DOMText nodes were removed, the performance improvement was noticeable
which is
good news.
This was a significant design change affecting hundreds of lines of code. It
may take
time to clean up loose ends (bugs).
Original comment by grick23@gmail.com
on 26 Sep 2009 at 9:38
There is still potential for sprite reduction, but the vast majority of savings
has
already been had. This work is essentially completed and fallout seems to be
minimal
so I am going to close this. Any further incremental improvements can be
tracked as
separate issues.
Original comment by grick23@gmail.com
on 11 Oct 2009 at 7:36
Original issue reported on code.google.com by
bradneub...@gmail.com
on 19 Sep 2009 at 11:57Attachments: