Merge pull request #323 from Crzyrndm/build_system_improvements

Build system improvements + stylesheet optimisations

.appveyor.yml

@@ -14,6 +14,9 @@ environment:
   matrix:
   - STATIC: OFF
   - STATIC: ON
+  
+matrix:
+  fast_finish: true
 
 init: []
 
@@ -23,10 +26,10 @@ before_build:
   - cmake -G "Visual Studio 14 2015" -D CMAKE_GENERATOR_PLATFORM=%platform% -D STATIC=%STATIC% -D SAMPLES=ON -D BENCHMARKS=ON ..
 
 build:
-  project: build/tests/xlnt.test.sln
+  project: build/xlnt_all.sln
   parallel: true
   verbosity: minimal
 
 test_script:
-  - cd tests\%configuration%
+  - tests\%configuration%\xlnt.test.exe
-  - xlnt.test.exe
+  - samples\%configuration%\sample-documentation.exe

.travis.yml

@@ -30,6 +30,7 @@ matrix:
         - COVERAGE=OFF
         - STATIC=OFF
         - SAMPLES=OFF
+        - BENCHMARKS=OFF
         
     # gcc-7, c++14, release build, static linking, samples + benchmarks compiled
     - os: linux
@@ -47,7 +48,8 @@ matrix:
         - BUILD_TYPE=Release
         - COVERAGE=OFF
         - STATIC=ON
-        - SAMPLES=ON
+        - SAMPLES=OFF
+        - BENCHMARKS=OFF
         
     # gcc-8, c++17, release build, static linking, samples + benchmarks compiled
     - os: linux
@@ -66,6 +68,7 @@ matrix:
         - COVERAGE=OFF
         - STATIC=ON
         - SAMPLES=ON
+        - BENCHMARKS=ON
 
     # =========== CLANG =============
     # clang 4, c++11, release build, dynamic linking
@@ -86,6 +89,7 @@ matrix:
         - COVERAGE=OFF
         - STATIC=OFF
         - SAMPLES=OFF
+        - BENCHMARKS=OFF
         
     # clang 5, c++14, release build, dynamic linking, samples + benchmarks compiled
     - os: linux
@@ -104,7 +108,8 @@ matrix:
         - BUILD_TYPE=Release
         - COVERAGE=OFF
         - STATIC=ON
-        - SAMPLES=ON
+        - SAMPLES=OFF
+        - BENCHMARKS=OFF
         
     # clang 6, c++17, release build, static linking, samples compiled
     - os: linux
@@ -124,6 +129,7 @@ matrix:
         - COVERAGE=OFF
         - STATIC=ON
         - SAMPLES=ON
+        - BENCHMARKS=ON
 
     # ============= CODE COVERAGE ===============
     # gcc-6, c++11, debug build, static linking, code coverage enabled
@@ -159,17 +165,21 @@ install:
   - cmake --version
 
 script:
-  - |
-    if [[ "${BUILD_TYPE}" == "Release" && "${STATIC}" == "ON" ]];
-      then BENCHMARKS="ON";
-    else BENCHMARKS="OFF";
-    fi
-    
   - mkdir build
   - cd build
   - cmake .. -DXLNT_CXX_LANG=${CXX_VER} -DSTATIC=$STATIC -DBENCHMARKS=$BENCHMARKS -DSAMPLES=$SAMPLES -DCOVERAGE=$COVERAGE -DCMAKE_BUILD_TYPE=$BUILD_TYPE
   - cmake --build . -- -j2
   - ./tests/xlnt.test
