diff --git a/apps/calculation/additional_outputs/complex_list_controller.cpp b/apps/calculation/additional_outputs/complex_list_controller.cpp index 9bfd3e960..2bbf450c2 100644 --- a/apps/calculation/additional_outputs/complex_list_controller.cpp +++ b/apps/calculation/additional_outputs/complex_list_controller.cpp @@ -26,10 +26,10 @@ void ComplexListController::setExpression(Poincare::Expression e) { } Poincare::Context * context = App::app()->localContext(); // Fill Calculation Store - m_calculationStore.push("im(z)", context); - m_calculationStore.push("re(z)", context); - m_calculationStore.push("arg(z)", context); - m_calculationStore.push("abs(z)", context); + m_calculationStore.push("im(z)", context, CalculationHeight); + m_calculationStore.push("re(z)", context, CalculationHeight); + m_calculationStore.push("arg(z)", context, CalculationHeight); + m_calculationStore.push("abs(z)", context, CalculationHeight); // Set Complex illustration // Compute a and b as in Expression::hasDefinedComplexApproximation to ensure the same defined result diff --git a/apps/calculation/additional_outputs/illustrated_list_controller.cpp b/apps/calculation/additional_outputs/illustrated_list_controller.cpp index 1b78bef20..8c74edc87 100644 --- a/apps/calculation/additional_outputs/illustrated_list_controller.cpp +++ b/apps/calculation/additional_outputs/illustrated_list_controller.cpp @@ -80,16 +80,7 @@ KDCoordinate IllustratedListController::rowHeight(int j) { } Shared::ExpiringPointer calculation = m_calculationStore.calculationAtIndex(calculationIndex); constexpr bool expanded = true; - KDCoordinate result = calculation->memoizedHeight(expanded); - if (result < 0) { - result = ScrollableThreeExpressionsCell::Height(calculation.pointer()); - if (result < 0) { - // Raise, because Height modified the calculation and failed. - Poincare::ExceptionCheckpoint::Raise(); - } - calculation->setMemoizedHeight(expanded, result); - } - return result + Metric::CellSeparatorThickness; + return calculation->height(expanded) + Metric::CellSeparatorThickness; } int IllustratedListController::typeAtLocation(int i, int j) { diff --git a/apps/calculation/additional_outputs/illustrated_list_controller.h b/apps/calculation/additional_outputs/illustrated_list_controller.h index 970341821..ea5b484de 100644 --- a/apps/calculation/additional_outputs/illustrated_list_controller.h +++ b/apps/calculation/additional_outputs/illustrated_list_controller.h @@ -10,8 +10,6 @@ namespace Calculation { class IllustratedListController : public ListController, public SelectableTableViewDelegate { -/* TODO There is factorizable code between this and - * Calculation::HistoryController (at least rowHeight). */ public: IllustratedListController(EditExpressionController * editExpressionController); @@ -35,6 +33,7 @@ public: constexpr static KDCoordinate k_illustrationHeight = 120; protected: + static KDCoordinate CalculationHeight(Calculation * c, bool expanded) { return ScrollableThreeExpressionsCell::Height(c); } Poincare::Expression m_savedExpression; CalculationStore m_calculationStore; private: diff --git a/apps/calculation/additional_outputs/scrollable_three_expressions_cell.cpp b/apps/calculation/additional_outputs/scrollable_three_expressions_cell.cpp index 63cf7b17e..563d49e1a 100644 --- a/apps/calculation/additional_outputs/scrollable_three_expressions_cell.cpp +++ b/apps/calculation/additional_outputs/scrollable_three_expressions_cell.cpp @@ -8,8 +8,8 @@ void ScrollableThreeExpressionsView::resetMemoization() { setLayouts(Poincare::Layout(), Poincare::Layout(), Poincare::Layout()); } -void ScrollableThreeExpressionsView::setCalculation(Calculation * calculation, bool * didForceOutput) { - assert(!didForceOutput || *didForceOutput == false); +// TODO: factorize with HistoryViewCell! +void ScrollableThreeExpressionsView::setCalculation(Calculation * calculation, bool canChangeDisplayOutput) { Poincare::Context * context = App::app()->localContext(); // Clean the layouts to make room in the pool @@ -24,13 +24,10 @@ void ScrollableThreeExpressionsView::setCalculation(Calculation * calculation, b bool couldNotCreateExactLayout = false; exactOutputLayout = calculation->createExactOutputLayout(&couldNotCreateExactLayout); if (couldNotCreateExactLayout) { - if (calculation->displayOutput(context) == ::Calculation::Calculation::DisplayOutput::ExactOnly) { - Poincare::ExceptionCheckpoint::Raise(); - } else { + if (canChangeDisplayOutput && calculation->displayOutput(context) != ::Calculation::Calculation::DisplayOutput::ExactOnly) { calculation->forceDisplayOutput(::Calculation::Calculation::DisplayOutput::ApproximateOnly); - if (didForceOutput) { - *didForceOutput = true; - } + } else { + Poincare::ExceptionCheckpoint::Raise(); } } } @@ -44,21 +41,18 @@ void ScrollableThreeExpressionsView::setCalculation(Calculation * calculation, b bool couldNotCreateApproximateLayout = false; approximateOutputLayout = calculation->createApproximateOutputLayout(context, &couldNotCreateApproximateLayout); if (couldNotCreateApproximateLayout) { - if (calculation->displayOutput(context) == ::Calculation::Calculation::DisplayOutput::ApproximateOnly) { - Poincare::ExceptionCheckpoint::Raise(); - } else { + if (canChangeDisplayOutput && calculation->displayOutput(context) != ::Calculation::Calculation::DisplayOutput::ApproximateOnly) { /* Set the display output to ApproximateOnly, make room in the pool by * erasing the exact layout, and retry to create the approximate layout */ calculation->forceDisplayOutput(::Calculation::Calculation::DisplayOutput::ApproximateOnly); - if (didForceOutput) { - *didForceOutput = true; - } exactOutputLayout = Poincare::Layout(); couldNotCreateApproximateLayout = false; approximateOutputLayout = calculation->createApproximateOutputLayout(context, &couldNotCreateApproximateLayout); if (couldNotCreateApproximateLayout) { Poincare::ExceptionCheckpoint::Raise(); } + } else { + Poincare::ExceptionCheckpoint::Raise(); } } @@ -74,16 +68,7 @@ void ScrollableThreeExpressionsView::setCalculation(Calculation * calculation, b KDCoordinate ScrollableThreeExpressionsCell::Height(Calculation * calculation) { ScrollableThreeExpressionsCell cell; - bool didForceOutput = false; - cell.setCalculation(calculation, &didForceOutput); - if (didForceOutput) { - /* We could not compute the height of the calculation as it is (the display - * output was forced to another value during the height computation). - * Warning: the display output of calculation was actually changed, so it - * will cause problems if we already did some computations with another - * display value. */ - return -1; - } + cell.setCalculation(calculation, true); KDRect leftFrame = KDRectZero; KDRect centerFrame = KDRectZero; KDRect approximateSignFrame = KDRectZero; @@ -103,8 +88,8 @@ void ScrollableThreeExpressionsCell::reinitSelection() { m_view.reloadScroll(); } -void ScrollableThreeExpressionsCell::setCalculation(Calculation * calculation, bool * didForceOutput) { - m_view.setCalculation(calculation, didForceOutput); +void ScrollableThreeExpressionsCell::setCalculation(Calculation * calculation, bool canChangeDisplayOutput) { + m_view.setCalculation(calculation, canChangeDisplayOutput); layoutSubviews(); } diff --git a/apps/calculation/additional_outputs/scrollable_three_expressions_cell.h b/apps/calculation/additional_outputs/scrollable_three_expressions_cell.h index e89b68da1..792c65e1d 100644 --- a/apps/calculation/additional_outputs/scrollable_three_expressions_cell.h +++ b/apps/calculation/additional_outputs/scrollable_three_expressions_cell.h @@ -19,7 +19,7 @@ public: setBackgroundColor(KDColorWhite); } void resetMemoization(); - void setCalculation(Calculation * calculation, bool * didForceOutput = nullptr); + void setCalculation(Calculation * calculation, bool canChangeDisplayOutput); void subviewFrames(KDRect * leftFrame, KDRect * centerFrame, KDRect * approximateSignFrame, KDRect * rightFrame) { return m_contentCell.subviewFrames(leftFrame, centerFrame, approximateSignFrame, rightFrame); } @@ -60,7 +60,7 @@ public: void setHighlighted(bool highlight) override { m_view.evenOddCell()->setHighlighted(highlight); } void resetMemoization() { m_view.resetMemoization(); } - void setCalculation(Calculation * calculation, bool * didForceOutput = nullptr); + void setCalculation(Calculation * calculation, bool canChangeDisplayOutput = false); void setDisplayCenter(bool display); ScrollableThreeExpressionsView::SubviewPosition selectedSubviewPosition() { return m_view.selectedSubviewPosition(); } void setSelectedSubviewPosition(ScrollableThreeExpressionsView::SubviewPosition subviewPosition) { m_view.setSelectedSubviewPosition(subviewPosition); } diff --git a/apps/calculation/additional_outputs/trigonometry_list_controller.cpp b/apps/calculation/additional_outputs/trigonometry_list_controller.cpp index a7e0ffa64..b8bdcb076 100644 --- a/apps/calculation/additional_outputs/trigonometry_list_controller.cpp +++ b/apps/calculation/additional_outputs/trigonometry_list_controller.cpp @@ -11,9 +11,9 @@ void TrigonometryListController::setExpression(Poincare::Expression e) { // Fill calculation store Poincare::Context * context = App::app()->localContext(); - m_calculationStore.push("sin(θ)", context); - m_calculationStore.push("cos(θ)", context); - m_calculationStore.push("θ", context); + m_calculationStore.push("sin(θ)", context, CalculationHeight); + m_calculationStore.push("cos(θ)", context, CalculationHeight); + m_calculationStore.push("θ", context, CalculationHeight); // Set trigonometry illustration float angle = Shared::PoincareHelpers::ApproximateToScalar(m_calculationStore.calculationAtIndex(0)->approximateOutput(context, Calculation::NumberOfSignificantDigits::Maximal), context); diff --git a/apps/calculation/calculation.cpp b/apps/calculation/calculation.cpp index cfa79bb4f..92d615b5f 100644 --- a/apps/calculation/calculation.cpp +++ b/apps/calculation/calculation.cpp @@ -120,12 +120,15 @@ Layout Calculation::createApproximateOutputLayout(Context * context, bool * coul } } -void Calculation::setMemoizedHeight(bool expanded, KDCoordinate height) { - if (expanded) { - m_expandedHeight = height; - } else { - m_height = height; - } +KDCoordinate Calculation::height(bool expanded) { + KDCoordinate h = expanded ? m_expandedHeight : m_height; + assert(h >= 0); + return h; +} + +void Calculation::setHeights(KDCoordinate height, KDCoordinate expandedHeight) { + m_height = height; + m_expandedHeight = expandedHeight; } Calculation::DisplayOutput Calculation::displayOutput(Context * context) { @@ -184,9 +187,9 @@ Calculation::DisplayOutput Calculation::displayOutput(Context * context) { } void Calculation::forceDisplayOutput(DisplayOutput d) { + // Heights haven't been computed yet + assert(m_height == -1 && m_expandedHeight == -1); m_displayOutput = d; - // Reset heights memoization as it might have changed when we modify the display output - resetHeightMemoization(); } bool Calculation::shouldOnlyDisplayExactOutput() { @@ -284,9 +287,4 @@ Calculation::AdditionalInformationType Calculation::additionalInformationType(Co return AdditionalInformationType::None; } -void Calculation::resetHeightMemoization() { - m_height = -1; - m_expandedHeight = -1; -} - } diff --git a/apps/calculation/calculation.h b/apps/calculation/calculation.h index 01c7f5d5e..ed6e9f2b2 100644 --- a/apps/calculation/calculation.h +++ b/apps/calculation/calculation.h @@ -81,9 +81,8 @@ public: Poincare::Layout createExactOutputLayout(bool * couldNotCreateExactLayout); Poincare::Layout createApproximateOutputLayout(Poincare::Context * context, bool * couldNotCreateApproximateLayout); - // Memoization of height - KDCoordinate memoizedHeight(bool expanded) { return expanded ? m_expandedHeight : m_height; } - void setMemoizedHeight(bool expanded, KDCoordinate height); + // Heights + KDCoordinate height(bool expanded); // Displayed output DisplayOutput displayOutput(Poincare::Context * context); @@ -97,7 +96,9 @@ private: static constexpr int k_numberOfExpressions = 4; static constexpr KDCoordinate k_heightComputationFailureHeight = 50; static constexpr const char * k_maximalIntegerWithAdditionalInformation = "10000000000000000"; - void resetHeightMemoization(); + + void setHeights(KDCoordinate height, KDCoordinate expandedHeight); + /* Buffers holding text expressions have to be longer than the text written * by user (of maximum length TextField::maxBufferSize()) because when we * print an expression we add omitted signs (multiplications, parenthesis...) */ diff --git a/apps/calculation/calculation_store.cpp b/apps/calculation/calculation_store.cpp index 47f5ebebf..00cef4b06 100644 --- a/apps/calculation/calculation_store.cpp +++ b/apps/calculation/calculation_store.cpp @@ -50,7 +50,7 @@ ExpiringPointer CalculationStore::calculationAtIndex(int i) { return calculationAtIndex(i); } -ExpiringPointer CalculationStore::push(const char * text, Context * context) { +ExpiringPointer CalculationStore::push(const char * text, Context * context, CalculationHeight height) { /* Compute ans now, before the buffer is slided and before the calculation * might be deleted */ Expression ans = ansExpression(context); @@ -84,7 +84,7 @@ ExpiringPointer CalculationStore::push(const char * text, Context * /* If the input does not fit in the store (event if the current * calculation is the only calculation), just replace the calculation with * undef. */ - return emptyStoreAndPushUndef(context); + return emptyStoreAndPushUndef(context, height); } nextSerializationLocation += strlen(nextSerializationLocation) + 1; } @@ -115,7 +115,7 @@ ExpiringPointer CalculationStore::push(const char * text, Context * * undef if it fits, else replace the whole calcualtion with undef. */ Expression undef = Undefined::Builder(); if (!pushSerializeExpression(undef, nextSerializationLocation, &newCalculationsLocation)) { - return emptyStoreAndPushUndef(context); + return emptyStoreAndPushUndef(context, height); } } nextSerializationLocation += strlen(nextSerializationLocation) + 1; @@ -132,7 +132,15 @@ ExpiringPointer CalculationStore::push(const char * text, Context * // Clean the memoization resetMemoizedModelsAfterCalculationIndex(-1); - return ExpiringPointer(reinterpret_cast(m_buffer)); + ExpiringPointer calculation = ExpiringPointer(reinterpret_cast(m_buffer)); + /* Heights are computed now to make sure that the display output is decided + * accordingly to the remaining size in the Poincare pool. Once it is, it + * can't change anymore: the calculation heights are fixed which ensures that + * scrolling computation is right. */ + calculation->setHeights( + height(calculation.pointer(), false), + height(calculation.pointer(), true)); + return calculation; } void CalculationStore::deleteCalculationAtIndex(int i) { @@ -251,12 +259,12 @@ const char * CalculationStore::lastCalculationPosition(const char * calculations return reinterpret_cast(c); } -Shared::ExpiringPointer CalculationStore::emptyStoreAndPushUndef(Context * context) { +Shared::ExpiringPointer CalculationStore::emptyStoreAndPushUndef(Context * context, CalculationHeight height) { /* We end up here as a result of a failed calculation push. The store * attributes are not necessarily clean, so we need to reset them. */ m_slidedBuffer = false; deleteAll(); - return push(Undefined::Name(), context); + return push(Undefined::Name(), context, height); } void CalculationStore::resetMemoizedModelsAfterCalculationIndex(int index) { diff --git a/apps/calculation/calculation_store.h b/apps/calculation/calculation_store.h index 9c2185f88..872115649 100644 --- a/apps/calculation/calculation_store.h +++ b/apps/calculation/calculation_store.h @@ -28,7 +28,8 @@ class CalculationStore { public: CalculationStore(); Shared::ExpiringPointer calculationAtIndex(int i); - Shared::ExpiringPointer push(const char * text, Poincare::Context * context); + typedef KDCoordinate (*CalculationHeight)(Calculation * c, bool expanded); + Shared::ExpiringPointer push(const char * text, Poincare::Context * context, CalculationHeight height); void deleteCalculationAtIndex(int i); void deleteAll(); int numberOfCalculations() const { return m_numberOfCalculations; } @@ -60,7 +61,7 @@ private: char * slideCalculationsToEndOfBuffer(); // returns the new position of the calculations size_t deleteLastCalculation(const char * calculationsStart = nullptr); const char * lastCalculationPosition(const char * calculationsStart) const; - Shared::ExpiringPointer emptyStoreAndPushUndef(Poincare::Context * context); + Shared::ExpiringPointer emptyStoreAndPushUndef(Poincare::Context * context, CalculationHeight height); char m_buffer[k_bufferSize]; const char * m_bufferEnd; diff --git a/apps/calculation/edit_expression_controller.cpp b/apps/calculation/edit_expression_controller.cpp index cd7702e93..2ae7929a9 100644 --- a/apps/calculation/edit_expression_controller.cpp +++ b/apps/calculation/edit_expression_controller.cpp @@ -122,7 +122,7 @@ bool EditExpressionController::inputViewDidReceiveEvent(Ion::Events::Event event if (!myApp->isAcceptableText(m_cacheBuffer)) { return true; } - m_calculationStore->push(m_cacheBuffer, myApp->localContext()); + m_calculationStore->push(m_cacheBuffer, myApp->localContext(), HistoryViewCell::Height); m_historyController->reload(); return true; } @@ -145,7 +145,7 @@ bool EditExpressionController::inputViewDidFinishEditing(const char * text, Layo } else { layoutR.serializeParsedExpression(m_cacheBuffer, k_cacheBufferSize, context); } - m_calculationStore->push(m_cacheBuffer, context); + m_calculationStore->push(m_cacheBuffer, context, HistoryViewCell::Height); m_historyController->reload(); m_contentView.expressionField()->setEditing(true, true); telemetryReportEvent("Input", m_cacheBuffer); diff --git a/apps/calculation/history_controller.cpp b/apps/calculation/history_controller.cpp index 070ecf1a1..16e68704d 100644 --- a/apps/calculation/history_controller.cpp +++ b/apps/calculation/history_controller.cpp @@ -206,20 +206,7 @@ KDCoordinate HistoryController::rowHeight(int j) { } Shared::ExpiringPointer calculation = calculationAtIndex(j); bool expanded = j == selectedRow() && selectedSubviewType() == SubviewType::Output; - KDCoordinate result = calculation->memoizedHeight(expanded); - if (result < 0) { - result = HistoryViewCell::Height(calculation.pointer(), expanded); - if (result < 0) { - // Raise, because Height modified the calculation and failed. - Poincare::ExceptionCheckpoint::Raise(); - } - calculation->setMemoizedHeight(expanded, result); - } - /* We might want to put an assertion here to check the memoization: - * assert(result == HistoryViewCell::Height(calculation.pointer(), expanded)); - * However, Height might fail due to pool memory exhaustion, in which case the - * assertion fails even if "result" had the right value. */ - return result; + return calculation->height(expanded); } int HistoryController::typeAtLocation(int i, int j) { diff --git a/apps/calculation/history_view_cell.cpp b/apps/calculation/history_view_cell.cpp index 2f982053d..e560ccbf3 100644 --- a/apps/calculation/history_view_cell.cpp +++ b/apps/calculation/history_view_cell.cpp @@ -36,16 +36,7 @@ void HistoryViewCellDataSource::setSelectedSubviewType(SubviewType subviewType, KDCoordinate HistoryViewCell::Height(Calculation * calculation, bool expanded) { HistoryViewCell cell(nullptr); - bool didForceOutput = false; - cell.setCalculation(calculation, expanded, &didForceOutput); - if (didForceOutput) { - /* We could not compute the height of the calculation as it is (the display - * output was forced to another value during the height computation). - * Warning: the display output of calculation was actually changed, so it - * will cause problems if we already did some computations with another - * display value. */ - return -1; - } + cell.setCalculation(calculation, expanded, true); KDRect ellipsisFrame = KDRectZero; KDRect inputFrame = KDRectZero; KDRect outputFrame = KDRectZero; @@ -246,8 +237,7 @@ void HistoryViewCell::resetMemoization() { m_calculationCRC32 = 0; } -void HistoryViewCell::setCalculation(Calculation * calculation, bool expanded, bool * didForceOutput) { - assert(!didForceOutput || *didForceOutput == false); +void HistoryViewCell::setCalculation(Calculation * calculation, bool expanded, bool canChangeDisplayOutput) { uint32_t newCalculationCRC = Ion::crc32Byte((const uint8_t *)calculation, ((char *)calculation->next()) - ((char *) calculation)); if (newCalculationCRC == m_calculationCRC32 && m_calculationExpanded == expanded) { return; @@ -273,11 +263,8 @@ void HistoryViewCell::setCalculation(Calculation * calculation, bool expanded, b bool couldNotCreateExactLayout = false; exactOutputLayout = calculation->createExactOutputLayout(&couldNotCreateExactLayout); if (couldNotCreateExactLayout) { - if (calculation->displayOutput(context) != ::Calculation::Calculation::DisplayOutput::ExactOnly) { + if (canChangeDisplayOutput && calculation->displayOutput(context) != ::Calculation::Calculation::DisplayOutput::ExactOnly) { calculation->forceDisplayOutput(::Calculation::Calculation::DisplayOutput::ApproximateOnly); - if (didForceOutput) { - *didForceOutput = true; - } } else { /* We should only display the exact result, but we cannot create it * -> raise an exception. */ @@ -294,21 +281,18 @@ void HistoryViewCell::setCalculation(Calculation * calculation, bool expanded, b bool couldNotCreateApproximateLayout = false; approximateOutputLayout = calculation->createApproximateOutputLayout(context, &couldNotCreateApproximateLayout); if (couldNotCreateApproximateLayout) { - if (calculation->displayOutput(context) == ::Calculation::Calculation::DisplayOutput::ApproximateOnly) { - Poincare::ExceptionCheckpoint::Raise(); - } else { + if (canChangeDisplayOutput && calculation->displayOutput(context) != ::Calculation::Calculation::DisplayOutput::ApproximateOnly) { /* Set the display output to ApproximateOnly, make room in the pool by * erasing the exact layout, and retry to create the approximate layout */ calculation->forceDisplayOutput(::Calculation::Calculation::DisplayOutput::ApproximateOnly); - if (didForceOutput) { - *didForceOutput = true; - } exactOutputLayout = Poincare::Layout(); couldNotCreateApproximateLayout = false; approximateOutputLayout = calculation->createApproximateOutputLayout(context, &couldNotCreateApproximateLayout); if (couldNotCreateApproximateLayout) { Poincare::ExceptionCheckpoint::Raise(); } + } else { + Poincare::ExceptionCheckpoint::Raise(); } } } diff --git a/apps/calculation/history_view_cell.h b/apps/calculation/history_view_cell.h index abe2d6ac8..5e296f529 100644 --- a/apps/calculation/history_view_cell.h +++ b/apps/calculation/history_view_cell.h @@ -51,7 +51,7 @@ public: Poincare::Layout layout() const override; KDColor backgroundColor() const override { return m_even ? KDColorWhite : Palette::WallScreen; } void resetMemoization(); - void setCalculation(Calculation * calculation, bool expanded, bool * didForceOutput = nullptr); + void setCalculation(Calculation * calculation, bool expanded, bool canChangeDisplayOutput = false); int numberOfSubviews() const override { return 2 + displayedEllipsis(); } View * subviewAtIndex(int index) override; void layoutSubviews(bool force = false) override;