From 700710135728b6e5c77b53a68dc99b364c618b8b Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 29 Nov 2022 02:00:41 +0100 Subject: [PATCH] Timeline: safely calculate the fraction of a very large timespan ...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??) --- src/lib/rational.hpp | 7 ++ src/lib/time/time.cpp | 5 +- src/lib/time/timevalue.hpp | 1 + src/stage/model/zoom-window.hpp | 50 +++++++-- tests/stage/model/zoom-window-test.cpp | 31 +++++- wiki/thinkPad.ichthyo.mm | 137 ++++++++++++++++++++++++- 6 files changed, 215 insertions(+), 16 deletions(-) diff --git a/src/lib/rational.hpp b/src/lib/rational.hpp index 9fbe260e6..03b6d2a12 100644 --- a/src/lib/rational.hpp +++ b/src/lib/rational.hpp @@ -139,6 +139,13 @@ namespace util { < 63; } + inline bool + can_represent_Product (Rat a, Rat b) + { + return can_represent_Product(a.numerator(), b.numerator()) + and can_represent_Product(a.denominator(), b.denominator()); + } + inline bool can_represent_Sum (Rat a, Rat b) { diff --git a/src/lib/time/time.cpp b/src/lib/time/time.cpp index 4991a454b..c25ba27c7 100644 --- a/src/lib/time/time.cpp +++ b/src/lib/time/time.cpp @@ -283,8 +283,11 @@ namespace time { /** constant to indicate "no duration" */ - const Duration Duration::NIL (Time::ZERO); + const Duration Duration::NIL {Time::ZERO}; + /** maximum possible temporal extension */ + const Duration Duration::MAX {Offset{Time::MIN, Time::MAX}}; + }} // namespace lib::Time diff --git a/src/lib/time/timevalue.hpp b/src/lib/time/timevalue.hpp index f56983f2e..85946fcfb 100644 --- a/src/lib/time/timevalue.hpp +++ b/src/lib/time/timevalue.hpp @@ -480,6 +480,7 @@ namespace time { { } static const Duration NIL; + static const Duration MAX ; void accept (Mutation const&); diff --git a/src/stage/model/zoom-window.hpp b/src/stage/model/zoom-window.hpp index 42f39cc1e..5d59ff262 100644 --- a/src/stage/model/zoom-window.hpp +++ b/src/stage/model/zoom-window.hpp @@ -111,6 +111,8 @@ namespace model { using util::Rat; using util::rational_cast; + using util::can_represent_Product; + using util::reQuant; using util::min; using util::max; @@ -166,7 +168,7 @@ namespace model { const FSecs DEFAULT_CANVAS{23}; const Rat DEFAULT_METRIC{25}; const uint MAX_PX_WIDTH{1000000}; - const FSecs MAX_TIMESPAN{_FSecs(Time::MAX-Time::MIN)}; + const FSecs MAX_TIMESPAN{_FSecs(Duration::MAX)}; const FSecs MICRO_TICK{1_r/Time::SCALE}; /** Maximum quantiser to be handled in fractional arithmetics without hazard. @@ -174,15 +176,15 @@ namespace model { * DENOMINATOR * Time::Scale has to stay below INT_MAX, with some safety margin */ const int64_t LIM_HAZARD{int64_t{1} << 40 }; + const int64_t HAZARD_DEGREE{util::ilog2(LIM_HAZARD)}; inline int - toxicDegree (Rat poison) + toxicDegree (Rat poison, const int64_t THRESHOLD =HAZARD_DEGREE) { - const int64_t HAZARD_DEGREE{util::ilog2(LIM_HAZARD)}; int64_t magNum = util::ilog2(abs(poison.numerator())); int64_t magDen = util::ilog2(abs(poison.denominator())); int64_t degree = max (magNum, magDen); - return max (0, degree - HAZARD_DEGREE); + return max (0, degree - THRESHOLD); } } @@ -426,7 +428,7 @@ namespace model { setVisiblePos (Rat percentage) { FSecs canvasDuration{afterAll_-startAll_}; - anchorWindowAtPosition (canvasDuration*percentage); + anchorWindowAtPosition (scaleSafe (canvasDuration, percentage)); fireChangeNotification(); } @@ -495,18 +497,48 @@ namespace model { * is thus specific to the ZoomWindow implementation. To sanitise, the denominator * is reduced logarithmically (bit-shift) sufficiently and then used as new quantiser, * thus ensuring that both denominator (=quantiser) and numerator are below limit. + * @warning the rational number must not be to large overall; this heuristic will fail + * on fractions with very large numerator and small denominator — however, for + * the ZoomWindow, this case is not relevant, since the zoom factor is limited, + * and other usages of rational numbers can be range checked explicitly. * @note the check is based on the 2-logarithm of numerator and denominator, which is * pretty much the fastest possibility (even a simple comparison would have - * to do the same). Values below threshold are simply passed-through. + * to do the same). Values below threshold are simply passed-through. */ static Rat detox (Rat poison) { int toxicity = toxicDegree (poison); - return toxicity ? util::reQuant(poison, poison.denominator() >> toxicity) + return toxicity ? reQuant (poison, max (poison.denominator() >> toxicity, 64)) : poison; } + /** + * Scale a possibly large time duration by a rational factor, while attempting to avoid + * integer wrap-around. Obviously this is only a heuristic, yet adequate within the + * framework of ZoomWindow, where the end result is pixel aligned anyway. + */ + static FSecs + scaleSafe (FSecs duration, Rat factor) + { + auto approx = [](Rat r){ return rational_cast (r); }; + + if (not util::can_represent_Product(duration, factor)) + { + if (approx(MAX_TIMESPAN) < approx(duration) * approx (factor)) + return MAX_TIMESPAN; // exceeds limits of time representation => cap the result + + // slightly adjust the factor so that the time-base denominator cancels out, + // allowing to calculate the product without dangerous multiplication of large numbers + factor = 1_r / reQuant (1_r/factor, duration.denominator()); + return detox (duration * factor); + } + else + // just calculate ordinary numbers... + return duration * factor; + } + + static Rat establishMetric (uint pxWidth, Time startWin, Time afterWin) { @@ -732,7 +764,7 @@ namespace model { FSecs duration{afterWin_-startWin_}; Rat posFactor = canvasOffset / FSecs{afterAll_-startAll_}; posFactor = parabolicAnchorRule (posFactor); // also limited 0...1 - FSecs partBeforeAnchor = posFactor * duration; + FSecs partBeforeAnchor = scaleSafe (duration, posFactor); startWin_ = startAll_ + (canvasOffset - partBeforeAnchor); establishWindowDuration (duration); startAll_ = min (startAll_, startWin_); @@ -815,6 +847,8 @@ namespace model { parabolicAnchorRule (Rat posFactor) { posFactor = util::limited (0, posFactor, 1); + if (toxicDegree(posFactor, 20)) // prevent integer wrap + posFactor = util::reQuant(posFactor, 1 << 20); posFactor = (2*posFactor - 1); // -1 ... +1 posFactor = posFactor*posFactor*posFactor; // -1 ... +1 but accelerating towards boundaries posFactor = (posFactor + 1) / 2; // 0 ... 1 diff --git a/tests/stage/model/zoom-window-test.cpp b/tests/stage/model/zoom-window-test.cpp index 623f88c53..08323ad11 100644 --- a/tests/stage/model/zoom-window-test.cpp +++ b/tests/stage/model/zoom-window-test.cpp @@ -399,9 +399,11 @@ namespace test { win.setVisiblePos(0.50); CHECK (win.visible() == TimeSpan(_t((40/2-16) -8), FSecs(16))); // window positioned to centre of canvas + CHECK (win.visible().start() == _t(-4)); // (canvas was already positioned asymmetrically) win.setVisiblePos(-0.50); CHECK (win.visible() == TimeSpan(_t(-16-40/2), FSecs(16))); // relative positioning not limited at lower bound + CHECK (win.visible().start() == _t(-36)); // (causing also further expansion of canvas) win.setVisiblePos(_t(200)); // absolute positioning likewise not limited CHECK (win.visible() == TimeSpan(_t(200-16), FSecs(16))); // but anchored according to relative anchor pos CHECK (win.px_per_sec() == 80); // metric retained @@ -580,13 +582,16 @@ namespace test { } - /** @test verify ZoomWindow code can handle "poisonous" Zoom-Factor parameters + /** @test verify ZoomWindow code can handle "poisonous" fractional number parameters + * - toxic zoom factor passed through ZoomWindow::setMetric(Rat) + * - toxic proportion factor passed to ZoomWindow::setVisiblePos(Rat) + * - indirectly cause toxic posFactor in ZoomWindow::anchorWindowAtPosition(FSecs) + * by providing a target position very far off current window location */ void safeguard_poisonousMetric() { - - ZoomWindow win{}; + ZoomWindow win{}; CHECK (win.visible() == win.overallSpan()); // by default window spans complete canvas CHECK (win.visible().duration() == _t(23)); // ...and has just some handsome extension CHECK (win.px_per_sec() == 25); @@ -624,6 +629,26 @@ namespace test { CHECK(856.350708f == rational_cast (_FSecs(win.visible().duration()))); CHECK (win.px_per_sec() == 575000000_r/856350691); // the new metric however is comprised of sanitised fractional numbers CHECK (win.pxWidth() == 575); // and the existing pixel width was not changed + + SHOW_EXPR(win.overallSpan()); + SHOW_EXPR(_raw(win.visible().duration())); + SHOW_EXPR(win.px_per_sec()); + SHOW_EXPR(win.pxWidth()); + win.setVisiblePos (poison); // Yet another way to sneak in our toxic value... + SHOW_EXPR(win.overallSpan()); + SHOW_EXPR(_raw(win.overallSpan())); + SHOW_EXPR(_raw(win.overallSpan().duration())); + SHOW_EXPR(_raw(win.visible().duration())); + SHOW_EXPR(win.px_per_sec()); + SHOW_EXPR(win.pxWidth()); + SHOW_EXPR(_raw(win.overallSpan().duration()) * rational_cast (poison)) + Time targetPos{TimeValue{gavl_time_t(_raw(win.overallSpan().duration()) * rational_cast (poison))}}; + SHOW_EXPR(targetPos); + SHOW_EXPR(_raw(targetPos)); + SHOW_EXPR(_raw(win.visible().start())) + SHOW_EXPR(_raw(win.visible().end())) + SHOW_EXPR(bool(win.visible().start() < targetPos)) + SHOW_EXPR(bool(win.visible().end() > targetPos)) } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 84d681183..cf66a25cd 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -39764,9 +39764,48 @@ + + + + + + + + + + + + + + + + + +

