| Age | Commit message (Collapse) | Author |
|---|
|
Basically everywhere the
TIL_MODULE_HERMETIC|TIL_MODULE_EXPERIMENTAL exclusions were being
applied needed TIL_MODULE_BUILTIN added.
Mostly this is to prevent randomizers from tripping over builtins
in the available modules lists they draw from.
Because builtins aren't visually interesting by themselves, and
in some cases don't currently even have a means of being
randomized properly like ref's path setting.
This wasn't needed previously since builtins were kept off the
modules list altogether. But since 1a6210be that changed and
they must be explicitly filtered by flag instead.
Note I deliberately left the rkt case with just a TODO comment.
It's not a randomizer situation, and it might be acceptable to
let rkt just show everything all the time in the module lists.
That whole situation there is for advanced users.
Also note that without this, rtv et al would easily trigger an
assert on NULL setup due to the "none" builtin. Since there's
still work to be done there in callers properly handling NULL
@res_setup on a successful finalize. But this commit mitigates
that by avoiding the builtins in the randomizers.
|
|
Particularly with nested modules it's annoying to have to stow
the module separate from the setup during the setup process.
If the baked setup included the module pointer in the
non-module-specific-setup part of the setup, then nested settings
could finalize using the generic module setup wrapper and just
rely on this til_setup_t.creator pointer to contain the
appropriate module. Which should enable tossing out a bunch of
copy-n-pasta surrounding nested modules setup.
Note this has to be a void* since til_setup_t is a generic thing
used equally by both the fb code and the module code. Hence why
this is called "creator" and not "module", as well as the void*
as opposed to til_module_t*.
Also if rototiller ever grows a sound backend, the setup
machinery will be reused there as well, and it'll be yet another
creator handle that isn't an til_fb_ops_t or a til_module_t.
It's assumed that the callers producing these setups won't be
trying to pass them to the wrong places i.e. a module setup
getting passed to an fb backend and vice versa.
I'm mildly annoyed about having to move the various til_module_t
blocks to above the module's foo_setup(), but it seemed like the
least annoying option. This may be revisited.
|
|
Until channel context paths are distinct it's buggy to let the
contexts linger while constructing the next channel's contexts.
Originally when the gc was added here the intention was to
support stuff like the "ref" module and get the channels settings
wired up immediately with more focus on rtv's details in this
area. Supporting stuff like contexts backing some layers
persisting across channels, while the others were swapped out,
seemed potentially interesting (and it still is).
But the rkt stuff became prioritized as rtv is more like a fuzzer
than anything despite being the default module. And rkt related
activities will continue for now, so let's just get rtv less
likely to crash.
A reliable repro for triggering an ASAN UAF bug without this
commit is:
--seed=0x64af3b05
'--module=rtv,duration=1,context_duration=1,channels=compose,caption_duration=2,snow_duration=0,snow_module=none,log_channels=on'
'--video=sdl,fullscreen=off,size=640x480'
A few channels in blinds will UAF while updating taps stored in a
freed context, because the previous channel has a blinds in the
same layer as the newly setup channel, putting the contexts at
exactly the same paths on-stream. There's probably another bug
in here that I need to dig into, but coexisting contexts at the
same path on-stream was never the intention. The syncronous
immediate gc ensures nothing remains of the previous channel
before constructing the new one at the same path.
|
|
The channel module name was being used as the settings instance
name, which is redundant to the entries[0] "value as label"
behavior with the module name also @ entries[0].
The resulting paths resembled:
/module/rtv/compose/compose/layers/[N]/...
/module/rtv/compose/compose/texture/...
Now they are:
/module/rtv/channel/compose/layers/[N]/...
/module/rtv/channel/compose/texture/...
There's still work to be done in this area of rtv. It's unclear
if even a static "/module/rtv/channel" is correct, or if it
should show the subscript of the channels[] entry currently being
used when that becomes a proper nested settings situation, i.e.
/module/rtv/channels/[0]/compose/layers/[N]/...
...
/module/rtv/channels/[1]/spiro
...
/module/rtv/channels/[2]/roto
...
for a "channels=compose,spiro,roto" kind of hypothetical
another option which might make sense is to have the channel path
more like an auto-increment identifier so the channel contexts
have unique paths and don't collide on the stream. This could be
important for scenarios utilizing the "ref" module and trying to
reuse parts of the context from one channel to the next. When
the paths collide things tend to break in weird ways presently.
this area needs more thought
|
|
In the interests of improving error handling of interactive
setups, return the setting that was invalid when setup returns
-EINVAL.
For now this is only supported for non-finalized/non-baking setup
calls, i.e. (!res_setup).
In the future I want this also supported for finalizing when
res_setup is set. A lot of the -EINVAL returns are actually
during that stage (or need to be added) due to that being when
the more sensitive parsing occurs going from strings to native
numeric types and such.
The main reason it's not already being done in this commit is
it'll churn quite a bit to get there, since the setup funcs don't
generally have the setting pointers onhand at that phase. It'll
require changing the string-value-centric local variables to
instead all be the til_setting_t* holding those values.
And if setup funcs aren't going to be so value-centric, the
settings API will likely change to not even return the values
directly anymore and only return the full-blown til_settings_t as
the main return pointer, perhaps rid of their res_setting use
even.
Anyway, while here I did some random little cleanups too.
|
|
This probably needs more work, but this at a minimum should
prevent us from leaking contexts in the stream at the myriad
paths we construct them at.
Context registration replaces what's at the existing path, but
rtv produces all sorts of setup paths, and I haven't added any
explicit unregistration of contexts at this time. That might
change, but for now let's just use this gc mechanism even if it's
a temporary hack.
|
|
Somewhere along the line this leak was created, there's been a
lot of activity surrounding this stuff so it's unsurprising.
A little janky surrounding the conditional on snow module, but
that's just how snow is handled today - it doesn't get randomized
like the other channels do.
|
|
Rather than creating an orphaned settings instance private to
til_module_setup_randomize(), the function now requires the
settings instance be provided.
The one remaining caller of this function is modules::rtv. Now
that rtv is responsible for creating the settings beforehand, and
the settings may be created with a path prefix, rtv gets its
til_module_context_t->setup.path prefixed for all the channel
settings.
Another improvement is now the channel settings instance gets
created from the module name as the settings string. So while
it's not yet possible to sparsely specify settings with others
being randomized, at least now when log_channels=on is in effect,
the printed args include the top-level channel module.
Having proper complete paths for the rtv channel modules is
especially visible in --print-paths output FYI.
An interesting test for exercising all this is:
```
$ src/rototiller --module=rtv,duration=0,context_duration=0,snow_module=none,channels=all,log_channels=on --print-pipes --defaults --go 2>/tmp/channels
in another terminal:
$ tail -F /tmp/channels
```
watch the chaos unfold
|
|
Mechanical rename just to make this consistent with
til_module_setup()/til_module_setup_finalize()
I should probably do a cleanup pass throughout the til APIs to
standardize on a subject-verb-object or subject-object-verb
order... Things have become a little inconsistent organically
over time
|
|
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 replaces the few ad-hoc til_module_t.setup() setup-baking
callers with the new til_module_setup_finalize() which always
produces a til_setup_t having an appropriate path, even when
there is no til_module_t.setup() method.
|
|
This commit adds passing the settings instance to til_setup_new()
which is used for deriving a path for the setup via
til_settings_print_path() on the supplied settings.
That path gets an allocated copy left in the returned
til_setup_t at til_setup_t.path
This path will exist for the lifetime of the til_setup_t, to be
freed along with the rest of the baked setup instance when the
refcount reaches 0.
The incoming til_settings_t is only read @ til_setup_new() in
constructing the path, no reference is kept. Basically the
til_settings_t* is just passed in for convenience reasons, since
constructing the path needs memory and may fail, this approach
lets the existing til_setup_new() call error handling also
capture the path allocation failures as-is turning
til_setup_new() into a bit more of a convenience helper.
Note that now all code may assume a til_setup_t has a set and
valid til_setup_t.path, which should be useful for context
creates when a setup is available.
|
|
Commit 7c8086020 switched the til_setup_new() api to support NULL
free_func for free().
This mechanical change pivots to that instead of the awkwardly
cast free() parameters.
|
|
Like modules/checkers required for fill_module, we need to do the
same for for rtv::snow_module.
There's more work to do on rtv::channels, but that's still
unsettled stuff in terms of settings syntax since rtv randomizes
settings. It's desirable to have the rtv settings able to
specify which settings to hold constant at a specific value
per-channel, leaving everything else for randomizing on channel
switch.
But there's no syntax for that kind of stuff currently, and it
seems like there's a need to communicate during the setup_func
dance when we're in a "settings optional because we'll fill them
in automatically at time of use later" to the front-end.
It's not strictly a front-end issue though - because the back-end
setup_func actually controls the forward progress. From the
current setup_func's perspective, everything's important to it
and must be fulfilled. And we certainly want the setup_func to
continue informing the setup process.
So it's more like the channel settings being populated via rtv
still need to all get populated, rtv just needs a way to add an
attribute to mark which settings are static vs. which should get
randomized on every use.
Perhaps there should just be a special value syntax reserved for
saying "random value" and the front-end can apply that, but then
a til_module_randomize_setup() could detect that too in a
per-setting flag the front-end set. That way the value gets
re-randomized, while the ones without that value set get left
alone.
Yes, I know this isn't the appropriate place for such commentary.
But nobody is reading these things anyways on my toy side project.
|
|
This is harmless as long as rtv stays hermetic.
But if rtv gets used in a composite scenario in the future by
either removing the hermetic flag, or allowing forced overrides,
n_cpus=0 will cause the nested module contexts to become threaded
on SMP machines.
That's problematic if the outer module context is already a
threaded render. What's appropriate here is to just propagate
the n_cpus down so if an upper layer has already gone threaded,
it will be sending down n_cpus=1 to serialize the nested
instances.
In practice, as-is, this change basically changes nothing, but
prepares for a potential future where rtv participates in
threaded compositions.
Through a lens of "rtv just rejiggers scenes and there settings
on a timer from a settings-specified subset of modules and
settings" it's arguably useful as just another module. Sometimes
you want something to change itself up periodically in say a
compose layer.
So preparing for this possibility isn't really all that
far-fetched/hypothetical.
|
|
Using on/off for boolean settings is the established convention
in rototiller, rtv went rogue here.
Just make it consistent
Sometimes it feels like this should be more flexible and support
0/1 yes/no true/false on/off by just having "boolean" typed
settings. But I think it may actually pay off long-term to be so
opinionated here and making serialized settings heirarchies
directly hashable/comparable without major normalization steps
(nothing beyond say... case normalization).
|
|
As-is it's not great for rtv to randomly wind up in compositions,
see comment in commit for more context.
|
|
first step towards settings-izing rtv, channels[] remains
|
|
This just does the obvious pulling in of til_setup_t, holding the
reference throughout the lifetime of the module context.
|
|
There was a time when it made sense for context creates needing
setups but not receiving them to still be functional with some
sane defaults.
But with recursive settings, we really shouldn't ever have
orphaned nested module uses unreachable by a proper setup.
So let's just get rid of this fallback, and exclusively rely on
the baked setups provided by the .setup() methods. They still
have preferred defaults, and the proper setup production
machinery is what should be responsible for applying those
at runtime where they may also be overridden or otherwise
influenced.
|
|
This introduces a boolean style log_channels= setting for
enabling logging of channel settings on channel switch.
It might be nice to change this to accept stdout/stderr/fdnum as
the setting instead of always directing at stderr.
This also doesn't capture the seed state so it's not exactly
logging everything needed to reproduce wholly what is being
shown. Some compositions depend more on rand than others, so
it matters at varying degrees.
It'd be nice for settings syntax to have some global syntax
supported where a seed can always be embedded to be loaded.
Introducing such things as global settings to the settings syntax
is a pending TODO item... right now the only way to load seed
state is at startup passed to main as --seed=. That's not gonna
cut it long-term.
This is an easy big step in the right direction though. Trying
to make sense of what's on-screen from the truncated captions is
impossible. Even if the captions wrapped the settings, it would
be tricky to catch the settings without recording the output or
screenshotting.
This also immediately makes me wonder about the voting system for
rtv where we log settings of favorites... then roll those into
playlists.
|
|
For recursive settings the individual setting being described
needs to get added to a potentially different settings instance
than the one being operated on at the top of the current
setup_func phase.
The settings instance being passed around for a setup_func to
operate on is constified, mainly to try ensure modules don't
start directly mucking with the settings. They're supposed to
just describe what they want next and iterate back and forth,
with the front-end creating the settings from the returned descs
however is appropriate, eventually building up the settings to
completion.
But since it's the setup_func that decides which settings
instance is appropriate for containing the setting.. at some
point it must associate a settings instance with the desc it's
producing, one that is going to be necessarily written to.
So here I'm just turning the existing til_setting_desc_t to a
"spec", unchanged. And introducing a new til_setting_desc_t
embedding the spec, accompanied by a non-const til_settings_t*
"container".
Now what setup_funcs use to express settings are a spec,
otherwise identically to before. Instead of cloning a desc to
allocate it for returning to the front-end, the desc is created
from a spec with the target settings instance passed in.
This turns the desc step where we take a constified settings
instance and cast it into a non-const a more formal act of going
from spec->desc, binding the spec to a specific settings
instance. It will also serve to isolate that hacky cast to a
til_settings function, and all the accessors of
til_setting_desc_t needing to operate on the containing settings
instance can just do so.
As of this commit, the container pointer is just sitting in the
desc_t but isn't being made use of or even assigned yet. This is
just to minimize the amount of churn happening in this otherwise
mostly mechanical and sprawling commit.
There's also been some small changes surrounding the desc
generators and plumbing of the settings instance where there
previously wasn't any. It's unclear to me if desc generators
will stay desc generators or turn into spec generators. For now
those are mostly just used by the drm_fb stuff anyways, modules
haven't made use of them, so they can stay a little crufty
harmlessly for now.
|
|
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 allows explicit listing of such modules as channels, while
protecting the automagic/defaults scenario.
If there's a future --experimental flag or such added, then the
TIL_MODULE_EXPERIMENTAL check will have to become conditional on
it.
|
|
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).
|
|
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
|
|
Preparatory commit for enabling cloneable/swappable fragments
There's an outstanding issue with the til_fb_page_t submission,
see comments. Doesn't matter for now since cloning doesn't happen
yet, but will need to be addressed before they do.
|
|
These modules have been doing their work in prepare_frame(), but
aren't actually threaded modules and don't return a frame plan
from prepare_frame() nor do they provide a render_fragment() to
complement the prepare_frame().
The convention thus far has been that single-threaded modules
just provide a render_fragment and by not providing a
prepare_frame they will be executed serially.
These two modules break the contract in a sense by using
prepare_frame() without following up with render_fragment().
I'm not sure why it happened that way, maybe at one time
prepare_frame() had access to some things that render_fragment()
didn't.
In any case, just make these use render_fragment() like any other
simple non-threaded module would.
This was actually causing a crash when n_cpus=1 because
module_render_fragment() was assuming the prepare_frame() branch
would include a render_fragment(). It should probably be
asserting as such.
|
|
just fixing up some vestigial rand() invocations to use the seed
|
|
til_setting_desc_t.random() and til_module_randomize_setup() now
take seeds.
Note they are not taking a pointer to a shared seed, but instead
receive the seed by value.
If a caller wishes the seed to evolve on every invocation into
these functions, it should simply insert a rand_r(&seed) in
producing the supplied seed value.
Within a given randomizer, the seed evolves when appropriate.
But isolating the effects by default seems appropriate, so
callers can easily have determinism within their respective scope
regardless of how much nested random use occurs.
|
|
modules/checkers w/fill_module=$module requires a consistent
mapping of cpu to fragnum since it creates a per-cpu
til_module_context_t for the fill_module.
The existing implementation for threaded rendering maximizes
performance by letting *any* scheduled to run thread advance
fragnum atomically and render the acquired fragnum
indiscriminately. A side effect of this is any given frame, even
rendered by the same module, will have a random mapping of
cpus/threads to fragnums.
With this change, the simple til_module_t.prepare_frame() API of
returning a bare fragmenter function is changed to instead return
a "frame plan" in til_frame_plan_t. Right now til_frame_plan_t
just contains the same fragmenter as before, but also has a
.cpu_affinity member for setting if the frame requires a stable
relationship of cpu/thread to fragnum.
Setting .cpu_affinity should be avoided if unnecessary, and that
is the default if you don't mention .cpu_affinity at all when
initializing the plan in the ergonomic manner w/designated
initializers. This is because the way .cpu_affinity is
implemented will leave threads spinning while they poll for
*their* next fragnum using atomic intrinsics. There's probably
some room for improvement here, but this is good enough for now
to get things working and correct.
|
|
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.
|
|
- modules now allocate their contexts using
til_module_context_new() instead of [cm]alloc().
- modules simply embed til_module_context_t at the start of their
respective private context structs, if they do anything with
contexts
- modules that do nothing with contexts (lack a create_context()
method), will now *always* get a til_module_context_t supplied
to their other methods regardless of their create_context()
presence. So even if you don't have a create_context(), your
prepare_frame() and/or render_fragment() methods can still
access seed and n_cpus from within the til_module_context_t
passed in as context, *always*.
- modules that *do* have a create_context() method, implying they
have their own private context type, will have to cast the
til_module_context_t supplied to the other methods to their
private context type. By embedding the til_module_context_t at
the *start* of their private context struct, a simple cast is
all that's needed. If it's placed somewhere else, more
annoying container_of() style macros are needed - this is
strongly discouraged, just put it at the start of struct.
- til_module_create_context() now takes n_cpus, which may be set
to 0 for automatically assigning the number of threads in its
place. Any non-zero value is treated as an explicit n_cpus,
primarily intended for setting it to 1 for single-threaded
contexts necessary when embedded within an already-threaded
composite module.
- modules like montage which open-coded a single-threaded render
are now using the same til_module_render_fragment() as
everything else, since til_module_create_context() is accepting
n_cpus.
- til_module_create_context() now produces a real type, not void
*, that is til_module_context_t *. All the other module
context functions now operate on this type, and since
til_module_context_t.module tracks the module this context
relates to, those functions no longer require both the module
and context be passed in. This is especially helpful for
compositing modules which do a lot of module context creation
and destruction; the module handle is now only needed to create
the contexts. Everything else operating on that context only
needs the single context pointer, not module+context pairs,
which was unnecessarily annoying.
- if your module's context can be destroyed with a simple free(),
without any deeper knowledge or freeing of nested pointers, you
can now simply omit destroy_context() altogether. When
destroy_context() is missing, til_module_context_free() will
automatically use libc's free() on the pointer returned from
your create_context() (or on the pointer that was automatically
created if you omitted create_context() too, for the
bare til_module_context_t that got created on your behalf
anyways).
For the most part, these changes don't affect module creation.
In some ways this eases module creation by making it more
convenient access seed and n_cpus if you had no further
requirement for a context struct.
In other ways it's slightly annoying to have to do type-casts
when you're working with your own context type, since before it
was all void* and didn't require casts when assigning to your
typed context variables.
The elimination for requiring a destroy_context() method in
simple free() of private context scenarios removes some
boilerplate in simple cases.
I think it's a wash for module writers, or maybe a slight win for
the simple cases.
|
|
I don't think rototiller is an appropriate place for being so
uncooperative, if someone gets the case wrong anywhere just make
it work. We should avoid making different things so subtly
different that case alone is the distinction anyways, so I don't
see this creating any future namespace collision problems.
|
|
This is a mostly mechanical change of using rand_r() in place of
rand(), using the provided seed as the seed state.
There's some outstanding rand()s outside of create_context()
which should probably get switched over, with the seed being
stowed in the context struct. I didn't bother going deeper on
this at the moment in the interests of getting to sleep soon.
|
|
In the recent surge of ADD-style rtv+compose focused development,
a bunch of modules were changed to randomize initial states at
context_create() so they wouldn't be so repetitive.
But the way this was done in a way that made it impossible to
suppress the randomized initial state, which sometimes may be
desirable in compositions. Imagine for instance something like
the checkers module, rendering one module in the odd cells, and
another module into the even cells. Imagine if these modules are
actually the same, but if checkers used one seed for all the odd
cells and another seed for all the even cells. If the modules
used actually utilized the seed provided, checkers would be able
to differentiate the odd from even by seeding them differently
even when the modules are the same.
This commit is a step in that direction, but rototiller and all
the composite modules (rtv,compose,montage) are simply passing
rand() as the seeds. Also none of the modules have yet been
modified to actually make use of these seeds.
Subsequent commits will update modules to seed their
pseudo-randomized initial state from the seed value rather than
always calling things like rand() themselves.
|
|
|
|
There's more to cleanup in rtv destruction, but this is the major
one.
|
|
Also shortened the durations across the board, and defaulted snow
to "none".
With "compose" being another meta module, and the randomized
settings, this actually gives a more interesting grand tour of
everything possible.
|
|
It's no longer necessary to specify both snow_duration=0 AND
snow_module=none to disable snow, just specify snow_module=none.
A future commit should really make describing the "snow_duration"
setting contingent on snow_module != "none".
|
|
Just one case, modules/submit, was using 32x32 tiles and is now
using 64x64. I don't expect it to make any difference.
While here I fixed up the num_cpus/n_cpus naming inconsistencies,
normalizing on n_cpus.
|
|
The code was deliberately showing a single frame of snow when
snow_duration=0, since the durations are integral seconds ther
was no way to have a sub 1-second snow otherwise.
snow_module=none just means cleared for the snow_duration, not no
snow mechanism whatsoever. So the combination of 0 duration and
"none" was still flashing a single blank frame.
With this commit the combination of "none" and 0 snow_duration
prevents even a single frame of blank snow from being rendered.
|
|
cosmetic indentation fixup
|
|
Mechanical rename to something less ambiguous in a world with
til_settings_t, til_setting_t, and til_setup_t etc.
|
|
This should plug a bulk of the setup leaks. Some of the
free_funcs still need to be changed to bespoke ones in modules
that allocate nested things in their respective setup, so those
are still leaking the nested things which are usually just a
small strdup of some kind.
|
|
This brings something resembling an actual type to the private
objects returrned in *res_setup. Internally libtil/rototiller
wants this to be a til_setup_t, and it's up to the private users
of what's returned in *res_setup to embed this appropriately and
either use container_of() or casting when simply embedded at the
start to go between til_setup_t and their private containing
struct.
Everywhere *res_setup was previously allocated using calloc() is
now using til_setup_new() with a free_func, which til_setup_new()
will initialize appropriately. There's still some remaining work
to do with the supplied free_func in some modules, where free()
isn't quite appropriate.
Setup freeing isn't actually being performed yet, but this sets
the foundation for that to happen in a subsequent commit that
cleans up the setup leaks.
Many modules use a static default setup for when no setup has
been provided. In those cases, the free_func would be NULL,
which til_setup_new() refuses to do. When setup freeing actually
starts happening, it'll simply skip freeing when
til_setup_t.free_func is NULL.
|
|
This makes the arg return optional by using a res_arg pointer,
instead returning -ENOMEM when it would have returned NULL on
allocation failures.
This also makes it possible to detect when no setup was
performed, by returning 0 in such a case. Now returns 1 when
setup occurs and res pointers populated.
|
|
This commit pulls the setup randomizer out of rtv into libtil
proper, so other modules may make use of it.
Other than adding an assert no functional changes occurred.
It may make sense to split this into two functions; one which
takes a til_module_t as-is, and a lower-level bare setup function
callback based function that doesn't know about til_module_t the
former would call into. That way generic setup randomization can
occur (the same setup machinery is used in video contexts for
example) without necessarily having a til_module_t on hand.
|
|
Mechanical renaming of "zero" to "clear" throughout for this
context.
|
|
Now modules allocate and return an opaque setup pointer in
res_setup when they implement a setup method.
Defaults are utilized when ${module}_create_context() receives a
NULL setup. The default setup used in this case should match the
defaults/preferred values emitted by the module's setup method.
But performing setup should always be optional, so a NULL setup
provided to create_context() is to be expected.
No cleanup of these setup instances is currently performed, so
it's a small memory leak for now. Since these are opaque and may
contain nested references to other allocations, simply using
free() somewhere in the frontend is insufficient. There will
probably need to be something like a til_module_t.setup_free()
method added in the future which modules may assign libc's free()
to when appropriate, or their own more elaborate version.
Lifecycle for the settings is very simple; the setup method
returns an instance, the caller is expected to free it when no
longer needed (once free is implemented). The create_context
consumer of a given setup must make its own copy of the settings
if necessary, and may not keep a reference - it must assume the
setup will be freed immediately after create_context() returns.
This enables the ability to reuse a setup instance across
multiple create_context() calls if desired, one can imagine
something like running the same module with the same settings
multiple times across multiple displays for instance. If the
module has significant entropy the output will differ despite
being configured identically...
With this commit one may change settings for any of the modules
*while* the modules are actively rendering a given context, and
the settings should *not* be visible. They should only affect
the context they're supplied to.
|
