From 40f003a9621fdf4bedd77ee0f32535d9946ff306 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 9 Dec 2022 23:13:27 +0100 Subject: [PATCH] Timeline: stress-test with excessive zoom-out reveals further weakness 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() --- src/stage/model/zoom-window.hpp | 47 ++----- tests/stage/model/zoom-window-test.cpp | 8 ++ wiki/thinkPad.ichthyo.mm | 184 ++++++++++++++++++++++++- 3 files changed, 198 insertions(+), 41 deletions(-) diff --git a/src/stage/model/zoom-window.hpp b/src/stage/model/zoom-window.hpp index 544187266..7d2a19a49 100644 --- a/src/stage/model/zoom-window.hpp +++ b/src/stage/model/zoom-window.hpp @@ -679,39 +679,21 @@ namespace model { /** Assertion helper: resulting pxWidth matches expectations */ static void - __assertMatchesExpectedPixWidth (Rat zoomFactor, FSecs duration, uint pxWidth) + ENSURE_matchesExpectedPixWidth (Rat zoomFactor, FSecs duration, uint pxWidth) { - auto sizeAtRequestedScale = calcPixelsForDurationAtScale (zoomFactor, duration); + auto sizeAtRequestedScale = approx(zoomFactor) * approx(duration); ENSURE (abs(pxWidth - sizeAtRequestedScale) <= 1 ,"ZoomWindow: established size or metric misses expectation " "by more than 1px. %upx != %1.6f expected pixel." , pxWidth, sizeAtRequestedScale); } - static void - __assertMetricConforms2pxWidth (Rat zoomFactor, FSecs duration, uint pxWidth) - { - if (util::can_represent_Product(zoomFactor, duration)) - { - ENSURE (pxWidth == rational_cast (zoomFactor*duration) - ,"ZoomWindow: established zoom factor misses expected " - "width in pixels: %upx != %upx (resulting)" - , pxWidth, rational_cast (zoomFactor*duration)); - } - else - { - auto sizeAtRequestedScale = approx(zoomFactor) * approx(duration); - ENSURE (pxWidth == uint(sizeAtRequestedScale + 0.5) - ,"ZoomWindow: established zoom factor misses expected " - "width in pixels: %upx != %1.8f px (resulting)" - , pxWidth, sizeAtRequestedScale); - } - } - - /** @remark indirect calculation path to avoid overflow on large durations */ + /** calculate `rational_cast (zoomFactor * duration)` + * @remark indirect calculation path to avoid overflow on large durations + */ static int64_t calcPixelsForDurationAtScale (Rat zoomFactor, FSecs duration) - {// calculate rational_cast (zoomFactor * duration) + {// break down the integer division into several steps... auto zn = zoomFactor.numerator(); auto zd = zoomFactor.denominator(); auto dn = duration.numerator(); @@ -776,7 +758,7 @@ namespace model { REQUIRE (changedMetric > 0); REQUIRE (afterWin_> startWin_); FSecs dur{afterWin_-startWin_}; - uint pxWidth = rational_cast (px_per_sec_*dur); + uint pxWidth = calcPixelsForDurationAtScale (px_per_sec_, dur); dur = Rat(pxWidth) / detox (changedMetric); dur = min (dur, MAX_TIMESPAN); dur = max (dur, MICRO_TICK); // prevent window going void @@ -791,7 +773,7 @@ namespace model { // re-check metric to maintain precise pxWidth px_per_sec_ = conformMetricToWindow (pxWidth); ENSURE (_FSecs(afterWin_-startWin_) < MAX_TIMESPAN); - __assertMatchesExpectedPixWidth (changedMetric, dur, pxWidth); + ENSURE_matchesExpectedPixWidth (changedMetric, afterWin_-startWin_, pxWidth); } /** @@ -939,18 +921,7 @@ namespace model { changedMetric = max (changedMetric, px / maxSaneWinExtension(px)); changedMetric = min (detox(changedMetric), ZOOM_MAX_RESOLUTION); if (changedMetric == px_per_sec_) return; - - Rat changeFactor{changedMetric / px_per_sec_}; - FSecs dur{afterWin_ - startWin_}; - dur /= changeFactor; - if (dur > FSecs{afterAll_ - startAll_}) - {// limit to the overall timespan... - startWin_ = startAll_; - afterWin_ = afterAll_; - px_per_sec_ = conformMetricToWindow(px); - } - else - mutateDuration (dur, px); + conformWindowToMetric (changedMetric); ensureInvariants (px); } diff --git a/tests/stage/model/zoom-window-test.cpp b/tests/stage/model/zoom-window-test.cpp index fb98b4411..5737c5205 100644 --- a/tests/stage/model/zoom-window-test.cpp +++ b/tests/stage/model/zoom-window-test.cpp @@ -743,6 +743,14 @@ namespace test { CHECK (rational_cast(100000000000_r/109951052826533333) == 9.09495611e-07f); // theoretical value CHECK (rational_cast(win.px_per_sec()) == 9.09503967e-07f); // value actually chosen CHECK (win.px_per_sec() == 508631_r/559239974093); + + /*--Test-6-----------*/ + win.setMetric (bruteZoom); // And now put one on top by requesting excessive zoom-out: + CHECK (_raw(win.overallSpan().duration()) == 614891469123651720); // overall canvas duration not changed + CHECK (_raw(win.visible().duration()) == 109951162777600000); // window duration only slightly increased + CHECK (508631_r/559239974093 > 15625_r/17179869184); // zoom factor slightly reduced + CHECK (win.px_per_sec() == 15625_r/17179869184); // and now hitting again the minimum limit + CHECK (win.pxWidth() == MAX_PX_WIDTH); // pixel count unchanged at maximum } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 87839c7fa..a4ffeeda8 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -40057,7 +40057,9 @@ - + + + @@ -40227,17 +40229,18 @@

+ - + - + @@ -40362,6 +40365,181 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ eindeutig: wrap-around in der relativen Änderung +

+ + + + + + + + + + + + + + + + + + + +

+ ...welche ich inzwischen nahezu komplett einmal umgepflügt habe... +

+ + + + + + + + + + + + +

+ weil hier... +

+
    +
  • + zum Einen eine sehr große Zahl auf einen beliebigen Bruchfaktor trifft +
    • +
  • + und außerdem als Resultat eine Beliebige Zahl entsteht, die nicht auf Time::SCALE quantisiert ist +
    • +
+ + + + + + + + + +

+ und redundant: macht nicht conformWindowToMetric()  inzwischen genau dassselbe?? +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ und die Testsuite ist auf Anhieb GRÜN +

+ + + + + + + + + + + + + + + + + + + + + + + + + +