Age | Commit message (Collapse) | Author |
|
More setup_func conversion to returning the failed setting on
errors during res_setup baking.
|
|
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.
|
|
Mechanical change switching til_fragmenter_slice_per_cpu() users
over to til_fragmenter_slice_per_cpu_x16(), except sparkler where
it's quite detrimental to performance.
|
|
I had benchmarked this change and it showed no difference at all
on my 2c/4c i7 X230.
But having just tried it on an RPi4B where it moved the test case
from 54FPS to 60FPS, a +10% improvement, it's worth the
readability loss.
It's interesting how Intel's cleverness discourages optimizing in
ways that benefit probably *all* the competition... even when the
optimization is such a minor change in terms of effort.
|
|
Major gain comes from eliminating the cosf() from the inner loop...
There's still a bunch left on the table for moire but even just
these changes turn 19FPS into 81FPS over here for:
'--module=compose,layers=moire\\\,centers\\\=2\,moire\\\,centers\\\=2\,moire\\\,centers\\\=2\,moire\\\,centers\\\=2,texture=none' '--video=mem,size=1366x768'
|
|
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.
|
|
With setup refcounting and a reference bound to the context, we
should just dereference the single instance. The way setups are
used it just as a read-only thing to affect context behavior...
Note I've left the module-type-specific setup pointer despite it
duplicating the setup pointer in the module_context. This is
just a convenience thing so the accessors don't have to cast the
general til_setup_t* to my_module_setup_t* everywhere.
|
|
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.
|
|
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.
|
|
This introduces a very naive unoptimized moire interference
pattern module, it's rather slow complete with a sqrtf() per
pixel per center.
|