+  - echo "samples" && echo 'travis_fold:start:samples'
+  - if [[ "${SAMPLES}" == "ON" ]]; then ./samples/sample-decrypt; fi
+  - if [[ "${SAMPLES}" == "ON" ]]; then ./samples/sample-img2xlsx "../samples/data/cafe.jpg" "img.xlsx"; fi
+  - if [[ "${SAMPLES}" == "ON" ]]; then ./samples/sample-documentation; fi
+  - echo 'travis_fold:end:samples'
+  - echo "benchmarks" && echo 'travis_fold:start:benchmarks'
+  - if [[ "${BENCHMARKS}" == "ON" ]]; then ./benchmarks/benchmark-styles; fi
+  - if [[ "${BENCHMARKS}" == "ON" ]]; then ./benchmarks/benchmark-writer; fi
+  - echo 'travis_fold:end:benchmarks'
+    
 
 after_success:
  - |

benchmarks/writer.cpp

@@ -39,11 +39,10 @@ void writer(int cols, int rows)
 
     for(int index = 0; index < rows; index++)
     {
-        if ((index + 1) % (rows / 10) == 0)
+        if (rows >= 10 && (index + 1) % (rows / 10) == 0)
         {
             std::string progress = std::string((index + 1) / (1 + rows / 10), '.');
             std::cout << "\r" << progress;
-            std::cout.flush();
         }
 
 		for (int i = 0; i < cols; i++)
@@ -51,8 +50,7 @@ void writer(int cols, int rows)
 			ws.cell(xlnt::cell_reference(i + 1, index + 1)).value(i);
 		}
     }
-
+    std::cout << '\n';
-    std::cout << std::endl;
 
     auto filename = "benchmark.xlsx";
     wb.save(filename);
@@ -63,33 +61,30 @@ void writer(int cols, int rows)
 // Time from the best of three is taken.
 void timer(std::function<void(int, int)> fn, int cols, int rows)
 {
-    using xlnt::benchmarks::current_time;
-
     const auto repeat = std::size_t(3);
-    auto time = std::numeric_limits<std::size_t>::max();
+    std::chrono::duration<double, std::milli> time{};
-
     std::cout << cols << " cols " << rows << " rows" << std::endl;
+    fn(rows, cols); // 1 cold run
 
     for(int i = 0; i < repeat; i++)
     {
-        auto start = current_time();
+        auto start = std::chrono::high_resolution_clock::now();
         fn(cols, rows);
-        time = std::min(current_time() - start, time);
+        time += std::chrono::high_resolution_clock::now() - start;
     }
 
-    std::cout << time / 1000.0 << std::endl;
+    std::cout << time.count() / repeat << " ms per iteration" << '\n' << '\n';
 }
 
 } // namespace
 
 int main()
 {
+    timer(&writer, 10000, 1);
+    timer(&writer, 1000, 10);
     timer(&writer, 100, 100);
-    timer(&writer, 1000, 100);
+    timer(&writer, 10, 1000);
-    timer(&writer, 4000, 100);
+    timer(&writer, 1, 10000);
-    timer(&writer, 8192, 100);
-    timer(&writer, 10, 10000);
-    timer(&writer, 4000, 1000);
 
     return 0;
 }

include/xlnt/cell/cell_reference.hpp

@@ -26,7 +26,7 @@
 #include <cstdint>
 #include <string>
 #include <utility>
-
+#include <tuple>
 #include <xlnt/xlnt_config.hpp>
 #include <xlnt/cell/index_types.hpp>
 
@@ -255,3 +255,17 @@ private:
 };
 
 } // namespace xlnt
+
+namespace std
+{
+template <>
+struct hash<xlnt::cell_reference>
+{
+    size_t operator()(const xlnt::cell_reference &x) const
+    {
+        static_assert(std::is_same<decltype(x.row()), std::uint32_t>::value, "this hash function expects both row and column to be 32-bit numbers");
+        static_assert(std::is_same<decltype(x.column_index()), std::uint32_t>::value, "this hash function expects both row and column to be 32-bit numbers");
+        return hash<std::uint64_t>{}(x.row() | static_cast<std::uint64_t>(x.column_index()) << 32);
+    }
+};
+}

samples/CMakeLists.txt

@@ -41,3 +41,4 @@ foreach(SAMPLE_SOURCE IN ITEMS ${SAMPLE_SOURCES})
       $<TARGET_FILE_DIR:${SAMPLE_EXECUTABLE}>)
   endif()
 endforeach()
