Rearrange the internal mutator functions to follow a common scheme,
so that most of the setters can be implemented by simple forwarding.
Move the change-listener triggering up into the actual setters.
This makes further test cases pass
- verify_setup
- verify_calibration
...implying that the pixel width is now retained
and basic behaviour matches expectations
Extensive tests with corner cases soon highlighted this problem
inherent to integer calculations with fractional numbers: it is
possible to derail the calculation by numeric overflow with values
not excessively large, but using large numbers as denominator.
This problem is typically triggered by addition and subtraction,
where you'd naively not expect any problems.
Thus changed the approach in the normalisation function, relying
on an explicitly coded test rather, and performing the adjustment
only after conversion back to simple integral micro-tick scale.
Getting all those requirements translated into code turns out to be a challenging task;
and the usual ascent to handle such a situation is to define **Invariants**
in conjunction with a normalisation scheme; each manipulation will then be
translated into invocation of one of the three fundamental mutators,
and these in turn always lead into the common normalisation sequence.
__Invariants__
- oriented and non-empty windows
- never alter given pxWidth
- zoom metric factor < max zoom
- visibleWindow ⊂ Canvas
Writing this specification unveiled a limitation of our internal
time base implementation, which is a 64bit microsecond grid.
As it turns out, any grid based time representation will always
be not precise enough to handle some relevant time specifications,
which are defined by a divisor. Most notably this affects the precise
display of frame duration in the GUI, and even more relevant,
the sample accurate editing of sound in the timeline.
Thus I decided to perform the internal computation in ZoomWindow
as rational numbers, based on boost::rational
Note: implementation stubbed only, test fails
This ZoomWindow_test highlights again the question about the intended usage
of the Lumiera time entities. In which way do we want to perform time calculations,
and under which circumstances is it adequate to perform arithmetic on
raw time values?
These questions made me think about rather far reaching concerns regarding
subsidiarity and implicit or explicit usage context. Basically I could
reconfirm the design choices taken some years ago -- while I must admit
that the project is headed towards a way larger scale and more loose
coupling of the parts, than I could imagine several years ago, at the
time when the design started...
As a side note: we can not avoid that some knowledge about the time implementation
leaks out from the support lib; time codes themselves are tightly coupled
to the usage scenario within the session and can not be used as means
for implementing UI concerns. And the more generic time frameworks,
like std::chrono (as much as it is desirable to have some integration here)
will not be of any help for most of our specific usage patterns.
The reason is, for film editing we do not have a global time scale,
rather the truth is when the film starts....
implement the first test case: nudge the zoom factor
⟹ scale factor doubled
⟹ visible window reduced to half size
⟹ visible window placed in the middle of the overall range
The header "format-cout.hpp" offers a convenience function
to print pretty much any object or data in human readable form.
However, the formatter for pointers used within this framework
switched std::cout into hexadecimal display of numbers and failed
to clean-up this state.
Since the "stickyness" of IOS stream manipulators is generally a problem,
we now provide a RAII helper to capture the previous stream state and
automatically restore it when leaving the scope.
Complete the investigation and turn the solution into a generic
mix-in-template, which can be used in flexible ways to support
this qualifier notation.
Moreover, recapitulate requirements for the ElementBoxWidget
- move construct into the buffer
- directly invoke the payload constructor through PlantingHandle
- reconsider type signature and size constraint
- extend the unit test
- document a corner case of c++ "perfect forwarding",
which caused me some grief here
...this extension was spurred by the previeous refactoring.
Since 'emplace' now clearly denotes an operation to move-embed an existing object,
we could as well offer a separate 'create' API, which would take forwarding
arguments as usual and just delegates to the placement-new operation 'create'
already available in the InPlaceBuffer class.
Such would be a convenience shortcut and is not strictly necessary,
since move-construction is typically optimised away; yet it would also
allow to support strictly non-copyable payload types.
This refactoring also highlights a fuzziness in the existing design,
where we just passed the interface type, while being sloppy about the
DEFAULT type. In fact this *is* relevant, since any kind of construction
might fail, necessitating to default-construct a placeholder, since
InPlaceBuffer was intended for zero-overhead usage and thus has in itself
no means to know about the state of its buffer's contents. Thus the
only sane contract is that there is always a valid object emplaced
into the buffer, which in turn forces us to provide a loophole for
class hierarchies with an abstract base class -- in such a case the
user has to provide a fallback type explicitly.
...for the operation on a PlantingHandle, which allows
to implant a sub type instance into the opaque buffer.
* "create" should be used for a constructor invocation
* "emplace" takes an existing object and move-constructs
this allows to avoid multi-step indirection
when translating mouse dragging pixel coordinates
into a time offset for the dragged clip widget.
Moreover this also improves the design,
since the handling of canvas metric is pretty much
a self contained, separate concern
some bugfixes,
but also a notable change: detect the completion of the gesture
directly when the button is released; this is necessary, because
seemingly we do not get motion_events when no button is pressed,
at least not in this test setup based on a Gtk::Button widget.
GTK doesn't expose a first-class API for this,
since -- by design -- the extension of a widget is negotiated.
Thus I'm looking for some kind of workaround for our specific use-case,
where a clip widget must be rendered with a well defined horizontal size,
corresponding to its length.
Thus far, we're only able to increase the size of the Button widget
used as placeholder, but we can not forcibly shrink that button,
probably because the embedded Gtk::Lable requires additional extension.
Partially as a leftover from the way more ambitious initial design,
we ended up with CanvasHook as an elaboration/specialisation of the
ViewHook abstraction. However, as it stands, this design is tilted,
since CanvasHook is not just an elaboration, but rather a variation
of the same basic idea.
And this is now more like a building pattern and less of a generic
framework, it seems adequate to separate these two variations completely,
even if this incurs a small amount of code duplication.
Actually this refactoring is necessary to resolve a bug, where
we ended up with the same Clip widgets attached two times to the
same Canvas control, one time through the ViewHook baseclass,
and a second time by the ctor of the "derived" CanvasHook
...in an attempt to clarify why numerous cross links are not generated.
In the end, this attempt was not very successful, yet I could find some breadcrumbs...
- file comments generally seem to have a problem with auto link generation;
only fully qualified names seem to work reliably
- cross links to entities within a namespace do not work,
if the corresponding namespace is not documented in Doxygen
- documentation for entities within anonymous namespaces
must be explicitly enabled. Of course this makes only sense
for detailed documentation (but we do generate detailed
documentation here, including implementation notes)
- and the notorious problem: each file needs a valid @file comment
- the hierarchy of Markdown headings must be consistent within each
documentation section. This entails also to individual documented
entities. Basically, there must be a level-one heading (prefix "#"),
otherwise all headings will just disappear...
- sometimes the doc/devel/doxygen-warnings.txt gives further clues
...by relying on the newly implemented automatic standard binding
Looks like a significant improvement for me, now the actual bindings
only details aspects, which are related to the target, and no longer
such technicalitis like how to place a Child-Mutator into a buffer handle
After this long break during the "Covid Year 2020",
I pick this clean-up task as a means to fresh up my knowledge about the code base
The point to note is, when looking at all the existing diff bindings,
seemingly there is a lot of redundancy on some technical details,
which do not cary much meaining or relevance at the usage site:
- the most prominent case is binding to a collection of DiffMutables hold by smart-ptr
- all these objects expose an object identity (getID() function), which can be used as »Matcher«
- and all these objects can just delegate to the child's buildMutator() function
for entering a recursive mutation.
...and the result was very much worth the effort,
leading to more focused and cleaner code.
- all the concerns of moving widgets and translating coordinates
are now confined to the second abstraction layer (CanvasHook)
- while the ViewHook now deals exclusively with attachment, detachment
and reordering of attachment sequence
As it turned out, it is rather easy to extend the existing listener
for structural changes to detect also value assignments. Actually
it seems we'd need both flavours, so be it.
Yeah, C++17, finally!
...not totally sure if we want to go that route.
However, the noise reduction in terms of code size at call site looks compelling
this draft commit reshifts the (meanwhile broken) test code from:
03c358fe86
Now the marker Buttons are injected again, but without any detailed
positioning code at call site. This demonstrates the viability of the
Structure-Change / ViewHook refactoring.
To make this change viable, it was necessary to remove the ViewHooked<>
marker template from the rehook() callback. As it turns out, this was
added rather for logical reasons, and is in fact not necessary in
any of the existing ViewHook implementations (and I don't expect any
other implementations to come)
BUT the actual positioning coordinates are still wrong (which seems
to re related to other conceptual problems in coordinate offset handling)
the reason for the failure, as it turned out,
is that 'noexcept' is part of the function signature since C++17
And, since typically a STL container has const and non-const variants
of the begin() and end() function, the match to a member function pointer
became ambuguous, when probing with a signature without 'noexcept'
However, we deliberately want to support "any STL container like" types,
and this IMHO should include types with a possibly throwing iterator.
The rationale is, sometimes we want to expose some element *generator*
behind a container-like interface.
At this point I did an investigation if we can emulate something
in the way of a Concept -- i.e. rather than checking for the presence
of some functions on the interface, better try to cover the necessary
behaviour, like in a type class.
Unfortunately, while doable, this turns out to become quite technical;
and this highlights why the C++20 concepts are such an important addition
to the language.
So for the time being, we'll amend the existing solution
and look ahead to C++20
as it turns out, "almost" the whole codebase compiles in C++17 mode.
with the exception of two metaprogramming-related problems:
- our "duck detector" for STL containers does not trigger anymore
- the Metafunction to dissect Function sigantures (meta::_Fun) flounders
"%broken" is not broken anymore, but renders a boolean,
and we configured the formatter not to complain on missing values.
Fortunately "%madness" is still broken ;-)
When drafting the time handling framework some years ago,
I foresaw the possible danger of mixing up numbers relating
to fractional seconds, with other plain numbers intended as
frame counts or as micro ticks. Thus I deliberately picked
an incompatible integer type for FSecs = boost::rational<long>
However, using long is problematic in itself, since its actual
bit length is not fixed, and especially on 32bit platforms long
is quite surprisingly defined to be the same as int.
However, meanwhile, using the new C++ features, I have blocked
pretty much any possible implicit conversion path, requiring
explicit conversions in the relevant ctor invocations. So,
after weighting in the alternatives, FSecs is now defined
as boost::rational<int64_t>.
basically the solution was a bit too naive and assumed everything is similar to a vector.
It is not, and this leads to some insidious problems with std::map, which hereby
are resolved by introducing ContainerTraits
All of the existing "simple" tests for the »Diff Framework« are way to much low-level;
they might indeed be elementary, but not introductory and simple to grasp.
We need a very simplistic example to show off the idea of mutation by diff,
and this simple example can then be used to build further usage test cases.
My actual goal for #1206 to have such a very basic usage demonstration and then
to attach a listener to this setup, and verify it is actually triggered.
PS: the name "GenNodeBasic_test" is somewhat pathetic, this test covers a lot
of ground and is anything but "basic". GenNode in fact became a widely used
fundamental data structure within Lumiera, and -- admittedly -- the existing
implementation might be somewhat simplistic, while the whole concept as such
is demanding, and we should accept that as the state of affairs
now the lifecycle of widget and hook are tightly interwoven.
Indeed the test uncovered a situation where a call into the
already destroyed Canvas might halt the application.
...basically it occurred to me that in practice we will never have to deal
with isolated ViewHooks, rather with widgets-combinded-with-a-hook.
So the idea is to combine both into a template ViewHooked<W>
basically this attempts to work around an "impedance mismatch" caused by relying on Lumiera's Diff framework.
Applying a diff might alter the structural order of components, without those componets
being aware of the change. If especially those components are attached into some
UI layout, or otherwise delegate to display widgets, we need a dedicated mechanism
to reestablish those display elements in proper order after applying the change.
The typical examples is a sequence of sub-Tracks, which might have been reordert due
to applying rules down in the Steam Layer. The resulting diff will propagate the
new order of sub-Tracks up into the UI, yet now all of the elaborate layout and
space allocation done in the presentation code needs to be adjusted or even
recomputed to accomodate the change.
