- protection against negative numbers seems adequate
- a possible concern are handling of very large time spans
- definitively have to guard against "poisonous" fractions
(e.g. n / INT_MAX)
- some test definitions were simply numerically wrong
- changed some aspects of the specified behaviour, to be more consistent
+ scrolling is more liberal and always allows to extend canvas
+ setting window to a given duration expands around anchor point
This function allows to move the visible range such that it contains
a given time position; the relative location of this point within
the visible range however is in turn determined by relating it to
the current overall canvas: if we are close to the beginning, the
position is also located rather to the left side, and if we're
approaching the canvas end, the position tends to the right side...
(and yes, I am aware that the variant taking a rational number
can be derailed by causing internal numeric overflow, when passing
a maliciously crafted rational number, like INT_MAX-1 / INT_MAX )
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
Since conformWindowToMetric() is always called prior to performing
the complete invariant-reestablishment sequence, we can even integrate
the rule for relative scaling into this central function, which
simplifies the mutation implementation significantly. Should
relative positioning go south, the following sanity checks
will push the window back into bounds.
With these changes, the verify_simpleUsage() test passes!
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 solution is to provide a standard implementation in the form of a mix-in,
which directly houses a `ZoomWindow` instance. Moreover, the latter
is deemed a prominent use case for the time::Control, allowing other
components to attach and push changes of the zoom state or register
as listeners to react to state changes.
Actually, the `TimelineLayout`, which hosts all the actual visible
widgets forming the timeline-UI, now integrates this mix-in; and since
`TimelineLayout` is passed to `TimelineController` and used there as
reference-`CanvasHook` for the root track, this implementation of
the `DisplayMetric` interface will ''effectively be used by all
widgets'' attached to the timeline canvas.
Reading my work notes from two years ago, the concept can be validated.
Clarify the relation of the interfaces involved, and the role foreseen
for the upcoming `ZoomWindow` abstraction. This solution approach
will lead to multiple-stage indirect calls, which however are deemed
not to be overly concerning and will be investigated later, to avoid
premature optimisation (see #1254)
- `DisplayMetric` is a focused special purpose abstraction
- it belongs into the general abstraction of the `DisplayManager`
- it is rather linked by use to the other abstraction, the `CanvasHook`
- while the `RelativeCanvasHook` is not an interface, but an implementation mix-in
- and the actual `DisplayMetric` implementation can likewise be provided
as mix-in, since it will typically be implemented in terms of a `ZoomWindow`
Using this scheme, it will be possible to avoid some of the indirect cally
by making this mix-in visible higher up the call graph -- in case the
actual need for optimisation can be confirmed in practice.
According to plan, this was more or less a drop-in replacement.
However, this first integration prototype highlights some design problems
* `ElementBoxWidget` is designed ''constructor-centric''
* but the population by diff messages will supply crucial information later
* and seemingly the size-constraint code is now invoked prior to widget realisation \\
⟹ Assertion Failure
Basically we want to create ElementBoxWidgets according to a
preconfigured layout scheme, yet we'll need to pass some additional
qualifiers and optional features, and these need to be checked
and used in accordance with the chosen flavour...
Investigating a possible solution based on additional ctor parameters,
which are given as "algebraic terms", and actually wrap a functor
to manipulate a builder configuration record
The ClipPresenter can access the CanvasHook wired into its actual ClipDelegate (widget).
And this in turn exposes the DisplayMetric, with the ability to transform
presentation coordinates (pixels) into a model representation (Time)
The actual translation is still hardwired placeholder code,
since it is planned to build an generic component "ZoomWindow"
to provide all the typical zomming and view window translations
found in every timeline editor
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
...previously this was modelled as part of the CanvasHook abstraction,
and in fact it will in any case be implemented by delegation to the
TimelineLayout or some kind of display manager.
We need this to tanslate mouse pixel movements into a time change
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
After some in-depth analysis, it seems best to reattach the Clips and Marker
top-down through the control structure, rather than building some additional
magic callback into the CanvasHook. Thus the 3rd DisplayEvaluation pass
now not only has to rebalance track header and body, but also
reatach or move each attached widget within the body, using its
nominal coordinates. This should then pick up the changed
layout decoration size
...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
...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
The reason was: each further ViewRefHook added again the full offset.
Need to change the hierarchy and allow for this chained hooking already
starting from the base interface ViewHook onward (with trivial default impl)
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)
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 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.
...which serves to solve the problem with Canvas access.
Basically we do not want each and every Clip widget to be aware of the concrete canvas implementation widget;
and in addition, automated removal of widgets from the Canvas seems desirable
Note however, we will not plaster our UI code and CSS with mangled-out selectors
on each and every single element. This is what cascading was meant to be used for.
- most notably the NOBUG logging flags have been renamed now
- but for the configuration, I'll stick to "GUI" for now,
since "Stage" would be bewildering for an occasional user
- in a similar vein, most documentation continues to refer to the GUI