+#

samples/documentation.cpp

@@ -0,0 +1,226 @@
+// Copyright (c) 2017-2018 Thomas Fussell
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+
+#include <xlnt/xlnt.hpp>
+#include "helpers/path_helper.hpp"
+
+// Readme
+// from https://tfussell.gitbooks.io/xlnt/content/ and https://github.com/tfussell/xlnt/blob/master/README.md
+void sample_readme_example1()
+{
+    xlnt::workbook wb;
+    xlnt::worksheet ws = wb.active_sheet();
+
+    ws.cell("A1").value(5);
+    ws.cell("B2").value("string data");
+    ws.cell("C3").formula("=RAND()");
+
+    ws.merge_cells("C3:C4");
+    ws.freeze_panes("B2");
+
+    wb.save("sample.xlsx");
+}
+
+// Simple - reading from an existing xlsx spread sheet.
+// from https://tfussell.gitbooks.io/xlnt/content/docs/introduction/Examples.html
+void sample_read_and_print_example()
+{
+    xlnt::workbook wb;
+    wb.load(path_helper::sample_file("documentation-print.xlsx")); // modified to use the test data directory
+    auto ws = wb.active_sheet();
+    std::clog << "Processing spread sheet" << std::endl;
+    for (auto row : ws.rows(false))
+    {
+        for (auto cell : row)
+        {
+            std::clog << cell.to_string() << std::endl;
+        }
+    }
+    std::clog << "Processing complete" << std::endl;
+}
+
+// Simple - storing a spread sheet in a 2 dimensional C++ Vector for further processing
+// from https://tfussell.gitbooks.io/xlnt/content/docs/introduction/Examples.html
+void sample_read_into_vector_example()
+{
+    xlnt::workbook wb;
+    wb.load(path_helper::sample_file("documentation-print.xlsx")); // modified to use the test data directory
+    auto ws = wb.active_sheet();
+    std::clog << "Processing spread sheet" << std::endl;
+    std::clog << "Creating a single vector which stores the whole spread sheet" << std::endl;
+    std::vector<std::vector<std::string>> theWholeSpreadSheet;
+    for (auto row : ws.rows(false))
+    {
+        std::clog << "Creating a fresh vector for just this row in the spread sheet" << std::endl;
+        std::vector<std::string> aSingleRow;
+        for (auto cell : row)
+        {
+            std::clog << "Adding this cell to the row" << std::endl;
+            aSingleRow.push_back(cell.to_string());
+        }
+        std::clog << "Adding this entire row to the vector which stores the whole spread sheet" << std::endl;
+        theWholeSpreadSheet.push_back(aSingleRow);
+    }
+    std::clog << "Processing complete" << std::endl;
+    std::clog << "Reading the vector and printing output to the screen" << std::endl;
+    for (int rowInt = 0; rowInt < theWholeSpreadSheet.size(); rowInt++)
+    {
+        for (int colInt = 0; colInt < theWholeSpreadSheet.at(rowInt).size(); colInt++)
+        {
+            std::cout << theWholeSpreadSheet.at(rowInt).at(colInt) << std::endl;
+        }
+    }
+}
+
+// Simple - writing values to a new xlsx spread sheet.
+// from https://tfussell.gitbooks.io/xlnt/content/docs/introduction/Examples.html
+void sample_write_sheet_to_file_example()
+{
+    // Creating a 2 dimensional vector which we will write values to
+    std::vector<std::vector<std::string>> wholeWorksheet;
+    //Looping through each row (100 rows as per the second argument in the for loop)
+    for (int outer = 0; outer < 100; outer++)
+    {
+        //Creating a fresh vector for a fresh row
+        std::vector<std::string> singleRow;
+        //Looping through each of the columns (100 as per the second argument in the for loop) in this particular row
+        for (int inner = 0; inner < 100; inner++)
+        {
+            //Adding a single value in each cell of the row
+            std::string val = std::to_string(inner + 1);
+            singleRow.push_back(val);
+        }
+        //Adding the single row to the 2 dimensional vector
+        wholeWorksheet.push_back(singleRow);
+        std::clog << "Writing to row " << outer << " in the vector " << std::endl;
+    }
+    //Writing to the spread sheet
+    //Creating the output workbook
+    std::clog << "Creating workbook" << std::endl;
+    xlnt::workbook wbOut;
+    //Setting the destination output file name
+    std::string dest_filename = "output.xlsx";
+    //Creating the output worksheet
+    xlnt::worksheet wsOut = wbOut.active_sheet();
+    //Giving the output worksheet a title/name
+    wsOut.title("data");
+    //We will now be looping through the 2 dimensional vector which we created above
+    //In this case we have two iterators one for the outer loop (row) and one for the inner loop (column)
+    std::clog << "Looping through vector and writing to spread sheet" << std::endl;
+    for (int fOut = 0; fOut < wholeWorksheet.size(); fOut++)
+    {
+        std::clog << "Row" << fOut << std::endl;
+        for (int fIn = 0; fIn < wholeWorksheet.at(fOut).size(); fIn++)
+        {
+            //Take notice of the difference between accessing the vector and accessing the work sheet
+            //As you may already know Excel spread sheets start at row 1 and column 1 (not row 0 and column 0 like you would expect from a C++ vector)
+            //In short the xlnt cell reference starts at column 1 row 1 (hence the + 1s below) and the vector reference starts at row 0 and column 0
+            wsOut.cell(xlnt::cell_reference(fIn + 1, fOut + 1)).value(wholeWorksheet.at(fOut).at(fIn));
+            //Further clarification to avoid confusion
+            //Cell reference arguments are (column number, row number); e.g. cell_reference(fIn + 1, fOut + 1)
+            //Vector arguments are (row number, column number); e.g. wholeWorksheet.at(fOut).at(fIn)
+        }
+    }
+    std::clog << "Finished writing spread sheet" << std::endl;
+    wbOut.save(dest_filename);
+}
+
+// Number Formatting
+// from https://tfussell.gitbooks.io/xlnt/content/docs/advanced/Formatting.html
+void sample_number_formatting_example()
+{
+    xlnt::workbook wb;
+    auto cell = wb.active_sheet().cell("A1");
+    cell.number_format(xlnt::number_format::percentage());
+    cell.value(0.513);
+    std::cout << '\n'
+              << cell.to_string() << std::endl;
+}
+// Properties
+// from https://tfussell.gitbooks.io/xlnt/content/docs/advanced/Properties.html
+void sample_properties_example()
+{
+    xlnt::workbook wb;
+    wb.core_property(xlnt::core_property::category, "hors categorie");
+    wb.core_property(xlnt::core_property::content_status, "good");
+    wb.core_property(xlnt::core_property::created, xlnt::datetime(2017, 1, 15));
+    wb.core_property(xlnt::core_property::creator, "me");
+    wb.core_property(xlnt::core_property::description, "description");
+    wb.core_property(xlnt::core_property::identifier, "id");
+    wb.core_property(xlnt::core_property::keywords, {"wow", "such"});
+    wb.core_property(xlnt::core_property::language, "Esperanto");
+    wb.core_property(xlnt::core_property::last_modified_by, "someone");
+    wb.core_property(xlnt::core_property::last_printed, xlnt::datetime(2017, 1, 15));
+    wb.core_property(xlnt::core_property::modified, xlnt::datetime(2017, 1, 15));
+    wb.core_property(xlnt::core_property::revision, "3");
+    wb.core_property(xlnt::core_property::subject, "subject");
+    wb.core_property(xlnt::core_property::title, "title");
+    wb.core_property(xlnt::core_property::version, "1.0");
+    wb.extended_property(xlnt::extended_property::application, "xlnt");
+    wb.extended_property(xlnt::extended_property::app_version, "0.9.3");
+    wb.extended_property(xlnt::extended_property::characters, 123);
+    wb.extended_property(xlnt::extended_property::characters_with_spaces, 124);
+    wb.extended_property(xlnt::extended_property::company, "Incorporated Inc.");
+    wb.extended_property(xlnt::extended_property::dig_sig, "?");
+    wb.extended_property(xlnt::extended_property::doc_security, 0);
+    wb.extended_property(xlnt::extended_property::heading_pairs, true);
+    wb.extended_property(xlnt::extended_property::hidden_slides, false);
+    wb.extended_property(xlnt::extended_property::h_links, 0);
+    wb.extended_property(xlnt::extended_property::hyperlink_base, 0);
+    wb.extended_property(xlnt::extended_property::hyperlinks_changed, true);
+    wb.extended_property(xlnt::extended_property::lines, 42);
+    wb.extended_property(xlnt::extended_property::links_up_to_date, false);
+    wb.extended_property(xlnt::extended_property::manager, "johnny");
+    wb.extended_property(xlnt::extended_property::m_m_clips, "?");
+    wb.extended_property(xlnt::extended_property::notes, "note");
+    wb.extended_property(xlnt::extended_property::pages, 19);
+    wb.extended_property(xlnt::extended_property::paragraphs, 18);
+    wb.extended_property(xlnt::extended_property::presentation_format, "format");
+    wb.extended_property(xlnt::extended_property::scale_crop, true);
+    wb.extended_property(xlnt::extended_property::shared_doc, false);
+    wb.extended_property(xlnt::extended_property::slides, 17);
+    wb.extended_property(xlnt::extended_property::template_, "template!");
+    wb.extended_property(xlnt::extended_property::titles_of_parts, {"title"});
+    wb.extended_property(xlnt::extended_property::total_time, 16);
+    wb.extended_property(xlnt::extended_property::words, 101);
+    wb.custom_property("test", {1, 2, 3});
+    wb.custom_property("Editor", "John Smith");
+    wb.save("lots_of_properties.xlsx");
+}
+
+int main()
+{
+    sample_readme_example1();
+    std::clog << '\n';
+    sample_read_and_print_example();
+    std::clog << '\n';
+    sample_read_into_vector_example();
+    std::clog << '\n';
+    sample_write_sheet_to_file_example();
+    std::clog << '\n';
+    sample_number_formatting_example();
+    std::clog << '\n';
+    sample_properties_example();
+    std::clog.flush();
+    return 0;
+}

