Merge pull request #421 from Crzyrndm/experimental/sheet-data-parser

Accelerated worksheet parsing

benchmarks/spreadsheet-load.cpp

@@ -0,0 +1,33 @@
+#include <xlnt/xlnt.hpp>
+#include <chrono>
+#include <helpers/path_helper.hpp>
+
+namespace {
+using milliseconds_d = std::chrono::duration<double, std::milli>;
+
+void run_test(const xlnt::path &file, int runs = 10)
+{
+    std::cout << file.string() << "\n\n";
+
+    xlnt::workbook wb;
+    std::vector<std::chrono::steady_clock::duration> test_timings;
+
+    for (int i = 0; i < runs; ++i)
+    {
+        auto start = std::chrono::steady_clock::now();
+        wb.load(file);
+
+        auto end = std::chrono::steady_clock::now();
+        wb.clear();
+        test_timings.push_back(end - start);
+
+        std::cout << milliseconds_d(test_timings.back()).count() << " ms\n";
+    }
+}
+} // namespace
+
+int main()
+{
+    run_test(path_helper::benchmark_file("large.xlsx"));
+    run_test(path_helper::benchmark_file("very_large.xlsx"));
+}

include/xlnt/styles/format.hpp

@@ -214,6 +214,7 @@ public:
 private:
     friend struct detail::stylesheet;
     friend class detail::xlsx_producer;
+    friend class detail::xlsx_consumer;
     friend class cell;
 
     /// <summary>

include/xlnt/utils/numeric.hpp

@@ -28,9 +28,12 @@
 #include <limits>
 #include <sstream>
 #include <type_traits>
