[Webkit-unassigned] [Bug 10490] remove broken SVGAnimated* code

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=10490


macdome at opendarwin.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjs at apple.com,
                   |                            |darin at apple.com




------- Comment #8 from macdome at opendarwin.org  2006-08-26 02:07 PDT -------
mjs, wildfox/rwlbuis and/or darin should take a breeze through this patch &
script.

I'm not looking for a formal review.  Just an architectural commentary.

The old system, involved SVGAnimated* objects which were held by RefPtrs for
each property for each element.  This was an 8byte waste for every property,
because a pointer to the real SVG* object was kept for both the base and anim
value, even though the anim value was almost never used.  This also lead to
extra-ugly c++ code like x()->baseValue()->value() just to get the damn float
value out of a coordinate.

The new system is ugly in its own way.  It uses macros to define custom
accessors for every animated property on every SVGElement subclass which needs
them.  The good part here is that this results in a space savings of 8 bytes as
we no longer keep these intermediate pointers.  Base values are now stored off
in their own hash in the SVGDocumentExtensions, animVals are just the normal
values stored in the renderer/dom objects.  We also have cleaner access to
these values in the c++ code:
x() => gives you the real float anim value
xBaseValue() => gives you the real float base value

There is one problem with this patch which causes crashes currently.  That is
old code like this:
foo()->baseVal()->doSomething()
would assume that baseVal() would end up lazy_create-ing a new default value on
first access (lazy_create is an old macro)
the modern equivilent re-write of that code (from the script) is:
fooBaseValue()->doSomething()
which ends up crashing.  fooBaseValue() has no default value (currently) so
just returns 0.

There are three ways to fix this:
1.  initialize all default values in the constructors
2.  bring back lazy_create, by passing a default value to the macros which
define these accessors
3.  change all the callsites of this type to call setFooBaseValue() instead.

#3 is probably actually the "cleanest" approach.  I'll probably go with #2 for
now though (as that should be the smallest code change).


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.