samples/sample.cpp

@@ -1,41 +0,0 @@
-// Copyright (c) 2017-2018 Thomas Fussell
-//
-// Permission is hereby granted, free of charge, to any person obtaining a copy
-// of this software and associated documentation files (the "Software"), to deal
-// in the Software without restriction, including without limitation the rights
-// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-// copies of the Software, and to permit persons to whom the Software is
-// furnished to do so, subject to the following conditions:
-//
-// The above copyright notice and this permission notice shall be included in
-// all copies or substantial portions of the Software.
-//
-// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
-// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
-// THE SOFTWARE
-//
-// @license: http://www.opensource.org/licenses/mit-license.php
-// @author: see AUTHORS file
-
-#include <xlnt/xlnt.hpp>
-
-int main()
-{
-    xlnt::workbook wb;
-    xlnt::worksheet ws = wb.active_sheet();
-
-    ws.cell("A1").value(5);
-    ws.cell("B2").value("string data");
-    ws.cell("C3").formula("=RAND()");
-
-    ws.merge_cells("C3:C4");
-    ws.freeze_panes("B2");
-
-    wb.save("sample.xlsx");
-
-    return 0;
-}

source/CMakeLists.txt

@@ -33,6 +33,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") # all warnings
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra") # extra warnings
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unknown-pragmas") # ignore MSVC and Clang pragmas
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-maybe-uninitialized") # GCC diagnostic with lots of false positives
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything") # all warnings
   # blacklist warnings that are not relevant

