| Age | Commit message (Collapse) | Author |
|---|
|
This was a UAF bug when the context being freed contained a
driving tap... just careless placement of the untap call.
It needs to occur before destroying the context, because module
contexts are often used to store the til_tap_t instances.
|
|
And just maintain it as the last ticks value after rendering with
the context...
A couple modules were already doing this manually in an ad-hoc
fashion, just make it a general thing.
Updated those modules to reflect the new situation
Especially in a rkt world with modules::mixer doing fades, it
becomes common to render the same context twice in the same frame
for the blending. We need to prevent accelerated animations in
such situations. For now let's just rely on ticks in a delta-T
fashion to prevent animating the context when ticks is the same.
modules::stars in particular needs this fixed up, upcoming commit
|
|
This becomes necessary in a world with
externall-discoverable-on-stream-contexts that can be arbitrarily
referenced by other contexts.
There will probably be more complexity necessary for invalidating
references, but this is a start.
|
|
This changes til_setup_t* from optional to required for
til_module_context_t creation, while dropping the separate path
parameter construction and passing throughout.
|
|
This just does the obvious pulling in of til_setup_t, holding the
reference throughout the lifetime of the module context.
|
|
Let's make it so til_module_context_t as returned from
til_module_context_new() can immediately be freed via
til_module_context_free().
Previously it was only after the context propagated out to
til_module_context_create() that it could be freed that way, as
that was where the module member was being assigned.
With this change, and wiring up the module pointer into
til_module_t.create_context() as well for convenient providing to
til_module_context_new(), til_module_t.create_context() error
paths can easily cleanup via `return til_module_context_free()`
But this does require the til_module_t.destroy_context() be able
to safely handle partially constructed contexts, since the
mid-create failure freeing won't necessarily have all the members
initialized. There will probably be some NULL derefs to fix up,
but at least the contexts are zero-initialized @ new.
|
|
This was mostly done out of convenience at the expense of turning
the fragment struct into more of a junk drawer.
But properly cleaning up owned stream pipes on context destroy
makes the inappropriateness of being part of til_fb_fragment_t
glaringly apparent.
Now the stream is just a separate thing passed to context create,
with a reference kept in the context for use throughout. Cleanup
of the owned pipes on the stream supplied to context create is
automagic when the context gets destroyed.
Note that despite there being a stream in the module context, the
stream to use is still supplied to all the rendering family
functions (prepare/render/finish) and it's the passed-in stream
which should be used by these functions. This is done to support
the possibility of switching out the stream frame-to-frame, which
may be interesting. Imagine doing things like a latent stream
and a future stream and switching between them on the fly for
instance. If there's a sequencing composite module, it could
flip between multiple sets of tracks or jump around multiple
streams with the visuals immediately flipping accordingly.
This should fix the --print-pipes crashing issues caused by lack
of cleanup when contexts were removed (like rtv does so often).
|
|
The purpose of the context path is to aid in locating the context
instance. The initial application of this will be in service of
the taps, which require their module's path as a sort of
containing directory of the tap name. It'd be convenient to
simply add the path hash with the tap hash to produce the tap's
"absolute path" hash when looking up in the hash table.
|
|
There needs to be a way to address module context instances
by name externally, in a manner complementary to settings and
taps.
This commit adds a string-based path to til_module_context_t, and
modifies til_module_create_context() to accept a parent path
which is then concatenated with the name of the module to produce
the module instance's new path.
The name separator used in the paths is '/' just like filesystem
paths, but these paths have no relationship to filesystems or
files.
The root module context creation in rototiller's main simply
passes "" as the parent path, resulting in a "/" root as one
would expect.
There are some obvious complications introduced here however:
- checkers in particular creates a context per cpu, simply using
the same seed and setup to try make the contexts identical at
the same ticks value. With this commit I'm simply passing the
incoming path as the parent for creating those contexts, but
it's unclear to me if that will work OK. With an eye towards
taps deriving their parent path from the context path, I guess
these taps would all get the same parent and hash to the same
value despite being duplicated. Maybe it Just Works, but one
thing is clear - there won't be any way to address the per-cpu
taps as-is. Maybe that's desirable though, there's probably
not much use in trying to control the taps at the CPU
granularity.
- when the recursive settings stuff lands, it should bring along
the ability to explicitly name settings blocks. Those names
should override the module name in constructing the path.
I've noted as such in the code.
- these paths probably need to be hashed @ initialization time
so there needs to be a hash function added to til, and a hash
value accompanying the name in the module context. It'd be
dumb to keep recomputing the hash when these paths get used
for hash table lookups multiple times per frame...
there's probably more I'm forgetting right now, but this seems
like a good first step.
fixup root path
|
|
Also wire this up to the til_module_context_new() helper and
all its callers.
This is in preparation for modules doing more correct delta-T
derived animation.
|
|
Preparatory commit for embedding a til type in the module
contexts, similar to til_setup_t for the module setups.
This will provide a convenient way to embed seed and n_cpus in
every module context, without having to implement that yourself.
But it also makes it so modules with no need for a context can
continue not implementing those methods, without obstructing
libtil from transparently doing it anyways with a bare
til_module_context_t.
This is kind of important as the current architecture made it
difficult to do things like create contexts with explicit n_cpus,
like in composite/meta modules which are already threaded and
wish to run embedded modules with n_cpus=1.
With this addition, n_cpus could be specified at context create
time, and it will always become remembered in the
til_module_context_t regardless of what the module implements.
That way it can definitely be carried into the prepare/render
methods, with no opportunity for disconnect between what was
passed to context create and what goes to the render methods.
Nothing is functionally changed in this commit alone, subsequent
commits will actually make use of it and realize what I've said
above.
|