+#include <cassert>
+#include <algorithm>
 
 namespace xlnt {
 namespace detail {
+
 /// <summary>
 /// Takes in any number and outputs a string form of that number which will
 /// serialise and deserialise without loss of precision
@@ -84,8 +87,7 @@ constexpr typename std::common_type<NumberL, NumberR>::type min(NumberL lval, Nu
 /// </summary>
 template <typename EpsilonType = float, // the type to extract epsilon from
     typename LNumber, typename RNumber> // parameter types (deduced)
-bool
+bool float_equals(const LNumber &lhs, const RNumber &rhs,
-float_equals(const LNumber &lhs, const RNumber &rhs,
     int epsilon_scale = 20) // scale the "fuzzy" equality. Higher value gives a more tolerant comparison
 {
     // a type that lhs and rhs can agree on
@@ -115,5 +117,46 @@ float_equals(const LNumber &lhs, const RNumber &rhs,
     return ((lhs + scaled_fuzz) >= rhs) && ((rhs + scaled_fuzz) >= lhs);
 }
 
+struct number_converter
+{
+    explicit number_converter()
+        : should_convert_to_comma(std::use_facet<std::numpunct<char>>(std::locale{}).decimal_point() == ',')
+    {
+    }
+
+    double stold(std::string &s) const noexcept
+    {
+        assert(!s.empty());
+        if (should_convert_to_comma)
+        {
+            auto decimal_pt = std::find(s.begin(), s.end(), '.');
+            if (decimal_pt != s.end())
+            {
+                *decimal_pt = ',';
+            }
+        }
+        return strtod(s.c_str(), nullptr);
+    }
+
+    double stold(const std::string &s) const
+    {
+        assert(!s.empty());
+        if (!should_convert_to_comma)
+        {
+            return strtod(s.c_str(), nullptr);
+        }
+        std::string copy(s);
+        auto decimal_pt = std::find(copy.begin(), copy.end(), '.');
+        if (decimal_pt != copy.end())
+        {
+            *decimal_pt = ',';
+        }
+        return strtod(copy.c_str(), nullptr);
+    }
+
+private:
+    bool should_convert_to_comma = false;
+};
+
 } // namespace detail
 } // namespace xlnt

include/xlnt/utils/optional.hpp

@@ -25,7 +25,7 @@
 
 #include "xlnt/xlnt_config.hpp"
 #include "xlnt/utils/exceptions.hpp"
-#include "../source/detail/numeric_utils.hpp"
+#include "xlnt/utils/numeric.hpp"
 #include <type_traits>
 
 namespace xlnt {

include/xlnt/worksheet/sheet_format_properties.hpp

@@ -25,7 +25,7 @@
 
 #include <xlnt/xlnt_config.hpp>
 #include <xlnt/utils/optional.hpp>
-#include "../source/detail/numeric_utils.hpp"
+#include <xlnt/utils/numeric.hpp>
 
 namespace xlnt {
 

source/detail/header_footer/header_footer_code.cpp

@@ -22,7 +22,7 @@
 // @author: see AUTHORS file
 
 #include <detail/header_footer/header_footer_code.hpp>
-#include <detail/numeric_utils.hpp>
+//#include <detail/numeric_utils.hpp>
 
 namespace xlnt {
 namespace detail {

source/detail/implementations/cell_impl.hpp

@@ -33,7 +33,7 @@
 #include <xlnt/utils/optional.hpp>
 #include <detail/implementations/format_impl.hpp>
 #include <detail/implementations/hyperlink_impl.hpp>
-#include "../numeric_utils.hpp"
+//#include "../numeric_utils.hpp"
 
 namespace xlnt {
 namespace detail {

source/detail/serialization/xlsx_consumer.cpp

@@ -44,24 +44,33 @@
 #include <detail/serialization/xlsx_consumer.hpp>
 #include <detail/serialization/zstream.hpp>
 
-namespace std {
-
-/// <summary>
-/// Allows xml::qname to be used as a key in a std::unordered_map.
-/// </summary>
-template <>
-struct hash<xml::qname>
-{
-    std::size_t operator()(const xml::qname &k) const
-    {
-        static std::hash<std::string> hasher;
-        return hasher(k.string());
-    }
-};
-
-} // namespace std
-
 namespace {
+/// string_equal
+/// for comparison between std::string and string literals
+/// improves on std::string::operator==(char*) by knowing the length ahead of time
+template <size_t N>
+inline bool string_arr_loop_equal(const std::string &lhs, const char (&rhs)[N])
+{
+    for (size_t i = 0; i < N - 1; ++i)
+    {
+        if (lhs[i] != rhs[i])
+        {
+            return false;
+        }
+    }
+    return true;
+}
+
+template <size_t N>
+inline bool string_equal(const std::string &lhs, const char (&rhs)[N])
+{
+    if (lhs.size() != N - 1)
+    {
+        return false;
+    }
+    // split function to assist with inlining of the size check
+    return string_arr_loop_equal(lhs, rhs);
+}
 
 xml::qname &qn(const std::string &namespace_, const std::string &name)
 {
@@ -107,25 +116,6 @@ bool is_true(const std::string &bool_string)
 #endif
 }
 
-struct number_converter
-{
-    number_converter()
-    {
-        stream.imbue(std::locale("C"));
-    }
-
-    double stold(const std::string &s)
-    {
-        stream.str(s);
-        stream.clear();
-        stream >> result;
-        return result;
-    }
-
-    std::istringstream stream;
-    double result;
-};
-
 using style_id_pair = std::pair<xlnt::detail::style_impl, std::size_t>;
 
 /// <summary>
@@ -143,6 +133,302 @@ void set_style_by_xfid(const std::vector<style_id_pair> &styles,
     }
 }
 
+/// parsing assumptions used by the following functions
+/// - on entry, the start element for the element has been consumed by parser->next
+/// - on exit, the closing element has been consumed by parser->next
+/// using these assumptions, the following functions DO NOT use parser->peek (SLOW!!!)
+/// probable further gains from not building an attribute map and using the attribute events instead as the impl just iterates the map
+
+/// 'r' == cell reference e.g. 'A1'
+/// https://docs.microsoft.com/en-us/openspecs/office_standards/ms-oe376/db11a912-b1cb-4dff-b46d-9bedfd10cef0
+///
+/// a lightweight version of xlnt::cell_reference with no extre functionality (absolute/relative, ...)
+/// many thousands are created during parsing, so even minor overhead is noticable
+struct Cell_Reference
+{
+    // not commonly used, added as the obvious ctor
+    explicit Cell_Reference(xlnt::row_t row_arg, xlnt::column_t::index_t column_arg) noexcept
+        : row(row_arg), column(column_arg)
+    {
+    }
+    // the common case. row # is already known during parsing (from parent <row> element)
+    // just need to evaluate the column
+    explicit Cell_Reference(xlnt::row_t row_arg, const std::string &reference) noexcept
+        : row(row_arg)
+    {
+        // only three characters allowed for the column
+        // assumption:
+        // - regex pattern match: [A-Z]{1,3}\d{1,7}
+        const char *iter = reference.c_str();
+        int temp = *iter - 'A' + 1; // 'A' == 1
+        ++iter;
+        if (*iter >= 'A') // second char
+        {
+            temp *= 26; // LHS values are more significant
+            temp += *iter - 'A' + 1; // 'A' == 1
+            ++iter;
+            if (*iter >= 'A') // third char
+            {
+                temp *= 26; // LHS values are more significant
+                temp += *iter - 'A' + 1; // 'A' == 1
+            }
+        }
+        column = static_cast<xlnt::column_t::index_t>(temp);
+    }
+
+    xlnt::row_t row; // range:[1, 1048576]
+    xlnt::column_t::index_t column; // range:["A", "ZZZ"] -> [1, 26^3] -> [1, 17576]
+};
+
+// <c> inside <row> element
+// https://docs.microsoft.com/en-us/dotnet/api/documentformat.openxml.spreadsheet.cell?view=openxml-2.8.1
+struct Cell
+{
+    bool is_phonetic = false; // 'ph'
+    xlnt::cell::type type = xlnt::cell::type::number; // 't'
+    int cell_metatdata_idx = -1; // 'cm'
+    int style_index = -1; // 's'
+    Cell_Reference ref{0, 0}; // 'r'
+    std::string value; // <v> OR <is>
+    std::string formula_string; // <f>
+};
+
+// <sheetData> element
+struct Sheet_Data
+{
+    std::vector<std::pair<xlnt::row_properties, xlnt::row_t>> parsed_rows;
+    std::vector<Cell> parsed_cells;
+};
+
+xlnt::cell::type type_from_string(const std::string &str)
+{
+    if (string_equal(str, "s"))
+    {
+        return xlnt::cell::type::shared_string;
+    }
+    else if (string_equal(str, "n"))
+    {
+        return xlnt::cell::type::number;
+    }
+    else if (string_equal(str, "b"))
+    {
+        return xlnt::cell::type::boolean;
+    }
+    else if (string_equal(str, "e"))
+    {
+        return xlnt::cell::type::error;
+    }
+    else if (string_equal(str, "inlineStr"))
+    {
+        return xlnt::cell::type::inline_string;
+    }
+    else if (string_equal(str, "str"))
+    {
+        return xlnt::cell::type::formula_string;
+    }
+    return xlnt::cell::type::shared_string;
+}
+
+Cell parse_cell(xlnt::row_t row_arg, xml::parser *parser)
+{
+    Cell c;
+    for (auto &attr : parser->attribute_map())
+    {
+        if (string_equal(attr.first.name(), "r"))
+        {
+            c.ref = Cell_Reference(row_arg, attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "t"))
+        {
+            c.type = type_from_string(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "s"))
+        {
+            c.style_index = static_cast<int>(strtol(attr.second.value.c_str(), nullptr, 10));
+        }
+        else if (string_equal(attr.first.name(), "ph"))
+        {
+            c.is_phonetic = is_true(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "cm"))
+        {
+            c.cell_metatdata_idx = static_cast<int>(strtol(attr.second.value.c_str(), nullptr, 10));
+        }
+    }
+    int level = 1; // nesting level
+        // 1 == <c>
+        // 2 == <v>/<is>/<f>
+        // exit loop at </c>
+    while (level > 0)
+    {
+        xml::parser::event_type e = parser->next();
+        switch (e)
+        {
+        case xml::parser::start_element:
+        {
+            ++level;
+            break;
+        }
+        case xml::parser::end_element:
+        {
+            --level;
+            break;
+        }
+        case xml::parser::characters:
+        {
+            // only want the characters inside one of the nested tags
+            // without this a lot of formatting whitespace can get added
+            if (level == 2)
+            {
+                // <v> -> numeric values
+                // <is> -> inline string
+                if (string_equal(parser->name(), "v") || string_equal(parser->name(), "is"))
+                {
+                    c.value += std::move(parser->value());
+                }
+                // <f> formula
+                else if (string_equal(parser->name(), "f"))
+                {
+                    c.formula_string += std::move(parser->value());
+                }
+            }
+            break;
+        }
+        case xml::parser::start_namespace_decl:
+        case xml::parser::end_namespace_decl:
+        case xml::parser::start_attribute:
+        case xml::parser::end_attribute:
+        case xml::parser::eof:
+        default:
+        {
+            throw xlnt::exception("unexcpected XML parsing event");
+        }
+        }
+    }
+    return c;
+}
+
+// <row> inside <sheetData> element
+std::pair<xlnt::row_properties, int> parse_row(xml::parser *parser, xlnt::detail::number_converter &converter, std::vector<Cell> &parsed_cells)
+{
+    std::pair<xlnt::row_properties, int> props;
+    for (auto &attr : parser->attribute_map())
+    {
+        if (string_equal(attr.first.name(), "dyDescent"))
+        {
+            props.first.dy_descent = converter.stold(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "spans"))
+        {
+            props.first.spans = attr.second.value;
+        }
+        else if (string_equal(attr.first.name(), "ht"))
+        {
+            props.first.height = converter.stold(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "s"))
+        {
+            props.first.style = strtoul(attr.second.value.c_str(), nullptr, 10);
+        }
+        else if (string_equal(attr.first.name(), "hidden"))
+        {
+            props.first.hidden = is_true(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "customFormat"))
+        {
+            props.first.custom_format = is_true(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "ph"))
+        {
+            is_true(attr.second.value);
+        }
+        else if (string_equal(attr.first.name(), "r"))
+        {
+            props.second = static_cast<int>(strtol(attr.second.value.c_str(), nullptr, 10));
+        }
+        else if (string_equal(attr.first.name(), "customHeight"))
+        {
+            props.first.custom_height = is_true(attr.second.value.c_str());
+        }
+    }
+
+    int level = 1;
+    while (level > 0)
+    {
+        xml::parser::event_type e = parser->next();
+        switch (e)
+        {
+        case xml::parser::start_element:
+        {
+            parsed_cells.push_back(parse_cell(props.second, parser));
+            break;
+        }
+        case xml::parser::end_element:
+        {
+            --level;
+            break;
+        }
+        case xml::parser::characters:
+        {
+            // ignore whitespace
+            break;
+        }
+        case xml::parser::start_namespace_decl:
+        case xml::parser::start_attribute:
+        case xml::parser::end_namespace_decl:
+        case xml::parser::end_attribute:
+        case xml::parser::eof:
+        default:
+        {
+            throw xlnt::exception("unexcpected XML parsing event");
+        }
+        }
+    }
+    return props;
+}
+
+// <sheetData> inside <worksheet> element
+Sheet_Data parse_sheet_data(xml::parser *parser, xlnt::detail::number_converter &converter)
+{
+    Sheet_Data sheet_data;
+    int level = 1; // nesting level
+        // 1 == <sheetData>
+        // 2 == <row>
+
+    while (level > 0)
+    {
+        xml::parser::event_type e = parser->next();
+        switch (e)
+        {
+        case xml::parser::start_element:
+        {
+            sheet_data.parsed_rows.push_back(parse_row(parser, converter, sheet_data.parsed_cells));
+            break;
+        }
+        case xml::parser::end_element:
+        {
+            --level;
+            break;
+        }
+        case xml::parser::characters:
+        {
+            // ignore, whitespace formatting normally
+            break;
+        }
+        case xml::parser::start_namespace_decl:
+        case xml::parser::start_attribute:
+        case xml::parser::end_namespace_decl:
+        case xml::parser::end_attribute:
+        case xml::parser::eof:
+        default:
+        {
+            throw xlnt::exception("unexcpected XML parsing event");
+        }
+        }
+    }
+    return sheet_data;
+}
+
 } // namespace
 
 /*
@@ -204,7 +490,7 @@ cell xlsx_consumer::read_cell()
 
         if (parser().attribute_present("ht"))
         {
-            row_properties.height = parser().attribute<double>("ht");
+            row_properties.height = converter_.stold(parser().attribute("ht"));
         }
 
         if (parser().attribute_present("customHeight"))
@@ -219,10 +505,11 @@ cell xlsx_consumer::read_cell()
 
         if (parser().attribute_present(qn("x14ac", "dyDescent")))
         {
-            row_properties.dy_descent = parser().attribute<double>(qn("x14ac", "dyDescent"));
+            row_properties.dy_descent = converter_.stold(parser().attribute(qn("x14ac", "dyDescent")));
         }
 
-        if (parser().attribute_present("spans")) {
+        if (parser().attribute_present("spans"))
+        {
             row_properties.spans = parser().attribute("spans");
         }
 
@@ -310,8 +597,6 @@ cell xlsx_consumer::read_cell()
         cell.formula(formula_value_string);
     }
 
-    number_converter converter;
-
     if (has_value)
     {
         if (type == "str")
@@ -326,7 +611,7 @@ cell xlsx_consumer::read_cell()
         }
         else if (type == "s")
         {
-            cell.d_->value_numeric_ = converter.stold(value_string);
+            cell.d_->value_numeric_ = converter_.stold(value_string);
             cell.data_type(cell::type::shared_string);
         }
         else if (type == "b") // boolean
@@ -335,7 +620,7 @@ cell xlsx_consumer::read_cell()
         }
         else if (type == "n") // numeric
         {
-            cell.value(converter.stold(value_string));
+            cell.value(converter_.stold(value_string));
         }
         else if (!value_string.empty() && value_string[0] == '#')
         {
@@ -587,23 +872,23 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
             if (parser().attribute_present("baseColWidth"))
             {
                 ws.d_->format_properties_.base_col_width =
-                    parser().attribute<double>("baseColWidth");
+                    converter_.stold(parser().attribute("baseColWidth"));
             }
             if (parser().attribute_present("defaultColWidth"))
             {
                 ws.d_->format_properties_.default_column_width =
-                    parser().attribute<double>("defaultColWidth");
+                    converter_.stold(parser().attribute("defaultColWidth"));
             }
             if (parser().attribute_present("defaultRowHeight"))
             {
                 ws.d_->format_properties_.default_row_height =
-                    parser().attribute<double>("defaultRowHeight");
+                    converter_.stold(parser().attribute("defaultRowHeight"));
             }
 
             if (parser().attribute_present(qn("x14ac", "dyDescent")))
             {
                 ws.d_->format_properties_.dy_descent =
-                    parser().attribute<double>(qn("x14ac", "dyDescent"));
+                    converter_.stold(parser().attribute(qn("x14ac", "dyDescent")));
             }
 
             skip_attributes();
@@ -620,10 +905,10 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
                 auto max = static_cast<column_t::index_t>(std::stoull(parser().attribute("max")));
 
                 // avoid uninitialised warnings in GCC by using a lambda to make the conditional initialisation
-                optional<double> width = [](xml::parser &p) -> xlnt::optional<double> {
+                optional<double> width = [this](xml::parser &p) -> xlnt::optional<double> {
                     if (p.attribute_present("width"))
                     {
-                        return (p.attribute<double>("width") * 7 - 5) / 7;
+                        return (converter_.stold(p.attribute("width")) * 7 - 5) / 7;
                     }
                     return xlnt::optional<double>();
                 }(parser());
@@ -682,163 +967,76 @@ std::string xlsx_consumer::read_worksheet_begin(const std::string &rel_id)
 
 void xlsx_consumer::read_worksheet_sheetdata()
 {
-    auto ws = worksheet(current_worksheet_);
-
     if (stack_.back() != qn("spreadsheetml", "sheetData"))
     {
         return;
     }
-
+    Sheet_Data ws_data = parse_sheet_data(parser_, converter_);
-    number_converter converter;
+    // NOTE: parse->construct are seperated here and could easily be threaded
-
+    // with a SPSC queue for what is likely to be an easy performance win
-    while (in_element(qn("spreadsheetml", "sheetData")))
+    for (auto &row : ws_data.parsed_rows)
     {
-        expect_start_element(qn("spreadsheetml", "row"), xml::content::complex); // CT_Row
+        current_worksheet_->row_properties_.emplace(row.second, std::move(row.first));
-        auto row_index = parser().attribute<row_t>("r");
+    }
-        auto &row_properties = ws.row_properties(row_index);
+    auto impl = detail::cell_impl();
-
+    for (Cell &cell : ws_data.parsed_cells)
-        if (parser().attribute_present("ht"))
     {
-            row_properties.height = parser().attribute<double>("ht");
+        impl.parent_ = current_worksheet_;
-        }
+        impl.column_ = cell.ref.column;
-
+        impl.row_ = cell.ref.row;
-        if (parser().attribute_present("customHeight"))
+        detail::cell_impl *ws_cell_impl = &current_worksheet_->cell_map_.emplace(cell_reference(impl.column_, impl.row_), std::move(impl)).first->second;
+        if (cell.style_index != -1)
         {
-            row_properties.custom_height = is_true(parser().attribute("customHeight"));
+            ws_cell_impl->format_ = target_.format(static_cast<size_t>(cell.style_index)).d_;
         }
-
+        if (cell.cell_metatdata_idx != -1)
-        if (parser().attribute_present("hidden") && is_true(parser().attribute("hidden")))
         {
-            row_properties.hidden = true;
         }
-
+        ws_cell_impl->phonetics_visible_ = cell.is_phonetic;
-        if (parser().attribute_present(qn("x14ac", "dyDescent")))
+        if (!cell.formula_string.empty())
         {
-            row_properties.dy_descent = parser().attribute<double>(qn("x14ac", "dyDescent"));
+            ws_cell_impl->formula_ = cell.formula_string[0] == '=' ? cell.formula_string.substr(1) : std::move(cell.formula_string);
         }
-
+        if (!cell.value.empty())
-        if (parser().attribute_present("s"))
         {
-            row_properties.style.set(static_cast<std::size_t>(std::stoull(parser().attribute("s"))));
+            ws_cell_impl->type_ = cell.type;
-        }
+            switch (cell.type)
-        if (parser().attribute_present("customFormat"))
             {
-            row_properties.custom_format.set(parser().attribute<bool>("customFormat"));
+            case cell::type::boolean:
-        }
-
-        if (parser().attribute_present("spans")) {
-            row_properties.spans = parser().attribute("spans");
-        }
-
-        skip_attributes({"customFont",
-            "outlineLevel", "collapsed", "thickTop", "thickBot",
-            "ph"});
-
-        while (in_element(qn("spreadsheetml", "row")))
             {
-            expect_start_element(qn("spreadsheetml", "c"), xml::content::complex);
+                ws_cell_impl->value_numeric_ = is_true(cell.value) ? 1.0 : 0.0;
-            auto cell = ws.cell(cell_reference(parser().attribute("r")));
+                break;
-
+            }
-            auto has_type = parser().attribute_present("t");
+            case cell::type::empty:
-            auto type = has_type ? parser().attribute("t") : "n";
+            case cell::type::number:
-
-            if (parser().attribute_present("s"))
             {
-                cell.format(target_.format(static_cast<std::size_t>(std::stoull(parser().attribute("s")))));
+                ws_cell_impl->value_numeric_ = converter_.stold(cell.value);
+                break;
             }
-
+            case cell::type::shared_string:
-            if (parser().attribute_present("ph"))
             {
-                cell.d_->phonetics_visible_ = parser().attribute<bool>("ph");
+                ws_cell_impl->value_numeric_ = static_cast<double>(strtol(cell.value.c_str(), nullptr, 10));
+                break;
             }
-
+            case cell::type::inline_string:
-            auto has_value = false;
-            auto value_string = std::string();
-
-            auto has_formula = false;
-            auto has_shared_formula = false;
-            auto formula_value_string = std::string();
-
-            while (in_element(qn("spreadsheetml", "c")))
             {
-                auto current_element = expect_start_element(xml::content::mixed);
+                ws_cell_impl->value_text_ = std::move(cell.value);
-
+                break;
-                if (current_element == qn("spreadsheetml", "v")) // s:ST_Xstring
+            }
+            case cell::type::formula_string:
             {
-                    has_value = true;
+                ws_cell_impl->value_text_ = std::move(cell.value);
-                    value_string = read_text();
+                break;
             }
-                else if (current_element == qn("spreadsheetml", "f")) // CT_CellFormula
+            case cell::type::error:
             {
-                    has_formula = true;
+                ws_cell_impl->value_text_.plain_text(cell.value, false);
-
+                break;
-                    if (parser().attribute_present("t"))
-                    {
-                        has_shared_formula = parser().attribute("t") == "shared";
-                    }
-
-                    skip_attributes(
-                        {"aca", "ref", "dt2D", "dtr", "del1", "del2", "r1", "r2", "ca", "si", "bx"});
-
-                    formula_value_string = read_text();
-                }
-                else if (current_element == qn("spreadsheetml", "is")) // CT_Rst
-                {
-                    expect_start_element(qn("spreadsheetml", "t"), xml::content::simple);
-                    value_string = read_text();
-                    expect_end_element(qn("spreadsheetml", "t"));
-                }
-                else
-                {
-                    unexpected_element(current_element);
-                }
-
-                expect_end_element(current_element);
-            }
-
-            expect_end_element(qn("spreadsheetml", "c"));
-
-            if (has_formula && !has_shared_formula)
-            {
-                cell.formula(formula_value_string);
-            }
-
-            if (has_value)
-            {
-                if (type == "str")
-                {
-                    cell.d_->value_text_ = value_string;
-                    cell.data_type(cell::type::formula_string);
-                }
-                else if (type == "inlineStr")
-                {
-                    cell.d_->value_text_ = value_string;
-                    cell.data_type(cell::type::inline_string);
-                }
-                else if (type == "s")
-                {
-                    cell.d_->value_numeric_ = converter.stold(value_string);
-                    cell.data_type(cell::type::shared_string);
-                }
-                else if (type == "b") // boolean
-                {
-                    cell.value(is_true(value_string));
-                }
-                else if (type == "n") // numeric
-                {
-                    cell.value(converter.stold(value_string));
-                }
-                else if (!value_string.empty() && value_string[0] == '#')
-                {
-                    cell.error(value_string);
             }
             }
         }
-
-        expect_end_element(qn("spreadsheetml", "row"));
     }
-
+    stack_.pop_back();
-    expect_end_element(qn("spreadsheetml", "sheetData"));
 }
 
 worksheet xlsx_consumer::read_worksheet_end(const std::string &rel_id)
@@ -1012,12 +1210,12 @@ worksheet xlsx_consumer::read_worksheet_end(const std::string &rel_id)
         {
             page_margins margins;
 
-            margins.top(parser().attribute<double>("top"));
+            margins.top(converter_.stold(parser().attribute("top")));
-            margins.bottom(parser().attribute<double>("bottom"));
+            margins.bottom(converter_.stold(parser().attribute("bottom")));
-            margins.left(parser().attribute<double>("left"));
+            margins.left(converter_.stold(parser().attribute("left")));
-            margins.right(parser().attribute<double>("right"));
+            margins.right(converter_.stold(parser().attribute("right")));
-            margins.header(parser().attribute<double>("header"));
+            margins.header(converter_.stold(parser().attribute("header")));
-            margins.footer(parser().attribute<double>("footer"));
+            margins.footer(converter_.stold(parser().attribute("footer")));
 
             ws.page_margins(margins);
         }
@@ -2124,7 +2322,7 @@ void xlsx_consumer::read_stylesheet()
                     while (in_element(qn("spreadsheetml", "gradientFill")))
                     {
                         expect_start_element(qn("spreadsheetml", "stop"), xml::content::complex);
-                        auto position = parser().attribute<double>("position");
+                        auto position = converter_.stold(parser().attribute("position"));
                         expect_start_element(qn("spreadsheetml", "color"), xml::content::complex);
                         auto color = read_color();
                         expect_end_element(qn("spreadsheetml", "color"));
@@ -2172,7 +2370,7 @@ void xlsx_consumer::read_stylesheet()
 
                     if (font_property_element == qn("spreadsheetml", "sz"))
                     {
-                        new_font.size(parser().attribute<double>("val"));
+                        new_font.size(converter_.stold(parser().attribute("val")));
                     }
                     else if (font_property_element == qn("spreadsheetml", "name"))
                     {
@@ -2741,7 +2939,7 @@ void xlsx_consumer::read_drawings(worksheet ws, const path &part)
 
     auto sd = drawing::spreadsheet_drawing(parser());
 
-    for (const auto image_rel_id : sd.get_embed_ids())
+    for (const auto &image_rel_id : sd.get_embed_ids())
     {
         auto image_rel = std::find_if(images.begin(), images.end(),
             [&](const relationship &r) { return r.id() == image_rel_id; });
@@ -2976,7 +3174,7 @@ rich_text xlsx_consumer::read_rich_text(const xml::qname &parent)
 
                         if (current_run_property_element == xml::qname(xmlns, "sz"))
                         {
-                            run.second.get().size(parser().attribute<double>("val"));
+                            run.second.get().size(converter_.stold(parser().attribute("val")));
                         }
                         else if (current_run_property_element == xml::qname(xmlns, "rFont"))
                         {
@@ -3114,7 +3312,7 @@ xlnt::color xlsx_consumer::read_color()
 
     if (parser().attribute_present("tint"))
     {
-        result.tint(parser().attribute<double>("tint"));
+        result.tint(converter_.stold(parser().attribute("tint")));
     }
 
     return result;

source/detail/serialization/xlsx_consumer.hpp

@@ -34,6 +34,7 @@
 
 #include <detail/external/include_libstudxml.hpp>
 #include <detail/serialization/zstream.hpp>
+#include <xlnt/utils/numeric.hpp>
 
 namespace xlnt {
 
@@ -409,6 +410,7 @@ private:
     detail::cell_impl *current_cell_;
 
     detail::worksheet_impl *current_worksheet_;
+    number_converter converter_;
 };
 
 } // namespace detail

source/detail/serialization/xlsx_producer.cpp

@@ -33,10 +33,10 @@
 #include <detail/serialization/vector_streambuf.hpp>
 #include <detail/serialization/xlsx_producer.hpp>
 #include <detail/serialization/zstream.hpp>
-#include <detail/numeric_utils.hpp>
 #include <xlnt/cell/cell.hpp>
 #include <xlnt/cell/hyperlink.hpp>
 #include <xlnt/packaging/manifest.hpp>
+#include <xlnt/utils/numeric.hpp>
 #include <xlnt/utils/path.hpp>
 #include <xlnt/utils/scoped_enum_hash.hpp>
 #include <xlnt/workbook/workbook.hpp>

source/worksheet/page_margins.cpp

@@ -22,7 +22,7 @@
 // @license: http://www.opensource.org/licenses/mit-license.php
 // @author: see AUTHORS file
 #include <xlnt/worksheet/page_margins.hpp>
-#include "detail/numeric_utils.hpp"
+#include <xlnt/utils/numeric.hpp>
 
 namespace xlnt {
 

source/worksheet/page_setup.cpp

@@ -22,7 +22,7 @@
 // @license: http://www.opensource.org/licenses/mit-license.php
 // @author: see AUTHORS file
 #include <xlnt/worksheet/page_setup.hpp>
-#include "detail/numeric_utils.hpp"
+#include <xlnt/utils/numeric.hpp>
 
 namespace xlnt {
 

source/worksheet/worksheet.cpp

@@ -46,7 +46,7 @@
 #include <xlnt/worksheet/column_properties.hpp>
 #include <detail/constants.hpp>
 #include <detail/default_case.hpp>
-#include <detail/numeric_utils.hpp>
+#include <xlnt/utils/numeric.hpp>
 #include <detail/unicode.hpp>
 #include <detail/implementations/cell_impl.hpp>
 #include <detail/implementations/workbook_impl.hpp>

tests/detail/numeric_util_test_suite.cpp

@@ -21,7 +21,7 @@
 // @license: http://www.opensource.org/licenses/mit-license.php
 // @author: see AUTHORS file
 
-#include "../../source/detail/numeric_utils.hpp"
+#include <xlnt/utils/numeric.hpp>
 #include <helpers/test_suite.hpp>
 
 class numeric_test_suite : public test_suite

tests/workbook/serialization_test_suite.cpp

@@ -90,6 +90,7 @@ public:
         register_test(test_round_trip_rw_encrypted_numbers);
         register_test(test_streaming_read);
         register_test(test_streaming_write);
+        register_test(test_load_save_german_locale);
     }
 
     bool workbook_matches_file(xlnt::workbook &wb, const xlnt::path &file)
@@ -708,5 +709,12 @@ public:
         b2.value("should not change");
         c3.value("C3!");
     }
+
+    void test_load_save_german_locale()
+    {
+       /* std::locale current(std::locale::global(std::locale("de-DE")));
+        test_round_trip_rw_custom_heights_widths();
+        std::locale::global(current);*/
+    }
 };
 static serialization_test_suite x;