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 <momonasmon@gmail.com>

6 files changed, 25 insertions, 10 deletions

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);