source/detail/implementations/stylesheet.hpp

@@ -178,33 +178,18 @@ struct stylesheet
 	}
     
     template<typename T, typename C>
-    std::size_t find_or_add(C &container, const T &item, bool *added = nullptr)
+    std::size_t find_or_add(C &container, const T &item)
     {
-        if (added != nullptr)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wsign-conversion"
+        auto iter = std::find(container.begin(), container.end(), item);
+        if (iter != container.end())
         {
-            *added = false;
+            return std::size_t(iter - container.begin());
         }
-
+        iter = container.emplace(container.end(), item);
-        std::size_t i = 0;
+        return std::size_t(iter - container.begin());
-        
+#pragma GCC diagnostic pop
-        for (auto iter = container.begin(); iter != container.end(); ++iter)
-        {
-            if (*iter == item)
-            {
-                return i;
-            }
-
-            ++i;
-        }
-
-        if (added != nullptr)
-        {
-            *added = true;
-        }
-
-        container.emplace(container.end(), item);
-
-        return container.size() - 1;
     }
     
     template<typename T>
@@ -235,7 +220,6 @@ struct stylesheet
         if (!garbage_collection_enabled) return;
         
         auto format_iter = format_impls.begin();
-
         while (format_iter != format_impls.end())
         {
             auto &impl = *format_iter;
@@ -399,12 +383,18 @@ struct stylesheet
 
     format_impl *find_or_create(format_impl &pattern)
     {
-        auto iter = format_impls.begin();
-        bool added = false;
         pattern.references = 0;
-        auto id = find_or_add(format_impls, pattern, &added);
+        std::size_t id = 0;
-        std::advance(iter, static_cast<std::list<format_impl>::difference_type>(id));
+        auto iter = format_impls.begin();
-        
+        while (iter != format_impls.end() && !(*iter == pattern))
+        {
+            ++id;
+            ++iter;
+        }
+        if (iter == format_impls.end())
+        {
+            iter = format_impls.emplace(format_impls.end(), pattern);
+        }
         auto &result = *iter;
 
         result.parent = this;
@@ -426,7 +416,10 @@ struct stylesheet
     {
         format_impl new_format = *pattern;
         new_format.style = style_name;
-
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
 
@@ -435,7 +428,10 @@ struct stylesheet
         format_impl new_format = *pattern;
         new_format.alignment_id = find_or_add(alignments, new_alignment);
         new_format.alignment_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
 
@@ -444,7 +440,10 @@ struct stylesheet
         format_impl new_format = *pattern;
         new_format.border_id = find_or_add(borders, new_border);
         new_format.border_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
     
@@ -453,7 +452,10 @@ struct stylesheet
         format_impl new_format = *pattern;
         new_format.fill_id = find_or_add(fills, new_fill);
         new_format.fill_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
     
@@ -462,7 +464,10 @@ struct stylesheet
         format_impl new_format = *pattern;
         new_format.font_id = find_or_add(fonts, new_font);
         new_format.font_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
     
@@ -475,7 +480,10 @@ struct stylesheet
         }
         new_format.number_format_id = new_number_format.id();
         new_format.number_format_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
     
@@ -484,7 +492,10 @@ struct stylesheet
         format_impl new_format = *pattern;
         new_format.protection_id = find_or_add(protections, new_protection);
         new_format.protection_applied = applied;
-        
+        if (pattern->references == 0)
+        {
+            *pattern = new_format;
+        }
         return find_or_create(new_format);
     }
 

source/detail/implementations/worksheet_impl.hpp

@@ -88,12 +88,9 @@ struct worksheet_impl
         sheet_properties_ = other.sheet_properties_;
         print_options_ = other.print_options_;
 
-        for (auto &row : cell_map_)
+        for (auto &cell : cell_map_)
         {
-            for (auto &cell : row.second)
+            cell.second.parent_ = this;
-            {
-                cell.second.parent_ = this;
-            }
         }
     }
 
