sc/qa/unit/subsequent_export_test4.cxx | 5 ++ sc/source/core/data/drwlayer.cxx | 4 - sc/source/core/data/postit.cxx | 39 ++++++++---------- svx/inc/sdr/primitive2d/sdrattributecreator.hxx | 3 - svx/source/sdr/contact/viewcontactofsdrcaptionobj.cxx | 6 +- svx/source/sdr/primitive2d/sdrattributecreator.cxx | 6 +- 6 files changed, 34 insertions(+), 29 deletions(-)
New commits: commit a9a2ace53625a8fb3feb1c050648a875a98cb7a8 Author: Maxim Monastirsky <momonas...@gmail.com> AuthorDate: Fri Apr 14 14:58:09 2023 +0300 Commit: Maxim Monastirsky <momonas...@gmail.com> CommitDate: Tue Apr 18 09:33:28 2023 +0200 sc drawstyles: Comment shadow should depends on shadow attribute The shadow of comments differs from the shadow of normal callout shapes, as it shows only for the text rectangle. However the way it's implemented is weird: The shadow attribute is set to false, which turns off the "normal" shadow, and then the special shadow is drawn unconditionally. There is also a code that forces the shadow attribute to false on import, to handle some old files that used to have the shadow on. The confusion begins when one shows the comment, and looks at right click > Area... > Shadow, which (rightfully) shows the shadow as off, but doesn't align with what is visible on the document canvas. Moreover, the other shadow settings on this page do affect the comment shadow, but in order to change them it is needed to check the "Use shadow" checkbox first. But leaving that checkbox as checked will result with a double shadow being drawn for the text rectangle, and an unnecessary shadow drawn for the arrow part. The problem becomes now more visible, as there is a Note style listed in the sidebar. One possible approach could be to draw the special shadow only when the shadow attribute is on, and patch existing files on import to "shadow on" instead of "shadow off". But this will cause the "double shadow" problem when opened in older versions (unless the comment is hidden, in which case we used to override the shadow attribute). But now there's an opportunity for a better solution: As we assign the default comment style to imported comments, we can just clear the shadow attribute DF, instead of forcing it to some value. As a result, the "shadow on" attribute of the style will be in effect in newer versions, while in older versions it will fallback to the "shadow off" pool default. Change-Id: I4b35bc1e8e2e12ed35a82b0c2e9aabcf28b46270 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150353 Tested-by: Jenkins Reviewed-by: Maxim Monastirsky <momonas...@gmail.com> diff --git a/sc/qa/unit/subsequent_export_test4.cxx b/sc/qa/unit/subsequent_export_test4.cxx index 663767c7374f..eb3af3a2b2bf 100644 --- a/sc/qa/unit/subsequent_export_test4.cxx +++ b/sc/qa/unit/subsequent_export_test4.cxx @@ -1557,6 +1557,11 @@ CPPUNIT_TEST_FIXTURE(ScExportTest4, testCommentStyles) auto pCaption = pNote->GetCaption(); CPPUNIT_ASSERT(pCaption); + // Check that we don't keep the shadow attribute as DF + // (see ScNoteUtil::CreateNoteFromCaption) + CPPUNIT_ASSERT_LESSEQUAL(SfxItemState::DEFAULT, + pCaption->GetMergedItemSet().GetItemState(SDRATTR_SHADOW, false)); + auto pStyleSheet = &pDoc->GetStyleSheetPool()->Make("MyStyle1", SfxStyleFamily::Frame); auto& rSet = pStyleSheet->GetItemSet(); rSet.Put(SvxFontHeightItem(1129, 100, EE_CHAR_FONTHEIGHT)); diff --git a/sc/source/core/data/drwlayer.cxx b/sc/source/core/data/drwlayer.cxx index 3940d09906ee..8191dfcede66 100644 --- a/sc/source/core/data/drwlayer.cxx +++ b/sc/source/core/data/drwlayer.cxx @@ -382,9 +382,7 @@ void ScDrawLayer::CreateDefaultStyles() pSet->Put(SdrCaptionEscDirItem(SdrCaptionEscDir::BestFit)); // shadow - /* SdrShadowItem has false, instead the shadow is set for the rectangle - only with SetSpecialTextBoxShadow() when the object is created. */ - pSet->Put(makeSdrShadowItem(false)); + pSet->Put(makeSdrShadowItem(true)); pSet->Put(makeSdrShadowXDistItem(100)); pSet->Put(makeSdrShadowYDistItem(100)); diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index b4acfb35f205..66f034087678 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -112,8 +112,10 @@ void ScCaptionUtil::SetExtraItems( SdrCaptionObj& rCaption, const SfxItemSet& rE SfxItemSet aItemSet = rCaption.GetMergedItemSet(); aItemSet.Put(rExtraItemSet); - // reset shadow items - aItemSet.Put( makeSdrShadowItem( false ) ); + // reset shadow visibility (see also ScNoteUtil::CreateNoteFromCaption) + aItemSet.ClearItem(SDRATTR_SHADOW); + // ... but not distance, as that will fallback to wrong values + // if the comment is shown and then opened in older versions: aItemSet.Put( makeSdrShadowXDistItem( 100 ) ); aItemSet.Put( makeSdrShadowYDistItem( 100 ) ); @@ -885,6 +887,11 @@ ScPostIt* ScNoteUtil::CreateNoteFromCaption( { if (auto pStyleSheet = rDoc.GetStyleSheetPool()->Find(ScResId(STR_STYLENAME_NOTE), SfxStyleFamily::Frame)) aNoteData.mxCaption->SetStyleSheet(static_cast<SfxStyleSheet*>(pStyleSheet), true); + + /* We used to show a shadow despite of the shadow item being set to false. + Clear the existing item, so it inherits the true setting from the style. + Setting explicitly to true would corrupt the shadow when opened in older versions. */ + aNoteData.mxCaption->ClearMergedItem(SDRATTR_SHADOW); } return pNote; diff --git a/svx/inc/sdr/primitive2d/sdrattributecreator.hxx b/svx/inc/sdr/primitive2d/sdrattributecreator.hxx index f372741fa68c..92a2b102ffdb 100644 --- a/svx/inc/sdr/primitive2d/sdrattributecreator.hxx +++ b/svx/inc/sdr/primitive2d/sdrattributecreator.hxx @@ -93,7 +93,8 @@ namespace drawinglayer::primitive2d attribute::SdrLineFillEffectsTextAttribute createNewSdrLineFillEffectsTextAttribute( const SfxItemSet& rSet, const SdrText* pText, - bool bHasContent); // used from OLE and graphic + bool bHasContent, // used from OLE and graphic + bool bSuppressShadow = false); // used from SC notes attribute::SdrLineFillShadowAttribute3D createNewSdrLineFillShadowAttribute( const SfxItemSet& rSet, diff --git a/svx/source/sdr/contact/viewcontactofsdrcaptionobj.cxx b/svx/source/sdr/contact/viewcontactofsdrcaptionobj.cxx index daadd83c5942..b03056ec5944 100644 --- a/svx/source/sdr/contact/viewcontactofsdrcaptionobj.cxx +++ b/svx/source/sdr/contact/viewcontactofsdrcaptionobj.cxx @@ -35,6 +35,7 @@ #include <svx/xfltrit.hxx> #include <svx/xlineit0.hxx> #include <svx/sdmetitm.hxx> +#include <svx/sdooitm.hxx> #include <svx/sdprcitm.hxx> #include <basegfx/polygon/b2dpolygontools.hxx> #include <sdr/primitive2d/sdrdecompositiontools.hxx> @@ -62,7 +63,7 @@ namespace sdr::contact drawinglayer::primitive2d::createNewSdrLineFillEffectsTextAttribute( rItemSet, rCaptionObj.getText(0), - false)); + false, rCaptionObj.GetSpecialTextBoxShadow())); // take unrotated snap rect (direct model data) for position and size const tools::Rectangle aRectangle(rCaptionObj.GetGeoRect()); @@ -134,10 +135,11 @@ namespace sdr::contact if(!aFill.isDefault() && 1.0 != aFill.getTransparence()) { // add shadow offset to object matrix + const bool bShadow(rItemSet.Get(SDRATTR_SHADOW).GetValue()); const sal_uInt32 nXDist(rItemSet.Get(SDRATTR_SHADOWXDIST).GetValue()); const sal_uInt32 nYDist(rItemSet.Get(SDRATTR_SHADOWYDIST).GetValue()); - if(nXDist || nYDist) + if (bShadow && (nXDist || nYDist)) { // #119750# create object and shadow outline, clip shadow outline // on object outline. If there is a rest, create shadow. Do this to diff --git a/svx/source/sdr/primitive2d/sdrattributecreator.cxx b/svx/source/sdr/primitive2d/sdrattributecreator.cxx index 89493e4d5a45..e1f6fc10c45a 100644 --- a/svx/source/sdr/primitive2d/sdrattributecreator.cxx +++ b/svx/source/sdr/primitive2d/sdrattributecreator.cxx @@ -1121,7 +1121,8 @@ namespace drawinglayer::primitive2d attribute::SdrLineFillEffectsTextAttribute createNewSdrLineFillEffectsTextAttribute( const SfxItemSet& rSet, const SdrText* pText, - bool bHasContent) + bool bHasContent, + bool bSuppressShadow) { attribute::SdrLineAttribute aLine; attribute::SdrFillAttribute aFill; @@ -1171,7 +1172,8 @@ namespace drawinglayer::primitive2d if(bHasContent || !aLine.isDefault() || !aFill.isDefault() || !aText.isDefault()) { // try shadow - const attribute::SdrShadowAttribute aShadow = createNewSdrShadowAttribute(rSet); + const attribute::SdrShadowAttribute aShadow = !bSuppressShadow ? + createNewSdrShadowAttribute(rSet) : attribute::SdrShadowAttribute(); // glow const attribute::SdrGlowAttribute aGlow = createNewSdrGlowAttribute(rSet); commit 90fbb14e517c95036a133697d7300fc662d1b8b5 Author: Maxim Monastirsky <momonas...@gmail.com> AuthorDate: Fri Apr 14 15:13:32 2023 +0300 Commit: Maxim Monastirsky <momonas...@gmail.com> CommitDate: Tue Apr 18 09:33:14 2023 +0200 sc drawstyles: Don't consider the default cell style formatting We no longer use the text formatting of the default cell style for new comments, nor we follow that style changes, so there is no reason to do that on import. And this formatting will be overwritten anyway in most cases with the imported formatting. (There is currently a bug in xlsx import that makes the author name appear with the default font in visible comments, and that case will look differently after this commit. However, the truth is that *both* old and new looks are wrong. The correct font can be seen in the temp popup when hovering the comment marker, or after copying & pasting the cell with the comment to somewhere else. But even if this wasn't a bug, it was *our* decision to use the cell style font, not something imported or done by Excel, so I believe we're free to change something like this.) Change-Id: I92b95d7dfcfb747144eb710c76ec5f755f70400f Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150490 Tested-by: Jenkins Reviewed-by: Maxim Monastirsky <momonas...@gmail.com> diff --git a/sc/source/core/data/postit.cxx b/sc/source/core/data/postit.cxx index f74937377d8e..b4acfb35f205 100644 --- a/sc/source/core/data/postit.cxx +++ b/sc/source/core/data/postit.cxx @@ -80,8 +80,8 @@ public: static void SetBasicCaptionSettings( SdrCaptionObj& rCaption, bool bShown ); /** Stores the cell position of the note in the user data area of the caption. */ static void SetCaptionUserData( SdrCaptionObj& rCaption, const ScAddress& rPos ); - /** Sets all default formatting attributes to the caption object. */ - static void SetDefaultItems( SdrCaptionObj& rCaption, ScDocument& rDoc, const SfxItemSet* pExtraItemSet ); + /** Sets all hard formatting attributes to the caption object. */ + static void SetExtraItems( SdrCaptionObj& rCaption, const SfxItemSet& rExtraItemSet ); }; void ScCaptionUtil::SetCaptionLayer( SdrCaptionObj& rCaption, bool bShown ) @@ -107,28 +107,17 @@ void ScCaptionUtil::SetCaptionUserData( SdrCaptionObj& rCaption, const ScAddress pObjData->meType = ScDrawObjData::CellNote; } -void ScCaptionUtil::SetDefaultItems( SdrCaptionObj& rCaption, ScDocument& rDoc, const SfxItemSet* pExtraItemSet ) +void ScCaptionUtil::SetExtraItems( SdrCaptionObj& rCaption, const SfxItemSet& rExtraItemSet ) { SfxItemSet aItemSet = rCaption.GetMergedItemSet(); - const ScPatternAttr& rDefPattern = rDoc.GetPool()->GetDefaultItem( ATTR_PATTERN ); - rDefPattern.FillEditItemSet( &aItemSet ); - - if (pExtraItemSet) - { - /* Updates caption item set according to the passed item set while removing shadow items. */ - - aItemSet.Put(*pExtraItemSet); - // reset shadow items - aItemSet.Put( makeSdrShadowItem( false ) ); - aItemSet.Put( makeSdrShadowXDistItem( 100 ) ); - aItemSet.Put( makeSdrShadowYDistItem( 100 ) ); - } + aItemSet.Put(rExtraItemSet); + // reset shadow items + aItemSet.Put( makeSdrShadowItem( false ) ); + aItemSet.Put( makeSdrShadowXDistItem( 100 ) ); + aItemSet.Put( makeSdrShadowYDistItem( 100 ) ); rCaption.SetMergedItemSet( aItemSet ); - - if (pExtraItemSet) - rCaption.SetSpecialTextBoxShadow(); } /** Helper for creation and manipulation of caption drawing objects independent @@ -680,8 +669,9 @@ void ScPostIt::CreateCaptionFromInitData( const ScAddress& rPos ) const if (auto pStyleSheet = mrDoc.GetStyleSheetPool()->Find(ScResId(STR_STYLENAME_NOTE), SfxStyleFamily::Frame)) maNoteData.mxCaption->SetStyleSheet(static_cast<SfxStyleSheet*>(pStyleSheet), true); - // copy all items or set default items; reset shadow items - ScCaptionUtil::SetDefaultItems( *maNoteData.mxCaption, mrDoc, xInitData->moItemSet ? &*xInitData->moItemSet : nullptr ); + // copy all items and reset shadow items + if (xInitData->moItemSet) + ScCaptionUtil::SetExtraItems(*maNoteData.mxCaption, *xInitData->moItemSet); } // set position and size of the caption object