fix(ooxml): an xlsx fill shows its colour, and a font its toggles

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- **Fix**: many xlsx fills showed a wrong colour. A cell now shows its fill,
+  theme colours and tints, italic, underline, strikethrough and every
+  alignment.
+
 - **Fix**: a cell fill disappeared in the dark color scheme. The cell now keeps
   its fill in a dark version with the same hue, and the text stays readable on
   it.

CMakeLists.txt

@@ -341,6 +341,7 @@ set(ODR_SOURCE_FILES
         "src/odr/internal/text/text_file.cpp"
 
         "src/odr/internal/util/byte_util.cpp"
+        "src/odr/internal/util/color_util.cpp"
         "src/odr/internal/util/byte_stream_util.cpp"
         "src/odr/internal/util/byte_string.cpp"
         "src/odr/internal/util/file_util.cpp"

src/odr/internal/html/common.cpp

@@ -3,6 +3,7 @@
 #include <odr/internal/abstract/file.hpp>
 #include <odr/internal/crypto/crypto_util.hpp>
 #include <odr/internal/html/html_writer.hpp>
+#include <odr/internal/util/color_util.hpp>
 #include <odr/internal/util/stream_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 #include <odr/internal/xml/xml_util.hpp>
@@ -360,49 +361,6 @@ std::string html::color(const Color &color) {
 
 namespace {
 
-struct Oklab {
-  double l{0};
-  double a{0};
-  double b{0};
-};
-
-double to_linear(const std::uint8_t channel) {
-  const double c = channel / 255.0;
-  return c <= 0.04045 ? c / 12.92 : std::pow((c + 0.055) / 1.055, 2.4);
-}
-
-double from_linear(const double c) {
-  return c <= 0.0031308 ? c * 12.92 : 1.055 * std::pow(c, 1.0 / 2.4) - 0.055;
-}
-
-Oklab to_oklab(const Color &color) {
-  const double r = to_linear(color.red);
-  const double g = to_linear(color.green);
-  const double b = to_linear(color.blue);
-  const double l =
-      std::cbrt(0.4122214708 * r + 0.5363325363 * g + 0.0514459929 * b);
-  const double m =
-      std::cbrt(0.2119034982 * r + 0.6806995451 * g + 0.1073969566 * b);
-  const double s =
-      std::cbrt(0.0883024619 * r + 0.2817188376 * g + 0.6299787005 * b);
-  return {0.2104542553 * l + 0.7936177850 * m - 0.0040720468 * s,
-          1.9779984951 * l - 2.4285922050 * m + 0.4505937099 * s,
-          0.0259040371 * l + 0.7827717662 * m - 0.8086757660 * s};
-}
-
-/// Linear srgb, possibly outside the gamut.
-std::array<double, 3> to_linear_rgb(const Oklab &lab) {
-  const double l =
-      std::pow(lab.l + 0.3963377774 * lab.a + 0.2158037573 * lab.b, 3);
-  const double m =
-      std::pow(lab.l - 0.1055613458 * lab.a - 0.0638541728 * lab.b, 3);
-  const double s =
-      std::pow(lab.l - 0.0894841775 * lab.a - 1.2914855480 * lab.b, 3);
-  return {4.0767416621 * l - 3.3077115913 * m + 0.2309699292 * s,
-          -1.2684380046 * l + 2.6097574011 * m - 0.3413193965 * s,
-          -0.0041960863 * l - 0.7034186147 * m + 1.7076147010 * s};
-}
-
 bool in_gamut(const std::array<double, 3> &rgb) {
   return std::ranges::all_of(
       rgb, [](const double c) { return c >= -1e-9 && c <= 1 + 1e-9; });
@@ -413,31 +371,34 @@ bool in_gamut(const std::array<double, 3> &rgb) {
 Color html::dark_fill(const Color &color) {
   // White lands on the page of `document-dark.css`, black on the lightest
   // ground its `#e6edf3` text reads on at 4.5:1.
-  static const Oklab page = to_oklab(Color(0x16, 0x1b, 0x22));
+  static const util::color::Oklab page =
+      util::color::Oklab::from_color(Color(0x16, 0x1b, 0x22));
   static constexpr double lightest = 0.5;
 
-  const Oklab source = to_oklab(color);
+  const util::color::Oklab source = util::color::Oklab::from_color(color);
   const double t = 1 - source.l;
-  Oklab lab{page.l + t * (lightest - page.l), page.a * (1 - t) + source.a,
-            page.b * (1 - t) + source.b};
+  util::color::Oklab lab{page.l + t * (lightest - page.l),
+                         page.a * (1 - t) + source.a,
+                         page.b * (1 - t) + source.b};
 
   // Less chroma until the color fits, so the hue stays.
   double low = 0;
   double high = 1;
-  if (!in_gamut(to_linear_rgb(lab))) {
+  if (!in_gamut(lab.to_linear_rgb())) {
     for (int i = 0; i < 20; ++i) {
       const double mid = (low + high) / 2;
-      (in_gamut(to_linear_rgb({lab.l, lab.a * mid, lab.b * mid})) ? low
-                                                                  : high) = mid;
+      (in_gamut(
+           util::color::Oklab{lab.l, lab.a * mid, lab.b * mid}.to_linear_rgb())
+           ? low
+           : high) = mid;
     }
     lab.a *= low;
     lab.b *= low;
   }
 
-  const std::array<double, 3> rgb = to_linear_rgb(lab);
+  const std::array<double, 3> rgb = lab.to_linear_rgb();
   const auto channel = [](const double c) {
-    return static_cast<std::uint8_t>(
-        std::lround(std::clamp(from_linear(c), 0.0, 1.0) * 255));
+    return util::color::to_byte(util::color::linear_to_srgb(c));
   };
   return {channel(rgb[0]), channel(rgb[1]), channel(rgb[2]), color.alpha};
 }

src/odr/internal/html/pdf_file.cpp

@@ -25,6 +25,7 @@
 #include <odr/internal/pdf/pdf_document_parser.hpp>
 #include <odr/internal/pdf/pdf_file.hpp>
 #include <odr/internal/pdf/pdf_page_extractor.hpp>
+#include <odr/internal/util/color_util.hpp>
 #include <odr/internal/util/number_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 #include <odr/internal/xml/xml_util.hpp>
@@ -291,12 +292,6 @@ void write_page_links(HtmlWriter &out, const std::vector<LinkOut> &links) {
   }
 }
 
-/// Clamp a colour component in [0, 1] to an 8-bit channel value.
-std::int32_t to255(const double v) {
-  return static_cast<std::int32_t>(
-      std::lround(std::clamp(v, 0.0, 1.0) * 255.0));
-}
-
 /// A PDF device color as CSS `rgb(...)`. Other spaces are already converted at
 /// extract time; only `unknown` reaches here, falling back to black.
 std::string device_color_to_css(const pdf::GraphicsState::Color &color) {
@@ -305,19 +300,19 @@ std::string device_color_to_css(const pdf::GraphicsState::Color &color) {
   std::int32_t b = 0;
   switch (color.space) {
   case pdf::ColorSpace::device_grey:
-    r = g = b = to255(color.grey);
+    r = g = b = util::color::to_byte(color.grey);
     break;
   case pdf::ColorSpace::device_rgb:
-    r = to255(color.rgb[0]);
-    g = to255(color.rgb[1]);
-    b = to255(color.rgb[2]);
+    r = util::color::to_byte(color.rgb[0]);
+    g = util::color::to_byte(color.rgb[1]);
+    b = util::color::to_byte(color.rgb[2]);
     break;
   case pdf::ColorSpace::device_cmyk: {
     const std::array<double, 3> rgb = pdf::cmyk_to_rgb(
         color.cmyk[0], color.cmyk[1], color.cmyk[2], color.cmyk[3]);
-    r = to255(rgb[0]);
-    g = to255(rgb[1]);
-    b = to255(rgb[2]);
+    r = util::color::to_byte(rgb[0]);
+    g = util::color::to_byte(rgb[1]);
+    b = util::color::to_byte(rgb[2]);
     break;
   }
   case pdf::ColorSpace::unknown:
@@ -332,7 +327,8 @@ std::string device_color_to_css(const pdf::GraphicsState::Color &color) {
 /// `rgb(...)`.
 std::string rgb_to_css(const std::array<double, 3> &rgb) {
   std::ostringstream s;
-  s << "rgb(" << to255(rgb[0]) << ',' << to255(rgb[1]) << ',' << to255(rgb[2])
+  s << "rgb(" << +util::color::to_byte(rgb[0]) << ','
+    << +util::color::to_byte(rgb[1]) << ',' << +util::color::to_byte(rgb[2])
     << ')';
   return std::move(s).str();
 }

src/odr/internal/ooxml/ooxml_util.cpp

@@ -3,6 +3,7 @@
 #include <odr/internal/abstract/filesystem.hpp>
 #include <odr/internal/common/path.hpp>
 #include <odr/internal/html/common.hpp>
+#include <odr/internal/util/color_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 #include <odr/internal/xml/xml_util.hpp>
 
@@ -209,6 +210,22 @@ ooxml::read_color_attribute(const pugi::xml_attribute attribute) {
   return {};
 }
 
+std::optional<Color>
+ooxml::read_drawing_rgb_color(const pugi::xml_node parent) {
+  if (const std::optional<Color> color =
+          read_color_attribute(parent.child("a:srgbClr").attribute("val"))) {
+    return color;
+  }
+  return read_color_attribute(parent.child("a:sysClr").attribute("lastClr"));
+}
+
+Color ooxml::apply_tint(const Color &color, const double tint) {
+  util::color::Hsl hsl = util::color::Hsl::from_color(color);
+  hsl.lightness =
+      tint < 0 ? hsl.lightness * (1 + tint) : hsl.lightness * (1 - tint) + tint;
+  return hsl.to_color(color.alpha);
+}
+
 std::optional<Measure>
 ooxml::read_half_point_attribute(const pugi::xml_attribute attribute) {
   if (!attribute) {

src/odr/internal/ooxml/ooxml_util.hpp

@@ -51,6 +51,12 @@ double points(const Measure &length);
 
 std::optional<std::string> read_string_attribute(pugi::xml_attribute);
 std::optional<Color> read_color_attribute(pugi::xml_attribute);
+/// The `a:srgbClr` of @p parent, else the value its `a:sysClr` last resolved
+/// to. [ECMA-376] 20.1.2.3.32, 20.1.2.3.33
+std::optional<Color> read_drawing_rgb_color(pugi::xml_node parent);
+/// [ECMA-376] 18.8.19: a negative @p tint moves the lightness toward black, a
+/// positive one toward white.
+Color apply_tint(const Color &color, double tint);
 std::optional<Measure> read_half_point_attribute(pugi::xml_attribute);
 std::optional<Measure> read_eighth_point_attribute(pugi::xml_attribute);
 std::optional<Measure> read_hundredth_point_attribute(pugi::xml_attribute);

src/odr/internal/ooxml/presentation/ooxml_presentation_style.cpp

@@ -13,16 +13,6 @@ namespace odr::internal::ooxml::presentation {
 
 namespace {
 
-/// A literal colour, or a system colour that names the value it last resolved
-/// to. [ECMA-376] 20.1.2.3.32, 20.1.2.3.33
-std::optional<Color> read_theme_color_(const pugi::xml_node slot) {
-  if (const std::optional<Color> color =
-          read_color_attribute(slot.child("a:srgbClr").attribute("val"))) {
-    return color;
-  }
-  return read_color_attribute(slot.child("a:sysClr").attribute("lastClr"));
-}
-
 /// `a:spcPct` in thousandths of a percent, or `a:spcPts` in hundredths of a
 /// point. [ECMA-376] 21.1.2.2.12
 std::optional<Measure> read_line_spacing(const pugi::xml_node node) {
@@ -58,7 +48,7 @@ namespace odr::internal::ooxml {
 presentation::ColorScheme::ColorScheme(const pugi::xml_node color_scheme,
                                        const pugi::xml_node color_map) {
   for (const pugi::xml_node slot : color_scheme.children()) {
-    if (const std::optional<Color> color = read_theme_color_(slot)) {
+    if (const std::optional<Color> color = read_drawing_rgb_color(slot)) {
       // the slot names are `a:dk1`, `a:lt1`, `a:accent1`, …
       const std::string_view name = slot.name();
       const std::size_t colon = name.find(':');
@@ -88,7 +78,7 @@ presentation::ColorScheme::resolve(const char *name) const {
 std::optional<Color>
 presentation::read_drawing_color(const pugi::xml_node parent,
                                  const ColorScheme *color_scheme) {
-  if (const std::optional<Color> color = read_theme_color_(parent)) {
+  if (const std::optional<Color> color = read_drawing_rgb_color(parent)) {
     return color;
   }
   if (const pugi::xml_attribute scheme_color =

src/odr/internal/ooxml/spreadsheet/AGENTS.md

@@ -47,7 +47,9 @@ loads `fonts`, `fills`, `borders`, `cellStyleXfs` and `cellXfs`. A cell's `s`
 attribute indexes `cellXfs`. The `xf` picks `fontId`, `fillId`, `borderId` and
 `alignment`, each resolved via its vector. Font, border and alignment are gated
 by `applyFont`, `applyBorder` and `applyAlignment`. Fill is applied
-unconditionally. The legacy indexed colour palette is hardcoded.
+unconditionally, and a pattern paints its `fgColor`. A colour is `rgb`, a slot
+of the hardcoded legacy palette, or a `theme` slot of the workbook's
+`a:clrScheme`, counted `lt1`, `dk1`, `lt2`, `dk2` first, and `tint` moves it.
 `cellStyleXfs` is loaded but never consulted.
 
 **Writing a cell replaces its children, and a written string goes inline.**
@@ -87,8 +89,7 @@ formula.
 2. `sheet_content` ignores the requested range and returns the full
    `<dimension>`.
 3. No `cellStyleXfs` inheritance. Borders render as `0.75pt solid` whatever
-   the style. Font italic, underline and strike are not read. Cell
-   `protection` is read and dropped.
+   the style. Cell `protection` is read and dropped.
 4. `sheet_set_cell` writes a number, a string or a boolean. `text_set_content`
    throws `UnsupportedOperation`. A `<hyperlink>` is not modelled, so
    `link_href` is empty. Comments are not modelled.

src/odr/internal/ooxml/spreadsheet/README.md

@@ -36,11 +36,11 @@ through the `cellXfs`, `fonts`, `fills` and `borders` indices
   - [x] size
   - [x] bold
   - [x] color
-  - [ ] italic
-  - [ ] underline, strike through
+  - [x] italic
+  - [x] underline, strike through
   - [ ] superscript, subscript
 - [x] cell
-  - [x] background / fill color
+  - [x] background / fill color, theme colors and tint
   - [x] borders (`0.75pt solid` only)
   - [x] alignment (horizontal, vertical, text rotation)
   - [ ] protection

src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp

@@ -11,6 +11,7 @@
 #include <odr/internal/formula/formula_ast.hpp>
 #include <odr/internal/formula/formula_parser.hpp>
 #include <odr/internal/formula/formula_writer.hpp>
+#include <odr/internal/ooxml/ooxml_util.hpp>
 #include <odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.hpp>
 #include <odr/internal/util/number_util.hpp>
 #include <odr/internal/xml/xml_util.hpp>
@@ -86,7 +87,13 @@ Document::Document(std::shared_ptr<abstract::ReadableFilesystem> files)
     }
   }
 
-  m_style_registry = StyleRegistry(styles_xml.document_element());
+  pugi::xml_node theme_root;
+  if (const std::optional<AbsPath> theme_path =
+          parse_relationship_target(*m_files, workbook_path, "theme");
+      theme_path && m_files->is_file(*theme_path)) {
+    theme_root = parse_xml_(*theme_path).first.document_element();
+  }
+  m_style_registry = StyleRegistry(styles_xml.document_element(), theme_root);
 
   const ParseContext parse_context(workbook_path, workbook_relations,
                                    m_xml_documents_and_relations,