@@ -134,7 +131,7 @@ struct worksheet_impl
     std::unordered_map<column_t, column_properties> column_properties_;
     std::unordered_map<row_t, row_properties> row_properties_;
 
-    std::unordered_map<row_t, std::unordered_map<column_t, cell_impl>> cell_map_;
+    std::unordered_map<cell_reference, cell_impl> cell_map_;
 
     optional<page_setup> page_setup_;
     optional<range_reference> auto_filter_;

source/detail/serialization/xlsx_producer.cpp

@@ -2180,7 +2180,8 @@ void xlsx_producer::write_worksheet(const relationship &rel)
                 return true;
             }
 
-            for (auto row = ws.lowest_row(); row <= ws.highest_row(); ++row)
+            auto highest = ws.highest_row();
+            for (auto row = ws.lowest_row(); row <= highest; ++row)
             {
                 if (ws.has_row_properties(row) && ws.row_properties(row).dy_descent.is_set())
                 {

source/styles/color.cpp

@@ -310,7 +310,7 @@ bool color::operator==(const xlnt::color &other) const
     case color_type::theme:
         return theme_.index() == other.theme_.index();
     case color_type::rgb:
-        return rgb_.hex_string() == other.rgb_.hex_string();
+        return rgb_.rgba() == other.rgb_.rgba();
     }
 
     return false;

source/worksheet/worksheet.cpp

@@ -196,32 +196,18 @@ const workbook &worksheet::workbook() const
 
 void worksheet::garbage_collect()
 {
-    auto cell_map_iter = d_->cell_map_.begin();
+    auto cell_iter = d_->cell_map_.begin();
 
-    while (cell_map_iter != d_->cell_map_.end())
+    while (cell_iter != d_->cell_map_.end())
     {
-        auto cell_iter = cell_map_iter->second.begin();
+        if (xlnt::cell(&cell_iter->second).garbage_collectible())
-
-        while (cell_iter != cell_map_iter->second.end())
         {
-            class cell current_cell(&cell_iter->second);
+            cell_iter = d_->cell_map_.erase(cell_iter);
-
-            if (current_cell.garbage_collectible())
-            {
-                cell_iter = cell_map_iter->second.erase(cell_iter);
-                continue;
-            }
-
-            cell_iter++;
         }
-
+        else
-        if (cell_map_iter->second.empty())
         {
-            cell_map_iter = d_->cell_map_.erase(cell_map_iter);
+            ++cell_iter;
-            continue;
         }
-
-        cell_map_iter++;
     }
 }
 
@@ -391,25 +377,22 @@ cell_reference worksheet::active_cell() const
 
 cell worksheet::cell(const cell_reference &reference)
 {
-    auto &row = d_->cell_map_[reference.row()];
+    auto match = d_->cell_map_.find(reference);
-    auto match = row.find(reference.column_index());
+    if (match == d_->cell_map_.end())
-
-    if (match == row.end())
     {
-        match = row.emplace(reference.column_index(), detail::cell_impl()).first;
+        auto impl = detail::cell_impl();
-        auto &impl = match->second;
-
         impl.parent_ = d_;
         impl.column_ = reference.column_index();
         impl.row_ = reference.row();
-    }
 
+        match = d_->cell_map_.emplace(reference, impl).first;
+    }
     return xlnt::cell(&match->second);
 }
 
 const cell worksheet::cell(const cell_reference &reference) const
 {
-    return xlnt::cell(&d_->cell_map_.at(reference.row()).at(reference.column_index()));
+    return xlnt::cell(&d_->cell_map_.at(reference));
 }
 
 cell worksheet::cell(xlnt::column_t column, row_t row)
