Closed GoogleCodeExporter closed 8 years ago
I have created the attached patch (to revision 1133) for this issue which goes
somewhat further by supporting
additional smil features and fixing issues found along the way. I'll summarize
the changes, added support,
and bug fixes in this message and plan to flesh it out in later messages. If
anyone is interested I can upload a
built svg.swf incorporating the patch.
Overview: The overall structure of the code is basically intact with the major
difference being the processing
of time intervals in onSVGDocTimeUpdate. The TimeInterval class has been
replaced by two time instance
arrays (begin and end) which are processed to determine the current time
interval. Also,
start/end/repeatInterval events propagate the instance time to dependent nodes
(instead of the dependent
node using the current time when it handles the event).
Added support:
- restart attribute
- end attribute
- extend event time spec parsing to include - "click", "click+1s"
- inherited from,to, values (eg values="none;inline;inherit")
- by animation (ie missing from)
- to animation (ie missing from)
- optimization - store frozen values to avoid recalculation
- from-by animation in SVGAnimateTransformNode
- beginElement,beginElementAt, endElement,endElementAt at the level of
SVGTimedNode
(see Issue 489)
Fixes:
- SVGSetNode: return the toParameter for the duration of the animation
- parseEventTimeSpec
- not parsing time offsets with decimal points correctly
- EventTimeSpec
- eventName "repeatEvent" should be "repeat"
- SVGNode.as
- getAnimAttribute - additive colors not added correctly
- SVGTimedNode
- repeatCount,repeatDur processing incorrect when repeatCount is not specified
- parseMinIntervalDuration,parseMaxIntervalDuration
- added error checking for invalid values
- SVGSVGNode
- added "display" to the list of attributes caught in getAttribute() when not specified
- required by animations which specify a value as "inherit"
- SVGAnimateNode
- missing from attribute causes exception when it is a color or number
- getAnimValue: colors returned as a number string, need to be returned as a color
string so that they continue to be treated as colors (instead of numbers)
Original comment by k...@svgmaker.com
on 20 May 2010 at 3:52
Attachments:
Wow, thanks for the patch! Hopefully this spares me from addressing Issue 489
myself. Integrating this patch is now my highest priority.
Original comment by grick23@gmail.com
on 20 May 2010 at 5:19
Patch integrated in r1134.
A few minor fixes were required but overall this was a great patch!
In addition, I added support for document time seeking and I exposed
beginElement,
endElement, beginElementAt, and endElementAt in the DOM (Issue 489).
Original comment by grick23@gmail.com
on 26 May 2010 at 3:06
Thanks, Rick. That's terrific!
Just a few comments...
1. In timeIntervalStarted() I noticed you put repeatIntervalStarted() in there.
First, I don't think it belongs there
since the repeat event should only be fired "each time the element repeats
after the first iteration". Second,
repeatIntervalStarted() only does anything if the currentRepeatIndex > 0.
currentRepeatIndex is set to 0 in timeIntervalStarted().
2. onSVGDocTimeUpdate() - // TODO: How does this affect cumulative animations?
Could you elaborate? I'm not sure what you mean.
3. onSVGDocTimeSeek(). If the seek is forward in time then there is nothing to
do. (ie no need to replay from 0s -
just process forward as normal to the new document time).
For seeking back in time, I found the SMIL spec less than clear (or
predictable) and requires somewhat more
baggage to implement. (this is more a comment on the spec than the code)
http://www.w3.org/TR/2005/REC-SMIL2-20051213/smil-timing.html#Timing-HyperlinksA
ndTiming
[[
. Resolved begin times (e.g. a begin associated with an event) are not cleared
or lost by seeking to an earlier time.
Resolved end times associated with events, repeat-values, accesskey-values or
added via DOM method calls are
cleared when seeking to time earlier than the resolved end time. This follows
the semantics for resetting element
state.
]]
This point means that begin and end time instances in the seek interval (seek
time -> hyperlink time) are treated
differently. The implication being that the time instances need to carry
around what type of event they are
associated with. Then moving forward again, does this mean that begin clicks
are remembered but end clicks not?
The SVG spec mentions hyperlinks targeting animation elements in one place only
- when describing the
"indefinite" value for the begin attribute. If this were the only case to
consider then no problem - no seeking
required, just a begin on the targeted element. In the tests/svggen directory,
the only test cases for hyperlinking
use begin="indefinite" (animate-elem-20-t.svg, animate-elem-21-t.svg,
animate-elem-29-t.svg).
Thanks again.
Original comment by k...@svgmaker.com
on 26 May 2010 at 5:39
1. As noted in the comment, repeatIntervalStarted is used by SVGAudioNode and
SVGVideoNode to start playing media. These elements currently use the repeat
time
span to control what they play, so they need to use repeatIntervalStarted to
know
when to play and they need to start on the first interval. As you mentioned,
that
routine only fires events after the first interval, as it should. So, I don't
see a
problem with the way this works. Perhaps you are under the impression that the
only
intention of repeatIntervalStarted was to throw an event, but that is not the
case,
and that is why the index is checked before throwing an event.
2. I was reading this in the spec: "Animations that are additive apply (i.e.
add to)
to the result of the earlier-activated animations." This made me think that if
an
animation is activated more than once, it should remember the additions caused
by a
previous activation. If we just skip these intervals because they were in the
past,
perhaps we are discarding additive behavior. However, I just tested Batik and
animations are apparently not additive against themselves, so this is not an
issue.
3. Agreed. The spec is less than clear and forces you into an implementation
that has
baggage that I'd rather not deal with. Apparently the browsers agreed because
they
have not implemented document seeking as the spec calls for. So, I have tried to
replicate the behavior of Batik, which does implement document seeking. You can
run
batik against samples/svg-files/anim1.svg and play with clicking up some dynamic
animations and then seek to the beginning. Batik replays the dynamic event
animations! The document seek hack I added to your code replicates Batik's
behavior
as far as I can tell. As you noticed, I did not put a lot of time into it.
Perhaps I should not have preserved the end times. You are probably right about
that.
Also, we may need to preserve more information to seek properly. The list of
simple
time values may need to evolve into a list of objects in order to retain that
state.
It would take a lot more thought and testing to see where the current
implementation
differs from Batik. We are pretty close. I probably won't have time to spend in
this
area in the short term but if you'd like to play around with Batik and submit a
patch
to more precisely replicate it's behavior, that would be appreciated. Given
that we
are digging into the obscure areas of SMIL functionality, I would understand if
you
decide move on to other work.
The spec I have been working from is http://www.w3.org/TR/smil-animation and the
hyperlinking is described in section 3.6.3 through 3.6.5. You are pointing to
the
SMIL 2.1 spec which I have never read, to be honest. I have enough trouble with
the
SMIL Animation spec, let alone the far more complicated "time container" stuff
indicated by SMIL 2.x. I am not sure those specs will ever be a realistic
target for
this project. It would bloat our code and I am not sure the functionality would
be
worth it.
Thanks again for your patch - and I have not forgotten all the text positioning
work
you did as well. Your work is really helping SVG Web.
Original comment by grick23@gmail.com
on 27 May 2010 at 2:14
1. Thanks for the explanation.
2. "Animations that are additive apply (i.e. add to) to the result of the
earlier-activated animations."
I think that refers to other earlier-activated animations. I wondered if
additive animations built on frozen
values until I read the following for restart - "When an animation restarts,
the defining semantic is that it
behaves as though this were the first time the animation had begun, independent
of any earlier behavior."
Which means the final or frozen value of a previous active interval has no
effect on subsequent active
intervals.
As for the ordering of additive animations, that gets resolved at
getAttribute(). Perhaps the effective
animations collected in getAttribute should be sorted on currentBeginTime?
3. I have been using the same SMIL spec as you referenced. But occasionally I
look at the 2.1 spec because
SVG Tiny 1.2 references it. For now, animateMotion is a more pressing feature
for me than is document
seeking.
Original comment by k...@svgmaker.com
on 27 May 2010 at 3:46
Original issue reported on code.google.com by
k...@svgmaker.com
on 20 Apr 2010 at 7:21Attachments: