From a738e3f82e34b9980e21cc3dba01258aa7200f71 Mon Sep 17 00:00:00 2001 From: whitequark Date: Wed, 18 Jul 2018 23:11:49 +0000 Subject: [PATCH] Eliminate imperative redraws. This commit removes Platform::Window::Redraw function, and rewrites its uses to run on timer events. Most UI toolkits have obscure issues with recursive event handling loops, and Emscripten is purely event- driven and cannot handle imperative redraws at all. As a part of this change, the Platform::Timer::WindUp function is split into three to make the interpretation of its argument less magical. The new functions are RunAfter (a regular timeout, setTimeout in browser terms), RunAfterNextFrame (an animation request, requestAnimationFrame in browser terms), and RunAfterProcessingEvents (a request to run something after all events for the current frame are processed, used for coalescing expensive operations in face of input event queues). This commit changes two uses of Redraw(): the AnimateOnto() and ScreenStepDimGo() functions. The latter was actually broken in that on small sketches, it would run very quickly and not animate the dimension change at all; this has been fixed. While we're at it, get rid of unused Platform::Window::NativePtr function as well. --- src/graphicswin.cpp | 48 ++++++++++++++------------- src/group.cpp | 3 +- src/platform/gui.h | 7 ++-- src/platform/guigtk.cpp | 11 +------ src/platform/guimac.mm | 11 +------ src/platform/guinone.cpp | 2 +- src/platform/guiwin.cpp | 11 +------ src/solvespace.cpp | 24 +++++++------- src/solvespace.h | 6 ++-- src/textscreens.cpp | 71 +++++++++++++++++++++++++--------------- src/ui.h | 15 +++++++-- 11 files changed, 106 insertions(+), 103 deletions(-) diff --git a/src/graphicswin.cpp b/src/graphicswin.cpp index 9c85f35..b44f1a9 100644 --- a/src/graphicswin.cpp +++ b/src/graphicswin.cpp @@ -423,6 +423,9 @@ void GraphicsWindow::AnimateOntoWorkplane() { Quaternion quatf = w->Normal()->NormalGetNum(); Vector offsetf = (SK.GetEntity(w->point[0])->PointGetNum()).ScaledBy(-1); + // If the view screen is open, then we need to refresh it. + SS.ScheduleShowTW(); + AnimateOnto(quatf, offsetf); } @@ -441,34 +444,37 @@ void GraphicsWindow::AnimateOnto(Quaternion quatf, Vector offsetf) { double mo = (offset0.Minus(offsetf)).Magnitude()*scale; // Animate transition, unless it's a tiny move. + int64_t t0 = GetMilliseconds(); int32_t dt = (mp < 0.01 && mo < 10) ? (-20) : (int32_t)(100 + 1000*mp + 0.4*mo); // Don't ever animate for longer than 2000 ms; we can get absurdly // long translations (as measured in pixels) if the user zooms out, moves, // and then zooms in again. if(dt > 2000) dt = 2000; - int64_t tn, t0 = GetMilliseconds(); - double s = 0; Quaternion dq = quatf.Times(quat0.Inverse()); - do { - offset = (offset0.ScaledBy(1 - s)).Plus(offsetf.ScaledBy(s)); - Quaternion quat = (dq.ToThe(s)).Times(quat0); - quat = quat.WithMagnitude(1); - projRight = quat.RotationU(); - projUp = quat.RotationV(); - window->Redraw(); + if(!animateTimer) { + animateTimer = Platform::CreateTimer(); + } + animateTimer->onTimeout = [=] { + int64_t tn = GetMilliseconds(); + if((tn - t0) < dt) { + animateTimer->RunAfterNextFrame(); - tn = GetMilliseconds(); - s = (tn - t0)/((double)dt); - } while((tn - t0) < dt); + double s = (tn - t0)/((double)dt); + offset = (offset0.ScaledBy(1 - s)).Plus(offsetf.ScaledBy(s)); + Quaternion quat = (dq.ToThe(s)).Times(quat0).WithMagnitude(1); - projRight = quatf.RotationU(); - projUp = quatf.RotationV(); - offset = offsetf; - Invalidate(); - // If the view screen is open, then we need to refresh it. - SS.ScheduleShowTW(); + projRight = quat.RotationU(); + projUp = quat.RotationV(); + } else { + projRight = quatf.RotationU(); + projUp = quatf.RotationV(); + offset = offsetf; + } + window->Invalidate(); + }; + animateTimer->RunAfterNextFrame(); } void GraphicsWindow::HandlePointForZoomToFit(Vector p, Point2d *pmax, Point2d *pmin, @@ -695,7 +701,6 @@ void GraphicsWindow::MenuView(Command id) { case Command::ONTO_WORKPLANE: if(SS.GW.LockedInWorkplane()) { SS.GW.AnimateOntoWorkplane(); - SS.ScheduleShowTW(); break; } // if not in 2d mode fall through and use ORTHO logic case Command::NEAREST_ORTHO: @@ -760,8 +765,8 @@ void GraphicsWindow::MenuView(Command id) { // Offset is the selected point, quaternion is same as before Vector pt = SK.GetEntity(SS.GW.gs.point[0])->PointGetNum(); quat0 = Quaternion::From(SS.GW.projRight, SS.GW.projUp); - SS.GW.AnimateOnto(quat0, pt.ScaledBy(-1)); SS.GW.ClearSelection(); + SS.GW.AnimateOnto(quat0, pt.ScaledBy(-1)); } else { Error(_("Select a point; this point will become the center " "of the view on screen.")); @@ -1175,9 +1180,8 @@ void GraphicsWindow::MenuRequest(Command id) { break; } // Align the view with the selected workplane - SS.GW.AnimateOntoWorkplane(); SS.GW.ClearSuper(); - SS.ScheduleShowTW(); + SS.GW.AnimateOntoWorkplane(); break; } case Command::FREE_IN_3D: diff --git a/src/group.cpp b/src/group.cpp index b906069..bddd2e4 100644 --- a/src/group.cpp +++ b/src/group.cpp @@ -291,9 +291,8 @@ void Group::MenuGroup(Command id, Platform::Path linkFile) { gg->activeWorkplane = gg->h.entity(0); } gg->Activate(); - SS.GW.AnimateOntoWorkplane(); TextWindow::ScreenSelectGroup(0, gg->h.v); - SS.ScheduleShowTW(); + SS.GW.AnimateOntoWorkplane(); } void Group::TransformImportedBy(Vector t, Quaternion q) { diff --git a/src/platform/gui.h b/src/platform/gui.h index c3b971f..a2512a7 100644 --- a/src/platform/gui.h +++ b/src/platform/gui.h @@ -135,7 +135,9 @@ public: virtual ~Timer() {} - virtual void WindUp(unsigned milliseconds) = 0; + virtual void RunAfter(unsigned milliseconds) = 0; + virtual void RunAfterNextFrame() { RunAfter(1); } + virtual void RunAfterProcessingEvents() { RunAfter(0); } }; typedef std::unique_ptr TimerRef; @@ -261,9 +263,6 @@ public: virtual void SetScrollbarPosition(double pos) = 0; virtual void Invalidate() = 0; - virtual void Redraw() = 0; - - virtual void *NativePtr() = 0; }; typedef std::shared_ptr WindowRef; diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp index 927e933..ec5df52 100644 --- a/src/platform/guigtk.cpp +++ b/src/platform/guigtk.cpp @@ -202,7 +202,7 @@ class TimerImplGtk : public Timer { public: sigc::connection _connection; - void WindUp(unsigned milliseconds) override { + void RunAfter(unsigned milliseconds) override { if(!_connection.empty()) { _connection.disconnect(); } @@ -982,15 +982,6 @@ public: void Invalidate() override { gtkWindow.get_gl_widget().queue_render(); } - - void Redraw() override { - Invalidate(); - Gtk::Main::iteration(/*blocking=*/false); - } - - void *NativePtr() override { - return >kWindow; - } }; WindowRef CreateWindow(Window::Kind kind, WindowRef parentWindow) { diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index fa2bb14..43e2b67 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -165,7 +165,7 @@ public: TimerImplCocoa() : timer(NULL) {} - void WindUp(unsigned milliseconds) override { + void RunAfter(unsigned milliseconds) override { SSFunction *callback = [[SSFunction alloc] initWithFunction:&this->onTimeout]; NSInvocation *invocation = [NSInvocation invocationWithMethodSignature: [callback methodSignatureForSelector:@selector(run)]]; @@ -984,15 +984,6 @@ public: void Invalidate() override { ssView.needsDisplay = YES; } - - void Redraw() override { - Invalidate(); - CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0, YES); - } - - void *NativePtr() override { - return (__bridge void *)ssView; - } }; WindowRef CreateWindow(Window::Kind kind, WindowRef parentWindow) { diff --git a/src/platform/guinone.cpp b/src/platform/guinone.cpp index fb047a1..d415e4f 100644 --- a/src/platform/guinone.cpp +++ b/src/platform/guinone.cpp @@ -85,7 +85,7 @@ SettingsRef GetSettings() { class TimerImplDummy : public Timer { public: - void WindUp(unsigned milliseconds) override {} + void RunAfter(unsigned milliseconds) override {} }; TimerRef CreateTimer() { diff --git a/src/platform/guiwin.cpp b/src/platform/guiwin.cpp index f8678af..469e052 100644 --- a/src/platform/guiwin.cpp +++ b/src/platform/guiwin.cpp @@ -244,7 +244,7 @@ public: } } - void WindUp(unsigned milliseconds) override { + void RunAfter(unsigned milliseconds) override { // FIXME(platform/gui): use SetCoalescableTimer when it's available (8+) sscheck(SetTimer(WindowHandle(), (UINT_PTR)this, milliseconds, &TimerImplWin32::TimerFunc)); @@ -1310,15 +1310,6 @@ public: void Invalidate() override { sscheck(InvalidateRect(hWindow, NULL, /*bErase=*/FALSE)); } - - void Redraw() override { - Invalidate(); - sscheck(UpdateWindow(hWindow)); - } - - void *NativePtr() override { - return hWindow; - } }; WindowRef CreateWindow(Window::Kind kind, WindowRef parentWindow) { diff --git a/src/solvespace.cpp b/src/solvespace.cpp index df93444..87a5318 100644 --- a/src/solvespace.cpp +++ b/src/solvespace.cpp @@ -111,15 +111,15 @@ void SolveSpaceUI::Init() { SetLocale(locale); } - timerGenerateAll = Platform::CreateTimer(); - timerGenerateAll->onTimeout = std::bind(&SolveSpaceUI::GenerateAll, &SS, Generate::DIRTY, + generateAllTimer = Platform::CreateTimer(); + generateAllTimer->onTimeout = std::bind(&SolveSpaceUI::GenerateAll, &SS, Generate::DIRTY, /*andFindFree=*/false, /*genForBBox=*/false); - timerShowTW = Platform::CreateTimer(); - timerShowTW->onTimeout = std::bind(&TextWindow::Show, &TW); + showTWTimer = Platform::CreateTimer(); + showTWTimer->onTimeout = std::bind(&TextWindow::Show, &TW); - timerAutosave = Platform::CreateTimer(); - timerAutosave->onTimeout = std::bind(&SolveSpaceUI::Autosave, &SS); + autosaveTimer = Platform::CreateTimer(); + autosaveTimer->onTimeout = std::bind(&SolveSpaceUI::Autosave, &SS); // The default styles (colors, line widths, etc.) are also stored in the // configuration file, but we will automatically load those as we need @@ -275,15 +275,15 @@ void SolveSpaceUI::Exit() { } void SolveSpaceUI::ScheduleGenerateAll() { - timerGenerateAll->WindUp(0); + generateAllTimer->RunAfterProcessingEvents(); } void SolveSpaceUI::ScheduleShowTW() { - timerShowTW->WindUp(0); + showTWTimer->RunAfterProcessingEvents(); } void SolveSpaceUI::ScheduleAutosave() { - timerAutosave->WindUp(autosaveInterval * 60 * 1000); + autosaveTimer->RunAfter(autosaveInterval * 60 * 1000); } double SolveSpaceUI::MmPerUnit() { @@ -646,9 +646,9 @@ void SolveSpaceUI::MenuAnalyze(Command id) { if(gs.constraints == 1 && gs.n == 0) { Constraint *c = SK.GetConstraint(gs.constraint[0]); if(c->HasLabel() && !c->reference) { - SS.TW.shown.dimFinish = c->valA; - SS.TW.shown.dimSteps = 10; - SS.TW.shown.dimIsDistance = + SS.TW.stepDim.finish = c->valA; + SS.TW.stepDim.steps = 10; + SS.TW.stepDim.isDistance = (c->type != Constraint::Type::ANGLE) && (c->type != Constraint::Type::LENGTH_RATIO) && (c->type != Constraint::Type::LENGTH_DIFFERENCE); diff --git a/src/solvespace.h b/src/solvespace.h index 6714334..ee4605a 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -789,9 +789,9 @@ public: // the sketch! bool allConsistent; - Platform::TimerRef timerShowTW; - Platform::TimerRef timerGenerateAll; - Platform::TimerRef timerAutosave; + Platform::TimerRef showTWTimer; + Platform::TimerRef generateAllTimer; + Platform::TimerRef autosaveTimer; void ScheduleShowTW(); void ScheduleGenerateAll(); void ScheduleAutosave(); diff --git a/src/textscreens.cpp b/src/textscreens.cpp index bec125a..0614cb6 100644 --- a/src/textscreens.cpp +++ b/src/textscreens.cpp @@ -550,38 +550,57 @@ void TextWindow::ShowGroupSolveInfo() { void TextWindow::ScreenStepDimFinish(int link, uint32_t v) { SS.TW.edit.meaning = Edit::STEP_DIM_FINISH; std::string edit_value; - if(SS.TW.shown.dimIsDistance) { - edit_value = SS.MmToString(SS.TW.shown.dimFinish); + if(SS.TW.stepDim.isDistance) { + edit_value = SS.MmToString(SS.TW.stepDim.finish); } else { - edit_value = ssprintf("%.3f", SS.TW.shown.dimFinish); + edit_value = ssprintf("%.3f", SS.TW.stepDim.finish); } SS.TW.ShowEditControl(12, edit_value); } void TextWindow::ScreenStepDimSteps(int link, uint32_t v) { SS.TW.edit.meaning = Edit::STEP_DIM_STEPS; - SS.TW.ShowEditControl(12, ssprintf("%d", SS.TW.shown.dimSteps)); + SS.TW.ShowEditControl(12, ssprintf("%d", SS.TW.stepDim.steps)); } void TextWindow::ScreenStepDimGo(int link, uint32_t v) { hConstraint hc = SS.TW.shown.constraint; Constraint *c = SK.constraint.FindByIdNoOops(hc); if(c) { SS.UndoRemember(); - double start = c->valA, finish = SS.TW.shown.dimFinish; - int i, n = SS.TW.shown.dimSteps; - for(i = 1; i <= n; i++) { - c = SK.GetConstraint(hc); - c->valA = start + ((finish - start)*i)/n; - SS.MarkGroupDirty(c->group); - SS.GenerateAll(); - if(!SS.ActiveGroupsOkay()) { - // Failed to solve, so quit - break; - } - SS.GW.window->Redraw(); + + double start = c->valA, finish = SS.TW.stepDim.finish; + SS.TW.stepDim.time = GetMilliseconds(); + SS.TW.stepDim.step = 1; + + if(!SS.TW.stepDim.timer) { + SS.TW.stepDim.timer = Platform::CreateTimer(); } + SS.TW.stepDim.timer->onTimeout = [=] { + if(SS.TW.stepDim.step <= SS.TW.stepDim.steps) { + c->valA = start + ((finish - start)*SS.TW.stepDim.step)/SS.TW.stepDim.steps; + SS.MarkGroupDirty(c->group); + SS.GenerateAll(); + if(!SS.ActiveGroupsOkay()) { + // Failed to solve, so quit + return; + } + SS.TW.stepDim.step++; + + const int64_t STEP_MILLIS = 50; + int64_t time = GetMilliseconds(); + if(time - SS.TW.stepDim.time < STEP_MILLIS) { + SS.TW.stepDim.timer->RunAfterNextFrame(); + } else { + SS.TW.stepDim.timer->RunAfter(time - SS.TW.stepDim.time - STEP_MILLIS); + } + SS.TW.stepDim.time = time; + } else { + SS.TW.GoToScreen(Screen::LIST_OF_GROUPS); + SS.ScheduleShowTW(); + } + SS.GW.Invalidate(); + }; + SS.TW.stepDim.timer->RunAfterNextFrame(); } - SS.GW.Invalidate(); - SS.TW.GoToScreen(Screen::LIST_OF_GROUPS); } void TextWindow::ShowStepDimension() { Constraint *c = SK.constraint.FindByIdNoOops(shown.constraint); @@ -593,17 +612,17 @@ void TextWindow::ShowStepDimension() { Printf(true, "%FtSTEP DIMENSION%E %s", c->DescriptionString().c_str()); - if(shown.dimIsDistance) { + if(stepDim.isDistance) { Printf(true, "%Ba %Ftstart%E %s", SS.MmToString(c->valA).c_str()); Printf(false, "%Bd %Ftfinish%E %s %Fl%Ll%f[change]%E", - SS.MmToString(shown.dimFinish).c_str(), &ScreenStepDimFinish); + SS.MmToString(stepDim.finish).c_str(), &ScreenStepDimFinish); } else { Printf(true, "%Ba %Ftstart%E %@", c->valA); Printf(false, "%Bd %Ftfinish%E %@ %Fl%Ll%f[change]%E", - shown.dimFinish, &ScreenStepDimFinish); + stepDim.finish, &ScreenStepDimFinish); } Printf(false, "%Ba %Ftsteps%E %d %Fl%Ll%f%D[change]%E", - shown.dimSteps, &ScreenStepDimSteps); + stepDim.steps, &ScreenStepDimSteps); Printf(true, " %Fl%Ll%fstep dimension now%E", &ScreenStepDimGo); @@ -762,16 +781,16 @@ void TextWindow::EditControlDone(std::string s) { case Edit::STEP_DIM_FINISH: if(Expr *e = Expr::From(s, /*popUpError=*/true)) { - if(shown.dimIsDistance) { - shown.dimFinish = SS.ExprToMm(e); + if(stepDim.isDistance) { + stepDim.finish = SS.ExprToMm(e); } else { - shown.dimFinish = e->Eval(); + stepDim.finish = e->Eval(); } } break; case Edit::STEP_DIM_STEPS: - shown.dimSteps = min(300, max(1, atoi(s.c_str()))); + stepDim.steps = min(300, max(1, atoi(s.c_str()))); break; case Edit::TANGENT_ARC_RADIUS: diff --git a/src/ui.h b/src/ui.h index 4dbe607..483476c 100644 --- a/src/ui.h +++ b/src/ui.h @@ -272,9 +272,6 @@ public: hStyle style; hConstraint constraint; - bool dimIsDistance; - double dimFinish; - int dimSteps; struct { int times; @@ -435,6 +432,15 @@ public: static void ScreenAllowRedundant(int link, uint32_t v); + struct { + bool isDistance; + double finish; + int steps; + + Platform::TimerRef timer; + int64_t time; + int step; + } stepDim; static void ScreenStepDimSteps(int link, uint32_t v); static void ScreenStepDimFinish(int link, uint32_t v); static void ScreenStepDimGo(int link, uint32_t v); @@ -572,8 +578,11 @@ public: Vector ProjectPoint4(Vector p, double *w); Vector UnProjectPoint(Point2d p); Vector UnProjectPoint3(Vector p); + + Platform::TimerRef animateTimer; void AnimateOnto(Quaternion quatf, Vector offsetf); void AnimateOntoWorkplane(); + Vector VectorFromProjs(Vector rightUpForward); void HandlePointForZoomToFit(Vector p, Point2d *pmax, Point2d *pmin, double *wmin, bool usePerspective,