@@ -424,13 +407,8 @@ const cell worksheet::cell(xlnt::column_t column, row_t row) const
 
 bool worksheet::has_cell(const cell_reference &reference) const
 {
-    const auto row = d_->cell_map_.find(reference.row());
+    const auto cell = d_->cell_map_.find(reference);
-    if (row == d_->cell_map_.cend()) return false;
+    return cell != d_->cell_map_.cend();
-
-    const auto col = row->second.find(reference.column_index());
-    if (col == row->second.cend()) return false;
-
-    return true;
 }
 
 bool worksheet::has_row_properties(row_t row) const
@@ -477,12 +455,9 @@ column_t worksheet::lowest_column() const
 
     auto lowest = constants::max_column();
 
-    for (auto &row : d_->cell_map_)
+    for (auto &cell : d_->cell_map_)
     {
-        for (auto &c : row.second)
+        lowest = std::min(lowest, cell.first.column());
-        {
-            lowest = std::min(lowest, c.first);
-        }
     }
 
     return lowest;
@@ -514,9 +489,9 @@ row_t worksheet::lowest_row() const
 
     auto lowest = constants::max_row();
 
-    for (auto &row : d_->cell_map_)
+    for (auto &cell : d_->cell_map_)
     {
-        lowest = std::min(lowest, row.first);
+        lowest = std::min(lowest, cell.first.row());
     }
 
     return lowest;
