Skip to content

Commit

Permalink
Correctly import solid fill color.
Browse files Browse the repository at this point in the history
In orcus, a solid fill type uses the foreground color only and ignores
the background color.  Also, let's not use the alpha component as it
would cause the color to not get rendered at all.

Some patches are applied against liborcus in order to adjust the ODF
styles import code for this change.  These changes will be incorporated
in 0.13.3.

Change-Id: I9e8c243cc6a7f366de2393e7b7ecf77366f5f9ea
Reviewed-on: https://gerrit.libreoffice.org/49071
Reviewed-by: Kohei Yoshida <libreoffice@kohei.us>
Tested-by: Kohei Yoshida <libreoffice@kohei.us>
  • Loading branch information
kohei-us authored and Kohei Yoshida committed Feb 2, 2018
1 parent 8cf603b commit 20945a9
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 5 deletions.
@@ -0,0 +1,50 @@
From 98d2b3377da71b713a37f9004acff3c02c22ce2b Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <kohei.yoshida@gmail.com>
Date: Wed, 31 Jan 2018 22:11:25 -0500
Subject: [PATCH 1/2] Alpha value of 0 means fully transparent. I'm sure 255
was intended.

(cherry picked from commit f7953a814d6a43205791b6cc01c528ef5d4b1ce3)
---
src/liborcus/gnumeric_sheet_context.cpp | 4 ++--
src/liborcus/odf_styles_context.cpp | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/liborcus/gnumeric_sheet_context.cpp b/src/liborcus/gnumeric_sheet_context.cpp
index 8659cc3..6bd1471 100644
--- a/src/liborcus/gnumeric_sheet_context.cpp
+++ b/src/liborcus/gnumeric_sheet_context.cpp
@@ -132,7 +132,7 @@ public:
{
spreadsheet::color_elem_t red, green, blue;
gnumeric_helper::parse_RGB_color_attribute(red, green, blue, attr.value);
- m_styles.set_fill_fg_color(0, red, green, blue);
+ m_styles.set_fill_fg_color(255, red, green, blue);

m_fill = true;

@@ -145,7 +145,7 @@ public:
{
spreadsheet::color_elem_t red, green, blue;
gnumeric_helper::parse_RGB_color_attribute(red, green, blue, attr.value);
- m_styles.set_fill_bg_color(0, red, green, blue);
+ m_styles.set_fill_bg_color(255, red, green, blue);

m_fill = true;
}
diff --git a/src/liborcus/odf_styles_context.cpp b/src/liborcus/odf_styles_context.cpp
index d988f7d..f9c422a 100644
--- a/src/liborcus/odf_styles_context.cpp
+++ b/src/liborcus/odf_styles_context.cpp
@@ -739,7 +739,7 @@ void styles_context::start_element(xmlns_id_t ns, xml_token_t name, const std::v
{
spreadsheet::color_elem_t red, green, blue;
func.get_background_color(red, green, blue);
- mp_styles->set_fill_bg_color(0, red, green, blue);
+ mp_styles->set_fill_bg_color(255, red, green, blue);
}

size_t fill_id = mp_styles->commit_fill();
--
2.7.4

@@ -0,0 +1,49 @@
From 473526e1ca3a7117e2daf977e1b82a0a3977fc84 Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <kohei.yoshida@gmail.com>
Date: Wed, 31 Jan 2018 22:24:45 -0500
Subject: [PATCH 2/2] We are supposed to use the foreground color for solid
fill.

(cherry picked from commit f821995022df8dd1e580dd22cf131584b2b1ac4f)
---
src/liborcus/odf_styles_context.cpp | 3 ++-
src/liborcus/odf_styles_context_test.cpp | 9 +++++----
2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/liborcus/odf_styles_context.cpp b/src/liborcus/odf_styles_context.cpp
index f9c422a..e5f1cc6 100644
--- a/src/liborcus/odf_styles_context.cpp
+++ b/src/liborcus/odf_styles_context.cpp
@@ -739,7 +739,8 @@ void styles_context::start_element(xmlns_id_t ns, xml_token_t name, const std::v
{
spreadsheet::color_elem_t red, green, blue;
func.get_background_color(red, green, blue);
- mp_styles->set_fill_bg_color(255, red, green, blue);
+ mp_styles->set_fill_pattern_type(ORCUS_ASCII("solid"));
+ mp_styles->set_fill_fg_color(255, red, green, blue);
}

size_t fill_id = mp_styles->commit_fill();
diff --git a/src/liborcus/odf_styles_context_test.cpp b/src/liborcus/odf_styles_context_test.cpp
index 4c7eab5..7255a54 100644
--- a/src/liborcus/odf_styles_context_test.cpp
+++ b/src/liborcus/odf_styles_context_test.cpp
@@ -48,10 +48,11 @@ void test_odf_fill(orcus::spreadsheet::import_styles &styles)
std::cerr << std::hex << (int)fill;
const orcus::spreadsheet::fill_t* cell_fill = styles.get_fill(fill);
assert(cell_fill);
- std::cerr << std::hex << (int)cell_fill->bg_color.red;
- assert(cell_fill->bg_color.red == 0xfe);
- assert(cell_fill->bg_color.green == 0xff);
- assert(cell_fill->bg_color.blue == 0xcc);
+ std::cerr << std::hex << (int)cell_fill->fg_color.red;
+ assert(cell_fill->fg_color.red == 0xfe);
+ assert(cell_fill->fg_color.green == 0xff);
+ assert(cell_fill->fg_color.blue == 0xcc);
+ assert(cell_fill->pattern_type == "solid");
}

void test_odf_border(orcus::spreadsheet::import_styles &styles)
--
2.7.4

2 changes: 2 additions & 0 deletions external/liborcus/UnpackedTarball_liborcus.mk
Expand Up @@ -18,6 +18,8 @@ $(eval $(call gb_UnpackedTarball_update_autoconf_configs,liborcus))
$(eval $(call gb_UnpackedTarball_add_patches,liborcus,\
external/liborcus/0001-workaround-a-linking-problem-on-windows.patch \
external/liborcus/rpath.patch.0 \
external/liborcus/0001-Alpha-value-of-0-means-fully-transparent.-I-m-sure-2.patch \
external/liborcus/0002-We-are-supposed-to-use-the-foreground-color-for-soli.patch \
))

ifeq ($(OS),WNT)
Expand Down
15 changes: 10 additions & 5 deletions sc/source/filter/orcus/interface.cxx
Expand Up @@ -1148,7 +1148,8 @@ void ScOrcusStyles::fill::applyToItemSet(SfxItemSet& rSet) const
return;
}

rSet.Put(SvxBrushItem(maBgColor, ATTR_BACKGROUND));
if (maPattern.equalsIgnoreAsciiCase("solid"))
rSet.Put(SvxBrushItem(maFgColor, ATTR_BACKGROUND));
}

