include/docmodel/color/ComplexColor.hxx | 2 - include/docmodel/theme/FormatScheme.hxx | 18 +++++------ include/oox/export/ColorExportUtils.hxx | 3 - oox/source/export/ColorExportUtils.cxx | 7 +++- sc/qa/unit/ucalc_DocumentThemes.cxx | 1 sc/source/filter/excel/xestyle.cxx | 13 ++++---- sc/source/ui/drawfunc/drawsh.cxx | 6 +-- sc/source/ui/theme/ThemeColorChanger.cxx | 49 +++++++++++++++++-------------- 8 files changed, 53 insertions(+), 46 deletions(-)
New commits: commit 03bc5199f92e70b6168e4f79600ac288aa7b26ec Author: Tomaž Vajngerl <tomaz.vajng...@collabora.co.uk> AuthorDate: Mon Aug 28 18:36:10 2023 +0200 Commit: Tomaž Vajngerl <qui...@gmail.com> CommitDate: Fri Sep 1 22:52:16 2023 +0200 various theme and complex color related fixes Most of changes are a results of code reviews. Added comments and clarifications, removed unneeded methods, renamed variables to be more clear, protection against nullptr dereference,... Change-Id: Iae2b6abfa90b3bbd3f28328ca7fcf429765cbe9d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/156203 Tested-by: Jenkins Reviewed-by: Tomaž Vajngerl <qui...@gmail.com> diff --git a/include/docmodel/color/ComplexColor.hxx b/include/docmodel/color/ComplexColor.hxx index 4cec9e972503..0fc99a12617b 100644 --- a/include/docmodel/color/ComplexColor.hxx +++ b/include/docmodel/color/ComplexColor.hxx @@ -180,8 +180,6 @@ public: meType = ColorType::HSL; } - void setPlaceholder() { meType = ColorType::Placeholder; } - void setSystemColor(SystemColorType eSystemColorType, sal_Int32 nRGB) { maLastColor = ::Color(ColorTransparency, nRGB); diff --git a/include/docmodel/theme/FormatScheme.hxx b/include/docmodel/theme/FormatScheme.hxx index 96c8afc48214..d6812e749aba 100644 --- a/include/docmodel/theme/FormatScheme.hxx +++ b/include/docmodel/theme/FormatScheme.hxx @@ -474,19 +474,19 @@ public: { FillStyle* pFillStyle = pThis->addFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } { FillStyle* pFillStyle = pThis->addFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } { FillStyle* pFillStyle = pThis->addFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } } @@ -517,7 +517,7 @@ public: pLineStyle->maLineDash.mePresetType = PresetDashType::Solid; pLineStyle->maLineJoin.meType = LineJoinType::Miter; auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pLineStyle->maLineFillStyle.mpFill = pFill; } { @@ -529,7 +529,7 @@ public: pLineStyle->maLineDash.mePresetType = PresetDashType::Solid; pLineStyle->maLineJoin.meType = LineJoinType::Miter; auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pLineStyle->maLineFillStyle.mpFill = pFill; } { @@ -541,7 +541,7 @@ public: pLineStyle->maLineDash.mePresetType = PresetDashType::Solid; pLineStyle->maLineJoin.meType = LineJoinType::Miter; auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pLineStyle->maLineFillStyle.mpFill = pFill; } } @@ -591,19 +591,19 @@ public: { FillStyle* pFillStyle = pThis->addBackgroundFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } { FillStyle* pFillStyle = pThis->addBackgroundFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } { FillStyle* pFillStyle = pThis->addBackgroundFillStyle(); auto pFill = std::make_shared<SolidFill>(); - pFill->maColor.setPlaceholder(); + pFill->maColor.setThemePlaceholder(); pFillStyle->mpFill = pFill; } } diff --git a/include/oox/export/ColorExportUtils.hxx b/include/oox/export/ColorExportUtils.hxx index f9dafe260b46..01b3503e00a3 100644 --- a/include/oox/export/ColorExportUtils.hxx +++ b/include/oox/export/ColorExportUtils.hxx @@ -14,9 +14,6 @@ namespace model { class ComplexColor; -} -namespace model -{ enum class ThemeColorType : sal_Int32; } diff --git a/oox/source/export/ColorExportUtils.cxx b/oox/source/export/ColorExportUtils.cxx index 2b9f7baabc53..d46ab493b5ab 100644 --- a/oox/source/export/ColorExportUtils.cxx +++ b/oox/source/export/ColorExportUtils.cxx @@ -45,11 +45,16 @@ sal_Int32 convertThemeColorTypeToExcelThemeNumber(model::ThemeColorType eType) if (eType == model::ThemeColorType::Unknown) return -1; + // Change position of text1 and text2 and background1 and background2 - needed because of an bug in excel, where + // the text and background index positions are switched. + // 0 -> 1, 1 -> 0 + // 2 -> 3, 3 -> 2 + // everything else stays the same static constexpr std::array<sal_Int32, 12> constThemeColorMapToXmlMap = { 1, 0, 3, 2, 4, 5, 6, 7, 8, 9, 10, 11 }; return constThemeColorMapToXmlMap[sal_Int32(eType)]; } -} +} // end oox /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/qa/unit/ucalc_DocumentThemes.cxx b/sc/qa/unit/ucalc_DocumentThemes.cxx index 664b06a879f9..d0bd29052c16 100644 --- a/sc/qa/unit/ucalc_DocumentThemes.cxx +++ b/sc/qa/unit/ucalc_DocumentThemes.cxx @@ -71,6 +71,7 @@ CPPUNIT_TEST_FIXTURE(DocumentThemesTest, testChangeTheme) aNewPattern.GetItemSet().Put(SvxColorItem(aColor, aComplexColor, ATTR_FONT_COLOR)); } + // Apply the pattern to cells C3:E5 (2,2 - 4,4) on the first sheet m_pDoc->ApplyPatternAreaTab(2, 2, 4, 4, 0, aNewPattern); { diff --git a/sc/source/filter/excel/xestyle.cxx b/sc/source/filter/excel/xestyle.cxx index f7f24c17d223..a68d6408edd9 100644 --- a/sc/source/filter/excel/xestyle.cxx +++ b/sc/source/filter/excel/xestyle.cxx @@ -1845,7 +1845,9 @@ static const char* ToLineStyle( sal_uInt8 nLineStyle ) return "*unknown*"; } -static void lcl_WriteBorder(XclExpXmlStream& rStrm, sal_Int32 nElement, sal_uInt8 nLineStyle, const Color& rColor, model::ComplexColor const& rComplexColor) +namespace +{ +void lcl_WriteBorder(XclExpXmlStream& rStrm, sal_Int32 nElement, sal_uInt8 nLineStyle, const Color& rColor, model::ComplexColor const& rComplexColor) { sax_fastparser::FSHelperPtr& rStyleSheet = rStrm.GetCurrentStream(); if( nLineStyle == EXC_LINE_NONE ) @@ -1853,7 +1855,7 @@ static void lcl_WriteBorder(XclExpXmlStream& rStrm, sal_Int32 nElement, sal_uInt rStyleSheet->singleElement(nElement); return; } - else if (rColor == Color(0, 0, 0)) + else if (rColor == Color(0, 0, 0) && !rComplexColor.isValidThemeType()) { rStyleSheet->singleElement(nElement, XML_style, ToLineStyle(nLineStyle)); return; @@ -1863,6 +1865,7 @@ static void lcl_WriteBorder(XclExpXmlStream& rStrm, sal_Int32 nElement, sal_uInt oox::xls::writeComplexColor(rStyleSheet, XML_color, rComplexColor, rColor); rStyleSheet->endElement(nElement); } +} // end anonymous namespace void XclExpCellBorder::SaveXml(XclExpXmlStream& rStream) const { @@ -1998,7 +2001,7 @@ void XclExpCellArea::SaveXml( XclExpXmlStream& rStrm ) const { { Color aColor = rPalette.GetColor(mnForeColor); - if (maForegroundComplexColor.isValidThemeType()) + if (maForegroundComplexColor.isValidThemeType() || mnForeColor != 0) oox::xls::writeComplexColor(rStyleSheet, XML_fgColor, maForegroundComplexColor, aColor); else if (mnForeColor != 0) oox::xls::writeComplexColor(rStyleSheet, XML_fgColor, maForegroundComplexColor, aColor); @@ -2006,9 +2009,7 @@ void XclExpCellArea::SaveXml( XclExpXmlStream& rStrm ) const { Color aColor = rPalette.GetColor(mnBackColor); - if (maBackgroundComplexColor.isValidThemeType()) - oox::xls::writeComplexColor(rStyleSheet, XML_bgColor, maBackgroundComplexColor, aColor); - else if (mnForeColor != 0) + if (maBackgroundComplexColor.isValidThemeType() || mnBackColor != 0) oox::xls::writeComplexColor(rStyleSheet, XML_bgColor, maBackgroundComplexColor, aColor); } } diff --git a/sc/source/ui/drawfunc/drawsh.cxx b/sc/source/ui/drawfunc/drawsh.cxx index fde34e9d6686..2369ad0884c9 100644 --- a/sc/source/ui/drawfunc/drawsh.cxx +++ b/sc/source/ui/drawfunc/drawsh.cxx @@ -260,9 +260,9 @@ void ScDrawShell::ExecDrawAttr( SfxRequest& rReq ) if( pView->AreObjectsMarked() ) { - std::unique_ptr<SfxItemSet> aNewArgs = rReq.GetArgs()->Clone(); - lcl_convertStringArguments(*aNewArgs); - pView->SetAttrToMarked(*aNewArgs, false); + std::unique_ptr<SfxItemSet> pNewArgs = rReq.GetArgs()->Clone(); + lcl_convertStringArguments(*pNewArgs); + pView->SetAttrToMarked(*pNewArgs, false); } else pView->SetDefaultAttr( *rReq.GetArgs(), false); diff --git a/sc/source/ui/theme/ThemeColorChanger.cxx b/sc/source/ui/theme/ThemeColorChanger.cxx index 8af2d4885abf..c9b88652dcc3 100644 --- a/sc/source/ui/theme/ThemeColorChanger.cxx +++ b/sc/source/ui/theme/ThemeColorChanger.cxx @@ -117,7 +117,7 @@ bool changeCellItems(SfxItemSet& rItemSet, model::ColorSet const& rColorSet) return bChanged; } -bool changeStyles(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> const& pColorSet) +bool changeStyles(ScDocShell& rDocShell, model::ColorSet const& rColorSet) { ScDocument& rDocument = rDocShell.GetDocument(); ScStyleSheetPool* pPool = rDocument.GetStyleSheetPool(); @@ -132,7 +132,7 @@ bool changeStyles(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> const& aOldData.InitFromStyle(pStyle); auto rItemSet = pStyle->GetItemSet(); - if (changeCellItems(rItemSet, *pColorSet)) + if (changeCellItems(rItemSet, rColorSet)) { if (rDocument.IsUndoEnabled()) { @@ -152,7 +152,7 @@ bool changeStyles(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> const& } bool changeSheets(ScDocShell& rDocShell, ScTabViewShell* pViewShell, ScDrawLayer* pModel, - std::shared_ptr<model::ColorSet> const& pColorSet) + model::ColorSet const& rColorSet) { ScDocument& rDocument = rDocShell.GetDocument(); bool bChanged = false; @@ -174,7 +174,7 @@ bool changeSheets(ScDocShell& rDocShell, ScTabViewShell* pViewShell, ScDrawLayer ScPatternAttr aNewPattern(*pPattern); auto& rItemSet = aNewPattern.GetItemSet(); - bool bItemChanged = changeCellItems(rItemSet, *pColorSet); + bool bItemChanged = changeCellItems(rItemSet, rColorSet); bChanged = bChanged || bItemChanged; if (bItemChanged && rDocument.IsUndoEnabled()) @@ -217,7 +217,7 @@ bool changeSheets(ScDocShell& rDocShell, ScTabViewShell* pViewShell, ScDrawLayer SdrObjListIter aIter(pPage, SdrIterMode::DeepNoGroups); for (SdrObject* pObject = aIter.Next(); pObject; pObject = aIter.Next()) { - svx::theme::updateSdrObject(*pColorSet, pObject, pView, rDocShell.GetUndoManager()); + svx::theme::updateSdrObject(rColorSet, pObject, pView, rDocShell.GetUndoManager()); } std::unique_ptr<SdrUndoGroup> pUndo = pModel->GetCalcUndo(); @@ -235,19 +235,19 @@ bool changeSheets(ScDocShell& rDocShell, ScTabViewShell* pViewShell, ScDrawLayer } model::ComplexColor modifyComplexColor(model::ComplexColor const& rComplexColor, - std::shared_ptr<model::ColorSet> const& pColorSet) + model::ColorSet const& rColorSet) { model::ComplexColor aComplexColor(rComplexColor); if (aComplexColor.isValidThemeType()) { - Color aColor = pColorSet->resolveColor(aComplexColor); + Color aColor = rColorSet.resolveColor(aComplexColor); aComplexColor.setFinalColor(aColor); } return aComplexColor; } -void changeSparklines(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> const& pColorSet) +void changeSparklines(ScDocShell& rDocShell, model::ColorSet const& rColorSet) { ScDocument& rDocument = rDocShell.GetDocument(); auto& rDocFunc = rDocShell.GetDocFunc(); @@ -261,18 +261,18 @@ void changeSparklines(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> co { auto aAttributes = rSparklineGroup->getAttributes(); - aAttributes.setColorAxis(modifyComplexColor(aAttributes.getColorAxis(), pColorSet)); + aAttributes.setColorAxis(modifyComplexColor(aAttributes.getColorAxis(), rColorSet)); aAttributes.setColorSeries( - modifyComplexColor(aAttributes.getColorSeries(), pColorSet)); + modifyComplexColor(aAttributes.getColorSeries(), rColorSet)); aAttributes.setColorNegative( - modifyComplexColor(aAttributes.getColorNegative(), pColorSet)); + modifyComplexColor(aAttributes.getColorNegative(), rColorSet)); aAttributes.setColorMarkers( - modifyComplexColor(aAttributes.getColorMarkers(), pColorSet)); - aAttributes.setColorHigh(modifyComplexColor(aAttributes.getColorHigh(), pColorSet)); - aAttributes.setColorLow(modifyComplexColor(aAttributes.getColorLow(), pColorSet)); + modifyComplexColor(aAttributes.getColorMarkers(), rColorSet)); + aAttributes.setColorHigh(modifyComplexColor(aAttributes.getColorHigh(), rColorSet)); + aAttributes.setColorLow(modifyComplexColor(aAttributes.getColorLow(), rColorSet)); aAttributes.setColorFirst( - modifyComplexColor(aAttributes.getColorFirst(), pColorSet)); - aAttributes.setColorLast(modifyComplexColor(aAttributes.getColorLast(), pColorSet)); + modifyComplexColor(aAttributes.getColorFirst(), rColorSet)); + aAttributes.setColorLast(modifyComplexColor(aAttributes.getColorLast(), rColorSet)); rDocFunc.ChangeSparklineGroupAttributes(rSparklineGroup, aAttributes); } } @@ -292,7 +292,8 @@ std::shared_ptr<model::Theme> getTheme(ScDocShell& rDocShell) return pTheme; } -void changeTheTheme(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> const& pColorSet) +void changeThemeColorInTheDocModel(ScDocShell& rDocShell, + std::shared_ptr<model::ColorSet> const& pColorSet) { auto pTheme = getTheme(rDocShell); std::shared_ptr<model::ColorSet> pNewColorSet = pColorSet; @@ -312,6 +313,10 @@ void changeTheTheme(ScDocShell& rDocShell, std::shared_ptr<model::ColorSet> cons void ThemeColorChanger::apply(std::shared_ptr<model::ColorSet> const& pColorSet) { + // Can't change to an empty color set + if (!pColorSet) + return; + m_rDocShell.MakeDrawLayer(); ScDocShellModificator aModificator(m_rDocShell); @@ -333,12 +338,12 @@ void ThemeColorChanger::apply(std::shared_ptr<model::ColorSet> const& pColorSet) } bool bChanged = false; - bChanged = changeStyles(m_rDocShell, pColorSet) || bChanged; + bChanged = changeStyles(m_rDocShell, *pColorSet) || bChanged; bChanged - = changeSheets(m_rDocShell, pViewShell, rDocument.GetDrawLayer(), pColorSet) || bChanged; - changeSparklines(m_rDocShell, pColorSet); + = changeSheets(m_rDocShell, pViewShell, rDocument.GetDrawLayer(), *pColorSet) || bChanged; + changeSparklines(m_rDocShell, *pColorSet); - changeTheTheme(m_rDocShell, pColorSet); + changeThemeColorInTheDocModel(m_rDocShell, pColorSet); if (bUndo) { @@ -349,6 +354,6 @@ void ThemeColorChanger::apply(std::shared_ptr<model::ColorSet> const& pColorSet) aModificator.SetDocumentModified(); } -} // end sw namespace +} // end sc namespace /* vim:set shiftwidth=4 softtabstop=4 expandtab: */