Run render pass asynchronously on UI thread

src/GraphControl/Control/Grapher.cpp

@@ -11,7 +11,6 @@ using namespace GraphControl;
 using namespace GraphControl::DX;
 using namespace Platform;
 using namespace Platform::Collections;
-using namespace std;
 using namespace Concurrency;
 using namespace Windows::Devices::Input;
 using namespace Windows::Foundation;
@@ -54,9 +53,9 @@ namespace
     // posX/posY are the pointer position elements and width,height are the dimensions of the graph surface.
     // The graphing engine interprets x,y position between the range [-1, 1].
     // Translate the pointer position to the [-1, 1] bounds.
-    __inline pair<double, double> PointerPositionToGraphPosition(double posX, double posY, double width, double height)
+    __inline std::pair<double, double> PointerPositionToGraphPosition(double posX, double posY, double width, double height)
     {
-        return make_pair((2 * posX / width - 1), (1 - 2 * posY / height));
+        return std::make_pair((2 * posX / width - 1), (1 - 2 * posY / height));
     }
 }
 
@@ -71,7 +70,7 @@ namespace GraphControl
 
         m_solver->ParsingOptions().SetFormatType(s_defaultFormatType);
         m_solver->FormatOptions().SetFormatType(s_defaultFormatType);
-        m_solver->FormatOptions().SetMathMLPrefix(wstring(L"mml"));
+        m_solver->FormatOptions().SetMathMLPrefix(L"mml");
 
         DefaultStyleKey = StringReference(s_defaultStyleKey);
 
@@ -95,21 +94,10 @@ namespace GraphControl
     {
         if (m_graph != nullptr && m_renderMain != nullptr)
         {
-            if (auto renderer = m_graph->GetRenderer())
+            if (auto renderer = m_graph->GetRenderer(); static_cast<bool>(renderer) && SUCCEEDED(renderer->ScaleRange(centerX, centerY, scale)))
             {
-                m_renderMain->GetCriticalSection().lock();
+                m_renderMain->RunRenderPass();
-
+                GraphViewChangedEvent(this, GraphViewChangedReason::Manipulation);
-                if (SUCCEEDED(renderer->ScaleRange(centerX, centerY, scale)))
-                {
-                    m_renderMain->GetCriticalSection().unlock();
-
-                    m_renderMain->RunRenderPass();
-                    GraphViewChangedEvent(this, GraphViewChangedReason::Manipulation);
-                }
-                else
-                {
-                    m_renderMain->GetCriticalSection().unlock();
-                }
             }
         }
     }