ScOrcusStyles::protection::protection():
Expand Down Expand Up @@ -1574,15 +1575,19 @@ void ScOrcusStyles::set_fill_pattern_type(const char* s, size_t n)
maCurrentFill.mbHasFillAttr = true;
}

void ScOrcusStyles::set_fill_fg_color(orcus::spreadsheet::color_elem_t alpha, orcus::spreadsheet::color_elem_t red, orcus::spreadsheet::color_elem_t green, orcus::spreadsheet::color_elem_t blue)
void ScOrcusStyles::set_fill_fg_color(
orcus::spreadsheet::color_elem_t /*alpha*/, orcus::spreadsheet::color_elem_t red, orcus::spreadsheet::color_elem_t green, orcus::spreadsheet::color_elem_t blue)
{
maCurrentFill.maFgColor = Color(alpha, red, green, blue);
// Ignore the alpha element for now.
maCurrentFill.maFgColor = Color(red, green, blue);
maCurrentFill.mbHasFillAttr = true;
}

void ScOrcusStyles::set_fill_bg_color(orcus::spreadsheet::color_elem_t alpha, orcus::spreadsheet::color_elem_t red, orcus::spreadsheet::color_elem_t green, orcus::spreadsheet::color_elem_t blue)
void ScOrcusStyles::set_fill_bg_color(
orcus::spreadsheet::color_elem_t /*alpha*/, orcus::spreadsheet::color_elem_t red, orcus::spreadsheet::color_elem_t green, orcus::spreadsheet::color_elem_t blue)
{
maCurrentFill.maBgColor = Color(alpha, red, green, blue);
// Ignore the alpha element for now.
maCurrentFill.maBgColor = Color(red, green, blue);
maCurrentFill.mbHasFillAttr = true;
}

Expand Down

0 comments on commit 20945a9

Please sign in to comment.