758984 - Cache nsSMILKeySplines in a central manager.

Status: RESOLVED WONTFIX

(Core :: SVG, defect)

Description

[David Zbarsky (:dzbarsky)]

Attachment: Patch

I need this for async css animations, so that the compositor thread can calculate spline values when it only has the Bezier coefficients.

Comment 1

[David Zbarsky (:dzbarsky)]

If we need to, we could also special case the built in timing functions to avoid any hashing or synchronization on them.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What is the point of caching these in a hash table?

Why not just copy all the data around to where you need it?

Comment 3

[David Zbarsky (:dzbarsky)]

Even if we had the data, we would need to reimplement the code in nsSMILKeySpline.cpp that uses the samples to actually calculate the spline value.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think we should move nsSMILKeySpline to gfx/thebes, rename it a bit, and use it from there in both layers and layout.

Comment 5

[David Zbarsky (:dzbarsky)]

Comment on attachment 627588 [details] [diff] [review]
Patch

I'm going to do this without using nsIHashKey.

Comment 6

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you explain why we should use a cache here? I don't understand why at all.

Comment 8

[David Zbarsky (:dzbarsky)]

cjones had concerns about serializing nsSMILKeySplines across threads.  Maybe he can explain.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I was under the impression that these pre-calculated tables stored a lot of data, but dz tells me they're 11 floats.  If they're very expensive to calculate we might win a bit from caching, but we can measure later.

My suggestion was
 - don't try to serialize the layout/style objects, but rather define the animation API in PLayers.ipdl as a conceptually separate interface, a "lowered" representation of what layout/ uses.
 - in the *implementation* of the compositor-driven animations, re-use as much of the layout/ code as possible

Sharing this code seems to make sense.  If we need to move/rename to make sharing more convenient, fine.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We don't need to serialize mSampleValues. We can just recompute it on the other side as needed. In fact it probably makes sense to make CalcSampleValues() lazy.

Comment 11

[David Zbarsky (:dzbarsky)]

Ok, we don't need this.