...the improved const correctness on STL iterators uncovered another
latent problem with out diagnositc format helper, which provide
consistently rounded float and double output, but failed to take
CV-qualifiaction into account
This is a subtle and far reaching fix, which hopefully removes
a roadblock regarding a Dispatcher pipeline: Our type rebinding
template used to pick up nested type definitions, especially
'value_type' and 'reference' from iterators and containers,
took an overly simplistic approach, which was then fixed
at various places driven by individual problems.
Now:
- value_type is conceptually the "thing" exposed by the iterator
- and pointers are treated as simple values, and no longer linked
to their pointee type; rather we handle the twist regarding
STL const_iterator direcly (it defines a non const value_type,
which is sensible from the STL point of view, but breaks our
generic iterator wrapping mechanism)
...in an attempt to resolve the deeply nested problems encountered
while building an iterator pipeline for the Dispatcher. It seems
that I was sloppy some years ago and just "bashed them into submission",
thereby mixing up two different meanings of "value_type"
Moreover I seemingly implemented the same helper trait template twice,
so the first step is to switch all usages to meta::TypeBinding
To complete the mock setup, the next step would be to extend the GenNode-based spec langage
to allow defining prerequisite Mock-JobTickets. Setting this up seems rather straight forward --
however, defining a simple testcase to cover this extension runs into surprisingly tricky problems..
- for one, the singleValIterator from Itertools has serious difficulties handling references
- but even more surprising, it seems impossible to make the "prerequisites iterator"
fit into the Tree-Explorer framework (which I intend to use as replacement
for the monadic approach)
after some extended analysis of generic types and template instances,
it seems that not TreeExplorer as such is the primary problem, but rather
there is a conceptual mismatch somewhere deep down in Itertools or Iter-Adapter
By reasoning and analysis I conclude that the differentiation into
multiple channels is likely misplaced in JobTicket; it belongs ratther
into the Segment and should provide a suitable JobTicket for each ModelPort
Handling of prerequisites also needs to be reshaped entirely after
switching to a pipeline builder for the Job-planning pipeline; as
preliminary access point, just add an iterator over the immediate
prerequisites, thereby shifting the exploration mechanism entirely
out of the JobTicket implementation
Testcase: A simple Sementation with a single and bounded Segment
As aside, figured out how to unpack an iterator such as to
tie a fixed number of references through a structural binding:
auto const& [s1,s2,s3] = seqTuple<3> (mockSegs.eachSeg());
...now able to build a mock segmentation which issues dummy jobs,
and is wired such as to verify the right job is invoked for each segment.
And this allows to build and verify the Dispatcher,
without being able to invoke actual render jobs yet.
- only the parts actually touched by the algo will be re-allocated
- when a segment is split, the clone copies carry on all data
Library: add function to check for a bare address (without type info)
This macro has turned out to be quite useful in cases
where a generic setup / algorithm / builder need to be customised
with λ adaptors for binding to local or custom types. It relies
on the metafunctions defined in lib/meta/function.hpp to match
the signature of "anything function-like"; so this seems the
proper place to provide that macro alongside
...this is something I should have done since YEARS, really...
Whenever working with symbolically represented data, tests
typically involve checking *hundreds* of expected results,
and thus it can be really hard to find out where the
failure actually happens; it is better for readability
to have the expected result string immediately in the
test code; now this expected result can be marked
with a user-defined literal, and then on mismatch
the expected and the real value will be printed.
The algorithm coded thus far turns out to be rather generic,
and thus it can be rewritten into a template, while all specific parts
are supplied as λ-Functors.
- instead of Time we use a generic ordering type
- the Iterator is likewise turned into a template parameter
- all the operations are directly supplied as functor types
- C++17 is able to pick up all those λ-Types from the ctor call
This change looks like "low hanging fruit"; the legibility of the code
is not seriously hampered, yet we get the benefit to test this rather
technical piece of logic by an isolated test (which for now is the
primary motivation), and we can hope to re-use it for similar tasks.
There are 12 distinct cases regarding the orientation of two intervals;
The Segmentation::splitSplice() operation shall insert a new Segment
and adjust / truncate / expand / split / delete existing segments
such as to retain the *Invariant* (seamless segmentation covering
the complete time axis)
right now we're lacking a complete working implementation of render node invocation,
and thus the Dispatcher implementation can only be verified with the help
of mocked jobs. However, at least a preliminary implementation of tagging the
invocation instance is available, and thus we're able to verify that
a given job instance indeed belongs to and is "backed" by a specific JobTicket.
This is prerequisite for building up a (likewise mocked) Fixture datastructure,
and this in turn was meant to form the basis for attacking an actual Scheduler
implementation, followed by a real render node invocation.
- can now create a Job from JobTicket::NIL
- on invocation this Job will to nothing
Only when the first real output backend is implemented,
we can decide if this simplistic implementation is enough,
or if an empty output must be explicitly generated...
* using a simplified preliminary implementation of hash chaining (see #1293)
* simplistic implementation of hashing for time values (half-rotation)
* for now just hashing the time into the upper part of the LUID
Maybe we can even live with that implementation for some time,
depending on how important uniform distribution of hash values is
for proper usage of the frame cache.
Needless to say, various further fine points need more consideration,
especially questions of portability (32bit anyone?). Moreover, since
frame times are typically quantised, the search space for the hashed
time values is drastically reduced; conceivably we should rather
research and implement a good hash function for 128bit and then combine
all information into a single hash key....
...using the MockJobTicket setup as point of reference,
since the actual invocation of render nodes will only be drafted
later in this "Vertical Slice" integration effort...
- introduce a JobTicket::NOP (null-object pattern)
- assuming that the function splitSplice() will retain complete coverage allways
Remark:
`Fixture::getPlaylistForRender()` is a leftover from the very early implementation drafts.
This function was more or less based on the way Cinelerra works; it is clear by now
that Lumiera can not possibly work this way, given that we'll build a low-level model
and dispatch precompiled render jobs....
The Fixture and the low-level model backbone deserve a distinct namespace on their own.
Since it's built by the Builder from the Session contents, and also used by the frame dispatch,
we can expect dependence on some types from Steam-Layer, and thus this namespace
needs to reside in Steam-Layer rather, while the actual low-level Model
might become part of Vault-Layer, creating a hierarchy of data structures.
(Remark: likely also the session related namespaces will need a reorganisation)
The idea is to escape a "design deadlock" by using a test-driven prototype
implementation of the data structure to back a further development
of the Dispatcher and Scheduler implementation, which then can be used
to gradually elaborate and switch over to an actual implementation
data structure
...requires a first attempt towards defining a `JobTiket`.
This turns out quite tricky, due to using those `LinkedElements`
(intrusive single linked list), which requires all added records
actually to live elsewhere. Since we want to use a custom allocator
later (the `AllocationCluster`), this boils down to allocating those
records only when about to construct the `JobTicket` itself.
What makes matters even worse: at the moment we use a separate spec
per Media channel (maybe these specs can be collapsed later non).
And thus we need to pass a collection -- or better an iterator
with raw specs, which in turn must reveal yet another nested
sequence for the prerequisite `JobTickets`.
Anyhow, now we're able at least to create an empty `JobTicket`,
backed by a dummy `JobFunctor`....
Looks like we'll actually retain and use this low-level solution
in cases where we just can not afford heap allocations but need
to keep polymorphic objects close to one another in memory.
Since single linked lists are filled by prepending, it is rather
common to need the reversed order of elements for traversal,
which can be achieved in linear time.
And while we're here, we can modernise the templated emplacement functions
- build the reworked Job-planning pipeline more or less from scratch
- back that with mocked `Dispatcher` and `JobTicket`
- then transfer this into a `RenderDrive`, which can be tested as well
- could continue then to a `CalcStream` integration test....
- introduce a new entity: RenderDrive
- it supersedes the CalcPlanCalculation, but is managed by CalcStream
- moreover, the RenderDrive will house a IterTreeExplorer-Pipeline
- define the concerns and relationships more clearly (see Drawing)
- prerequisite to disentangle the Job-planning "mechanics"
- decision: the Monad-style iteration framework will be abandoned
- the job-planning will be recast in terms of the iter-tree-explorer
- job-planning and frame dispatch will be disentangled
- the Scheduler will deliberately offer a high-level interface
- on this high-level, Scheduler will support dependency management
- the low-level implementation of the Scheduler will be based on Activity verbs
This finishes a long lasting effort to rework the top-level of the Lumiera GTK UI,
to adapt to GTK-3 and the new asynchronous message based architecture.
Special credits and thanks to
* Joel Holdsworth
* Stefan Kangas
Without their relentless foundational work, the Lumiera UI could
never be where it is now. Even if some code was rewritten and several
parts of the old GTK-2 implementation are now obsolete, numerous ideas
solutions and inspirations were drawn from those early contributions
and live on as part of the reworked GUI.
for sake of developement of the timeline body drawing code,
several tweaks were added to make the impact of the styling stand
out clearly. This changeset removes all those tweaks and restors
the code to intended neutral behaviour
Moreover, the cursom drawing of the timeline now requires some
basic aids to be present in the stylesheet, otherwise the track structure
will not be visible. Thus add some minimalistic styling to the
"light-theme-complement"-stylesheet, mostly based on the usual
predefined theme colours and some box-shadow settings.
This is by no means an adequate graphical solution,
yet it should be enough to get on with coding....
check private notes and mindmap and fix some remaining minor inconsistencies,
notably the calculations in the overlay renderer in the `BodyCanvasWidget`.
Not sure if the initial window width is used properly for calibration
of ZoomWindow "pixel width" base setting.
Follow-up to the layout logic established with this commit:
09714cfe28
An extended round of rebuilding and reworking the global UI structures
can be concluded now. A flexible recursive structure of Tracks has
been implemented for the new Timeline-UI, allowing to contro all
relevant aspects of structure composition by **Diff Messages** sent
up from the Steam-Layer into the UI.
Moreover, the ability to control the custom drawing code through regular
**CSS style rules** has been demonstrated, allowing for seamless integration
of Lumiera UI elements with the existing desktop theme.
This completes the initial implementation round for the TrackHead.
- arrangement and layout for nested sub-Tracks is now settled
- a graphical representation of scope nesting was implemented
Postponed for later...
- still some minor discrepancies on synchronisation of vertical space
between TrackHead and custom drawing in the body (off-by-one?)
- Expanding / Collapsing of Tracks
- Implement actual Controls to influence the Scope, e.g. Volume, Mix-Mode
- Dynamically indicate selection and Muting on the structure display
While by default the Cairo Context is scaled to device units,
we must not assume that the given Context is unscaled; rather
it might have been deliberately scaled...
A notable example is the Gtk Inspector, which offers a "Magnifier" feature
- pick up all relevant values from CSS
- also control the width of the StaveBracket
- observe the given overall height
Moreover, complete documentation drawing in Inkscape
and add a page to the TiddlyWiki, describing the principles
underlying this design and construction.
.timeline__head : The complete header container on the left side
.timeline__navi : navigation control at top
.timeline__pbay : container holding the patchbay on the left side
.fork__head : each individual TrackHeadWidget (possibly nested)
.fork__control : container for the control components for each track scope
.fork__bracket : the StaveBracket drawing to indicate the nesting structure
The tricky part is to derive the anchor point for the upper
and the lower cap of the bracket, taking into account possible padding.
There seems to be a bug hidden somewhere in this logic, since the
line turns out too short at the lower end....?
According to the documentation, we should be able to get a Pango
font description from the CSS style context, and from there we should
be able to retrieve a font-size specification (and a DPI for the display)
Running this experimental code yields a font-size value of 9pt,
leading to a scale factor of about ~6, which seems plausible.
after weighting in the pro, and cons,
I decided to follow the standard path and pick up values
for each StaveBracket instance individually from Gtk::StyleContext,
assuming that the GTK framework will care for caching and performance
...as it turned out, it suffices just to state desired behaviour explicitly
- make the StaveBracket expand=false
- declare the HeadControl expand=true
The reason lies in the fact that on leaf-tracks there are just two
adjacent cells in a single row, lacking any exterior source of layout guidance.
...just drawing a marker cross for now to indicate allocated size;
speaking of size -- GTK sometimes expands allocation horizontally,
while we'd prefer an absolutely fixed size for the purpose at hand.
Intention is to shift focus of development work down towards Player and Scheduler soon.
However, since the timeline display saw substantial improvement it seems prudent
to finish work on some open ends, notably
- the track head structure
- the drawing and styling code
While content rendering for Clip widgets can safely be postponed, regarding the TrackHeadWidget,
where custom drawing is planned to display a structural outline of the nested scopes, some
ground work should be completed to make those plannings explicit.
This both demonstrates the layout of the `TrackHeadWidget`
and puts `ElementBoxWidget` into intended use to indicate
the scope of a track and to provide a placement icon and
an expander/collapser button.
see #1018
see #1219
Layout calculation and balancing between body and track header
now works reasonably well, labels are placed properly and the
calculated layout remains stable when changing window size,
connected panes and scrollbars behave as expected now.
Quite a feat!
...to properly account for
- the "prelude" padding above the root track
- overview rulers located into a fixed separate canvas at top
- slope-down and slope-up borders around nested tracks.
After testing, results seem now to be accurate up to ± 1px
Finally (sigh)
As it turns out, we need to take the actual allocation of the cells
within the grid explicitly into account: combined cells will report
their extension for each of the underlying cells, thus leading to
excessively overestimated measurements.
So we now calculate the overall height based on the actual structure
- first row holds the label
- left column below used as expander
- right column holds individual content
Remaining problems:
- height of ruler canvas at top not taken into account (11px in this example)
- start of sub-track headers not synchronised with start of sub-tracks
in the body area
...seemingly the allocation of grid cells in the `TrackHeadWidget`
is not quite correct yet: even when there are nested sub-tracks,
we always need another row to hold the controls corresponding to
the track itself and the whole scope. And this row is also what
should be adjusted to match the vertical extension of the content
area.
As it turns out, the whole topic how to handle collapsed tracks
was not even considered yet; the calculation of the "track profile"
would need to be reworked to accommodate collapsed tracks, see: #1265
Sometimes, fractional seconds in the ZoomWindow metric can build up
to numerator and denominator values in the 64-bit integer domain.
Thus the division and truncation of the Window pixel with value
must be done in int64_t, while the result value is then
guaranteed to be a small integer < 100000
This caused the canvas to flicker and jump in size and the
resulting scrollbar change caused various cascading effects
on the further layout calculations...
Note: the actual root cause, why this re-entrance happens,
is due to another obvious numerics bug not yet solved.
Here, the canvas width was suddenly set to zero, causing
the scrollbar position to change and thus the ZoomWindow
to re-fire the structure change signal.
However, such invalidation of previously established baseline
values can never be totally excluded in advanced layout calculations,
and thus the evaluation mechanism must be prepared and re-triggered
to start over, until a stable layout is achieved.
- rearrange cell content and disable auto-expand to prevent
the content area from becoming oversized initially
- fix autocompletion error in signal binding,
causing segfault when moving the scrollbar
attempting to get the vertical space allocation in header and content area
synchronised; previously we conflated the content size and additional
padding, but even after distinguishing both, we still get a cyclic
dependency, leading to progressive increasing of allocated size...
After quite some tinkering, instead of extending the DisplayManager interface,
I now prefer to treat this connection rather as an intricate implementation detail:
The TimelineLayout implementatino now provides two translation functions,
which are directly wired as slots from the Signals emitted by moving the
hand of the scrollbar; the idea is that these functions mutate the ZoomWindow,
which then triggers a DisplayEvaltuation, which in turn causes the
drawing code to pick up and translate back the new metric and position.
Results look promising, insofar the DisplayEvaluation is now triggered
repeatedly, and the actual window width in pixel is propagated;
however, the response of the layout code is seemingly random at times,
the allocated height grows monontonously and the code Segfaults when
moving the scrollbar...
this makes the arrangement more symmetric and natural
and also makes the overview ruler scroll alongside the content pane,
thereby creating the (intended) impression of one uniform layout space
As it turns out, this happens as side-effect from the workaround 2019-08-22
fc5eaf857c
Obviously, just set_size() on the canvas is not sufficient for
GTK actually to establish a size-request (seemingly the canvas counts
as /empty/ and only real widgets would make a difference).
However, since the ruler canvas is directly placed into the box,
and not adapted by a ScrolledWindow, explicitly set_size_request()
also causes the enclosing Box to "inherit" this minimal required size,
thereby also spreading out the BodyCanvasWidget beyond the size
actually available. GTK handles this situation by hard-clipping
on the right side, which causes the vertical scrollbar to disappear
and keeps the horizontal scrollbar disabled (since nominally it still
spans the whole size available, even while this size is then clipped
subsequently).
This changeset adds a lot of debug printing and demonstrates this
behaviour by setting only a minimal horizontal size_request, so that
the window is no longer expanded and clipped.
This does not really change the logic of the DisplayEvaluation mechanism,
but makes it much more flexible: instead of having two hard wired components,
the DisplayEvaluation visitor now holds a collection of LayoutElement pointers.
This way, the Layout manager itself (on behalf of the ZoomWindow) can
participate in the process, and on activation will now establish the
window width in pixel.
This works now insofar the drawing on the canvas is adapted coherently;
however something with the setup of the Scrollbar is still not quite right;
some time ago I recall the scrollbar worked, but now it is blocked
and the canvas just clips to the right side.
It is now tied to the start of ZoomWindow::overallSpan(),
thereby defining the (technical) pixel coordinates within the window
and for drawing on the canvas to be always positive. Whenever ZoomWindow
re-calibrates, it's change signal will trigger, causing the
TimelineLayout to perform a new DisplayEvaluationPass,
which in turn prompts all embedded widgets to readjust
their positions accordingly.
Note: changing behaviour of TimeSpan to possibly flip start and end,
and also to use Offset as Offset and then re-orient,
since this seems the least surprising behaviour.
These changes carry over into changed default and limiting
on ZoomWindow constructor and various mutators, and most
notably shifting the time span always into allowed domain.
...the implementation was way too naive; in some cases we could go
into an infinite loop. In the end, using Newton approximation was not
necessary (and thus there is no loop anymore), but it helped me get
at a much better solution with very small error margin on average case.
All these corner cases are obviously "academic" to some degree,
but it turns out there is no clear-cut point where you'd be able
just so set a limit and be sure that fractional integer arithmetic
works flawless in all cases.
Thus the choice is
- give up (fractional) integers and work with floats and have to
deal with error accumulation
- or do something as chosen here, namely add a boundary zone, where
fractional integer arithmetic can be kept under control, while admitting
small errors, and in turn get the absolutely precise integers in all
everyday standard cases
The value used previously was too conservative, and prevented ZommWindow
from zooming out to the complete Time domain. This was due to missing the
Time::SCALE denominator, which increaded the limit by factor 1e6
In fact the code is able to handle even this extremely reduced limit,
but doing so seems over the top, since now detox() kicks in on several
calculations, leading to rather coarse grained errors.
Thus I decided to use a compromise: lower the limit only by factor 1000;
with typical screen pixel widths, we can reach the full time domain,
while most scaling and zoom calculations can be performed precisely,
without detox() kicking in. Obviously this change requires adjusting
a lot of the test case expectations, since we can now zoom out maximally.
As it turns out, the calculation path initially choosen for the mutateScale(Rat)
was needlessly indirect, and also duplicated several of the safeguards,
meanwhile implemented way better in conformWindowToMetric(Rat)
Thus, instead of relatively re-scaling the window, now we just
limit the given zoomFactor and pass it to conformWindowToMetric()
There is a built-in limitation, which now is even
lowered to 100000 pixels horizontally.
With the techniques introduced in this changeset, it seems possible
to support more -- yet this would be a case of unnecessary genricity;
handling such large numbers will drive more computations into the
danger zone, and doing so incurs cost in terms of testing and debugging.
Placing that into context, contemporary displays are not even 4K on
average, and it does not look likely even for cinema display to go
way beyond 8k -- so yes, I want display hardware with 100000 pixels!!
The key takeaway of this changeset:
- can calculate px = trunc(zoomFactor * duration) step wise,
even when the direct calculation would lead to wrap-around
- can safely adjust and fix the zoomFactor using Newton approximation
...even zooming out to span the complete time domain (~19000 years).
But only under the condition that the display window is sufficiently
large in terms of pixels, so we can handle the computation without
glitches.
This should not be a relevant limitation in practice, since a window
size of some 100 pixels is enough to handle Duration::MAX. Needless to add
that it's hard to imagine a media timeline of such tremendous size...
building on these Library changes, plus the safe-add function
developed some days ago, it is now possible to mark a large displacement
as `time::Offset`, and apply this to yield any valid time position,
even extreme negative values
...building on these Library changes, plus the safe-add function
developed some days ago, it is now possible to mark a large displacement
as `time::Offset`, and apply this to yield any valid time position,
even extreme negative values
The APIs for time quantisation were drafted in an early stage of the project
and then never followed-up. Especially Grid::gridAlign has no
real-world usage yet, and is only massaged in some tests.
When looking at QuantiserBasics_test, I was puzzled and led astray,
since this function suggests to materialise a continuous time into
a quantised time -- which it doesn't (there is another dedicated
function Quantiser::materialise() to that end); so, without engaging
into the discussion if this function is of any use, I'll hereby
choose a name better reflecting what it does.
This is a deep refactoring to allow to represent the distance
between all valid time points as a time::Offset or time::Duration.
By design this is possible, since Time::MAX was defined as 1/30 of
the maximum value technically representable as int64_t. However,
introducing a different limiter for offsets and durations turns
out difficult, due to the inconsistencies in the exiting hierarchy
of temporal entities. Which in turn seems to stem from the unfortunate
decision to make time entities immutable, see #1261
Since the limiter is hard wired into the `time::TimeValue` constructor,
we are forced to create a "backdoor" of sorts, to pass up values
with different limiting from child classes. This would not be so
much of a problem if calculations weren't forced to go through `TimeVar`,
which does not distinguish between time points and time durations.
This solution rearranges all checks to be performed now by time::Offset,
while time::Duration will only take the absolute value at construction,
based on the fact that there is no valid construction path to yield
a duration which does not go through an offset first.
Later, when we're ready to sort out the implementation base of time values
(see #1258), this design issue should be revisited
- either we'll allow derived classes explicitly to invoke the limiter functions
- or we may be able to have an automatic conversion path from clearly
marked base implementation types, in which case we wouldn't use the
buildRaw_() and _raw() "backdoor" functions any more...
While the calculation was already basically under control, I just was not content
with the achieved numeric precision -- and the fact that the test case in fact
misses the bar, making it difficult do demonstrate that the calculation
is not derailed. I just had the gut feeling that it must be somehow possible
to achieve an absolute error level, not just a relative error level of 1e-6
Thus I reworked this part into a generic helper function, see #1262
The end result is:
* partial failure. we can only ''guarantee'' the relative error margin of 1e-6
* but in most cases not out to the most extreme numbers, the sophisticated
solution achieves much better results way below the absolute error level of 1µ-Tick
Thus with using rational numbers, we have now a solution that is absolutely precise
in the regular case, and gradually introduces errors at the domain bound
but with an guaranteed relative error margin of 1e-6 (== Time::SCALE)
...now able to achieve the expected error bound of 1e-6
...this seems to be the worst-case for ZoomWindow::setVisiblePos(factor)
for extremely large timeline; seemingly not possible to achieve the
goal set for this special test case
...in a similar vein as done for the product calculation.
In this case, we need to check the dimensions carefully and pick
the best calculation path, but as long as the overall result can
be represented, it should be possible to carry out the calculation
with fractional values, albeit introducing a small error.
As a follow-up, I have now also refactored the re-quantisation
functions, to be usable for general requantisation to another grid,
and I used these to replace the *naive* implementation of the
conversion FSecs -> µ-Grid, which caused a lot of integer-wrap-around
However, while the test now works basically without glitch or wrap,
the window position is still numerically of by 1e-6, which becomes
quite noticeably here due to the large overall span used for the test.
...using a requantisation trick to cancel out some factors in the
product of two rational numbers, allowing to calculate the product
without actual multiplication of (dangerously large) numbers.
with these additional safeguards, the anchorWindowAtPosition()
succeeds without Integer-wrap, but the result is not fully correct
(some further calculation error hidden somewhere??)
- detailed documentation of known problematic behaviour
when working with rational fractions
- demonstrate the heuristic predicate to detect dangerous numbers
- add extensive coverage and microbenchmarks for the integer-logarithm
implementation, based on an example on Stackoverflow. Surprising result:
The std::ilog(double) function is of comparable speed, at least for
GCC-8 on Debian-Buster.
Especially rational numbers with large denominator can be insidious,
since they might cause numeric overflow on seemingly harmless operations,
like adding a small number.
A solution might be to *requantise* the number into a different,
way smaller denominator. Obviously this is a lossy operation;
yet a small and controlled numeric error is always better than
an uncontrolled numeric wrap-around.
- 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.
* restructure the widgets used to implement ElementBox
* inject a Gtk::EventBox top-level base type to capture all Gdk-Events
* push the Gtk::Frame one level down (TODO: API for managing children)
With these changes
* dragging of Clips in the timeline works as expected
* size constraints are observed precisely
* all warnings and assertion failures from GTK disappeared
Thus we can conclude that the solution approach for size constrained widgets
was successful and this challenging problem is solved.
...as it turned out, the ClipWidget did still invoke the
Gtk::Frame::set_label() function, thereby deactivate our
elaborate custom IDLabel and showing the label text
unconditionally instead (also violating our size constraint)
...as it turns out, this code basically works already when the
widget is not(yet) realized:
- when a widget is hidden, it responds with size=0
- when a widget is shown, it reponds with proper or at least
preliminary size requirement, irrespective if already drawn
After injecting the diff, the widgets are created and then adjusted
in several steps; however, this code all executes from within a single
call to the UI-bus, and thus just piles up a sequence of realize()
and resize() messages, which are only executed later, in case the
Application-UI as a whole is visible on screen.
*Remaining Problems*:
- size-constraint code not working correct in all cases
- dragging works only on the buttons, not on the background
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
- Test the new layout code with debugger + dump messages
- Experiment: live changes to the name-ID content
(send msg. "manip" -> Text changed and Layout properly revalidated)
devise a more fine grained algorithm for adapting the display of IDLabel
to a situation with size constrained layout, e.g. for a time calibrated canvas.
We still do not implement the shortening of ID labels (see #1242),
since doing so would be surprisingly expensive. But at least we
do proceed in several steps now
- first attempt to reduce the name-ID (for now: hide it)
- if this doesn't suffice, also hide the menu
- and as a last resort, hide the icon and thus make the IDLabel empty
We are using buttons now, but the standard theme introduces a lot of padding arount button's contents.
Thus we need to consider ways to address the compound of widgets forming an ElementBox; moreover,
this is the classical situation where the BEM notation helps to clarify the intention....
The problem leading to custom styling here is the padding within buttons;
the default stylesheet seemingly adds a min-width and min-height setting,
and some padding within the Button; based on systematic CSS class names,
it is possible to remove these settings specifically for buttons
within the IDLabel in general (no need to treat only the case of an EventBoxLabel
-- IDLabel could become a custom widget on its own
as it turns out, this is a self-contained separate concern,
and thus this arrangement of two icons plus a caption shall
now manage itself as a custom widget.
And while touching this subject, I have also reconsidered
the purpose and arrangement of those icons and completed
the specification with some decisions...
- context menus will be left-click, selection right-click (Blender!)
- we will always show those two icons, just allocate different graphics
- when there is no expander, the 2nd icon will just serve to open the menu
- so the button is almost redundant in that case (except when dragging)
Further extended GTK code survey to clarify the role of the minimum_size,
it is indeed ignored by most standard containers, but it is actually
used by Gtk::Layout as starting point for the query sequence. Thus
it does not make sense to treat minimum and natural size differently;
both queries should be responded by returning our size constraint.
Unless we define additional borders and margins in the CSS, we can be sure
that GTK will base the size allocation on the exact values returned
from the get_required_* functions.
These functions will be invoked only from within the Event Loop
and after the ctor is finished, but before the first "draw".
They will be re-invoked on each "size change" event and on each
focus change (since a focus change may change the style and thus
the actual extension).
...the key point is to ask the embedded box holding the label
about it's preferred_size() -- this info is updated immediately,
even at begin, when the nested child widgets did not yet receive
an allocation.
Even while the preferred-size is something different than the
actual allocation, it will always be smaller and is thus sufficient
to decide if the size constraint can be met
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.
...does not work reliably yet...
- on first invocation, the child allocation is still zero
- later on, there seem to be lots of further invocations,
always when the application window gains focus
- these further invocations somehow change the visible extension
of the widget's background
identify the various dimensions, which require flexibility
to support the intended use cases; try to come up with a
design draft, allowing to settle on a preliminary version
soon, while not hampering further development later on.
Obviously this is a very deep and challenging topic,
and we're far from even remotely addressing it adequately;
we just need to get to the point to use this drafted version
as building block, since these usages will then push us further
into the right direction...
The flexible custom styling yet needs to be definied.
Just adding a stock icon and a standard sized label field for now.
Widget can be constructed and successfully attached to a track.
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
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
Seems to work solid now, after switching to the root coordinates provided by GDK.
With local relative coordinates, the subject fidgets while being dragged,
for obvious reasons, since we're shifting the relative point of reference.
Also clarified a strange behaviour of the test drawing code:
Cairo is "turtle graphics", so we need to set the starting point explicitly.
...well, the metric translation is not quite correct,
so it doesn't yet stick to the mouse. But all the challenging
problems within the framework for implementing such a generic
gesture seem to be solved now.
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
- 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
...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
while dragging, effectively we have to translate a mouse position delta
into a TimeValue delta, and we want to avoid direct coupling to some
timeline display manager, to keep the gesture logic mostly generic.
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.
...because this is a prototype, but should fit in
with a future frameworks to handle complex interactions and gestures.
And no, we can not afford to rely on a UI toolkit for such a core concern
It is impossible that a framework like e.g. GTK will allow us to
support a custom made hardware controller and integrate it seamlessly
into getsture handling, thereby following a design philosophy that
is in accordance with our fundamental decisions.
...found out that GTK already implements an "implicit grab",
and thus the tricky situation that the mouse slides off the widget
can not happen at all; so in the end it's rather easy to build a trigger
for a dragging gesture.
The demo code is now activated only after the button is down
and just prints the position...
PS: did some research regarding the new Coroutines in C++
Setup the scaffolding necessary to get at the actual clip widget
and to establish a signal connection to the button_pressed signal.
The intention is to watch this in conjunction with mouse movements
for detection of the actual gesture.
At the moment, I am using button widgets as placeholder for the actual
clip widgets (not yet implemented...). And, as a tiny little success,
these buttons now invoke the gesture controller on right click
(left click is seemingly consumed by the button itself)
thus far my implementation concept seems to work as intended....
note: when populating the timeline with actual Clips,
the not-yet implemented linkSubject()-Function of the DragRelocateController
gets invoked (as it should), thereby killing Lumiera
...actually postpone to build a generic translation system and use hard wired relations for now;
it is acknowledged that we'll need some kind of translation system eventually,
once the GUI has to handle a lot of possibly configurable gestures.
..and thus there is now one dedicated source location,
where configuration of new clip widgets can be done reliably.
So all prerequisites are solved and we can start
building a prototypical drag-gesture implementation
A separate translation unit turns out to be unnecessary here,
since this is implementation level code included into one single
other translation unit (timeline-controller.cpp). In such a situation,
having the whole class definition at one code location improves
readability.
Moreover, there clearly is now another abstraction barrier,
insofar all of the clip widget's implementation technicalities
are buried within clip-widget.cpp
The specific twist with the clip display lies in the fact
that there might or might not be a dedicated clip widget,
based on the current presentation style and zoom level.
Consequently we need hook up the widget for dragging,
only when, and whenever a new clip widget is actually created.
This boils down to the requirement to detect whenever a state change
creates a dedicated widget -- and this can only be sensibly implemented
when all display state transitions are handled by a single function.
Previously, we had two specialised functions for this purpose:
one to initially create the delegate and one to switch the
implementation type for an already existing delegate.
This refactoring attempts to merge all this logic into a single function,
which now unfortunately became quite complex and hard to understand.
My planning thus far seems solid enough to start fleshing out one concrete gesture handling,
which can serve as a blueprint for a generic scheme to be worked out later.
Moreover, the implementation is limited to mouse interaction for the time being,
while the goal remains to treat "gestures" in a way to span several
Interaction-Schemes eventually (mouse, key sequence, pen...).
...even while keeping the focus to the actual problem at hand,
this solution must be built with the larger goal in mind, which
is the ability to support various editing gestures, transmitted
possibly through several control-systems (mouse, keybindings, pen...)
It is obvious that we'll need a dedicated controller for each kind of gesture;
what turns out as tricky is to maintain and bind a stateful context
and find the correct participants while a specific gesture is under way.
Now basically the header labels are aligned with the start of the corresponding body area.
However, there still seems to be some minor glitch hidden somewhere,
and the labels seem to be off by one pixel per track. Also the allocated
canvas size is to small after first evaluation, but somehow gets
corrected whenever the window is resized.
..now this more or less works and indeed crops the button widget
used here for a proof-of concept; however the label within that button
emits a lot of layout warnings on each event handling and drawing routine,
indicating that we violated its fundamental assumptions.
Not sure how to proceed from here; also not sure if this actually
becomes turns into a relevant issue in practice, since maybe in most cases
we'll rather increase the size, and all we really have to do is handle
the Clip's textual label properly. A clip smaller than some drop-down icon
should probably not be rendered explicitly, just as overview
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.
- fix a regression introduced with the 3rd DisplayEvaluation pass
- use references to pass the timings more efficiently to the ClipDelegate
- DisplayEvalutation in fact has a real LifeCycle and is not disposable
- generate the population diff for this test in canonical form
with these changes, essentially the clip is moved to the
new position established in the preceding DisplayEvaluation.
...there is still some problem when this DisplayEvaluation itself
is triggered from within draw(), because then GTK still uses the old
sub-widget coordinates within this draw code, pretty much as if
they were cached somewhere. The next draw() call then uses the
proper new coordinates.
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
This can only be a preliminary solution, since we do not know
the actual usage pattern of the ClipDelegate object yet.
We only know there will typically be a huge amount of clips
to represent in the UI, and we need to be careful to avoid
unneccesary reallocations.
Thus for now we use a data record as base class, and we
move the data record into the new allocation when switching gears.
However, this could easily be converted into a data delegate,
where we'd only transfer ownership without reallocation,
in case this turns out to be more efficient.
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
...as it turns out, a first preliminary clip display should be working by now;
seemingly I was able to the tough theoretical problems last spring,
and was at the point of actually allocating display extension space
within the custom drawing area of the timeline.
Thus a simple placeholder widget based on a Gtk::Button should show up
at the right position, when sending a suitable diff message. The only
thing missing seems to be a first rough draft for the function
determineRequiredVerticalExtension()
...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.
the actual translation is still TODO;
we should delegate the calculation to the DisplayManager,
which is in focus within the TrackBody, where the coordinate
translation hook is now located.
...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
while the actual selection logic for the appearance style still remains
to be coded, this changeset basically settles the tricky initialisation 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.
...to indicate how the setting up the delegate might decide upon the appearance style
WIP: this is more than half baked
- for one it seems doubtful to pass a hidden hint regarding appearance through that optional argument
- and then, most importantly, we should be passing a time::TimeSpan
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
...while the first solution looked as a nice API, abstracting away
the actual collections (and in fact helped me to sport and fix a problem
with type substitution), in the end I prefer a simpler solution.
Since we're now passing in a lambda for transform anyway, it is
completely pointless to create an abstracted iterator type, just
for the sole purpose of dereferencing an unique_ptr.
As it stands now, this is all tightly interwoven implementation code,
and the DisplayFrame is no longer intended to become an important
interface on it's own (this role has been taken by the ViewHook /
ViewHooked types).
Note: as an asside, this solution also highlights, that our
TreeExplorer framework has gradually turned into a generic
pipeline building framework, rendering the "monadic use" just
one usage scenario amongst others. And since C++20 will bring
us a language based framework for building iteration pipelines,
very similar to what we have here, we can expect to retrofit
this framework eventually. For this reason, I now start using
the simple name `lib::explore(IT)` as a synonym.
...there is no need for yet another indirection here,
since TrackPresenter is not much of an interface and
only included at into two other translation units.
Moreover, header-only code simplifies the use of
templated lambdas, which come in handy when dealing
with the various nested sub-collections.
- it seems such a feature is not possible to implement in a totally
sane and safe way, since intermixed other UI messages might cause
removal of some widgets for which we scheduled a change. And there
is no simple and performant mechanism available to track the lifecycle
of all the widgets involved
- as it stands, it is actually not necessary to schedule the resizing
for later, since the UI runs single-threaded, and thus GTK has no
opportunity to act on them while our evaluation pass is running
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)
...not fully conclusive yet.
However, the split into two canvas controls plays an important role here;
at some point we need to translate into the coordinates shifted by the height
of the first, pinned canvas (track profile "prefix").
This is an attempt to hide that away as a technical detail,
buried within the calculation of the track body height allocation.
the marked pars are diagnostics code anyway,
however, the first attempt used direct manipulation of the child offsets from "outside".
Now, after switching to the ViewHook-mechanism, such direct manipulation
of view innards is no longer neccessary, as can be verified by removing that test code now.
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)
...which erroneously assumed the list of timelines to be empty.
When sending a further population diff, this assumption is broken,
since the first diff resulted in adding a timeline element.
This misatke was detected by the new consistency check added with
9f3fe8a88
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
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>.
GCC8 now spots and warns about such mismatches.
And we should take such warnings seriously;
code produced by the newer GCC versions tends to segfault,
especially under -O2 and above, when a return statement is
actually missing, even if the return value is actually not
used at call site.
Here, a functor to unlock the active "guard" is passed into
a macro construct, which basically allows to abstract the
various kinds of "guards", be it mutex, condition variable
or the like.
Seemingly, the intention was to deal with a failure when
unlocking -- however all the real implementations prefer
to kill the whole application without much ado.
...to solve the problem with interwoven nested ctor invocation.
This interface also promises to help with nested invcations,
without being overly generic.
Our diff language requires a diff to handle the complete contents of the target.
Through this clean-up hook this is now in fact enforced.
The actual reason for adding this however was that I need to ensure
listeners are triggered
As it turned out, the reason was a missing move-ctor.
The base of the whole DSL-Stack, TreeMutator, is defined MoveOnly,
and this is also the intended use (build an anonymous instance
through the DSL and move it into the work buffer prior to diff application)
However, C++ does *cease to define* a move ctor implicitly,
whenever /one of the "big five" is defined explicitly/.
So Detector4StructuralChanges was the culprit, it defined a dtor,
but failed to define the move ctor explicitly.
So.... well, this did cost me several hours to track down,
yet I still rather do not want to write all those ctors explicitly all the time,
and so I am still in favour of implicitly generated ctors, even if they hurt sometimes.
with the new decorator layer, we suddenly trigger a chain of template instantiation errors.
At first sight, they are almost undecipherable, yet after some experimentation, it becomes clear
that they relate down to the base class (TreeMutator), which is defined MoveOnly
This seems to indicate that, at some point in the call chain, we are
digressing from the move-construction scheme and switch over to copy construction,
which in the end failst (and shall fail).
Inconclusive, to be investigated further
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
Basically the advice system is just some further helper component with
a lookup table, which "just works" transparently most of the time.
We'd need those diagnostic messages only when explicitly debugging
for some kind of access-after death, which should not happen as long
as we stick to the general policy not to allow any significant functionality
to be hooked up from dtors
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.
By applying a Diff, the children of some timeline element (track) may be re-ordered.
This imposes specific problems, since these elements hold onto slave-Widgets,
which are already attached into some elaborated and nested widget structure.
To keep complexity under control, we can not allow the TrackPresenter to have
any knowledge regarding the implementation structure of these target widgets.
Thus I am pondering the idea to represent that relation as an abstracted ViewHook link
...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
This is dummy/test/diagnostics code and should be removed when the track display code is complete!
It can be activated by sending a "mark"-Message via the UI-Bus, towards the
Timeline element to be tested (Tip: use the same ID as used when injecting
the Timeline via the TestControl Dialog box). When receiving this message
(asynchronously), the TimelineControler asks each nested TrackPresnter
to inject a Button with the corresponding track name onto the BodyCanvasWidget.
This allows us to verify the coordinate calculation and size allocation --
and indeed, the numbers are not yet correct (TODO)
admittedly this is a bit sketchy, but I don't have a better framework to hinge upon right now.
Thus we store the vertical start coordinates and the offset of the content area
as a side effect, while calculating the TrackProfile
...which has the nice additional effect of exposing box-shadow on the outside of the content area too.
Thus the content area now behaves equivalent to the rulers, and adjacent
content space of simple tracks without rulers and nesting can be slightly
offset from each other through a margin in CSS
In the end, I used the profile building pass to also calculate and sum up the vertical offsets.
Seems to be the only sane approach to get really precise values, since adjacent
upwards slopes can be combined at various places (and I do not want to use the
actual drawing code for this calculation)
need to investigate and probably need to store per track offset values
already while building the track profile. The primary reason for the
observed discrepancy seems to be the rather flexible combination of
slope borders.
Especially note the tricks we need to play in order to allow for (limited) usage of CSS3 box-shadows.
The reason is, all these CSS3 effects are rendered in one shot and combinend on the StyleContext::render_background() call
Thus we need to ensure that the background is properly aligned with the frames
seemingly, the Box with PACK_SHRINK allocates a zero height to the rulerCanvas initally,
which is correct at that point, since the widgets are not yet realised.
However, when we later set_size() on the rulerCanvas, the enclosing Box should reflow.
It does indeed if the child widget is a button or something similar, however,
somehow this reflowing does not work when we set_size on the canvas.
A workaround is to place a new set_size_request().
TODO: do this more precisely, and only on the rulerCanvas. To the contrary,
the mainCanvas is placed into a scolling-pane and thus does not need a size-Request.
Moreover, the latter automatically communicates with the hadjustment() / vadjustment() of
the enclosing scrollbars.
as can be verified with the debugger, it sets the correct sizes now.
And it is called only once (unless the content size actually changes).
TODO: however, the visible display of the GTK widgets is not adjusted
Indeed I had missed to connect the new "free standing" StyleContext to
some Gdk::Screen, typically the default screen (connected to the current
top level window). But seemingly this was not really necessary, since,
somehow magically, the style context must have connected itself to some
screen, otherwise it wouldn't be able to access the CSS cascade.
Anyhow, fixing this omission does not resolve our problem.
Nor does any combination of re-connecting, invalidating etc.
I poked around in the GTK (C) code a lot, but could not spot any obvious
missing initialisation step. To much magic around here. Without massive
debugging into GTK internals, I don't see any way to further this
investigation. And, moreover there is a viable workaround
(namely to set and remove the classes explicitly, which works as intended)
I posted a question on Stackoverflow and for now
I'll file this topic as "inconclusive"
https://stackoverflow.com/q/57342478
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.
DONE
- can now control the border size through a set of modifier classes
OPEN
- but context_save()/restore() does not work; seem to loose all styling
- not clear how to deal with CSS3 effects like box-shadow
...somehow does not yet work as intended...
- unable to control the border-width from code
- Gtk::StyleContext::add_class(name) does not seem to have any effect
The population message is just made up, in order to create more interesting structures
in the UI and so to further the development of the timeline display.
For the actual structure I choose to mirror my example drawing in draw/UI-TimelineLayout-1.png
which is also used in the TiddlyWiki, on the #GuiTimelineView tiddler
https://lumiera.org/wiki/renderengine.html#GuiTimelineView
We can add our custom classes to custom widgets, and we can set the
widget name, which can be used as #id selector from CSS
Unfortunately we can not set the main CSS node name for CustomWidgets defined through GTKmm (C++)
The latter is only possible when deriving the custom widget in plain-C, which is quite tedious.
On a second thought, this limitation is not so severe as it might seem, because
most of the time you actually do *not* want to change the CSS node name,
because you want to match against existing rules in the theme (e.g. box, or paned)
The actual case here would have been an exception to this rule, since here
it would be nice to anchor the whole custom timeline drawing in an "body.timeline" element
NOTE: Current state for the selector path is now:
window.background box.vertical box[2/3].horizontal widget[2/2] widget paned.vertical widget box.vertical notebook[1/1].frame paned.horizontal.timeline-page box.vertical.timeline.timeline-body fork.timeline
...and perform the initialisation once, when attaching the first timeline to the UI
Now our code produces the following Gtk::WidgetPath (note the last node, which our code added)
window:backdrop:dir-ltr.background box:backdrop:dir-ltr.vertical box:backdrop:dir-ltr[2/3].horizontal widget:backdrop:dir-ltr[2/2] widget:backdrop:dir-ltr paned:backdrop:dir-ltr.vertical widget:backdrop:dir-ltr box:backdrop:dir-ltr.vertical notebook:backdrop:dir-ltr[1/1].frame paned:backdrop:dir-ltr.horizontal box:backdrop:dir-ltr.vertical fork.timeline
For context: The »Advice System« was coined a long time ago, in 2010,
based on the vague impression that it might be useful for that kind of application
we are about to build here. And, as can be expected, none of the usage situations
envisioned at that time was brought to bear. Non the less, the facility came in
handy at times, precisely because it is cross-cutting and allows to pass
information without imposing any systematic relationship between the
communication partners.
And now we've got again such a situation.
The global style manager in the UI has to build a virtual CSS path,
which is needed by drawing code somewhere deep down, and we absolutely
do not want to pass a reference to the style manager over 20 recursive calls.
The alternatives would be
(1) to turn the style manager into a public service
(2) to have a static access function somewhere
(3) to use a global variable.
For rationale, (1) would be overblown, because we do not actually request
a service to do work for us, rather we need some global piece of information.
(2) would be equivalent to (1), just more confusing. And (3) is basically
what the Advice system does, with the added benefit of a clear-cut service
access point and a well defined lifecycle.
This changeset adds the ability to check if actual Advice has been published,
which allows us to invoke the (possibly expensive) GTK path building and
style context building code only once.
- at some (yet to be defined) location, a virtual WidgetPath is constructed
and used to build a Gtk::StyleContext in accordance to the curren CSS
- within the drawing routine, we use Lumiera's Advice-System to access this info
Mostly, std::regexp can be used as a drop-in replacement.
Note: unfortunately ECMA regexps do not support lookbehind assertions.
This lookbehind is necesary here because we want to allow parsing values
from strings with additional content, which means we need explicitly to
exclude mismatches due to invalid syntax.
We can work around that issue like "either line start, or *not* one of these characters.
Alternatively we could consider to make the match more rigid,
i.e we would require the string to conain *only* the timecode spec to be parsed.
The existing implementation created a Buffer-Type based on various traits,
including the constructor and destructor functions for the buffer content.
However, this necessitates calculating the hash_value of a std::function,
which (see #294) is generally not possible to implement.
So with this changeset we now store an additional identity hash value
right into the TypeHandler, based on the target type placed into the buffer
This was prompted by a test failing under Boost-1.65 (--> see #294)
When reviewed now, the whole idea of testing Steam-Layer Commands for
equivalence feels a bit sketchy.
Just the comparison for the command ''identity'' alone seems sufficient,
i.e. the test if a command-ID is associated with the same backend-handle
and thus the same functor binding.
<rant>
the "improved" boost::rational can no longer compute 1/x
quite brilliant
</rant>
well... the reason is again signed vs unsigned int.
FrameRate is based on unsigned int (since a negative frame rate makes no sense).
seemingly, the newer boost libraries added an internal type rational<I>::bool_type
together with an overload for the equality comparison operator.
Unfortunately this now renders a comparison ambiguous with the constant zero (i.e. int{0})
because in our use case we employ rational<uint>.
Workaround is to compare explicitly to a zero of the underlying integer type.
...when rendering this part, which shall be always visible.
And the rest of the profile needs to be rendered into a second canvas,
which is placed within a pane with scrollbar.
Implemented as a statefull iterator filter
TODO:
- actual draw operations not yet implemented
- find a way how to select the prelude / body part of the track profile
This is a consequence of subsuming the timeline ruler under the concept of an overview track
the template lib::PolymorphicValue seemingly picked the wrong
implementation strategy for "virtual copy support": In fact it is possible
to use the optimal strategy here, since our interface inherits from CloneSupport,
yet the metaprogramming logic picked the mix-in-adapter (which requires one additional "slot"
of storage plus a dynamic_cast at runtime).
The reason for this malfunction was the fact that we used META_DETECT_FUNCTION
to detect the presence of a clone-support-function. This is not correct, since
it can only detect a function in the *same* class, not an inherited function.
Thus, switching to META_DETECT_FUNCTION_NAME solves this problem
Well, this solution has some downsides, but since I intend to rewrite the
whole virtual copy support (#1197) anyway, I'll deem this acceptable for now
TODO / WIP: still some diagnostics code to clean up, plus a better solution for the EmptyBase
...which, in the end, can even be considered the more logical design choice,
since the "verb visitor" is a more elaborated and sophisiticated Verb-Token,
adding the special twist of embedded storage for variable function arguments
...but bad news on the main issue:
the workaround consumes the tuple and thus is not tenable!
And what is even worse: the textbook implementation of std::apply is
equivalent to our workaround and also consumes the argument tuple
A simple yet weird workaround (and basically equivalent to our helper function)
is to wrap the argument tuple itself into std::forward<Args> -- which has the
effect of exposing RValue references to the forwarding function, thus silencing
the compiler.
I am not happy with this result, since it contradicts the notion of perfect forwarding.
As an asside, the ressearch has sorted out some secondary suspicions..
- it is *not* the Varargs argument pack as such
- it is *not* the VerbToken type as such
The problem clearly is related to exposing tuple elements to a forwarding function.
this is a generalisation of what we use in the diff framework;
typically you'd package the VerbToken into some kind of container,
together with the concrete invocation argument.
However, the specific twist here is that we want *variable arguments*,
depending on the actual operation called on the interpreter interface.
Up to now, PolymorphicValue was always used as-is, packaged into a typedef.
Now we consider using it as building block within an adapter for visitor-like tokens.
Which requires to pass-down the ctor call directly from the subclass, at least if we
want to emplace the resulting entity directly into a stdlib container.
As an asside, PolymorphicValue also used explicit specialisations for N-arguments,
which meanwhile can be replaced by variadic templates
...which leads to a specific twist here; while in the simple version
we still could hope to get away with a simple uniform uint argument,
the situation has changed altogether now. The canvas has turned into
some generic component, since it is instantiated two times, onece for
the time ruler and once for the actual body content. Thus all of the
specifics of the drawing code need to be pushed into a new, dedicated
renderer component. And this more or less forces us to pass all the
actual presentation variations through the invocation arguments of
the visitor.
So we're now off again for a digression, we need a more generalised visitor
After thinking the whole concept over several times, it occurred to me that
a separate implementation of a time ruler would be quite redundant with the
envisioned feature of per-track overview rulers. Following this line of thought,
the time ruler would just be some specifically configured overview ruler.
This has the somewhat unfortunate consequence, that it becomes the responsibility
of the body canvas to render the overview ruler, thereby somehow delegating
to a common renderer implementation. Which makes the whole setup of the body canvas
way more complex, because now we get *two* canvas like painting areas, one
always visible at top, and the second one, the content area, fully scrollable
within the lower part.
- we got occasional hangups when waiting for disabled state
- the builder was not triggered properly, sometimes redundant, sometimes without timeout
As it turned out, the loop control logic is more like a state machine,
and the state variables need to be separated from the external influenced variables.
As a consequence, the inChange_ variable was not calculated properly when disabled in a race,
and then the loop went into infinite wait state, without propagating this to
the externally waiting client, which caused the deadlock
effectively we rely in the micro tick timescale promoted by libGAVL,
but it seems indicated to introduce our own constant definition.
And also clarify some comments and tests.
(this changeset does not change any values or functionality)
basically we can pick just any convention here, and so we should pick the convention in a way
that makes most sense informally, for a *human reader*. But what we previously did, was to pick
the condition such as to make it simple in some situations for the programmer....
With the predictable result: even with the disappointingly small number of usages we have up to now,
we got that condition backwards several times.
OK, so from now on!!!
Time::NEVER == Time::MAX, because "never" is as far as possible into the future
A classical carry-over of dirty values...
Problem arises, when starting an unconditional wait on the same object monitor,
which previously conducted a timed wait. Then the obsolete timeout from the previous
wait remained in place, causing our Sync-Wrapper (erroneously) to assume a timed wait
and then pthread to return immediately from this timed wait.
The result was permanent idle looping in the ProcDispatcher, after the first command was processed
When invoking the util::toString conversion, we indeed to want any conversion,
including explicit conversion operators. However, probing the possibility to build a string
can be dangerous, since there is a string constructor from characters, and
integral types can be converted to characters.
OTOH, leaving out explicit conversions is likewise not desirable, since there are
class types, which deliberately do not offer an implicit conversion, but allow
explicit conversion for dump and diagnostic output. The notorious example for
such a situation is the lib::idi::EntryID<TY>. We certainly do not want an
EntryID to be converted into a string without further notice, but we do want
an EntryID to be automatically rendered to string in diagnostic output, since
this will include the human readable ID part.
See especially: 8432420726
Now we'll attempt to get out of this dilemma by probing explicitly for the presence
of a string conversion operator, which will fail for any non-class types, thereby
ruling out all those nasty indirect type -> character -> string conversion paths.
The rationale is: if someone queries the predicate can_convertToString, the intention
is really to get an string rendering, and not just to invoke some random function
with an string argument.
- 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
the new structure causes them now to be installed into $TARGET/stage
which is simply not what I want. I still consider $TARGET/gui the better choice,
since an administrator or packager is not aware of our layer namings.
The existing solution was half baked anyway, it did not really replicate the source tree.
On the other hand, I want to retain the location of the CSS files within the GUI tree,
since I consider it a good practice, to keep "code-like" resources with the actual code,
and not far away in some arcane "data" directory.
No I've noticed, that the env.GuiResource() function is only used once, for this very task.
So, for the time being, we can keep it simple and deditaced to that task, i.e
we pick up all CSS files we find and install it into a single target directory.
NOTE: this issue has brought to my attention two further, completely unrelated issues
* Ticket #1192 (Lumiera hangs on failed GUI start)
* The ProcDispatcher does an idle wait, due to an error in timed-wait implementation
...in accordance to our general design guideline: we don't duplicate
actual model values within the controllers/presenters, since our widgets
act themselves as view-model
This change demonstrates how to deal properly with possible duplicate entities
with similar symbolic ID: define a RandomID (to guarantee a distinct hash on each instance).
In the actual implementation, this should happen already within the domain model,
not when constructing the diff (obviously of course...)
This change also adds a mutation sequence to inject the actual track name
same pattern as the existing EntryID, i.e. a human readable symbol plus a hash
but the hash is just random, instead of deriving it from the symbol text.
Use case is when we explicitly need a distinct identity, even when the
human readable symbolic name is the same. Actual example: the fork root in the timeline
...it should have been explicit from start, since there is no point
in converting an EntryID into a plain flat string without further notice
this became evident, when the compiler picked the string overload on
MakeRec().genNode(specialID)
...which is in compliance to the rules, since string is a direct match,
while BareEntryID would be an (slicing) upcast. However, obviously we
want the BareEntryID here, and not an implicit string conversion,
thereby discarding the special hash value hidden within the ID
so this seems to be the better approach for dealing with this insidious problem.
In some cases -- as here most prominently with the root track within the timeline --
we have to care within the domain model to prepare unique ids even for sub objects
treated as attributes. In the actual case, without that special attention,
all timelines would hold onto an attribute "fork" with the same ID, based
on the type of the nested object plus the string "fork". Thus all root track
representations in the GUI would end up listening to the same ID on the UI-Bus...
...rather extend the "object builder" DSL notation to allow passing in a given EntryID literally.
Rationale is, we should handle the problem of unique IDs on the level of the domain model.
If we attempt to "fix" this within GenNode, the price would be to make the ETD creation stateful
this is not a problem, strictly sepaking, locally.
But it becomes a problem once the GUI uses those attribute IDs
as Element-IDs for tangible UI entities, which need to be uniquely
addressable via the UI-Bus.
An obvious solution is to inject randomness into the Attribute ID hash
...otherwise we'll get several seemingly identical Timeline tabs in the UI,
since this fake function just sends an INS for each newly injected Timeline,
and there is no deduplication in the UI (we assume that in a real session
and timeline-IDs will be unique)
these recursively nested helper entities work together with the TimelineCanvas
and enable the latter to draw the track background in the Timeline Widget and
to find out about the vertical coordinates where to place content (Clip, Effects, Markers)
Gtk::Viewport allows to add the ability to scroll a partial view window
for a container larger than the available display area. The position
and movement of this window is controlled by Gtk::Adjustments,
which can be located elsewhere.
Here we use the existing Adjustments of the ScrolledWindow
holding the body canvas; this setup makes the header pane follow
the scroll movements of the body
bottom line is to do most autmatically, and to establish a slave-relation
navigation-area -> timeline-ruler
header-pane-content -> corresponding track-body
this can be accomplished mostly by connecting the aproprieate signals,
thus these widgets will live within the Layout-Manager, which consequently
is renamed into TimelineLayout
the solution idea is to use a helper frame, and an "anchor functor",
which is passed down from the respective parent context, and which
does the actual work of injecting the child widgets at the apropriate
position within the parent display.
seems to work surprisingly well...
the diff application poceeds in the GUI up to the point
where the TrackPresenter need to be inserted into a two-fold display context
To drive the timeline display in the UI ahead, the plan is to have
a faked action, which injects dummy population diff messages into the GUI,
resulting in the build-up of a typical simple session timeline
decision: for now we will represent *every* Timeline present in the Session.
Later it would also possible to skip some representation; however we'd need
a way to store such presentation state such that we'd be able to get at this
persisted stat right at this point here, when processing the Diff.
other than the regular way of building an object,
we do expect a minimal structure to be sent right within the INS message.
Rationale: the standard way would allow for too much leeway and created
unwanted intermediary states. The non-standard way decided upon here
is well within the limits of our diff language
As it turns out, using the functional-notation form conversion
with *parentheses* will fall back on a C-style (wild, re-interpret) cast
when the target type is *not* a class. As in the case in question here, where
it is a const& to a class. To the contrary, using *curly braces* will always
attempt to go through a constructor, and thus fail as expected, when there is
no conversion path available.
I wasn't aware of that pitfall. I noticed it since the recently introduced
class TimelineGui lacked a conversion operator to BareEntryID const& and just
happily used the TimelineGui object itself and did a reinterpret_cast into BareEntryID
Problem is, the InteractionDirector, being the representation of the model root,
needs to manage and maintain the collection of "timelines". However, these
can not be widgets, rather, they need to attach to widgets living within
the GUI widget structure proper, i.e. within the TimelinePanel
proposed solution is to build a smart handle based on WLink,
but also delegating the DiffMutable interface
This involves a fundamental decision about how to build structures in the Lumiera UI:
They shall be solely created in response to diff messages. Which leads us to
introduce a new (and quite challenging) concept: the »DiffConstituent«
...these magical strings are already spreading dangerously throughout the code base
PS: also fixup for c6b8811af0 (broken whitespace in test definition)
This marks start of actual work on this fundamental task.
Extensive planning from 2016 is available, together with an almost
complete diff binding for the entities involved into timeline display.
''a new hope''
This was quite a long way until we're back at the point of
re-building the timeline anew.
Stash the canvas research code to make room for new things to come
this turned out to be more tricky than expected.
When we initially configure the UI and invoke this->show_all(),
seemingly some draw-callbacks will be scheduled into the event loop.
Just set_visible(false) on the relevant buttons directly after that call
will have no effect (since the widget is still hidden at that point anyway,
it is not yet mapped and realised).
Thus we need to schedule a callback with the Glib::signal_idle(),
so our state detection runs after the initial mapping of the UI
NOTE: there is a minor itch, which I don't address right now:
when adding the error state and thus revealing the additional buttons,
the error log grabs some additional horizontal space, even while there
would be ample space for the additional buttons within the button bar.
When the error state is cleared and the buttons thus hidden again,
the additional horizontal space is dropped and the error log gets
narrower. Probably we'd need some special GTK call to re-allocate
the required space properly
solved by temporarily adding a CSS class.
Mostly this was an issue of writing the Stylesheet properly.
Hint: use the GTK+ inspector, i.e. run with
GTK_DEBUG=interactive target/lumiera
Even while we (still) have the goal to ship our own stylesheet and provide
the typical subdued media-aplication look, right now this porting and styling effort (#1023)
is unfinished and handled with rather low priority (writing code is more important
than toying with styles and looks).
This alternative stylesheet is meant to be used with a typical "light" desktop theme.
We'll add just the bare minimum of definitions to make lumiera work well in that setup.
And right now, I'll use that setup to continue with my development work
In GTK-2 you'd always use a theming engine.
During the transitional period, GTK-3 followed that path,
but at the point when the new CSS based system was mature enough,
this approach was discouraged, since it means to ship additional
executable code and defeats the purpose of configuring the look
of the application through one coherent set of rules.
As it turns out, debian continued to ship the 'adwaita' for some
extended transitional period, but in Stretch it is gone, which broke
our half-way ported stylesheet.
This changeset removes the use of a theming engine, and fixes tha
basic look by adding the background-color: inherit, which pushes aside
many (but not all) of the default theme definitions.
However, our stylesheet remains mostly broken and TODO for now
Basically we create a pair of marks, with left/right gravity and then
inject the content between. Unfortunately, when the insert position
is the very end of the buffer (which it always is), this trick
leads to nesting the marked regions into each other.
As a remedy, we first insert the trailing newline,
and then attach the insert position one step before
discard all other info log messages and retain only the entries marked as error.
This is also a proof-of-concept regarding position bookmarks and markup.
Implemented by populating a new buffer and swapping it into place.
== possible Scenario ==
1. Gui: sigTerm invoked
2. last Subsystem -> cleans all remaining Subsy entries
3. main-Thread wakens
4. leaves main() und undloads the GUI plug-in
5. which destroys the `DependencyFactory<LocationQuery>` placed in static memory
6. the Gui-Thread returns from sigTerm() and invokes `~UiManager()`
7. which indirectly deregisters through `InteractionDirector` the `LocationQuery` Service
8. `DependInject::Service::shutdown()` grabs the Lock ==> **BOOM**
== Solution ==
Ensure all dtors of the UI backbone are invoked ''prior'' to calling sigTerm()
and especially our provisional dummy code to execute some commands "right here"
should also check and raise captured exceptions from command invocation
As it turns out, several problems reinforce each other
- lumiera error does not properly propagate the cause message
- our test/dummy code does not check the ExecResult
- thus the exception is detected rather accidentally, when entring the next sync/wait state
- emergency shutdown is chaotic in its very nature (this is well known...)
- but especially triggerShutdown is not airtight and might die...
- causing the shutdown to hang....
And last but not least, a ZombieCheck tripwire got triggered,
but unfortunately I was unable to get hold of the zombie iteself
test_meta_markAction always produces a state mark with payload type string.
However, the model::Tangible expects a bool payload when handling the "expand" mark.
- add diagnostics to lib::variant to indicate expected and actual payload type
- attempt to fix with boost::lexical_cast; this is insufficient, since
you'd expect such a function to understand "true" and "false" etc.
Moreover, raising this exception causes emergency shutdown, which
flounders due to triggering a ZombieCheck. Interesting.
The very backbone structure of the Lumiera UI, the UI-Bus is now fully defined
and proven to be operative, including asynchronous dispatch of messages
an a generic notification mechanism
A communication chain, triggered from a button in a non-modal dialog box,
passing invocation into another thread, dispatched by the ProcDispatcher,
then again passing thread boundaries to push a response back into the UI.
This is a milestone, and integrates several components built during the last years.
- a text input field
- a trigger to invoke the showInfo function on GuiNotification
- triggers to send state mark messages via GuiNotification into the UI-Bus
- a combo box to define the action-ID within those state mark messages
With these controls, it should be possible to execute all the variations
of the Tangible element protocol and verify the respective behaviour
has been coded up properly within the receiving ErrorLogDisplay widget
Note the key point (and the next step to code up) is for #1099 to
invoke a dummy/demo command in Proc-Layer, which in turn pushes an
reaction via the GuiNotification facade back into the UI asynchronously...
wrap up the helpers and wire the connection to the UI-Bus.
Then attempt a direct invocation, still within the GTK thread.
While this might seem as just some silly experiment, in fact it is
*** THE FUCKING FIRST TIME to transmit a visible action to a real widget ***
this links together and integrates various efforts achieved during the last years
Gtk::Notebook is a quite powerful container foundation to build complex dialog widgets with multible pages on tabs.
Hower, the construction, wiring an setup is notoriously tedious, due to the repetitiveness
and the sheer amount of child widgets spread over various pages.
This design draft is an attempt to mitigate the required boilerplate, without
overly much obscuring the structure. The basic idea is to package each page into
a locally defined child struct, which is actually heap allocated and managed automatically.
This way, each child page gets its own namespace, and wiring to other components
is made explicit by passing named ctor arguments -- while the overall structure
of building and wiring of widgets stays close to the habits of Gtkmm programming.
...which gives us already the base functionality required to run the first tests
- can be triggered from the Help menu
- non-modal dialog (Gtk::Dialog)
- attached as child / slave-Window to the current active workspace window
- window manager hint to keep it on top
- have a notebook control within the dialog
- attached (passively) to the UI-Bus
...just to decide not to follow-up too much on that topic right now.
As it turns out, GTK seems to be lacking in that respect. I have plotted
some ideas how we could work around that discrepancy in future...
And for this simple DemoGuiRoundtrip, we'll just use direct styling,
but we'll store a table of bookmarks for the error entries, allowing
us to add further features later on top
after an extended digression to fix our matcher for tests on the EventLog,
the new helper abstractions gui::model::Expander and gui::model::Revealer
are now covered and ready for use.
In this special case here, the controller uses both the Expander and Revealer
inherited from model::Tangible; yet both are wired to access the actual
display widget via the getter, and delegate to the Expander rsp. Revealer
located within the widget. Which in turn are wired when creating the widget
within the InfoboxPanel.
Bottom line -- we have a generic scheme now, and the actual implementation
is filled in as lambda, at the point where the component or widget is created
well... reduction in size of the debug build objects
turns out not to be so large as I hoped. But it is significant anyway,
about 3-4MB on the most affected test classes. Plus from now on we
do not repeat that code on other tests using the same features.
up to now, EventLog was header only, which seems to cause
a significant bloat in terms of generated code size, especially
in debug builds. One major source for this kind of "template bloat"
is the IterChainSearch, rsp. the filter and transformer iterators.
And since EventLog is not meant for performance critical application code,
but rather serves as helper for writing unit tests, an obvious remedy is
to move that problematic part of the code down into a dedicate translation
unit, instead of using inline functions. To prepare this refactoring,
some var arg (templated) API funcitons need to be segregated.
For the before / after chaining search functions,
we now do one single step in the respective direction before evaluating
the new (next) filter condition. However, we also need to *deactivate* the
previous condition, otherwise that single "step" might cause us to jump
or even exhaust the underlying filter, due to the old filter condition
still being applied.
due to the lack of real backtracking, the existing solution
relied on a quirk, and started the before / after chained search
conditions /at/ the current element, not after / before it.
Now we're able to remove this somewhat surprising behaviour, yet to do so
we also need to introduce basic "just search" variations of all search
operations, in order to define the initial condition for a chained search.
Without that, the first condition in a chain would never be able to
match on the header entry of the log
- need to use dedicated steps in the chain for every added condition now
- seems to break the logic on tests on non-match.
This doesn't come as a surprise, since backtracking can be expected
to reveal additional solutions.
NOTE: some tests broken, to be investigated
est-event-log-test.cpp:228: thread_1: verify_callLogging: (log.ensureNot("fun").after("fun").after("fun2"))
...which can be achieved by checking the backtracking loop
always right after the non-backtracking iteration, exploiting
the fact that the guard conditions of both are complimentary.
So the only case when we'd actually enter the backtracking
loop after regular iteration would precisely be when
we drop down due to exahausting an upper layer.
The result now reads
"sausage-bacon-tomato-and-spam-spam-bacon-spam-tomato-and-spam-bacon-tomato-and-bacon-tomato-and-tomato-and"
...which sounds correct, yay!
...since usually such evaluation layers are finally wrapped into
an IterableDecorator and then presented as TreeEplorer -- an exercise
we do not want to perform here, since it is pointless in the typicall
use case. The IterChainSearch is already meant to be ready-for-use.
Thus, instead of wrapping again, the pragmatic solution is simply
to overload the missing operator++ and make it call the augmented
iterNext() function. Related to this, we also need to ensure
proper operation in case no further expansion is mandated
...seems basically sane now.
Just we still need to wrap it one more time into IterableDecorator;
which means the overall scheme how to build and package the whole pipeline
is not correct yet.
Maybe it is not possible to get it packaged all into one single class?
on closer investigation it turned out that the logic of the
first design attempt was broken altogether. It did not properly
support backtracking (which was the reason to start this whole
exercise) and it caused dangling references within the lambda
closure once the produced iterator pipeline was moved out
into the target location.
Reasoning from first principles then indicated that the only sane
way to build such a search evaluation component is to use *two*
closely collaborating layers. The actual filter configuration
and evaluation logic can not reside and work from within the
expander. Rather, it must sit in a layer on top and work in
a conventional, imperative way (with a while loop).
Sometimes, functional programming is *not* the natural way
of doing things, and we should then stop attempting to force
matters against their nature.
this is an rather obvious extension to the TreeExplorer framework.
In some cases, client code wants to define its own very specific
processing layers, beyond of what can be done with filters and
transformers. Obviously, writing such a custom layer requires
intimate knowledge about the internals of TreeExplorer
the actual use case prompting this extension is IterChainSearch;
it turned out that the original design can not be implemented,
unless the resulting object is non-copyable (which violates
the basic traits of a TreeExplorer based pipeline).
Up to now, we had a very simplistic configuration option just
to search for a match, and we had the complete full-blown reconfiguration
builder option, which accepts a functor to work on and reconfigure the
embedded Filter chain.
It occurred to me that in many cases you'd rather want some intermediary
level of flexibility: you want to replace the filter predicate entirely
by some explicitly given functor, yet you don't need the full ability
to re-shape the Filter chain as a whole. In fact the intended use case
for IterChainSearch (which is the EventLog I am about to augment with
backtracking capabilities) will only ever need that intermediate level.
Thus wer're adding this intermediary level of configurability now.
The only twist is that doing so requires us to pass an "arbitrary function like thing"
(captured by universal reference) through a "layer of lambdas". Which means,
we have to capture an "arbitrary thingie" by value.
Fortunately, as I just found out today, C++14 allows something which comes
close to that requirement: the value capture of a lambda is allowe to have
an intialiser. Which means, we can std::forward into the value captured
by the intermediary lambda. I just hope I never need to know or understand
the actual type this captured "value" takes on.... :-)
with the augmented TreeExplorer, we're now able to get rid of the
spurious base layer, and we're able to discard the filter and
continue with the unfiltered sequence starting from current position.
build a special feature into the Explorer component of TreeExplorer,
causing it to "lock into" the current child sequence and discard
all previous sequences from the stack of child explorations
There is an asymetry, insofar the base layer configuration is
evaluated immediately, causing the MutableFilter to be reconfigured
and forwarded to the first match.
to the contrary, when configuring an additional layer, we just
add it to the chain, but then need to iterate once to cause
this configuration actually to be unfolded onto the stack
...which just turns the pipeline into exhausted state,
instead of raising an Assertion failure
The point is, expandChildren() does not guard itself,
since it _requires_ an non-empty iterator as precondition.
Thus, any function downstream, which invokes expandChildren(),
has to check and guard this call apropriately.
In the concrete case at hand we just stop adding further constraints
when the pipeline is already in exhausted state
...the solution built thus far was logically broken, since it retained the unfiltered
source sequence within the base layer. Thus it would backtrack into this unfiltered
sequence eventually.
The idea was to build a special treatment for attaching the first filter condition;
in fact the first one does not need to be added to the chain, but rather should be
planted directly into the base layer.
WIP: this is a solution draft, but does not work yet
- when attaching the base layer, the filter is pulled twice
- an overconstrained filter raises an Assertion failure
(instead of just transitioning into exhausted state)
So we have now a reworked version of the internals of TreeExplorer in place.
It should be easier to debug template instantation traces now, since most
of the redundancy on the type parameters could be remove. Moreover, existing
pipelines can now be re-assigned with similarily built pipelines in many cases,
since the concrete type of the functor is now erased.
The price tag for this refactoring is that we have now to perform a call
through a function pointer on each functor invocation (due to the type erasure).
And seemingly the bloat in the debugging information has been increased slightly
(this overhead is removed by stripping the binary)
Here the design trardeoff becomes clearly visiblie
- on the plus side, we removed that spurous redundant info
from the template parameter, and we simplified functor rebinding
- but as a tradeoff, we now always have two std::function objects
nested into each other, which also means that at least the outer
object resides on the heap and /inevitably/ calls through a
function pointer, even in case the target function is a lambda,
simply because some type erasure happened, and the call site
does not know the actual type anymore
...step by step switch over to the new usage pattern.
Transformer should be the blueprint for all other functor usages.
The reworked solutions behaves as expected;
we see two functor invocations; the outer functor, which does
the argument adaptation, is allocated in heap memory
This does not touch the existing code-path,
but the idea is to use the _FunTraits directly from within the
constructor of the respective processing layer, and to confine the
knowledge of the actual FUN functor type to within that limited context.
Only the generic signature of the resulting std::function need to be
encoded into the type of the processing component, which should help
to simplify the type signatures
...which still needs to be the *concrete* signature of the funcition to pass,
but we'll attempt to loosen that requirement in the next refactoring steps
...and in preparation start with some renamings...
The overall goal is to simplify the type signatures and thereby
to make the generates pipelines more assignment compatible.
The debugging experience form the last days indicated that the
current design is not maintainable on the long run. Both the
template instantiation chains and the call stacks are way to
complicated and hard to understand and diagnose
It is essential that we pass the current state of the filter
into the expand functor, where it needs to be copied (once!)
to create a child state, which can then be augmented.
This augmented state is then pushed onto a stack, to enable backtracking.
Due to the flexible adapters and the wrapping into the TreeExplorer builder,
we ended up performing several spurious copies on the current state
...based on a monadic tree expansion: we define a single step,
which takes the current filter configuration and builds the next
filter configuration, based on a stored chain of configuration functions
The actual exhausting depth-first results just by the greedy application pattern,
and uses the stack embedded in the "Explorer" layer of TreeExplorer
..this resolves the most challenging part of the construction work;
we use the static helper functions to infer a type and construct a suitable
processing pipeline and we invoke the same helper to initialise the base class
in the ctor.
Incidentally... we can now drop all the placeholder stubs,
since we now inherit the full iterator and child explorer API.
The test now starts actually to work... we get spam and sausage!
TODO: now actually fill in the expand functor such as to pick the
concrete filter step in the chain from a sequence of preconfigured
filter bindings
...now matters start to get really nasty,
since we have to pick up an infered type from a partially built pipeline
and use it to construct the signature for a functor to bind into the more elaborate complete pipeline
this is a tricky undertaking, since our treeExplore() helper constructs
a complex wrapped type, depending on the actual builder expressions used.
Solution is to use decltype on the result of a helper function,
and let the _DecoratorTraits from TreeExplorer do the necessary type adaptations
when adapting a functor, the wrapper automatically decides
if this functor was meant to be used in "monadic style" (value -> new monad)
or in "manipulate state-core style" (iter& -> iter)
Unfortunately, in some cases functions accepting a partially built pipeline
will be classified in the wrong way, due to the fact that IterableDecorator
has a templated wildcard constructor and thus seems to accept the conversion
value_type -> IterableDecorator. This causes the "monadic style" to be chosen
erroneously, and leads to a template instantiation failure just at the point when
the generated functor will be used.
The solution is to explicitly *rule out* the monadic usage style
in all those cases, where the *argument* of the functor to bind can
in fact be directly constructed / converted from the source iterator
or state core. Because, if that is the case
- it is superfluous explicitly to dereference the source iterator;
the functor can do the same
- chances are that the "manipulation style" was intended
(as was the case in the concrete example at hand here)
...it should have been this way all the time.
Generic code might otherwise be ill guided to assume a conversion
from the Iterator to its value type, while in fact an explicit dereferentiation is necessary
Need to be more careful when eating trailing spaces after a std::string.
Because the full-blown type is a template, sometimes the compiler adds a spurious
additional space behind the closing angle bracket, due to the now obsolete
"maximum munch rule" of C++98 -- to prevent closing angle brackets to become '>>'.
But in other cases, some type adornments or other language identifiers follow the
string type, like e.g. 'const'. In those cases, the trailing space must be retained.
We solve this by a look-ahead assertion in the regular expression:
consume the trailing space _only_ if a non-word character follows (like '>').
The intention is to augment the iterator based (linear) search
used in EventLog to allow for real backtracking, based on a evaluation tree.
This should be rather staight forward to implement, relying on the
exploreChildren() functionality of TreeExplorer. The trick is to package
the chained search step as a monadic flatMap operation
while this is basically a drop-in replacement,
it marks the switch to the monadic evaluation technology,
which is prerequisite for building real backtracking into the search.
we did an unnecessary copy of the argument, which was uncovered
by the test case manipulating the state core.
Whew.
Now we have a beautiful new overengineered solution
outift the Filter base class with the most generic form of the Functor
wrapper, and rather wrap each functor argument individually. This allows
then to combine various kinds of functors
...this solution works, but has a shortcoming:
the type of the passed lambdas is effectively pinned to conform
with the signature of the first lambda used initially when building the filter.
Well, this is the standard use case, but it kind of turns all the
tricky warpping and re-binding into a nonsense excercise; in this form
the filter can only be used in the monadic case (value -> bool).
Especially this rules out all the advanced usages, where the filter
collaborates with the internals of the source.
while this is basically just code code cosmetics,
at least it marks this as a very distinct special case,
and keeps the API for the standard Filter layer clean.
a quite convoluted construct built from several nested generic lambdas.
When investigated in the debugger, the observed addresses and the
invoked code looks sane and as expected.
The intention is to switch from the itertools-based filter
to the filter available in the TreeExplorer framework.
Thus "basically" we just need to copy the solution over,
since both are conceptually equivalent.
However...... :-(
The TreeExplorer framework is designed to be way more generic
and accepts basically everything as argument and tries to adapt apropriately.
This means we have to use a lot of intricate boilerplate code,
just to get the same effect that was possible in Itertools with
a simple and elegant in-place lambda assignment
