From 90fa6f0850a857434269d98813c6fada306a1be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Mon, 17 Dec 2018 16:50:33 +0100 Subject: [PATCH 01/14] [poincare] Fix Integer::approximate Take the sign into account even for infinite values --- poincare/src/integer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/poincare/src/integer.cpp b/poincare/src/integer.cpp index 5b5e1a96c..77f4618cd 100644 --- a/poincare/src/integer.cpp +++ b/poincare/src/integer.cpp @@ -323,7 +323,7 @@ T Integer::approximate() const { */ if (isInfinity()) { - return INFINITY; + return m_negative ? -INFINITY : INFINITY; } native_uint_t lastDigit = m_numberOfDigits > 0 ? digit(m_numberOfDigits-1) : 0; From 9a254eb78b42066bb5fd11eb9a4827beaf9536ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Mon, 17 Dec 2018 17:19:26 +0100 Subject: [PATCH 02/14] [poincare] Fix Integer approximation Take into accound the sign even if the approximation is infinite --- poincare/src/integer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/poincare/src/integer.cpp b/poincare/src/integer.cpp index 77f4618cd..d69cd9617 100644 --- a/poincare/src/integer.cpp +++ b/poincare/src/integer.cpp @@ -334,7 +334,7 @@ T Integer::approximate() const { /* Escape case if the exponent is too big to be stored */ assert(m_numberOfDigits > 0); if (((int)m_numberOfDigits-1)*32+numberOfBitsInLastDigit-1> IEEE754::maxExponent()-IEEE754::exponentOffset()) { - return INFINITY; + return m_negative ? -INFINITY : INFINITY; } exponent += (m_numberOfDigits-1)*32; exponent += numberOfBitsInLastDigit-1; From 78e4c9066f574ac3a7bf5357002a22a7bec0bc99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Mon, 17 Dec 2018 18:01:52 +0100 Subject: [PATCH 03/14] [shared] Revert: Use Simplify instead of Reduce To approximate an expression, it is more precise to approximate its simplified form than its reduced form. Indeed, we want to minimize the number of nodes in the expression before approximating. For instance, a/b has fewer nodes than a*b^-1. --- apps/sequence/sequence.cpp | 4 ++-- apps/shared/expression_model.cpp | 2 +- apps/shared/poincare_helpers.h | 4 ---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/apps/sequence/sequence.cpp b/apps/sequence/sequence.cpp index 5cf80f9ff..d7bedbfef 100644 --- a/apps/sequence/sequence.cpp +++ b/apps/sequence/sequence.cpp @@ -96,14 +96,14 @@ void Sequence::setInitialRank(int rank) { Poincare::Expression Sequence::firstInitialConditionExpression(Context * context) const { if (m_firstInitialConditionExpression.isUninitialized()) { - m_firstInitialConditionExpression = PoincareHelpers::ParseAndReduce(m_firstInitialConditionText, *context); + m_firstInitialConditionExpression = PoincareHelpers::ParseAndSimplify(m_firstInitialConditionText, *context); } return m_firstInitialConditionExpression; } Poincare::Expression Sequence::secondInitialConditionExpression(Context * context) const { if (m_secondInitialConditionExpression.isUninitialized()) { - m_secondInitialConditionExpression = PoincareHelpers::ParseAndReduce(m_secondInitialConditionText, *context); + m_secondInitialConditionExpression = PoincareHelpers::ParseAndSimplify(m_secondInitialConditionText, *context); } return m_secondInitialConditionExpression; } diff --git a/apps/shared/expression_model.cpp b/apps/shared/expression_model.cpp index 4dfecbbe8..bc7362e85 100644 --- a/apps/shared/expression_model.cpp +++ b/apps/shared/expression_model.cpp @@ -21,7 +21,7 @@ const char * ExpressionModel::text() const { Poincare::Expression ExpressionModel::expression(Poincare::Context * context) const { if (m_expression.isUninitialized()) { - m_expression = PoincareHelpers::ParseAndReduce(m_text, *context); + m_expression = PoincareHelpers::ParseAndSimplify(m_text, *context); } return m_expression; } diff --git a/apps/shared/poincare_helpers.h b/apps/shared/poincare_helpers.h index 90c0745cd..a9a2b9ef6 100644 --- a/apps/shared/poincare_helpers.h +++ b/apps/shared/poincare_helpers.h @@ -37,10 +37,6 @@ inline T ApproximateToScalar(const char * text, Poincare::Context & context) { return Poincare::Expression::approximateToScalar(text, context, Poincare::Preferences::sharedPreferences()->angleUnit()); } -inline Poincare::Expression ParseAndReduce(const char * text, Poincare::Context & context) { - return Poincare::Expression::ParseAndReduce(text, context, Poincare::Preferences::sharedPreferences()->angleUnit()); -} - inline Poincare::Expression ParseAndSimplify(const char * text, Poincare::Context & context) { return Poincare::Expression::ParseAndSimplify(text, context, Poincare::Preferences::sharedPreferences()->angleUnit()); } From ec08f027c0cf72a60ccfcf71a64d3156607807eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89milie=20Feral?= Date: Tue, 18 Dec 2018 09:54:20 +0100 Subject: [PATCH 04/14] [poincare] Revert: Use Simplify instead of Reduce II To approximate an expression, it is more precise to approximate its simplified form than its reduced form. Indeed, we want to minimize the number of nodes in the expression before approximating. For instance, a/b has fewer nodes than a*b^-1. --- apps/shared/storage_expression_model.cpp | 7 ++++++- poincare/include/poincare/expression.h | 1 - poincare/src/expression.cpp | 10 +--------- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/apps/shared/storage_expression_model.cpp b/apps/shared/storage_expression_model.cpp index 66a143b2b..2f7ac146f 100644 --- a/apps/shared/storage_expression_model.cpp +++ b/apps/shared/storage_expression_model.cpp @@ -39,7 +39,12 @@ Expression StorageExpressionModel::expressionReduced(Poincare::Context * context if (m_expression.isUninitialized()) { assert(!isNull()); Ion::Storage::Record::Data recordData = value(); - m_expression = Expression::ExpressionFromAddress(expressionAddressForValue(recordData), expressionSizeForValue(recordData)).reduce(*context, Preferences::sharedPreferences()->angleUnit()); + m_expression = Expression::ExpressionFromAddress(expressionAddressForValue(recordData), expressionSizeForValue(recordData)); + PoincareHelpers::Simplify(&m_expression, *context); + // simplify might return an uninitialized Expression if interrupted + if (m_expression.isUninitialized()) { + m_expression = Expression::ExpressionFromAddress(expressionAddressForValue(recordData), expressionSizeForValue(recordData)); + } } return m_expression; } diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index aaca8f7bf..f6feb43f4 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -167,7 +167,6 @@ public: int serialize(char * buffer, int bufferSize, Preferences::PrintFloatMode floatDisplayMode = Preferences::PrintFloatMode::Decimal, int numberOfSignificantDigits = PrintFloat::k_numberOfStoredSignificantDigits) const; /* Simplification */ - static Expression ParseAndReduce(const char * text, Context & context, Preferences::AngleUnit); static Expression ParseAndSimplify(const char * text, Context & context, Preferences::AngleUnit angleUnit); Expression simplify(Context & context, Preferences::AngleUnit angleUnit); Expression reduce(Context & context, Preferences::AngleUnit angleUnit); diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp index 1363d40de..8df1b9a5b 100644 --- a/poincare/src/expression.cpp +++ b/poincare/src/expression.cpp @@ -312,14 +312,6 @@ int Expression::serialize(char * buffer, int bufferSize, Preferences::PrintFloat /* Simplification */ -Expression Expression::ParseAndReduce(const char * text, Context & context, Preferences::AngleUnit angleUnit) { - Expression exp = Parse(text); - if (exp.isUninitialized()) { - return Undefined(); - } - return exp.reduce(context, angleUnit); -} - Expression Expression::ParseAndSimplify(const char * text, Context & context, Preferences::AngleUnit angleUnit) { Expression exp = Parse(text); if (exp.isUninitialized()) { @@ -423,7 +415,7 @@ U Expression::approximateToScalar(Context& context, Preferences::AngleUnit angle template U Expression::approximateToScalar(const char * text, Context& context, Preferences::AngleUnit angleUnit) { - Expression exp = ParseAndReduce(text, context, angleUnit); + Expression exp = ParseAndSimplify(text, context, angleUnit); return exp.approximateToScalar(context, angleUnit); } From f6a5759a16a2c08c9225180239921c9b99c66d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 18 Dec 2018 15:48:40 +0100 Subject: [PATCH 05/14] [poincare] Fix the replacement of unknowns in the storage In the Graph app, define f(x) = cos(x) and g(x)=diff(f(x),x,x). The graph and table are perfectly computed, but in the Calculation apps, g(5) fails. --- apps/shared/storage_expression_model.cpp | 3 +- poincare/Makefile | 1 + poincare/include/poincare/derivative.h | 1 + poincare/include/poincare/expression.h | 3 ++ poincare/include/poincare/expression_node.h | 2 ++ poincare/include/poincare/integral.h | 2 ++ .../poincare/parametered_expression_helper.h | 28 ++++++++++++++++ poincare/include/poincare/product.h | 2 ++ poincare/include/poincare/sum.h | 2 ++ poincare/include/poincare/symbol.h | 2 ++ poincare/src/derivative.cpp | 7 +++- poincare/src/expression.cpp | 12 +++++++ poincare/src/expression_node.cpp | 4 +++ poincare/src/integral.cpp | 5 +++ .../src/parametered_expression_helper.cpp | 33 +++++++++++++++++++ poincare/src/product.cpp | 5 +++ poincare/src/sum.cpp | 5 +++ poincare/src/symbol.cpp | 10 ++++++ 18 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 poincare/include/poincare/parametered_expression_helper.h create mode 100644 poincare/src/parametered_expression_helper.cpp diff --git a/apps/shared/storage_expression_model.cpp b/apps/shared/storage_expression_model.cpp index 2f7ac146f..234875015 100644 --- a/apps/shared/storage_expression_model.cpp +++ b/apps/shared/storage_expression_model.cpp @@ -87,8 +87,7 @@ Ion::Storage::Record::ErrorStatus StorageExpressionModel::setContent(const char // Compute the expression to store, without replacing symbols expressionToStore = Expression::Parse(c); if (!expressionToStore.isUninitialized()) { - Symbol xUnknown = Symbol(Symbol::SpecialSymbols::UnknownX); - expressionToStore = expressionToStore.replaceSymbolWithExpression(Symbol("x", 1), xUnknown); + expressionToStore = expressionToStore.replaceUnknown(Symbol('x')); //TODO Beware of non x unknowns! (for instance whe sequences are in the storage) } } return setExpressionContent(expressionToStore); diff --git a/poincare/Makefile b/poincare/Makefile index 532933cf4..4f5baca15 100644 --- a/poincare/Makefile +++ b/poincare/Makefile @@ -97,6 +97,7 @@ objs += $(addprefix poincare/src/,\ nth_root.o\ number.o\ opposite.o\ + parametered_expression_helper.o\ parenthesis.o\ permute_coefficient.o\ power.o\ diff --git a/poincare/include/poincare/derivative.h b/poincare/include/poincare/derivative.h index 9ae929760..3e2458f7f 100644 --- a/poincare/include/poincare/derivative.h +++ b/poincare/include/poincare/derivative.h @@ -22,6 +22,7 @@ public: // Properties Type type() const override { return Type::Derivative; } int polynomialDegree(Context & context, const char * symbolName) const override; + Expression replaceUnknown(const Symbol & symbol) override; private: // Layout diff --git a/poincare/include/poincare/expression.h b/poincare/include/poincare/expression.h index f6feb43f4..77a6a5fc4 100644 --- a/poincare/include/poincare/expression.h +++ b/poincare/include/poincare/expression.h @@ -14,6 +14,7 @@ namespace Poincare { class Context; class SymbolAbstract; +class Symbol; class Expression : public TreeHandle { friend class AbsoluteValue; @@ -154,6 +155,8 @@ public: static constexpr int k_maxNumberOfPolynomialCoefficients = k_maxPolynomialDegree+1; int getPolynomialReducedCoefficients(const char * symbolName, Expression coefficients[], Context & context, Preferences::AngleUnit angleUnit) const; Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) { return node()->replaceSymbolWithExpression(symbol, expression); } + Expression replaceUnknown(const Symbol & symbol); + Expression defaultReplaceUnknown(const Symbol & symbol); /* Comparison */ /* isIdenticalTo is the "easy" equality, it returns true if both trees have diff --git a/poincare/include/poincare/expression_node.h b/poincare/include/poincare/expression_node.h index 82aa7677b..64d65bc67 100644 --- a/poincare/include/poincare/expression_node.h +++ b/poincare/include/poincare/expression_node.h @@ -13,6 +13,7 @@ namespace Poincare { * 'this' outdated. They should only be called in a wrapper on Expression. */ class SymbolAbstract; +class Symbol; class ExpressionNode : public TreeNode { friend class AdditionNode; @@ -103,6 +104,7 @@ public: virtual Sign sign() const { return Sign::Unknown; } virtual bool isNumber() const { return false; } /*!*/ virtual Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression); + /*!*/ virtual Expression replaceUnknown(const Symbol & symbol); /*!*/ virtual Expression setSign(Sign s, Context & context, Preferences::AngleUnit angleUnit); virtual int polynomialDegree(Context & context, const char * symbolName) const; /*!*/ virtual int getPolynomialCoefficients(Context & context, const char * symbolName, Expression coefficients[]) const; diff --git a/poincare/include/poincare/integral.h b/poincare/include/poincare/integral.h index 2fc2cb42b..9c2f08dfa 100644 --- a/poincare/include/poincare/integral.h +++ b/poincare/include/poincare/integral.h @@ -21,6 +21,8 @@ public: // ExpressionNode Type type() const override { return Type::Integral; } int polynomialDegree(Context & context, const char * symbolName) const override; + Expression replaceUnknown(const Symbol & symbol) override; + private: // Layout Layout createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const override; diff --git a/poincare/include/poincare/parametered_expression_helper.h b/poincare/include/poincare/parametered_expression_helper.h new file mode 100644 index 000000000..d56888026 --- /dev/null +++ b/poincare/include/poincare/parametered_expression_helper.h @@ -0,0 +1,28 @@ +#ifndef POINCARE_PARAMETERED_EXPRESSION_HELPER_H +#define POINCARE_PARAMETERED_EXPRESSION_HELPER_H + +#include + +namespace Poincare { + +class SymbolAbstract; +class Symbol; + +// Parametered expressions are Integral, Derivative, Sum and Product. + +namespace ParameteredExpressionHelper { + /* We sometimes replace 'x' by 'UnknownX' to differentiate between a variable + * and a function parameter. For instance, when defining the function + * f(x)=cos(x) in Graph, we want x to be an unknown, instead of having f be + * the constant function equal to cos(user variable that is named x). + * + * In parametered expression, we do not want to replace all the 'x' with + * unknowns: for instance, we want to change f(x)=diff(cos(x),x,x) into + * f(X)=diff(cos(x),x,X), X being an unknown. ReplaceUnknownInExpression does + * that. */ + Expression ReplaceUnknownInExpression(Expression e, const Symbol & symbolToReplace); +}; + +} + +#endif diff --git a/poincare/include/poincare/product.h b/poincare/include/poincare/product.h index 76e2fe4d2..fd98fa751 100644 --- a/poincare/include/poincare/product.h +++ b/poincare/include/poincare/product.h @@ -16,6 +16,8 @@ public: #endif Type type() const override { return Type::Product; } + Expression replaceUnknown(const Symbol & symbol) override; + private: float emptySequenceValue() const override { return 1.0f; } Layout createSequenceLayout(Layout argumentLayout, Layout symbolLayout, Layout subscriptLayout, Layout superscriptLayout) const override; diff --git a/poincare/include/poincare/sum.h b/poincare/include/poincare/sum.h index e39cffcd3..0ac12d2b1 100644 --- a/poincare/include/poincare/sum.h +++ b/poincare/include/poincare/sum.h @@ -16,6 +16,8 @@ public: #endif Type type() const override { return Type::Sum; } + Expression replaceUnknown(const Symbol & symbol) override; + private: float emptySequenceValue() const override { return 0.0f; } Layout createSequenceLayout(Layout argumentLayout, Layout symbolLayout, Layout subscriptLayout, Layout superscriptLayout) const override; diff --git a/poincare/include/poincare/symbol.h b/poincare/include/poincare/symbol.h index b7d1715c0..eb6a90389 100644 --- a/poincare/include/poincare/symbol.h +++ b/poincare/include/poincare/symbol.h @@ -20,6 +20,7 @@ public: // Expression Properties Type type() const override { return Type::Symbol; } Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression) override; + Expression replaceUnknown(const Symbol & symbol) override; int polynomialDegree(Context & context, const char * symbolName) const override; int getPolynomialCoefficients(Context & context, const char * symbolName, Expression coefficients[]) const override; int getVariables(Context & context, isVariableTest isVariable, char * variables, int maxSizeVariable) const override; @@ -77,6 +78,7 @@ public: // Expression Expression shallowReduce(Context & context, Preferences::AngleUnit angleUnit, ExpressionNode::ReductionTarget target); Expression replaceSymbolWithExpression(const SymbolAbstract & symbol, const Expression & expression); + Expression replaceUnknown(const Symbol & symbol); int getPolynomialCoefficients(Context & context, const char * symbolName, Expression coefficients[]) const; Expression shallowReplaceReplaceableSymbols(Context & context); private: diff --git a/poincare/src/derivative.cpp b/poincare/src/derivative.cpp index 38e0b16c6..33ac1b490 100644 --- a/poincare/src/derivative.cpp +++ b/poincare/src/derivative.cpp @@ -1,8 +1,9 @@ #include -#include #include +#include #include #include +#include #include #include #include @@ -26,6 +27,10 @@ int DerivativeNode::polynomialDegree(Context & context, const char * symbolName) return ExpressionNode::polynomialDegree(context, symbolName); } +Expression DerivativeNode::replaceUnknown(const Symbol & symbol) { + return ParameteredExpressionHelper::ReplaceUnknownInExpression(Derivative(this), symbol); +} + Layout DerivativeNode::createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const { return LayoutHelper::Prefix(this, floatDisplayMode, numberOfSignificantDigits, Derivative::s_functionHelper.name()); } diff --git a/poincare/src/expression.cpp b/poincare/src/expression.cpp index 8df1b9a5b..99a630389 100644 --- a/poincare/src/expression.cpp +++ b/poincare/src/expression.cpp @@ -281,6 +281,18 @@ int Expression::getPolynomialReducedCoefficients(const char * symbolName, Expres return degree; } +Expression Expression::replaceUnknown(const Symbol & symbol) { + return node()->replaceUnknown(symbol); +} + +Expression Expression::defaultReplaceUnknown(const Symbol & symbol) { + int nbChildren = numberOfChildren(); + for (int i = 0; i < nbChildren; i++) { + childAtIndex(i).replaceUnknown(symbol); + } + return *this; +} + /* Comparison */ bool Expression::isIdenticalTo(const Expression e) const { diff --git a/poincare/src/expression_node.cpp b/poincare/src/expression_node.cpp index f0c2f61b5..446b7caa5 100644 --- a/poincare/src/expression_node.cpp +++ b/poincare/src/expression_node.cpp @@ -9,6 +9,10 @@ Expression ExpressionNode::replaceSymbolWithExpression(const SymbolAbstract & sy return Expression(this).defaultReplaceSymbolWithExpression(symbol, expression); } +Expression ExpressionNode::replaceUnknown(const Symbol & symbol) { + return Expression(this).defaultReplaceUnknown(symbol); +} + Expression ExpressionNode::setSign(Sign s, Context & context, Preferences::AngleUnit angleUnit) { assert(false); return Expression(); diff --git a/poincare/src/integral.cpp b/poincare/src/integral.cpp index e4a001ddc..0c2c26fd5 100644 --- a/poincare/src/integral.cpp +++ b/poincare/src/integral.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -26,6 +27,10 @@ int IntegralNode::polynomialDegree(Context & context, const char * symbolName) c return ExpressionNode::polynomialDegree(context, symbolName); } +Expression IntegralNode::replaceUnknown(const Symbol & symbol) { + return ParameteredExpressionHelper::ReplaceUnknownInExpression(Integral(this), symbol); +} + Layout IntegralNode::createLayout(Preferences::PrintFloatMode floatDisplayMode, int numberOfSignificantDigits) const { return IntegralLayout( childAtIndex(0)->createLayout(floatDisplayMode, numberOfSignificantDigits), diff --git a/poincare/src/parametered_expression_helper.cpp b/poincare/src/parametered_expression_helper.cpp new file mode 100644 index 000000000..944003c16 --- /dev/null +++ b/poincare/src/parametered_expression_helper.cpp @@ -0,0 +1,33 @@ +#include +#include +#include +#include + +namespace Poincare { + +Expression ParameteredExpressionHelper::ReplaceUnknownInExpression(Expression e, const Symbol & symbolToReplace) { + assert(!e.isUninitialized()); + assert(e.type() == ExpressionNode::Type::Integral + || e.type() == ExpressionNode::Type::Derivative + || e.type() == ExpressionNode::Type::Sum + || e.type() == ExpressionNode::Type::Product); + assert(!symbolToReplace.isUninitialized()); + assert(symbolToReplace.type() == ExpressionNode::Type::Symbol); + + int numberOfChildren = e.numberOfChildren(); + assert(numberOfChildren > 2); + + Expression child1 = e.childAtIndex(1); + assert(child1.type() == ExpressionNode::Type::Symbol); + + Symbol& parameterChild = static_cast(child1); + if (strcmp(parameterChild.name(), symbolToReplace.name()) != 0) { + return e.defaultReplaceUnknown(symbolToReplace); + } + for (int i = 2; i < numberOfChildren; i++) { + e.childAtIndex(i).replaceUnknown(symbolToReplace); + } + return e; +} + +} diff --git a/poincare/src/product.cpp b/poincare/src/product.cpp index d5dd1ce27..721c64648 100644 --- a/poincare/src/product.cpp +++ b/poincare/src/product.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -13,6 +14,10 @@ namespace Poincare { constexpr Expression::FunctionHelper Product::s_functionHelper; +Expression ProductNode::replaceUnknown(const Symbol & symbol) { + return ParameteredExpressionHelper::ReplaceUnknownInExpression(Product(this), symbol); +} + Layout ProductNode::createSequenceLayout(Layout argumentLayout, Layout symbolLayout, Layout subscriptLayout, Layout superscriptLayout) const { return ProductLayout(argumentLayout, symbolLayout, subscriptLayout, superscriptLayout); } diff --git a/poincare/src/sum.cpp b/poincare/src/sum.cpp index c7bd4f496..9f35641bd 100644 --- a/poincare/src/sum.cpp +++ b/poincare/src/sum.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include extern "C" { #include @@ -13,6 +14,10 @@ namespace Poincare { constexpr Expression::FunctionHelper Sum::s_functionHelper; +Expression SumNode::replaceUnknown(const Symbol & symbol) { + return ParameteredExpressionHelper::ReplaceUnknownInExpression(Sum(this), symbol); +} + Layout SumNode::createSequenceLayout(Layout argumentLayout, Layout symbolLayout, Layout subscriptLayout, Layout superscriptLayout) const { return SumLayout(argumentLayout, symbolLayout, subscriptLayout, superscriptLayout); } diff --git a/poincare/src/symbol.cpp b/poincare/src/symbol.cpp index 47f2c4aed..dc9ecf035 100644 --- a/poincare/src/symbol.cpp +++ b/poincare/src/symbol.cpp @@ -27,6 +27,10 @@ Expression SymbolNode::replaceSymbolWithExpression(const SymbolAbstract & symbol return Symbol(this).replaceSymbolWithExpression(symbol, expression); } +Expression SymbolNode::replaceUnknown(const Symbol & symbol) { + return Symbol(this).replaceUnknown(symbol); +} + int SymbolNode::polynomialDegree(Context & context, const char * symbolName) const { if (strcmp(m_name,symbolName) == 0) { return 1; @@ -180,6 +184,12 @@ Expression Symbol::replaceSymbolWithExpression(const SymbolAbstract & symbol, co return *this; } +Expression Symbol::replaceUnknown(const Symbol & symbol) { + assert(!symbol.isUninitialized()); + assert(symbol.type() == ExpressionNode::Type::Symbol); + return replaceSymbolWithExpression(symbol, Symbol(SpecialSymbols::UnknownX)); +} + int Symbol::getPolynomialCoefficients(Context & context, const char * symbolName, Expression coefficients[]) const { if (strcmp(name(), symbolName) == 0) { coefficients[0] = Rational(0); From 5498c84ed7eafaacd997f88d9323f2d934c143c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 18 Dec 2018 12:25:54 +0100 Subject: [PATCH 06/14] [apps/regression] Add tests about regression navigation --- apps/Makefile | 5 + apps/regression/graph_controller.h | 7 +- apps/regression/test/graph.cpp | 176 +++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 apps/regression/test/graph.cpp diff --git a/apps/Makefile b/apps/Makefile index d6e8cffac..98141b448 100644 --- a/apps/Makefile +++ b/apps/Makefile @@ -86,4 +86,9 @@ $(app_objs): $(app_image_objs) epsilon.$(EXE): $(app_objs) $(app_image_objs) +TO_REMOVE := apps/main.o apps/i18n.o +TMP := $(app_objs) $(app_image_objs) +VAR := $(filter-out $(TO_REMOVE), $(TMP)) +test.$(EXE): $(VAR) + products += epsilon.$(EXE) $(app_objs) $(call INLINER_PRODUCTS,$(app_images)) diff --git a/apps/regression/graph_controller.h b/apps/regression/graph_controller.h index d5b45feef..b30ac092e 100644 --- a/apps/regression/graph_controller.h +++ b/apps/regression/graph_controller.h @@ -23,6 +23,11 @@ public: void viewWillAppear() override; void selectRegressionCurve(); int selectedSeriesIndex() const { return *m_selectedSeriesIndex; } + + // moveCursorHorizontally and Vertically are public to be used in tests + bool moveCursorHorizontally(int direction) override; + bool moveCursorVertically(int direction) override; + private: constexpr static int k_maxLegendLength = 16; constexpr static int k_maxNumberOfCharacters = 50; @@ -35,7 +40,6 @@ private: // SimpleInteractiveCurveViewController void reloadBannerView() override; - bool moveCursorHorizontally(int direction) override; Shared::InteractiveCurveViewRange * interactiveCurveViewRange() override; Shared::CurveView * curveView() override; bool handleEnter() override; @@ -43,7 +47,6 @@ private: // InteractiveCurveViewController void initRangeParameters() override; void initCursorParameters() override; - bool moveCursorVertically(int direction) override; uint32_t modelVersion() override; uint32_t rangeVersion() override; bool isCursorVisible() override; diff --git a/apps/regression/test/graph.cpp b/apps/regression/test/graph.cpp new file mode 100644 index 000000000..1162e3451 --- /dev/null +++ b/apps/regression/test/graph.cpp @@ -0,0 +1,176 @@ +#include +#include +#include +#include "../model/model.h" +#include "../graph_controller.h" +#include "../regression_context.h" +#include "../store.h" + +using namespace Poincare; +using namespace Regression; + +void load_in_store( + Regression::Store * store, + double * x0, double * y0, int numberOfPoints0, + double * x1 = nullptr, double * y1 = nullptr, int numberOfPoints1 = 0, + double * x2 = nullptr, double * y2 = nullptr, int numberOfPoints2 = 0) +{ + // Set the points and the regression type + if (numberOfPoints0 != 0 && x0 != nullptr && y0 != nullptr) { + for (int i = 0; i < numberOfPoints0; i++) { + store->set(x0[i], 0, 0, i); + store->set(y0[i], 0, 1, i); + } + } + if (numberOfPoints1 != 0 && x1 != nullptr && y1 != nullptr) { + for (int i = 0; i < numberOfPoints1; i++) { + store->set(x1[i], 1, 0, i); + store->set(y1[i], 1, 1, i); + } + } + if (numberOfPoints2 != 0 && x2 != nullptr && y2 != nullptr) { + for (int i = 0; i < numberOfPoints2; i++) { + store->set(x2[i], 2, 0, i); + store->set(y2[i], 2, 1, i); + } + } +} + +class NavigationEvent { +public: + enum class Direction { + Up, + Right, + Down, + Left + }; + NavigationEvent(Direction d, int series, int dot) : + direction(d), + expectedSelectedSeries(series), + expectedSelectedDot(dot) + {} + Direction direction; + int expectedSelectedSeries; + int expectedSelectedDot; +}; + +void assert_navigation_is( + int numberOfEvents, + NavigationEvent * events, + int startingDotSelection, + int startingSeriesSelection, + double * x0, double * y0, int numberOfPoints0, + double * x1 = nullptr, double * y1 = nullptr, int numberOfPoints1 = 0, + double * x2 = nullptr, double * y2 = nullptr, int numberOfPoints2 = 0) +{ + assert(startingDotSelection >= 0); + Store store; + Shared::CurveViewCursor cursor; + uint32_t dummy; + + int selectedDotIndex = startingDotSelection; + int selectedSeriesIndex = startingSeriesSelection; + + load_in_store(&store, x0, y0, numberOfPoints0, x1, y1, numberOfPoints1, x2, y2, numberOfPoints2); + + if (selectedDotIndex < store.numberOfPairsOfSeries(selectedSeriesIndex)) { + cursor.moveTo( + store.get(selectedSeriesIndex, 0, selectedDotIndex), + store.get(selectedSeriesIndex, 1, selectedDotIndex)); + } else { + cursor.moveTo( + store.meanOfColumn(selectedSeriesIndex, 0), + store.meanOfColumn(selectedSeriesIndex, 1)); + } + + AppsContainerStorage container; + // We do not really care about the snapshot + App app(&container, container.appSnapshotAtIndex(0)); + + GraphController graphController( + &app, &app, nullptr, + &store, + &cursor, + &dummy, &dummy, + &selectedDotIndex, + &selectedSeriesIndex); + + + for (int i = 0; i < numberOfEvents; i++) { + NavigationEvent event = events[i]; + if (event.direction == NavigationEvent::Direction::Up) { + graphController.moveCursorVertically(1); + } else if (event.direction == NavigationEvent::Direction::Right) { + graphController.moveCursorHorizontally(1); + } else if (event.direction == NavigationEvent::Direction::Down) { + graphController.moveCursorVertically(-1); + } else { + assert(event.direction == NavigationEvent::Direction::Left); + graphController.moveCursorHorizontally(-1); + } + quiz_assert(event.expectedSelectedDot == selectedDotIndex); + quiz_assert(event.expectedSelectedSeries == selectedSeriesIndex); + } +} + +QUIZ_CASE(regression_navigation_1) { + constexpr int numberOfPoints0 = 4; + double x0[numberOfPoints0] = {2.0, 3.0, 2.0, 1.0}; + double y0[numberOfPoints0] = {0.0, 1.0, 0.5, 5.0}; + + constexpr int numberOfPoints1 = 3; + double x1[numberOfPoints1] = {1.0, 10.0, 2.0}; + double y1[numberOfPoints1] = {0.0, 25.0, 0.5}; + + constexpr int numberOfEvents = 7; + NavigationEvent events[numberOfEvents] = { + NavigationEvent(NavigationEvent::Direction::Down, 0, -1), + NavigationEvent(NavigationEvent::Direction::Down, 0, 2), + NavigationEvent(NavigationEvent::Direction::Down, 1, 2), + NavigationEvent(NavigationEvent::Direction::Down, 0, 0), + NavigationEvent(NavigationEvent::Direction::Down, 1, 0), + NavigationEvent(NavigationEvent::Direction::Down, 1, -1), + NavigationEvent(NavigationEvent::Direction::Down, 1, -1) + }; + assert_navigation_is(numberOfEvents, events, numberOfPoints0, 0, x0, y0, numberOfPoints0, x1, y1, numberOfPoints1); +} + +QUIZ_CASE(regression_navigation_2) { + constexpr int numberOfPoints0 = 4; + double x0[numberOfPoints0] = {8.0, 9.0, 10.0, 10.0}; + double y0[numberOfPoints0] = {2.0, 5.0, 1.0, 2.0}; + + constexpr int numberOfPoints1 = 3; + double x1[numberOfPoints1] = {10.0, 12.0, 5.0}; + double y1[numberOfPoints1] = {2.0, 0.0, 8.0}; + + constexpr int numberOfEvents = 6; + NavigationEvent events[numberOfEvents] = { + /* FIXME + * Because of double computation error, the regression curve of the series 0 + * is above its mean point. + NavigationEvent(NavigationEvent::Direction::Down, 1, -1), + * */ + NavigationEvent(NavigationEvent::Direction::Down, 0, -1), + NavigationEvent(NavigationEvent::Direction::Down, 0, numberOfPoints0), + NavigationEvent(NavigationEvent::Direction::Down, 0, -1), + NavigationEvent(NavigationEvent::Direction::Down, 0, 3), + NavigationEvent(NavigationEvent::Direction::Down, 1, 0), + NavigationEvent(NavigationEvent::Direction::Down, 0, 2) + }; + assert_navigation_is(numberOfEvents, events, numberOfPoints1, 1, x0, y0, numberOfPoints0, x1, y1, numberOfPoints1); +} + +QUIZ_CASE(regression_navigation_3) { + constexpr int numberOfPoints = 4; + double x[numberOfPoints] = {1.0, 2.0, 3.0, 4.0}; + double y[numberOfPoints] = {2.0, 2.0, 2.0, 2.0}; + + constexpr int numberOfEvents = 3; + NavigationEvent events[numberOfEvents] = { + NavigationEvent(NavigationEvent::Direction::Down, 0, -1), + NavigationEvent(NavigationEvent::Direction::Down, 1, -1), + NavigationEvent(NavigationEvent::Direction::Down, 2, -1), + }; + assert_navigation_is(numberOfEvents, events, 2, 0, x, y, numberOfPoints, x, y, numberOfPoints, x, y, numberOfPoints); +} From 56504525c94f2c875f936781e635b241c6aad647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Tue, 18 Dec 2018 17:42:01 +0100 Subject: [PATCH 07/14] [Makfile] Remove superfluous test_objs We now build all objects even for test, so we do not need most of the previous test_objs --- apps/calculation/Makefile | 2 -- apps/probability/Makefile | 1 - apps/regression/Makefile | 28 ---------------------------- apps/sequence/Makefile | 2 -- apps/shared/Makefile | 2 -- apps/solver/Makefile | 2 -- apps/statistics/Makefile | 1 - 7 files changed, 38 deletions(-) diff --git a/apps/calculation/Makefile b/apps/calculation/Makefile index c2b9bc190..7b797bea7 100644 --- a/apps/calculation/Makefile +++ b/apps/calculation/Makefile @@ -24,7 +24,5 @@ i18n_files += $(addprefix apps/calculation/,\ tests += $(addprefix apps/calculation/test/,\ calculation_store.cpp\ ) -test_objs += $(addprefix apps/calculation/, calculation.o calculation_store.o) app_images += apps/calculation/calculation_icon.png - diff --git a/apps/probability/Makefile b/apps/probability/Makefile index 510b0cf62..7b0765032 100644 --- a/apps/probability/Makefile +++ b/apps/probability/Makefile @@ -39,7 +39,6 @@ i18n_files += $(addprefix apps/probability/,\ tests += $(addprefix apps/probability/test/,\ erf_inv.cpp\ ) -test_objs += $(addprefix apps/probability/law/, erf_inv.o) app_images += apps/probability/probability_icon.png diff --git a/apps/regression/Makefile b/apps/regression/Makefile index 57a950967..3065bc27f 100644 --- a/apps/regression/Makefile +++ b/apps/regression/Makefile @@ -45,33 +45,5 @@ i18n_files += $(addprefix apps/regression/,\ tests += $(addprefix apps/regression/test/,\ model.cpp\ ) -test_objs += $(addprefix apps/regression/,\ - regression_context.o\ - store.o\ -) -test_objs += $(addprefix apps/shared/,\ - store_context.o\ -) -test_objs += $(addprefix apps/shared/,\ - curve_view_cursor.o\ - curve_view_range.o\ - double_pair_store.o\ - interactive_curve_view_range.o\ - interactive_curve_view_range_delegate.o\ - memoized_curve_view_range.o\ - store_context.o\ -) -test_objs += $(addprefix apps/regression/model/,\ - cubic_model.o\ - exponential_model.o\ - linear_model.o\ - logarithmic_model.o\ - logistic_model.o\ - model.o\ - power_model.o\ - quadratic_model.o\ - quartic_model.o\ - trigonometric_model.o\ -) app_images += apps/regression/regression_icon.png diff --git a/apps/sequence/Makefile b/apps/sequence/Makefile index 2941b50fb..76354d74c 100644 --- a/apps/sequence/Makefile +++ b/apps/sequence/Makefile @@ -34,7 +34,5 @@ i18n_files += $(addprefix apps/sequence/,\ tests += $(addprefix apps/sequence/test/,\ sequence.cpp\ ) -test_objs += $(addprefix apps/sequence/, sequence.o sequence_store.o sequence_context.o cache_context.o) -test_objs += $(addprefix apps/shared/, expression_model.o expression_model_store.o function.o function_store.o) app_images += apps/sequence/sequence_icon.png diff --git a/apps/shared/Makefile b/apps/shared/Makefile index 35ed31960..db1d6dd7f 100644 --- a/apps/shared/Makefile +++ b/apps/shared/Makefile @@ -86,5 +86,3 @@ app_objs += $(addprefix apps/shared/,\ vertical_cursor_view.o\ zoom_parameter_controller.o\ ) - -test_objs += $(addprefix apps/shared/, storage_expression_model.o storage_function.o storage_cartesian_function.o) diff --git a/apps/solver/Makefile b/apps/solver/Makefile index c010c0697..a15ab0b36 100644 --- a/apps/solver/Makefile +++ b/apps/solver/Makefile @@ -23,7 +23,5 @@ i18n_files += $(addprefix apps/solver/,\ tests += $(addprefix apps/solver/test/,\ equation_store.cpp\ ) -test_objs += $(addprefix apps/solver/, equation.o equation_store.o) -test_objs += $(addprefix apps/shared/, expression_model.o expression_model_store.o) app_images += apps/solver/solver_icon.png diff --git a/apps/statistics/Makefile b/apps/statistics/Makefile index 003a98214..424fb48c0 100644 --- a/apps/statistics/Makefile +++ b/apps/statistics/Makefile @@ -34,6 +34,5 @@ i18n_files += $(addprefix apps/statistics/,\ tests += $(addprefix apps/statistics/test/,\ store.cpp\ ) -test_objs += $(addprefix apps/statistics/, store.o) app_images += apps/statistics/stat_icon.png From 6740198e067ae5a848a7d481a474b62b8a09fe7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Wed, 19 Dec 2018 15:14:23 +0100 Subject: [PATCH 08/14] [apps/shared] Constexpr reset value in storage_expr_model_list_ctrlr --- ...torage_expression_model_list_controller.cpp | 18 +++++++++--------- .../storage_expression_model_list_controller.h | 3 ++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/apps/shared/storage_expression_model_list_controller.cpp b/apps/shared/storage_expression_model_list_controller.cpp index dda78d027..6dfc64a85 100644 --- a/apps/shared/storage_expression_model_list_controller.cpp +++ b/apps/shared/storage_expression_model_list_controller.cpp @@ -9,8 +9,8 @@ namespace Shared { StorageExpressionModelListController::StorageExpressionModelListController(Responder * parentResponder, I18n::Message text) : ViewController(parentResponder), m_addNewModel(), - m_memoizedCellHeight {-1, -1, -1, -1, -1}, - m_cumulatedHeightForSelectedIndex(-1) + m_memoizedCellHeight {k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue}, + m_cumulatedHeightForSelectedIndex(k_resetedMemoizedValue) { m_addNewModel.setMessage(text); } @@ -20,7 +20,7 @@ void StorageExpressionModelListController::tableSelectionDidChange(int previousS int currentSelectedRow = selectedRow(); // The previously selected cell's height might have changed. - m_memoizedCellHeight[currentSelectedMemoizedIndex] = -1; + m_memoizedCellHeight[currentSelectedMemoizedIndex] = k_resetedMemoizedValue; // Update m_cumulatedHeightForSelectedIndex if we scrolled one cell up/down if (currentSelectedRow == previousSelectedRow + 1) { @@ -29,7 +29,7 @@ void StorageExpressionModelListController::tableSelectionDidChange(int previousS for (int i = 0; i < k_memoizedCellHeightsCount - 1; i++) { m_memoizedCellHeight[i] = m_memoizedCellHeight[i+1]; } - m_memoizedCellHeight[k_memoizedCellHeightsCount-1] = -1; + m_memoizedCellHeight[k_memoizedCellHeightsCount-1] = k_resetedMemoizedValue; // Update m_cumulatedHeightForSelectedIndex if (previousSelectedRow >= 0) { m_cumulatedHeightForSelectedIndex+= memoizedRowHeight(previousSelectedRow); @@ -43,7 +43,7 @@ void StorageExpressionModelListController::tableSelectionDidChange(int previousS for (int i = k_memoizedCellHeightsCount - 1; i > 0; i--) { m_memoizedCellHeight[i] = m_memoizedCellHeight[i-1]; } - m_memoizedCellHeight[0] = -1; + m_memoizedCellHeight[0] = k_resetedMemoizedValue; // Update m_cumulatedHeightForSelectedIndex if (currentSelectedRow >= 0) { m_cumulatedHeightForSelectedIndex-= memoizedRowHeight(currentSelectedRow); @@ -63,7 +63,7 @@ KDCoordinate StorageExpressionModelListController::memoizedRowHeight(int j) { constexpr int halfMemoizationCount = k_memoizedCellHeightsCount/2; if (j >= currentSelectedRow - halfMemoizationCount && j <= currentSelectedRow + halfMemoizationCount) { int memoizedIndex = j - (currentSelectedRow - halfMemoizationCount); - if (m_memoizedCellHeight[memoizedIndex] < 0) { + if (m_memoizedCellHeight[memoizedIndex] == k_resetedMemoizedValue) { m_memoizedCellHeight[memoizedIndex] = expressionRowHeight(j); } return m_memoizedCellHeight[memoizedIndex]; @@ -83,7 +83,7 @@ KDCoordinate StorageExpressionModelListController::memoizedCumulatedHeightFromIn return notMemoizedCumulatedHeightFromIndex(j); } // Recompute the memoized cumulatedHeight if needed - if (m_cumulatedHeightForSelectedIndex < 0) { + if (m_cumulatedHeightForSelectedIndex == k_resetedMemoizedValue) { m_cumulatedHeightForSelectedIndex = notMemoizedCumulatedHeightFromIndex(currentSelectedRow); } /* Compute the wanted cumulated height by adding/removing memoized cell @@ -226,9 +226,9 @@ bool StorageExpressionModelListController::isAddEmptyRow(int j) { } void StorageExpressionModelListController::resetMemoization() { - m_cumulatedHeightForSelectedIndex = -1; + m_cumulatedHeightForSelectedIndex = k_resetedMemoizedValue; for (int i = 0; i < k_memoizedCellHeightsCount; i++) { - m_memoizedCellHeight[i] = -1; + m_memoizedCellHeight[i] = k_resetedMemoizedValue; } } diff --git a/apps/shared/storage_expression_model_list_controller.h b/apps/shared/storage_expression_model_list_controller.h index 7b2353eae..21116c000 100644 --- a/apps/shared/storage_expression_model_list_controller.h +++ b/apps/shared/storage_expression_model_list_controller.h @@ -39,9 +39,10 @@ private: // Memoization static constexpr int k_memoizedCellHeightsCount = 5; static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount == 5, "Wrong array size in initialization of StorageExpressionModelListController::m_memoizedCellHeight."); -static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount % 2 == 1, "StorageExpressionModelListController::k_memoizedCellHeightsCount should be odd to be able to compute the middle element."); + static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount % 2 == 1, "StorageExpressionModelListController::k_memoizedCellHeightsCount should be odd to be able to compute the middle element."); void resetMemoization(); virtual KDCoordinate notMemoizedCumulatedHeightFromIndex(int j) = 0; + static constexpr int k_resetedMemoizedValue = -1; KDCoordinate m_memoizedCellHeight[k_memoizedCellHeightsCount]; KDCoordinate m_cumulatedHeightForSelectedIndex; }; From 13419f434fb841964e71a1095d2f59002eabbcfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Wed, 19 Dec 2018 15:16:00 +0100 Subject: [PATCH 09/14] [apps/shared] Memoize in StorageFuncListCtrl::indexFromCumulatedHeight --- ...storage_expression_model_list_controller.h | 6 ++- .../storage_function_list_controller.cpp | 37 +++++++++++++++++++ .../shared/storage_function_list_controller.h | 1 + 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/apps/shared/storage_expression_model_list_controller.h b/apps/shared/storage_expression_model_list_controller.h index 21116c000..063bcbf73 100644 --- a/apps/shared/storage_expression_model_list_controller.h +++ b/apps/shared/storage_expression_model_list_controller.h @@ -35,14 +35,16 @@ protected: virtual StorageExpressionModelStore * modelStore() = 0; virtual InputViewController * inputController() = 0; EvenOddMessageTextCell m_addNewModel; -private: +protected: // Memoization static constexpr int k_memoizedCellHeightsCount = 5; +private: + // Memoization + static constexpr int k_resetedMemoizedValue = -1; static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount == 5, "Wrong array size in initialization of StorageExpressionModelListController::m_memoizedCellHeight."); static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount % 2 == 1, "StorageExpressionModelListController::k_memoizedCellHeightsCount should be odd to be able to compute the middle element."); void resetMemoization(); virtual KDCoordinate notMemoizedCumulatedHeightFromIndex(int j) = 0; - static constexpr int k_resetedMemoizedValue = -1; KDCoordinate m_memoizedCellHeight[k_memoizedCellHeightsCount]; KDCoordinate m_cumulatedHeightForSelectedIndex; }; diff --git a/apps/shared/storage_function_list_controller.cpp b/apps/shared/storage_function_list_controller.cpp index a555a4c76..d15d10f8f 100644 --- a/apps/shared/storage_function_list_controller.cpp +++ b/apps/shared/storage_function_list_controller.cpp @@ -4,6 +4,7 @@ namespace Shared { static inline int max(int x, int y) { return x > y ? x : y; } +static inline int min(int x, int y) { return x < y ? x : y; } StorageFunctionListController::StorageFunctionListController(Responder * parentResponder, ButtonRowController * header, ButtonRowController * footer, I18n::Message text) : StorageExpressionModelListController(parentResponder, text), @@ -81,6 +82,42 @@ int StorageFunctionListController::indexFromCumulatedWidth(KDCoordinate offsetX) } } +int StorageFunctionListController::indexFromCumulatedHeight(KDCoordinate offsetY) { + if (offsetY == 0) { + return 0; + } + + /* We use memoization to speed up this method: if offsetY is "around" the + * memoized cumulatedHeightForIndex, we can compute its value easily by + * adding/substracting memoized row heights. */ + + int currentSelectedRow = selectedRow(); + int rowsCount = numberOfRows(); + if (rowsCount <= 1 || currentSelectedRow < 1) { + return TableViewDataSource::indexFromCumulatedHeight(offsetY); + } + + KDCoordinate currentCumulatedHeight = cumulatedHeightFromIndex(currentSelectedRow); + if (offsetY > currentCumulatedHeight) { + int iMax = min(k_memoizedCellHeightsCount/2 + 1, rowsCount - currentSelectedRow); + for (int i = 0; i < iMax; i++) { + currentCumulatedHeight+= rowHeight(currentSelectedRow + i); + if (offsetY <= currentCumulatedHeight) { + return currentSelectedRow + i; + } + } + } else { + int iMax = min(k_memoizedCellHeightsCount/2, currentSelectedRow); + for (int i = 1; i <= iMax; i++) { + currentCumulatedHeight-= rowHeight(currentSelectedRow-i); + if (offsetY > currentCumulatedHeight) { + return currentSelectedRow - i; + } + } + } + return TableViewDataSource::indexFromCumulatedHeight(offsetY); +} + int StorageFunctionListController::typeAtLocation(int i, int j) { if (isAddEmptyRow(j)){ return i + 2; diff --git a/apps/shared/storage_function_list_controller.h b/apps/shared/storage_function_list_controller.h index c47644ad1..c8e21fb30 100644 --- a/apps/shared/storage_function_list_controller.h +++ b/apps/shared/storage_function_list_controller.h @@ -26,6 +26,7 @@ public: KDCoordinate cumulatedWidthFromIndex(int i) override; KDCoordinate cumulatedHeightFromIndex(int j) override; int indexFromCumulatedWidth(KDCoordinate offsetX) override; + int indexFromCumulatedHeight(KDCoordinate offsetY) override; int typeAtLocation(int i, int j) override; HighlightCell * reusableCell(int index, int type) override; int reusableCellCount(int type) override; From 216b16fc1156dd281110013d0803eba44102f1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Wed, 19 Dec 2018 15:39:32 +0100 Subject: [PATCH 10/14] [apps/shared] Increase memoized heights count in StorageExpModelListCtrl --- ...orage_expression_model_list_controller.cpp | 5 ++--- ...storage_expression_model_list_controller.h | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/apps/shared/storage_expression_model_list_controller.cpp b/apps/shared/storage_expression_model_list_controller.cpp index 6dfc64a85..d11522dd1 100644 --- a/apps/shared/storage_expression_model_list_controller.cpp +++ b/apps/shared/storage_expression_model_list_controller.cpp @@ -8,10 +8,9 @@ namespace Shared { StorageExpressionModelListController::StorageExpressionModelListController(Responder * parentResponder, I18n::Message text) : ViewController(parentResponder), - m_addNewModel(), - m_memoizedCellHeight {k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue, k_resetedMemoizedValue}, - m_cumulatedHeightForSelectedIndex(k_resetedMemoizedValue) + m_addNewModel() { + resetMemoization(); m_addNewModel.setMessage(text); } diff --git a/apps/shared/storage_expression_model_list_controller.h b/apps/shared/storage_expression_model_list_controller.h index 063bcbf73..1d5b2961b 100644 --- a/apps/shared/storage_expression_model_list_controller.h +++ b/apps/shared/storage_expression_model_list_controller.h @@ -37,12 +37,26 @@ protected: EvenOddMessageTextCell m_addNewModel; protected: // Memoization - static constexpr int k_memoizedCellHeightsCount = 5; + static constexpr int k_memoizedCellHeightsCount = 7; + /* We use memoization to speed up indexFromHeight(offset) in the children + * classes: if offset is "around" the memoized cumulatedHeightForIndex, we can + * compute its value easily by adding/substracting memoized row heights. We + * thus need to memoize 3 cells (see under for explanation on the 3) above the + * selected one, and 3 under, which gives 7 cells. + * 3 is the maximal number of non selected visible rows if the selected cell + * is completely [on top/at the bottom] of the screen. To compute this value: + * (ScreenHeight - Metric::TitleBarHeight - Metric::TabHeight - ButtonRowHeight + * - currentSelectedRowHeight) / Metric::StoreRowHeight + * = (240-18-27-20-50)/50 = 2.5 */ + static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount % 2 == 1, "StorageExpressionModelListController::k_memoizedCellHeightsCount should be odd."); + /* We memoize values for indexes around the selectedRow index. + * k_memoizedCellHeightsCount needs to be odd to compute things such as: + * constexpr int halfMemoizationCount = k_memoizedCellHeightsCount/2; + * if (j < selectedRow - halfMemoizationCount + * || j > selectedRow + halfMemoizationCount) { ... } */ private: // Memoization static constexpr int k_resetedMemoizedValue = -1; - static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount == 5, "Wrong array size in initialization of StorageExpressionModelListController::m_memoizedCellHeight."); - static_assert(StorageExpressionModelListController::k_memoizedCellHeightsCount % 2 == 1, "StorageExpressionModelListController::k_memoizedCellHeightsCount should be odd to be able to compute the middle element."); void resetMemoization(); virtual KDCoordinate notMemoizedCumulatedHeightFromIndex(int j) = 0; KDCoordinate m_memoizedCellHeight[k_memoizedCellHeightsCount]; From 8830821d0ec35faed79f388144e9307a1c8c085d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Fri, 4 Jan 2019 15:20:02 +0100 Subject: [PATCH 11/14] [apps/code/python] Inline and indent some code --- apps/code/app.cpp | 32 ++++++++++++++++---------------- apps/code/console_controller.cpp | 2 +- python/port/port.cpp | 4 ---- python/port/port.h | 2 +- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/apps/code/app.cpp b/apps/code/app.cpp index efc7e6a94..4d18011c2 100644 --- a/apps/code/app.cpp +++ b/apps/code/app.cpp @@ -46,23 +46,23 @@ bool App::Snapshot::lockOnConsole() const { void App::Snapshot::setOpt(const char * name, char * value) { if (strcmp(name, "script") == 0) { - m_scriptStore.deleteAllScripts(); - char * separator = strchr(value, ':'); - if (!separator) { - return; - } - *separator = 0; - const char * scriptName = value; - /* We include the 0 in the scriptContent to represent the importation - * status. It is set to 1 after addScriptFromTemplate. Indeed, this '/0' - * char has two goals: ending the scriptName and representing the - * importation status; we cannot set it to 1 before adding the script to - * storage. */ - const char * scriptContent = separator; - Code::ScriptTemplate script(scriptName, scriptContent); - m_scriptStore.addScriptFromTemplate(&script); - m_scriptStore.scriptNamed(scriptName).toggleImportationStatus(); // set Importation Status to 1 + m_scriptStore.deleteAllScripts(); + char * separator = strchr(value, ':'); + if (!separator) { return; + } + *separator = 0; + const char * scriptName = value; + /* We include the 0 in the scriptContent to represent the importation + * status. It is set to 1 after addScriptFromTemplate. Indeed, this '/0' + * char has two goals: ending the scriptName and representing the + * importation status; we cannot set it to 1 before adding the script to + * storage. */ + const char * scriptContent = separator; + Code::ScriptTemplate script(scriptName, scriptContent); + m_scriptStore.addScriptFromTemplate(&script); + m_scriptStore.scriptNamed(scriptName).toggleImportationStatus(); // set Importation Status to 1 + return; } if (strcmp(name, "lock-on-console") == 0) { m_lockOnConsole = true; diff --git a/apps/code/console_controller.cpp b/apps/code/console_controller.cpp index b3cce8867..86075d0c7 100644 --- a/apps/code/console_controller.cpp +++ b/apps/code/console_controller.cpp @@ -33,7 +33,7 @@ ConsoleController::ConsoleController(Responder * parentResponder, App * pythonDe m_sandboxController(this), m_inputRunLoopActive(false) #if EPSILON_GETOPT - , m_locked(lockOnConsole) + , m_locked(lockOnConsole) #endif { m_selectableTableView.setMargins(0, Metric::CommonRightMargin, 0, Metric::TitleBarExternHorizontalMargin); diff --git a/python/port/port.cpp b/python/port/port.cpp index 68332186e..4c8a465a0 100644 --- a/python/port/port.cpp +++ b/python/port/port.cpp @@ -23,10 +23,6 @@ extern "C" { static MicroPython::ScriptProvider * sScriptProvider = nullptr; static MicroPython::ExecutionEnvironment * sCurrentExecutionEnvironment = nullptr; -MicroPython::ExecutionEnvironment::ExecutionEnvironment() : - m_sandboxIsDisplayed(false) -{ -} MicroPython::ExecutionEnvironment * MicroPython::ExecutionEnvironment::currentExecutionEnvironment() { return sCurrentExecutionEnvironment; diff --git a/python/port/port.h b/python/port/port.h index 697867c57..e28622c80 100644 --- a/python/port/port.h +++ b/python/port/port.h @@ -14,7 +14,7 @@ public: class ExecutionEnvironment { public: - ExecutionEnvironment(); + ExecutionEnvironment() : m_sandboxIsDisplayed(false) {} static ExecutionEnvironment * currentExecutionEnvironment(); void runCode(const char * ); virtual const char * inputText(const char * prompt) { From 33072eb9b1f50db4080601c6b618619a28a7a825 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Fri, 4 Jan 2019 15:21:29 +0100 Subject: [PATCH 12/14] [apps/code] Add some comments --- apps/code/console_controller.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/code/console_controller.cpp b/apps/code/console_controller.cpp index 86075d0c7..2fc42c634 100644 --- a/apps/code/console_controller.cpp +++ b/apps/code/console_controller.cpp @@ -85,17 +85,20 @@ const char * ConsoleController::inputText(const char * prompt) { AppsContainer * a = (AppsContainer *)(app()->container()); m_inputRunLoopActive = true; + // Set the prompt text m_selectableTableView.reloadData(); m_selectableTableView.selectCellAtLocation(0, m_consoleStore.numberOfLines()); m_editCell.setPrompt(prompt); m_editCell.setText(""); + // Run new input loop a->redrawWindow(); a->runWhile([](void * a){ ConsoleController * c = static_cast(a); return c->inputRunLoopActive(); }, this); + // Reset the prompt line flushOutputAccumulationBufferToStore(); m_consoleStore.deleteLastLineIfEmpty(); m_editCell.setPrompt(sStandardPromptText); From 1782326ed8dd4b15dcc8f3638c4e347ab5de0ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Fri, 4 Jan 2019 15:22:12 +0100 Subject: [PATCH 13/14] [apps/code] Clean ConsoleController code --- apps/code/app.cpp | 7 +++---- apps/code/console_controller.cpp | 27 ++++++++++++++++----------- apps/code/console_controller.h | 4 ++-- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/apps/code/app.cpp b/apps/code/app.cpp index 4d18011c2..afc1e7840 100644 --- a/apps/code/app.cpp +++ b/apps/code/app.cpp @@ -93,10 +93,9 @@ App::~App() { bool App::handleEvent(Ion::Events::Event event) { if (event == Ion::Events::Home && m_consoleController.inputRunLoopActive()) { - // We need to return true here because we want to actually exit from the - // input run loop, which requires ending a dispatchEvent cycle. - m_consoleController.askInputRunLoopTermination(); - m_consoleController.interrupt(); + /* We need to return true here because we want to actually exit from the + * input run loop, which requires ending a dispatchEvent cycle. */ + m_consoleController.terminateInputLoop(); if (m_modalViewController.isDisplayingModal()) { m_modalViewController.dismissModalViewController(); } diff --git a/apps/code/console_controller.cpp b/apps/code/console_controller.cpp index 2fc42c634..3d8d3c273 100644 --- a/apps/code/console_controller.cpp +++ b/apps/code/console_controller.cpp @@ -81,6 +81,12 @@ void ConsoleController::runAndPrintForCommand(const char * command) { m_consoleStore.deleteLastLineIfEmpty(); } +void ConsoleController::terminateInputLoop() { + assert(m_inputRunLoopActive); + m_inputRunLoopActive = false; + interrupt(); +} + const char * ConsoleController::inputText(const char * prompt) { AppsContainer * a = (AppsContainer *)(app()->container()); m_inputRunLoopActive = true; @@ -149,9 +155,8 @@ bool ConsoleController::handleEvent(Ion::Events::Event event) { } #if EPSILON_GETOPT if (m_locked && (event == Ion::Events::Home || event == Ion::Events::Back)) { - if (inputRunLoopActive()) { - askInputRunLoopTermination(); - interrupt(); + if (m_inputRunLoopActive) { + terminateInputLoop(); } return true; } @@ -238,10 +243,10 @@ bool ConsoleController::textFieldShouldFinishEditing(TextField * textField, Ion: } bool ConsoleController::textFieldDidReceiveEvent(TextField * textField, Ion::Events::Event event) { - if (event == Ion::Events::Up && inputRunLoopActive()) { - askInputRunLoopTermination(); - // We need to return true here because we want to actually exit from the - // input run loop, which requires ending a dispatchEvent cycle. + if (event == Ion::Events::Up && m_inputRunLoopActive) { + m_inputRunLoopActive = false; + /* We need to return true here because we want to actually exit from the + * input run loop, which requires ending a dispatchEvent cycle. */ return true; } if (event == Ion::Events::Up) { @@ -255,8 +260,8 @@ bool ConsoleController::textFieldDidReceiveEvent(TextField * textField, Ion::Eve } bool ConsoleController::textFieldDidFinishEditing(TextField * textField, const char * text, Ion::Events::Event event) { - if (inputRunLoopActive()) { - askInputRunLoopTermination(); + if (m_inputRunLoopActive) { + m_inputRunLoopActive = false; return false; } runAndPrintForCommand(text); @@ -271,8 +276,8 @@ bool ConsoleController::textFieldDidFinishEditing(TextField * textField, const c } bool ConsoleController::textFieldDidAbortEditing(TextField * textField) { - if (inputRunLoopActive()) { - askInputRunLoopTermination(); + if (m_inputRunLoopActive) { + m_inputRunLoopActive = false; } else { #if EPSILON_GETOPT /* In order to lock the console controller, we disable poping controllers diff --git a/apps/code/console_controller.h b/apps/code/console_controller.h index 287ea9012..87677bcc1 100644 --- a/apps/code/console_controller.h +++ b/apps/code/console_controller.h @@ -31,8 +31,8 @@ public: void autoImport(); void autoImportScript(Script script, bool force = false); void runAndPrintForCommand(const char * command); - bool inputRunLoopActive() { return m_inputRunLoopActive; } - void askInputRunLoopTermination() { m_inputRunLoopActive = false; } + bool inputRunLoopActive() const { return m_inputRunLoopActive; } + void terminateInputLoop(); // ViewController View * view() override { return &m_selectableTableView; } From 58f94f5e5ff692ccb4fb84fe349b820302c84d4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9a=20Saviot?= Date: Fri, 4 Jan 2019 16:00:28 +0100 Subject: [PATCH 14/14] [apps/code] Check that app can be exited before switching to DFU This fixes the following crash: create a script which contains only "input()". Execute it, then while in the input, plug in the calculator. When un-plugging it, the device crashes. --- apps/apps_container.cpp | 28 ++++++++++++++++++++-------- apps/code/app.cpp | 1 + apps/code/app.h | 7 +++++++ escher/include/escher/app.h | 3 +++ 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/apps/apps_container.cpp b/apps/apps_container.cpp index b4c41e6b0..2b038e6e5 100644 --- a/apps/apps_container.cpp +++ b/apps/apps_container.cpp @@ -152,14 +152,25 @@ bool AppsContainer::dispatchEvent(Ion::Events::Event event) { if (event == Ion::Events::USBEnumeration) { if (Ion::USB::isPlugged()) { App::Snapshot * activeSnapshot = (activeApp() == nullptr ? appSnapshotAtIndex(0) : activeApp()->snapshot()); - /* Just after a software update, the battery timer does not have time to - * fire before the calculator enters DFU mode. As the DFU mode blocks the - * event loop, we update the battery state "manually" here. */ - updateBatteryState(); - switchTo(usbConnectedAppSnapshot()); - Ion::USB::DFU(); - switchTo(activeSnapshot); - didProcessEvent = true; + if (activeApp() == nullptr || activeApp()->prepareForExit()) { + /* Just after a software update, the battery timer does not have time to + * fire before the calculator enters DFU mode. As the DFU mode blocks the + * event loop, we update the battery state "manually" here. */ + updateBatteryState(); + switchTo(usbConnectedAppSnapshot()); + Ion::USB::DFU(); + switchTo(activeSnapshot); + didProcessEvent = true; + } else { + /* activeApp()->prepareForExit() returned false, which means that the + * app needs another event loop to prepare for being switched off. + * Discard the current enumeration interruption. + * The USB host tries a few times in a row to enumerate the device, so + * hopefully the device will get another enumeration event soon and this + * time the device will be ready to go in DFU mode. Otherwise, the user + * needs to re-plug the device to go into DFU mode. */ + Ion::USB::clearEnumerationInterrupt(); + } } else { /* Sometimes, the device gets an ENUMDNE interrupts when being unplugged * from a non-USB communicating host (e.g. a USB charger). The interrupt @@ -212,6 +223,7 @@ bool AppsContainer::processEvent(Ion::Events::Event event) { } void AppsContainer::switchTo(App::Snapshot * snapshot) { + assert(activeApp() == nullptr || activeApp()->prepareForExit()); if (activeApp() && snapshot != activeApp()->snapshot()) { resetShiftAlphaStatus(); } diff --git a/apps/code/app.cpp b/apps/code/app.cpp index afc1e7840..bedd3d0f1 100644 --- a/apps/code/app.cpp +++ b/apps/code/app.cpp @@ -88,6 +88,7 @@ App::App(Container * container, Snapshot * snapshot) : } App::~App() { + assert(!m_consoleController.inputRunLoopActive()); deinitPython(); } diff --git a/apps/code/app.h b/apps/code/app.h index 58d007e3c..db1c0bfc6 100644 --- a/apps/code/app.h +++ b/apps/code/app.h @@ -37,6 +37,13 @@ public: ScriptStore m_scriptStore; }; ~App(); + bool prepareForExit() override { + if (m_consoleController.inputRunLoopActive()) { + m_consoleController.terminateInputLoop(); + return false; + } + return true; + } StackViewController * stackViewController() { return &m_codeStackViewController; } ConsoleController * consoleController() { return &m_consoleController; } diff --git a/escher/include/escher/app.h b/escher/include/escher/app.h index ed164c7f4..15790005a 100644 --- a/escher/include/escher/app.h +++ b/escher/include/escher/app.h @@ -50,6 +50,9 @@ public: void setFirstResponder(Responder * responder); Responder * firstResponder(); virtual bool processEvent(Ion::Events::Event event); + /* prepareForExit returns true if the app can be switched off in the current + * runloop step, else it prepares for a switch off and returns false. */ + virtual bool prepareForExit() { return true; } void displayModalViewController(ViewController * vc, float verticalAlignment, float horizontalAlignment, KDCoordinate topMargin = 0, KDCoordinate leftMargin = 0, KDCoordinate bottomMargin = 0, KDCoordinate rightMargin = 0); void dismissModalViewController();