@@ -543,9 +518,9 @@ row_t worksheet::highest_row() const
 {
     auto highest = constants::min_row();
 
-    for (auto &row : d_->cell_map_)
+    for (auto &cell : d_->cell_map_)
     {
-        highest = std::max(highest, row.first);
+        highest = std::max(highest, cell.first.row());
     }
 
     return highest;
@@ -572,12 +547,9 @@ column_t worksheet::highest_column() const
 {
     auto highest = constants::min_column();
 
-    for (auto &row : d_->cell_map_)
+    for (auto &cell : d_->cell_map_)
     {
-        for (auto &c : row.second)
+        highest = std::max(highest, cell.first.column());
-        {
-            highest = std::max(highest, c.first);
-        }
     }
 
     return highest;
@@ -744,13 +716,22 @@ const cell_vector worksheet::cells(bool skip_null) const
 
 void worksheet::clear_cell(const cell_reference &ref)
 {
-    d_->cell_map_.at(ref.row()).erase(ref.column());
+    d_->cell_map_.erase(ref);
     // TODO: garbage collect newly unreferenced resources such as styles?
 }
 
 void worksheet::clear_row(row_t row)
 {
-    d_->cell_map_.erase(row);
+    for (auto it = d_->cell_map_.begin(); it != d_->cell_map_.end();)
+    {
+        if (it->first.row() == row)
+        {
+            it = d_->cell_map_.erase(it);
+        }
+        else {
+            ++it;
+        }
+    }
     d_->row_properties_.erase(row);
     // TODO: garbage collect newly unreferenced resources such as styles?
 }
@@ -769,22 +750,16 @@ bool worksheet::compare(const worksheet &other, bool reference) const
 
     if (d_->parent_ != other.d_->parent_) return false;
 
-    for (auto &row : d_->cell_map_)
-    {
-        if (other.d_->cell_map_.find(row.first) == other.d_->cell_map_.end())
-        {
-            return false;
-        }
 
-        for (auto &cell : row.second)
+        for (auto &cell : d_->cell_map_)
         {
-            if (other.d_->cell_map_[row.first].find(cell.first) == other.d_->cell_map_[row.first].end())
+            if (other.d_->cell_map_.find(cell.first) == other.d_->cell_map_.end())
             {
                 return false;
             }
 
             xlnt::cell this_cell(&cell.second);
-            xlnt::cell other_cell(&other.d_->cell_map_[row.first][cell.first]);
+            xlnt::cell other_cell(&other.d_->cell_map_[cell.first]);
 
             if (this_cell.data_type() != other_cell.data_type())
             {
@@ -797,7 +772,6 @@ bool worksheet::compare(const worksheet &other, bool reference) const
                 return false;
             }
         }
-    }
 
     // todo: missing some comparisons