From 89c3fc3e4d6ba625d414399ad37b4b54f1d69725 Mon Sep 17 00:00:00 2001 From: Pepe Rivera Date: Tue, 3 Dec 2019 14:41:39 -0800 Subject: [PATCH] Add error handling to graph and equations (#827) * add error handling * Handle regraphing on certain errors * Fix high contrast * Hide KGF button in error state --- src/Calculator/App.xaml | 102 ++++++++++-- src/Calculator/Controls/EquationTextBox.cpp | 23 ++- src/Calculator/Controls/EquationTextBox.h | 3 + .../GraphingCalculator/EquationInputArea.xaml | 1 + .../GraphingCalculator.xaml.cpp | 48 +++++- .../GraphingCalculator.xaml.h | 1 - .../KeyGraphFeaturesPanel.xaml | 117 +------------ src/GraphControl/Control/Equation.cpp | 41 ++++- src/GraphControl/Control/Equation.h | 73 +++++++-- src/GraphControl/Control/EquationCollection.h | 10 +- src/GraphControl/Control/Grapher.cpp | 154 ++++++++++++++---- src/GraphControl/Control/Grapher.h | 14 +- src/GraphControl/DirectX/RenderMain.cpp | 10 +- src/GraphControl/DirectX/RenderMain.h | 2 +- 14 files changed, 397 insertions(+), 202 deletions(-) diff --git a/src/Calculator/App.xaml b/src/Calculator/App.xaml index fd53d4e..1028383 100644 --- a/src/Calculator/App.xaml +++ b/src/Calculator/App.xaml @@ -74,6 +74,8 @@ + + @@ -166,6 +168,8 @@ + + @@ -224,6 +228,8 @@ + + @@ -1687,7 +1693,7 @@ - - @@ -185,12 +72,12 @@ - Property == s_hasGraphErrorProperty) + { + propertyName = EquationProperties::HasGraphError; + } + else if (args->Property == s_isValidatedProperty) + { + propertyName = EquationProperties::IsValidated; + } eq->PropertyChanged(eq, propertyName); } @@ -398,4 +432,9 @@ namespace GraphControl { return L""s; } + + bool Equation::IsGraphableEquation() + { + return !Expression->IsEmpty() && IsLineEnabled && !HasGraphError; + } } diff --git a/src/GraphControl/Control/Equation.h b/src/GraphControl/Control/Equation.h index 39b04e4..2fd32bd 100644 --- a/src/GraphControl/Control/Equation.h +++ b/src/GraphControl/Control/Equation.h @@ -91,6 +91,44 @@ namespace GraphControl #pragma endregion +#pragma region bool HasGraphError DependencyProperty + static property Windows::UI::Xaml::DependencyProperty ^ HasGraphErrorProperty { Windows::UI::Xaml::DependencyProperty ^ get() { return s_hasGraphErrorProperty; } } + + property bool HasGraphError + { + bool get() + { + return static_cast(GetValue(s_hasGraphErrorProperty)); + } + + internal: + void set(bool value) + { + SetValue(s_hasGraphErrorProperty, value); + } + } + +#pragma endregion + +#pragma region bool IsValidated DependencyProperty + static property Windows::UI::Xaml::DependencyProperty ^ IsValidatedProperty { Windows::UI::Xaml::DependencyProperty ^ get() { return s_isValidatedProperty; } } + + property bool IsValidated + { + bool get() + { + return static_cast(GetValue(s_isValidatedProperty)); + } + + internal: + void set(bool value) + { + SetValue(s_isValidatedProperty, value); + } + } + +#pragma endregion + #pragma region Key Graph Features @@ -108,7 +146,7 @@ namespace GraphControl { return static_cast(GetValue(s_xInterceptProperty)); } - void set(Platform::String^ value) + internal: void set(Platform::String^ value) { SetValue(s_xInterceptProperty, value); } @@ -129,7 +167,7 @@ namespace GraphControl { return static_cast(GetValue(s_yInterceptProperty)); } - void set(Platform::String^ value) + internal: void set(Platform::String^ value) { SetValue(s_yInterceptProperty, value); } @@ -150,7 +188,7 @@ namespace GraphControl { return static_cast(GetValue(s_parityProperty)); } - void set(int value) + internal: void set(int value) { SetValue(s_parityProperty, value); } @@ -171,7 +209,7 @@ namespace GraphControl { return static_cast(GetValue(s_periodicityDirectionProperty)); } - void set(int value) + internal: void set(int value) { SetValue(s_periodicityDirectionProperty, value); } @@ -192,7 +230,7 @@ namespace GraphControl { return static_cast(GetValue(s_periodicityExpressionProperty)); } - void set(Platform::String ^ value) + internal: void set(Platform::String ^ value) { SetValue(s_periodicityExpressionProperty, value); } @@ -213,7 +251,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_minimaProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_minimaProperty, value); } @@ -234,7 +272,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_maximaProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_maximaProperty, value); } @@ -255,7 +293,7 @@ namespace GraphControl { return static_cast(GetValue(s_domainProperty)); } - void set(Platform::String^ value) + internal: void set(Platform::String^ value) { SetValue(s_domainProperty, value); } @@ -276,7 +314,7 @@ namespace GraphControl { return static_cast(GetValue(s_rangeProperty)); } - void set(Platform::String^ value) + internal: void set(Platform::String^ value) { SetValue(s_rangeProperty, value); } @@ -297,7 +335,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_inflectionPointsProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_inflectionPointsProperty, value); } @@ -318,7 +356,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_monotonicityProperty)); } - void set(Windows::Foundation::Collections::IObservableMap ^ value) + internal: void set(Windows::Foundation::Collections::IObservableMap ^ value) { SetValue(s_monotonicityProperty, value); } @@ -339,7 +377,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_verticalAsymptotesProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_verticalAsymptotesProperty, value); } @@ -360,7 +398,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_horizontalAsymptotesProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_horizontalAsymptotesProperty, value); } @@ -381,7 +419,7 @@ namespace GraphControl { return static_cast ^>(GetValue(s_obliqueAsymptotesProperty)); } - void set(Windows::Foundation::Collections::IVector ^ value) + internal: void set(Windows::Foundation::Collections::IVector ^ value) { SetValue(s_obliqueAsymptotesProperty, value); } @@ -402,7 +440,7 @@ namespace GraphControl { return static_cast(GetValue(s_tooComplexFeaturesProperty)); } - void set(int value) + internal: void set(int value) { SetValue(s_tooComplexFeaturesProperty, value); } @@ -423,7 +461,7 @@ namespace GraphControl { return static_cast(GetValue(s_analysisErrorProperty)); } - void set(int value) + internal: void set(int value) { SetValue(s_analysisErrorProperty, value); } @@ -434,6 +472,7 @@ namespace GraphControl internal : event PropertyChangedEventHandler ^ PropertyChanged; std::wstring GetRequest(); + bool IsGraphableEquation(); private: static void OnCustomDependencyPropertyChanged(Windows::UI::Xaml::DependencyObject ^ obj, Windows::UI::Xaml::DependencyPropertyChangedEventArgs ^ args); @@ -446,6 +485,8 @@ namespace GraphControl static Windows::UI::Xaml::DependencyProperty ^ s_expressionProperty; static Windows::UI::Xaml::DependencyProperty ^ s_lineColorProperty; static Windows::UI::Xaml::DependencyProperty ^ s_isLineEnabledProperty; + static Windows::UI::Xaml::DependencyProperty ^ s_hasGraphErrorProperty; + static Windows::UI::Xaml::DependencyProperty ^ s_isValidatedProperty; static Windows::UI::Xaml::DependencyProperty ^ s_xInterceptProperty; static Windows::UI::Xaml::DependencyProperty ^ s_yInterceptProperty; static Windows::UI::Xaml::DependencyProperty ^ s_parityProperty; diff --git a/src/GraphControl/Control/EquationCollection.h b/src/GraphControl/Control/EquationCollection.h index 5fac827..dc920c6 100644 --- a/src/GraphControl/Control/EquationCollection.h +++ b/src/GraphControl/Control/EquationCollection.h @@ -7,7 +7,7 @@ namespace GraphControl { - delegate void EquationChangedEventHandler(); + delegate void EquationChangedEventHandler(Equation ^ sender); delegate void VisibilityChangedEventHandler(Equation ^ sender); public @@ -151,19 +151,19 @@ public event EquationChangedEventHandler ^ EquationLineEnabledChanged; private: - void OnEquationPropertyChanged(GraphControl::Equation ^, Platform::String ^ propertyName) + void OnEquationPropertyChanged(GraphControl::Equation ^ sender, Platform::String ^ propertyName) { if (propertyName == EquationProperties::LineColor) { - EquationStyleChanged(); + EquationStyleChanged(sender); } else if (propertyName == EquationProperties::Expression) { - EquationChanged(); + EquationChanged(sender); } else if (propertyName == EquationProperties::IsLineEnabled) { - EquationLineEnabledChanged(); + EquationLineEnabledChanged(sender); } } diff --git a/src/GraphControl/Control/Grapher.cpp b/src/GraphControl/Control/Grapher.cpp index 105b840..01756d2 100644 --- a/src/GraphControl/Control/Grapher.cpp +++ b/src/GraphControl/Control/Grapher.cpp @@ -62,7 +62,7 @@ namespace GraphControl , m_Moving{ false } { m_solver->ParsingOptions().SetFormatType(s_defaultFormatType); - m_solver->FormatOptions().SetFormatType(FormatType::MathML); + m_solver->FormatOptions().SetFormatType(s_defaultFormatType); m_solver->FormatOptions().SetMathMLPrefix(wstring(L"mml")); DefaultStyleKey = StringReference(s_defaultStyleKey); @@ -144,7 +144,7 @@ namespace GraphControl m_renderMain = ref new RenderMain(swapChainPanel); } - UpdateGraph(); + TryUpdateGraph(); } void Grapher::RegisterDependencyProperties() @@ -229,19 +229,26 @@ namespace GraphControl ref new EquationChangedEventHandler(this, &Grapher::OnEquationLineEnabledChanged); } - UpdateGraph(); + PlotGraph(); } - void Grapher::OnEquationChanged() + void Grapher::OnEquationChanged(Equation ^ equation) { - UpdateGraph(); + // If the equation was previously valid, we should try to graph again in the event of the failure + bool shouldRetry = equation->IsValidated; + + // Reset these properties if the equation is requesting to be graphed again + equation->HasGraphError = false; + equation->IsValidated = false; + + TryPlotGraph(shouldRetry); } - void Grapher::OnEquationStyleChanged() + void Grapher::OnEquationStyleChanged(Equation ^) { if (m_graph) { - UpdateGraphOptions(m_graph->GetOptions(), GetValidEquations()); + UpdateGraphOptions(m_graph->GetOptions(), GetGraphableEquations()); } if (m_renderMain) @@ -250,14 +257,15 @@ namespace GraphControl } } - void Grapher::OnEquationLineEnabledChanged() + void Grapher::OnEquationLineEnabledChanged(Equation ^ equation) { - UpdateGraph(); - } + // If the equation is in an error state or is empty, it should not be graphed anyway. + if (equation->HasGraphError || equation->Expression->IsEmpty()) + { + return; + } - void Grapher::PlotGraph() - { - UpdateGraph(); + PlotGraph(); } void Grapher::AnalyzeEquation(Equation ^ equation) @@ -307,13 +315,43 @@ namespace GraphControl equation->AnalysisError = CalculatorApp::AnalysisErrorType::AnalysisCouldNotBePerformed; } - void Grapher::UpdateGraph() + void Grapher::PlotGraph() + { + TryPlotGraph(false); + } + + void Grapher::TryPlotGraph(bool shouldRetry) + { + if (TryUpdateGraph()) + { + SetEquationsAsValid(); + } + else + { + SetEquationErrors(); + + // If we failed to plot the graph, try again after the bad equations are flagged. + if (shouldRetry) + { + TryUpdateGraph(); + } + } + } + + bool Grapher::TryUpdateGraph() { optional>> initResult = nullopt; + bool successful = false; if (m_renderMain && m_graph != nullptr) { - auto validEqs = GetValidEquations(); + unique_ptr graphExpression; + wstring request; + + auto validEqs = GetGraphableEquations(); + + // Will be set to true if the previous graph should be kept in the event of an error + bool shouldKeepPreviousGraph = false; if (!validEqs.empty()) { @@ -323,6 +361,11 @@ namespace GraphControl int numValidEquations = 0; for (Equation ^ eq : validEqs) { + if (eq->IsValidated) + { + shouldKeepPreviousGraph = true; + } + if (numValidEquations++ > 0) { ss << L","; @@ -333,26 +376,78 @@ namespace GraphControl ss << s_getGraphClosingTags; - wstring request = ss.str(); - unique_ptr graphExpression; - if (graphExpression = m_solver->ParseInput(request)) + request = ss.str(); + } + + if (graphExpression = m_solver->ParseInput(request)) + { + initResult = m_graph->TryInitialize(graphExpression.get()); + + if (initResult != nullopt) { - initResult = m_graph->TryInitialize(graphExpression.get()); + UpdateGraphOptions(m_graph->GetOptions(), validEqs); + SetGraphArgs(); + + m_renderMain->Graph = m_graph; + + // It is possible that the render fails, in that case fall through to explicit empty initialization + if (m_renderMain->RunRenderPass()) + { + UpdateVariables(); + successful = true; + } + else + { + // If we failed to render then we have already lost the previous graph + shouldKeepPreviousGraph = false; + initResult = nullopt; + } } } if (initResult == nullopt) { - initResult = m_graph->TryInitialize(); + // Do not re-initialize the graph to empty if there are still valid equations graphed + if (!shouldKeepPreviousGraph) + { + initResult = m_graph->TryInitialize(); + + if (initResult != nullopt) + { + UpdateGraphOptions(m_graph->GetOptions(), validEqs); + SetGraphArgs(); + + m_renderMain->Graph = m_graph; + m_renderMain->RunRenderPass(); + + UpdateVariables(); + + // Initializing an empty graph is only a success if there were no equations to graph. + successful = (validEqs.size() == 0); + } + } } + } - if (initResult != nullopt) + // Return true if we were able to graph and render all graphable equations + return successful; + } + + void Grapher::SetEquationsAsValid() + { + for (Equation ^ eq : GetGraphableEquations()) + { + eq->IsValidated = true; + } + } + + void Grapher::SetEquationErrors() + { + for (Equation ^ eq : GetGraphableEquations()) + { + if (!eq->IsValidated) { - UpdateGraphOptions(m_graph->GetOptions(), validEqs); - SetGraphArgs(); - - UpdateVariables(); - m_renderMain->Graph = m_graph; + eq->HasGraphError = true; } } } @@ -417,6 +512,7 @@ namespace GraphControl void Grapher::UpdateVariables() { auto updatedVariables = ref new Map(); + if (m_graph) { auto graphVariables = m_graph->GetVariables(); @@ -484,13 +580,13 @@ namespace GraphControl } } - vector Grapher::GetValidEquations() + vector Grapher::GetGraphableEquations() { vector validEqs; for (Equation ^ eq : Equations) { - if (!eq->Expression->IsEmpty() && eq->IsLineEnabled) + if (eq->IsGraphableEquation()) { validEqs.push_back(eq); } @@ -501,7 +597,7 @@ namespace GraphControl void Grapher::OnForceProportionalAxesChanged(DependencyPropertyChangedEventArgs ^ args) { - UpdateGraph(); + TryUpdateGraph(); } void Grapher::OnBackgroundColorChanged(const Windows::UI::Color& color) diff --git a/src/GraphControl/Control/Grapher.h b/src/GraphControl/Control/Grapher.h index c16c0ac..a96f2dd 100644 --- a/src/GraphControl/Control/Grapher.h +++ b/src/GraphControl/Control/Grapher.h @@ -171,12 +171,13 @@ public void OnDependencyPropertyChanged(Windows::UI::Xaml::DependencyObject ^ obj, Windows::UI::Xaml::DependencyProperty ^ p); void OnEquationsChanged(Windows::UI::Xaml::DependencyPropertyChangedEventArgs ^ args); - void OnEquationChanged(); - void OnEquationStyleChanged(); - void OnEquationLineEnabledChanged(); - void UpdateGraph(); + void OnEquationChanged(GraphControl::Equation ^ equation); + void OnEquationStyleChanged(GraphControl::Equation ^ equation); + void OnEquationLineEnabledChanged(GraphControl::Equation ^ equation); + bool TryUpdateGraph(); + void TryPlotGraph(bool shouldRetry); void UpdateGraphOptions(Graphing::IGraphingOptions& options, const std::vector& validEqs); - std::vector GetValidEquations(); + std::vector GetGraphableEquations(); void SetGraphArgs(); std::shared_ptr GetGraph(GraphControl::Equation ^ equation); void UpdateVariables(); @@ -194,6 +195,9 @@ public void HandleTracingMovementTick(Object ^ sender, Object ^ e); void HandleKey(bool keyDown, Windows::System::VirtualKey key); + void SetEquationsAsValid(); + void SetEquationErrors(); + Windows::Foundation::Collections::IObservableVector ^ ConvertWStringVector(std::vector inVector); Windows::Foundation::Collections::IObservableMap ^ ConvertWStringIntMap(std::map inMap); diff --git a/src/GraphControl/DirectX/RenderMain.cpp b/src/GraphControl/DirectX/RenderMain.cpp index 3083c18..a9b373d 100644 --- a/src/GraphControl/DirectX/RenderMain.cpp +++ b/src/GraphControl/DirectX/RenderMain.cpp @@ -68,8 +68,6 @@ namespace GraphControl::DX renderer->SetGraphSize(static_cast(m_swapChainPanel->ActualWidth), static_cast(m_swapChainPanel->ActualHeight)); } } - - RunRenderPass(); } void RenderMain::BackgroundColor::set(Windows::UI::Color backgroundColor) @@ -125,12 +123,16 @@ namespace GraphControl::DX RunRenderPass(); } - void RenderMain::RunRenderPass() + bool RenderMain::RunRenderPass() { - if (Render()) + bool succesful = Render(); + + if (succesful) { m_deviceResources.Present(); } + + return succesful; } // Renders the current frame according to the current application state. diff --git a/src/GraphControl/DirectX/RenderMain.h b/src/GraphControl/DirectX/RenderMain.h index becde5b..b9ff322 100644 --- a/src/GraphControl/DirectX/RenderMain.h +++ b/src/GraphControl/DirectX/RenderMain.h @@ -46,7 +46,7 @@ namespace GraphControl::DX void CreateWindowSizeDependentResources(); - void RunRenderPass(); + bool RunRenderPass(); // Indicates if we are in active tracing mode (the tracing box is being used and controlled through keyboard input) property bool ActiveTracing