| Age | Commit message (Collapse) | Author |
|---|
|
More setup_func conversion to returning the failed setting on
errors during res_setup baking.
|
|
This needs a bit more work, but at least filtering the same-tick
renders avoids the many-movements-per-frame potential when used
as something like a checkers::fill_module
|
|
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.
|
|
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 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.
|
|
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.
|
|
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 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.
|
|
Wire up seed via til_module_context.seed in the obvious manner,
minimal change to the code, no functional difference besides
giving pixbounces instances an isolated random seed state.
|
|
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.
|
|
While testing a checkers change that fills cells using other
modules, ASAN kept tripping on pixbounce:
==147817==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fc78a31c10c at pc 0x55b30cd406e2 bp 0x7fc790afd0d0 sp 0x7fc790afd0c8
WRITE of size 4 at 0x7fc78a31c10c thread T2
#0 0x55b30cd406e1 in til_fb_fragment_put_pixel_unchecked pixbounce.c
#1 0x55b30cd3f8ae in pixbounce_render_fragment pixbounce.c
#2 0x55b30cd1dffb in module_render_fragment til.c
#3 0x55b30cd1d989 in til_module_render (/home/foo/src/rototiller/build/src/rototiller+0x134989)
#4 0x55b30cd22534 in checkers_render_fragment checkers.c
#5 0x55b30cd14681 in thread_func til_threads.c
#6 0x7fc792b3d5c1 in start_thread pthread_create.c
#7 0x7fc792bc2583 in __clone (/usr/lib/libc.so.6+0x112583)
0x7fc78a31c10c is located 2276 bytes to the right of 1228840-byte region [0x7fc78a1ef800,0x7fc78a31b828)
allocated by thread T0 here:
#0 0x55b30cccf219 in __interceptor_malloc (/home/foo/src/rototiller/build/src/rototiller+0xe6219)
#1 0x7fc792d0e528 (/usr/lib/libSDL2-2.0.so.0+0x39528)
Thread T2 created by T0 here:
#0 0x55b30cc3cfa8 in pthread_create (/home/foo/src/rototiller/build/src/rototiller+0x53fa8)
#1 0x55b30cd13fff in til_threads_create (/home/foo/src/rototiller/build/src/rototiller+0x12afff)
#2 0x55b30cd1d573 in til_init (/home/foo/src/rototiller/build/src/rototiller+0x134573)
#3 0x55b30cd08f6c in main (/home/foo/src/rototiller/build/src/rototiller+0x11ff6c)
#4 0x7fc792add30f in __libc_start_call_main libc-start.c
SUMMARY: AddressSanitizer: heap-buffer-overflow pixbounce.c in til_fb_fragment_put_pixel_unchecked
Shadow bytes around the buggy address:
0x0ff97145b7d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b7e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b7f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b810: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0ff97145b820: fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b830: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b840: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b850: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b860: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0ff97145b870: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==147817==ABORTING
---
Rather than spend time digging into pixbounce's arithmetic, just
using the checked variant for now.
|
|
- 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.
|
|
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Just adds TIL_FB_DRAW_FLAG_TEXTURABLE so callers can granularly
inhibit texturing if desired.
|
|
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.
|
|
|
|
|
|
|
|
|
|
In the interests of facilitating randomized automagic layered
compositing, tell the world when you're overlay-appropriate.
|
|
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.
|
|
Mechanical renaming of "zero" to "clear" throughout for this
context.
|
|
Just rely on til_init()'s srand() ensuring things are fresh.
|
|
This is a preparatory commit for cleaning up the existing sloppy
global-ish application of settings during the iterative _setup()
call sequences.
Due to how this has evolved from a very rudimentary thing
enjoying many assumptions about there ever only being a single
module instance being configured by the settings, there's a lot
of weirdness and inconsistency surrounding module setup WRT
changes being applied instantaneously to /all/ existing and
future context's renderings of a given module vs. requiring a new
context be created to realize changes.
This commit doesn't actually change any of that, but puts the
plumbing in place for the setup methods to allocate and
initialize a private struct encapsulating the parsed and
validated setup once the settings are complete. This opaque
setup pointer will then be provided to the associated
create_context() method as the setup pointer. Then the created
context can configure itself using the provided setup when
non-NULL, or simply use defaults when NULL.
A future commit will update the setup methods to allocate and
populate their respective setup structs, adding the structs as
needed, as well as updating their create_context() methods to
utilize those setups.
One consequence of these changes when fully realized will be that
every setting change will require a new context be created from
the changed settings for the change to be realized.
For settings appropriately manipulated at runtime the concept of
knobs was introduced but never finished. That will have to be
finished in the future to enable more immediate/interactive
changing of settings-like values appropriate for interactive
manipulation
|
|
Originally the thinking was that rototiller modules would become
dlopen()ed shared objects, and that it would make sense to let
them be licensed differently.
At this time only some modules I have written were gplv3, Phil's
modules are all gplv2, and I'm not inclined to pivot towards a
dlopen model.
So this commit drops the license field from til_module_t,
relicenses my v3 code to v2, and adds a gplv2 LICENSE file to the
source root dir. As of now rototiller+libtil and all its modules
are simply gplv2, and anything linking in libtil must use a gplv2
compatible license - the expectation is that you just use gplv2.
|
|
Largely mechanical rename of librototiller -> libtil, but
introducing a til_ prefix to all librototiller (now libtil)
functions and types where a rototiller prefix was absent.
This is just a step towards a more libized librototiller, and til
is just a nicer to type/read prefix than rototiller_.
|
|
This is a first approximation of separating the core modules and
threaded rendering from the cli-centric rototiller program and
its sdl+drm video backends.
Unfortunately this seemed to require switching over to libtool
archives (.la) to permit consolidating the per-lib and
per-module .a files into the librototiller.a and linking just
with librototiller.a to depend on the aggregate of
libs+modules+librototiller-glue in a simple fashion.
If an alternative to .la comes up I will switch over to it,
using libtool really slows down the build process.
Those are implementation/build system details though. What's
important in these changes is establishing something resembling a
librototiller API boundary, enabling creating alternative
frontends which vendor this tree as a submodule and link just to
librototiller.{la,a} for all the modules+threaded rendering of
them, while providing their own fb_ops_t for outputting into, and
their own settings applicators for driving the modules setup.
|
|
Minor cosmetic consistency fixup
|
|
Most modules find themselves wanting some kind of "t" value increasing
with time or frames rendered. It's common for them to create and
maintain this variable locally, incrementing it with every frame
rendered.
It may be interesting to introduce a global notion of ticks since
rototiller started, and have all modules derive their "t" value from
this instead of having their own private versions of it.
In future modules and general innovations it seems likely that playing
with time, like jumping it forwards and backwards to achieve some
visual effects, will be desirable. This isn't applicable to all
modules, but for many their entire visible state is derived from their
"t" value, making them entirely reversible.
This commit doesn't change any modules functionally, it only adds the
plumbing to pull a ticks value down to the modules from the core.
A ticks offset has also been introduced in preparation for supporting
dynamic shifting of the ticks value, though no API is added for doing
so yet.
It also seems likely an API will be needed for disabling the
time-based ticks advancement, with functions for explicitly setting
its value. If modules are created for incorporating external
sequencers and music coordination, they will almost certainly need to
manage the ticks value explicitly. When a sequencer jumps
forwards/backwards in the creative process, the module glue
responsible will need to keep ticks synchronized with the
sequencer/editor tool.
Before any of this can happen, we need ticks as a first-class core
thing shared by all modules.
Future commits will have to modify existing modules to use the ticks
appropriately, replacing their bespoke variants.
|
|
- fix bug for square fragments
- use create and destroy context hooks instead of globals
- refactor to allow fragment resizing
- handle extremely small fragments gracefully
|
|
Mostly mechanical change, though threads.c needed some jiggering to
make the logical cpu id available to the worker threads.
Now render_fragment() can easily addresss per-cpu data created by
create_context().
|
|
|