@@ -251,7 +239,7 @@ namespace GraphControl
 
             if (auto analyzer = graph->GetAnalyzer())
             {
-                vector<Equation ^> equationVector;
+                std::vector<Equation ^> equationVector;
                 equationVector.push_back(equation);
                 UpdateGraphOptions(graph->GetOptions(), equationVector);
                 bool variableIsNotX;
@@ -325,15 +313,15 @@ namespace GraphControl
 
     task<bool> Grapher::TryUpdateGraph(bool keepCurrentView)
     {
-        optional<vector<shared_ptr<IEquation>>> initResult = nullopt;
+        std::optional<std::vector<std::shared_ptr<IEquation>>> initResult;
         bool successful = false;
         m_errorCode = 0;
         m_errorType = 0;
 
         if (m_renderMain && m_graph != nullptr)
         {
-            unique_ptr<IExpression> graphExpression;
+            std::unique_ptr<IExpression> graphExpression;
-            wstring request;
+            std::wstring request;
 
             auto validEqs = GetGraphableEquations();
 
@@ -371,13 +359,12 @@ namespace GraphControl
                         co_return false;
                     }
 
-                    unique_ptr<IExpression> expr;
+                    std::wstring parsableEquation = s_getGraphOpeningTags;
-                    wstring parsableEquation = s_getGraphOpeningTags;
                     parsableEquation += equationRequest;
                     parsableEquation += s_getGraphClosingTags;
 
                     // Wire up the corresponding error to an error message in the UI at some point
-                    if (!(expr = m_solver->ParseInput(parsableEquation, m_errorCode, m_errorType)))
+                    if (auto expr = m_solver->ParseInput(parsableEquation, m_errorCode, m_errorType); !static_cast<bool>(expr))
                     {
                         co_return false;
                     }
@@ -392,11 +379,11 @@ namespace GraphControl
             {
                 initResult = TryInitializeGraph(keepCurrentView, graphExpression.get());
 
-                if (initResult != nullopt)
+                if (initResult.has_value())
                 {
-                    auto graphedEquations = initResult.value();
+                    auto graphedEquations = *initResult;
 
-                    for (int i = 0; i < validEqs.size(); i++)
+                    for (size_t i = 0; i < validEqs.size(); ++i)
                     {
                         validEqs[i]->GraphedEquation = graphedEquations[i];
                     }
@@ -407,8 +394,8 @@ namespace GraphControl
                     m_renderMain->Graph = m_graph;
 
                     // It is possible that the render fails, in that case fall through to explicit empty initialization
-                    co_await m_renderMain->RunRenderPassAsync(false);
+                    auto succ = co_await m_renderMain->RunRenderPassAsync(false);
-                    if (m_renderMain->IsRenderPassSuccesful())
+                    if (succ)
                     {
                         UpdateVariables();
                         successful = true;
@@ -417,7 +404,7 @@ namespace GraphControl
                     {
                         // If we failed to render then we have already lost the previous graph
                         shouldKeepPreviousGraph = false;
-                        initResult = nullopt;
+                        initResult.reset();
                         m_solver->HRErrorToErrorInfo(m_renderMain->GetRenderError(), m_errorCode, m_errorType);
                     }
                 }
@@ -427,15 +414,15 @@ namespace GraphControl
                 }
             }
 
-            if (initResult == nullopt)
+            if (!initResult.has_value())
             {
                 // Do not re-initialize the graph to empty if there are still valid equations graphed
                 if (!shouldKeepPreviousGraph)
                 {
                     initResult = TryInitializeGraph(false, nullptr);
-                    if (initResult != nullopt)
+                    if (initResult.has_value())
                     {
-                        UpdateGraphOptions(m_graph->GetOptions(), vector<Equation ^>());
+                        UpdateGraphOptions(m_graph->GetOptions(), {});
                         SetGraphArgs(m_graph);
 
                         m_renderMain->Graph = m_graph;
@@ -475,12 +462,10 @@ namespace GraphControl
         }
     }
 
-    void Grapher::SetGraphArgs(shared_ptr<IGraph> graph)
+    void Grapher::SetGraphArgs(std::shared_ptr<IGraph> graph)
     {
         if (graph != nullptr && m_renderMain != nullptr)
         {
-            critical_section::scoped_lock lock(m_renderMain->GetCriticalSection());
-
             for (auto variablePair : Variables)
             {
                 graph->SetArgValue(variablePair->Key->Data(), variablePair->Value->Value);
@@ -488,17 +473,17 @@ namespace GraphControl
         }
     }
 
-    shared_ptr<IGraph> Grapher::GetGraph(Equation ^ equation)
+    std::shared_ptr<IGraph> Grapher::GetGraph(Equation ^ equation)
     {
-        shared_ptr<Graphing::IGraph> graph = m_solver->CreateGrapher();
+        std::shared_ptr<Graphing::IGraph> graph = m_solver->CreateGrapher();
 
-        wstring request = s_getGraphOpeningTags;
+        std::wstring request = s_getGraphOpeningTags;
         request += equation->GetRequest()->Data();
         request += s_getGraphClosingTags;
 
-        if (unique_ptr<IExpression> graphExpression = m_solver->ParseInput(request, m_errorCode, m_errorType))
+        if (auto expr = m_solver->ParseInput(request, m_errorCode, m_errorType); static_cast<bool>(expr))
         {
-            if (graph->TryInitialize(graphExpression.get()))
+            if (graph->TryInitialize(expr.get()))
             {
                 return graph;
             }
@@ -557,19 +542,12 @@ namespace GraphControl
 
         if (m_graph != nullptr && m_renderMain != nullptr)
         {
-            auto workItemHandler = ref new WorkItemHandler([this, variableName, newValue](IAsyncAction ^ action) {
+            m_graph->SetArgValue(variableName->Data(), newValue);
-                m_renderMain->GetCriticalSection().lock();
+            m_renderMain->RunRenderPass();
-                m_graph->SetArgValue(variableName->Data(), newValue);
-                m_renderMain->GetCriticalSection().unlock();
-
-                m_renderMain->RunRenderPass();
-            });
-
-            ThreadPool::RunAsync(workItemHandler, WorkItemPriority::High, WorkItemOptions::None);
         }
     }
 
-    void Grapher::UpdateGraphOptions(IGraphingOptions& options, const vector<Equation ^>& validEqs)
+    void Grapher::UpdateGraphOptions(IGraphingOptions& options, const std::vector<Equation ^>& validEqs)
     {
         options.SetForceProportional(ForceProportionalAxes);
 
@@ -580,7 +558,7 @@ namespace GraphControl
 
         if (!validEqs.empty())
         {
-            vector<Graphing::Color> graphColors;
+            std::vector<Graphing::Color> graphColors;
             graphColors.reserve(validEqs.size());
             for (Equation ^ eq : validEqs)
             {
@@ -603,9 +581,9 @@ namespace GraphControl
         }
     }
 
-    vector<Equation ^> Grapher::GetGraphableEquations()
+    std::vector<Equation ^> Grapher::GetGraphableEquations()
     {
-        vector<Equation ^> validEqs;
+        std::vector<Equation ^> validEqs;
 
         for (Equation ^ eq : Equations)
         {
@@ -783,15 +761,10 @@ namespace GraphControl
                     translationX /= -width;
                     translationY /= height;
 
-                    m_renderMain->GetCriticalSection().lock();
-
                     if (FAILED(renderer->MoveRangeByRatio(translationX, translationY)))
                     {
-                        m_renderMain->GetCriticalSection().unlock();
                         return;
                     }
-
-                    m_renderMain->GetCriticalSection().unlock();
                     needsRenderPass = true;
                 }
 
@@ -805,15 +778,10 @@ namespace GraphControl
                     const auto& pos = e->Position;
                     const auto [centerX, centerY] = PointerPositionToGraphPosition(pos.X, pos.Y, width, height);
 
-                    m_renderMain->GetCriticalSection().lock();
-
                     if (FAILED(renderer->ScaleRange(centerX, centerY, scale)))
                     {
-                        m_renderMain->GetCriticalSection().unlock();
                         return;
                     }
-
-                    m_renderMain->GetCriticalSection().unlock();
                     needsRenderPass = true;
                 }
 
@@ -834,14 +802,14 @@ namespace GraphControl
         {
             if (auto renderer = m_graph->GetRenderer())
             {
-                shared_ptr<Graphing::IBitmap> BitmapOut;
+                std::shared_ptr<Graphing::IBitmap> BitmapOut;
                 bool hasSomeMissingDataOut = false;
                 HRESULT hr = E_FAIL;
                 hr = renderer->GetBitmap(BitmapOut, hasSomeMissingDataOut);
                 if (SUCCEEDED(hr))
                 {
                     // Get the raw data
-                    vector<BYTE> byteVector = BitmapOut->GetData();
+                    std::vector<BYTE> byteVector = BitmapOut->GetData();
                     auto arr = ArrayReference<BYTE>(byteVector.data(), (unsigned int)byteVector.size());
 
                     // create a memory stream wrapper
@@ -1085,13 +1053,13 @@ void Grapher::OnGraphBackgroundPropertyChanged(Windows::UI::Color /*oldValue*/,
     }
 }
 
-void Grapher::OnGridLinesColorPropertyChanged(Windows::UI::Color /*oldValue*/, Windows::UI::Color newValue)
+winrt::fire_and_forget Grapher::OnGridLinesColorPropertyChanged(Windows::UI::Color /*oldValue*/, Windows::UI::Color newValue)
 {
     if (m_renderMain != nullptr && m_graph != nullptr)
     {
         auto gridLinesColor = Graphing::Color(newValue.R, newValue.G, newValue.B, newValue.A);
         m_graph->GetOptions().SetGridColor(gridLinesColor);
-        m_renderMain->RunRenderPassAsync();
+        co_await m_renderMain->RunRenderPassAsync();
     }
 }
 
@@ -1110,16 +1078,15 @@ void Grapher::OnLineWidthPropertyChanged(double oldValue, double newValue)
     }
 }
 
-optional<vector<shared_ptr<Graphing::IEquation>>> Grapher::TryInitializeGraph(bool keepCurrentView, const IExpression* graphingExp)
+std::optional<std::vector<std::shared_ptr<Graphing::IEquation>>> Grapher::TryInitializeGraph(bool keepCurrentView, const IExpression* graphingExp)
 {
-    critical_section::scoped_lock lock(m_renderMain->GetCriticalSection());
     if (keepCurrentView || IsKeepCurrentView)
     {
         auto renderer = m_graph->GetRenderer();
         double xMin, xMax, yMin, yMax;
         renderer->GetDisplayRanges(xMin, xMax, yMin, yMax);
         auto initResult = m_graph->TryInitialize(graphingExp);
-        if (initResult != nullopt)
+        if (initResult.has_value())
         {
             if (IsKeepCurrentView)
             {

src/GraphControl/Control/Grapher.h

@@ -228,7 +228,6 @@ public enum class GraphViewChangedReason
                 {
                     if (auto render = m_graph->GetRenderer())
                     {
-                        Concurrency::critical_section::scoped_lock lock(m_renderMain->GetCriticalSection());
                         render->GetDisplayRanges(*xMin, *xMax, *yMin, *yMax);
                     }
                 }
@@ -280,7 +279,7 @@ public enum class GraphViewChangedReason
         void OnEquationsPropertyChanged(EquationCollection ^ oldValue, EquationCollection ^ newValue);
         void OnAxesColorPropertyChanged(Windows::UI::Color oldValue, Windows::UI::Color newValue);
         void OnGraphBackgroundPropertyChanged(Windows::UI::Color oldValue, Windows::UI::Color newValue);
-        void OnGridLinesColorPropertyChanged(Windows::UI::Color /*oldValue*/, Windows::UI::Color newValue);
+        winrt::fire_and_forget OnGridLinesColorPropertyChanged(Windows::UI::Color /*oldValue*/, Windows::UI::Color newValue);
         void OnLineWidthPropertyChanged(double oldValue, double newValue);
         void OnEquationChanged(Equation ^ equation);
         void OnEquationStyleChanged(Equation ^ equation);

src/GraphControl/DirectX/RenderMain.cpp

@@ -97,7 +97,7 @@ namespace GraphControl::DX
             bool wasPointRendered = m_Tracing;
             if (CanRenderPoint() || wasPointRendered)
             {
-                RunRenderPassAsync();
+                [](RenderMain ^ self) -> winrt::fire_and_forget { co_await self->RunRenderPassAsync(); }(this);
             }
         }
     }
@@ -111,7 +111,7 @@ namespace GraphControl::DX
             bool wasPointRendered = m_Tracing;
             if (CanRenderPoint() || wasPointRendered)
             {
-                RunRenderPassAsync();
+                [](RenderMain ^ self) -> winrt::fire_and_forget { co_await self->RunRenderPassAsync(); }(this);
             }
         }
     }
@@ -146,15 +146,6 @@ namespace GraphControl::DX
                 trackPoint = m_activeTracingPointerLocation;
             }
 
-            if (!m_criticalSection.try_lock())
-            {
-                return false;
-            }
-
-            m_criticalSection.unlock();
-
-            critical_section::scoped_lock lock(m_criticalSection);
-
             int formulaId = -1;
             double outNearestPointValueX, outNearestPointValueY;
             float outNearestPointLocationX, outNearestPointLocationY;
@@ -210,53 +201,37 @@ namespace GraphControl::DX
 
     bool RenderMain::RunRenderPass()
     {
-        // Non async render passes cancel if they can't obtain the lock immediatly
-        if (!m_criticalSection.try_lock())
-        {
-            return false;
-        }
-
-        m_criticalSection.unlock();
-
-        critical_section::scoped_lock lock(m_criticalSection);
-
         return RunRenderPassInternal();
     }
 
-    IAsyncAction ^ RenderMain::RunRenderPassAsync(bool allowCancel)
+    concurrency::task<bool> RenderMain::RunRenderPassAsync(bool allowCancel)
     {
-        // Try to cancel the renderPass that is in progress
+        if (allowCancel)
-        if (m_renderPass != nullptr && m_renderPass->Status == ::AsyncStatus::Started)
         {
-            m_renderPass->Cancel();
+            m_renderPassCts.cancel();
         }
+        m_renderPassCts = concurrency::cancellation_token_source{};
 
-        auto device = m_deviceResources;
+        bool result = true;
-        auto workItemHandler = ref new WorkItemHandler([this, allowCancel](IAsyncAction ^ action) {
+        co_await CoreWindow::GetForCurrentThread()->Dispatcher->RunAsync(
-            critical_section::scoped_lock lock(m_criticalSection);
+            CoreDispatcherPriority::High,
-
+            ref new DispatchedHandler(
-            // allowCancel is passed as false when the grapher relies on the render pass to validate that an equation can be succesfully rendered.
+                [this, &result, cancel = m_renderPassCts.get_token()]
-            // Passing false garauntees that another render pass doesn't cancel this one.
+                {
-            if (allowCancel && action->Status == ::AsyncStatus::Canceled)
+                    if (cancel.is_canceled())
-            {
+                        return;
-                return;
+                    result = RunRenderPassInternal();
-            }
+                }));
-
+        co_return result;
-            RunRenderPassInternal();
-        });
-
-        m_renderPass = ThreadPool::RunAsync(workItemHandler, WorkItemPriority::High, WorkItemOptions::None);
-
-        return m_renderPass;
     }
 
     bool RenderMain::RunRenderPassInternal()
     {
         // We are accessing Direct3D resources directly without Direct2D's knowledge, so we
         // must manually acquire and apply the Direct2D factory lock.
-        ID2D1Multithread* m_D2DMultithread;
+        winrt::com_ptr<ID2D1Multithread> d2dmultithread;
-        m_deviceResources.GetD2DFactory()->QueryInterface(IID_PPV_ARGS(&m_D2DMultithread));
+        m_deviceResources.GetD2DFactory()->QueryInterface(IID_PPV_ARGS(d2dmultithread.put()));
-        m_D2DMultithread->Enter();
+        d2dmultithread->Enter();
 
         bool succesful = Render();
 
@@ -267,10 +242,8 @@ namespace GraphControl::DX
 
         // It is absolutely critical that the factory lock be released upon
         // exiting this function, or else any consequent Direct2D calls will be blocked.
-        m_D2DMultithread->Leave();
+        d2dmultithread->Leave();
-
+        return succesful;
-        m_isRenderPassSuccesful = succesful;
-        return m_isRenderPassSuccesful;
     }
 
     // Renders the current frame according to the current application state.

src/GraphControl/DirectX/RenderMain.h

@@ -51,17 +51,7 @@ namespace GraphControl::DX
 
         bool RunRenderPass();
 
-        Windows::Foundation::IAsyncAction ^ RunRenderPassAsync(bool allowCancel = true);
+        concurrency::task<bool> RunRenderPassAsync(bool allowCancel = true);
-
-        Concurrency::critical_section& GetCriticalSection()
-        {
-            return m_criticalSection;
-        }
-
-        bool IsRenderPassSuccesful()
-        {
-            return m_isRenderPassSuccesful;
-        }
 
         HRESULT GetRenderError();
 
@@ -198,11 +188,7 @@ namespace GraphControl::DX
         // Are we currently showing the tracing value
         bool m_Tracing;
 
-        Concurrency::critical_section m_criticalSection;
+        concurrency::cancellation_token_source m_renderPassCts;
-
-        Windows::Foundation::IAsyncAction ^ m_renderPass = nullptr;
-
-        bool m_isRenderPassSuccesful;
 
         HRESULT m_HResult;
     };