+ und zwar dann, wenn auch noch die Window-Parameter extrem sind — dann sieht die Lage ziemlich hoffnungslos aus +

+ + + + + + + + + + +

+ ...denn dadurch würde man die kreuzweise Multiplikation verhindern +

+ + + + + + @@ -39883,8 +39922,12 @@ - + + + + + @@ -39904,7 +39947,7 @@ - + @@ -40275,12 +40318,37 @@ + + + + + + + + + + + + + + + + + +

+ Und zwar, wenn der Nenner viel kleiner ist als der Zähler, und der Zähler extrem groß. Dann würde nämlich die Ganzzahl-Division keine signifikante Verringerung der Dimension bewirken, und die anschließende re-Quantisierung das Ergebnis (bedingt durch die Normierung auf einen gemeinsamen Nenner) sogar noch vergrößern +

+ + + + + - - + + @@ -40300,6 +40368,26 @@ + + + + + + + + + + + + + + + + + + + + @@ -40307,6 +40395,47 @@ + + + + + + + + + + + + + + + +

+ #--◆--# _raw(win.overallSpan().duration()) ? = 307445734561825860 +

+

+ #--◆--# _raw(targetPos) ?                    = 206435633551724864 +

+

+ #--◆--# _raw(win.visible().start()) ?        =   2248731193323487 +

+

+ #--◆--# _raw(win.visible().end()) ?          =   2248732049674178 +

+

+ #--◆--# bool(win.visible().start() < targetPos) ? = 1 +

+

+ #--◆--# bool(win.visible().end() > targetPos) ? = 0 +

+ + + + + + + +