cool#13574 sw redline render mode: avoid coloring, set lightness

User A managed to format a piece of text as red, formatted with
underline and then user B is now confused why rejecting this tracked
change doesn't work.

This is working as intended for the normal redline render mode, but we
can try something different for the 'omit insert/delete' redline render
mode: when showing the old version, we can render deletes unchanged and
when showing the new version, we can render inserts unchanged. The rest
of the redlines can be semi-hidden.

That semi-hidden is a bit tricky to provide. Now that we don't add
layout-level colors in SwModule::GetInsertAuthorAttr(), we go with
automatic color, which gets resolved quite late. So first figure out if
we need to "omit" (semi-hide) the current redline portion in
SwTextPainter::DrawTextLine(), and then pass around a flag, so that once
SwFntObj::DrawText() is past ApplyAutoColor(), we can set lightness to a
medium value.

This is needed, because a typical auto color resolves to either white or
black, and changing saturation has no effect for those colors. And this
way we still get readable text in both light and dark mode.

Change-Id: I0ff57cd996fda80fadc315ad0c6c85e5af1ff3e7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197350
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

sw/CppunitTest_sw_core_text.mk

@@ -34,6 +34,7 @@ $(eval $(call gb_CppunitTest_use_libraries,sw_core_text, \
     sfx \
     subsequenttest \
     svl \
+    svt \
     sw \
 	swqahelper \
     test \

sw/qa/core/text/itrpaint.cxx

@@ -12,10 +12,13 @@
 #include <memory>
 
 #include <o3tl/string_view.hxx>
+#include <svtools/colorcfg.hxx>
 
 #include <docsh.hxx>
 #include <wrtsh.hxx>
 #include <ndtxt.hxx>
+#include <swmodule.hxx>
+#include <swdll.hxx>
 
 namespace
 {
@@ -29,8 +32,8 @@ public:
     }
 };
 
-/// #RRGGBB -> HSL saturation.
-sal_Int16 GetColorSaturation(std::u16string_view rRGB)
+/// #RRGGBB -> HSL lightness.
+sal_Int16 GetColorLightness(std::u16string_view rRGB)
 {
     Color aColor(o3tl::toInt32(rRGB.substr(1, 2), 16), o3tl::toInt32(rRGB.substr(3, 2), 16),
                  o3tl::toInt32(rRGB.substr(5, 2), 16));
@@ -38,12 +41,16 @@ sal_Int16 GetColorSaturation(std::u16string_view rRGB)
     sal_uInt16 nSaturation;
     sal_uInt16 nBrightness;
     aColor.RGBtoHSB(nHue, nSaturation, nBrightness);
-    return nSaturation;
+    return nBrightness;
 }
 
 CPPUNIT_TEST_FIXTURE(Test, testRedlineRenderModeOmitInsertDelete)
 {
-    // Default rendering: default, normal saturation, normal saturation.
+    // Reset redline author IDs to a predictable default.
+    SwGlobals::ensure();
+    SwModule::get()->ClearRedlineAuthors();
+
+    // Default rendering: default, normal lightness, normal lightness.
     createSwDoc("redline.docx");
 
     SwDocShell* pDocShell = getSwDocShell();
@@ -64,15 +71,17 @@ CPPUNIT_TEST_FIXTURE(Test, testRedlineRenderModeOmitInsertDelete)
     CPPUNIT_ASSERT_EQUAL(u"oldcontent"_ustr, aContent.copy(nIndex2, nLength2));
     OUString aColor2
         = getXPath(pXmlDoc, "(//textarray)[2]/preceding-sibling::textcolor[1]", "color");
-    CPPUNIT_ASSERT_GREATER(static_cast<sal_Int16>(50), GetColorSaturation(aColor2));
+    Color aRedlineColor = SwModule::get()->GetColorConfig().GetColorValue(svtools::AUTHOR1).nColor;
+    OUString aRedlineColorString = u"#"_ustr + aRedlineColor.AsRGBHexString();
+    CPPUNIT_ASSERT_EQUAL(aRedlineColorString, aColor2);
     sal_Int32 nIndex3 = getXPath(pXmlDoc, "(//textarray)[3]", "index").toInt32();
     sal_Int32 nLength3 = getXPath(pXmlDoc, "(//textarray)[3]", "length").toInt32();
     CPPUNIT_ASSERT_EQUAL(u"newcontent"_ustr, aContent.copy(nIndex3, nLength3));
     OUString aColor3
         = getXPath(pXmlDoc, "(//textarray)[3]/preceding-sibling::textcolor[1]", "color");
-    CPPUNIT_ASSERT_GREATER(static_cast<sal_Int16>(50), GetColorSaturation(aColor3));
+    CPPUNIT_ASSERT_EQUAL(aRedlineColorString, aColor3);
 
-    // Omit inserts: default, normal saturation, de-saturated.
+    // Omit inserts: default, normal lightness, increased lightness.
     SwWrtShell* pWrtShell = pDocShell->GetWrtShell();
     SwViewOption aOpt(*pWrtShell->GetViewOptions());
     aOpt.SetRedlineRenderMode(SwRedlineRenderMode::OmitInserts);
@@ -91,16 +100,16 @@ CPPUNIT_TEST_FIXTURE(Test, testRedlineRenderModeOmitInsertDelete)
     nLength2 = getXPath(pXmlDoc, "(//textarray)[2]", "length").toInt32();
     CPPUNIT_ASSERT_EQUAL(u"oldcontent"_ustr, aContent.copy(nIndex2, nLength2));
     aColor2 = getXPath(pXmlDoc, "(//textarray)[2]/preceding-sibling::textcolor[1]", "color");
-    CPPUNIT_ASSERT_GREATER(static_cast<sal_Int16>(50), GetColorSaturation(aColor2));
+    CPPUNIT_ASSERT_EQUAL(u"#000000"_ustr, aColor2);
     nIndex3 = getXPath(pXmlDoc, "(//textarray)[3]", "index").toInt32();
     nLength3 = getXPath(pXmlDoc, "(//textarray)[3]", "length").toInt32();
     CPPUNIT_ASSERT_EQUAL(u"newcontent"_ustr, aContent.copy(nIndex3, nLength3));
     aColor3 = getXPath(pXmlDoc, "(//textarray)[3]/preceding-sibling::textcolor[1]", "color");
     // Without the accompanying fix in place, this test would have failed with:
-    // - Expected less or equal than: 50
-    // - Actual  : 100
-    // i.e. the 3rd text portion was not de-saturated.
-    CPPUNIT_ASSERT_LESSEQUAL(static_cast<sal_Int16>(50), GetColorSaturation(aColor3));
+    // - Expected greater or equal than: 49
+    // - Actual  : 0
+    // i.e. the 3rd text portion had no increased lightness from black.
+    CPPUNIT_ASSERT_GREATEREQUAL(static_cast<sal_Int16>(49), GetColorLightness(aColor3));
 
     // Omit deletes: default, de-saturated, normal saturation.
     aOpt.SetRedlineRenderMode(SwRedlineRenderMode::OmitDeletes);
@@ -119,12 +128,12 @@ CPPUNIT_TEST_FIXTURE(Test, testRedlineRenderModeOmitInsertDelete)
     nLength2 = getXPath(pXmlDoc, "(//textarray)[2]", "length").toInt32();
     CPPUNIT_ASSERT_EQUAL(u"oldcontent"_ustr, aContent.copy(nIndex2, nLength2));
     aColor2 = getXPath(pXmlDoc, "(//textarray)[2]/preceding-sibling::textcolor[1]", "color");
-    CPPUNIT_ASSERT_LESSEQUAL(static_cast<sal_Int16>(50), GetColorSaturation(aColor2));
+    CPPUNIT_ASSERT_GREATEREQUAL(static_cast<sal_Int16>(49), GetColorLightness(aColor2));
     nIndex3 = getXPath(pXmlDoc, "(//textarray)[3]", "index").toInt32();
     nLength3 = getXPath(pXmlDoc, "(//textarray)[3]", "length").toInt32();
     CPPUNIT_ASSERT_EQUAL(u"newcontent"_ustr, aContent.copy(nIndex3, nLength3));
     aColor3 = getXPath(pXmlDoc, "(//textarray)[3]/preceding-sibling::textcolor[1]", "color");
-    CPPUNIT_ASSERT_GREATER(static_cast<sal_Int16>(50), GetColorSaturation(aColor3));
+    CPPUNIT_ASSERT_EQUAL(u"#000000"_ustr, aColor3);
 }
 }
 

sw/source/core/inc/drawfont.hxx

@@ -91,6 +91,7 @@ class SW_DLLPUBLIC SwDrawTextInfo
     // GetModelPositionForViewPoint should not return the next position if screen position is
     // inside second half of bound rect, used for Accessibility
     bool m_bPosMatchesBounds : 1 = false;
+    bool m_bOmitPaint = false;
 
 #ifdef DBG_UTIL
     // These flags should control that the appropriate Set-function has been
@@ -658,6 +659,9 @@ public:
     // as argument, the change if made to the font otherwise the font at the
     // output device is changed returns if the font has been changed
     bool ApplyAutoColor( vcl::Font* pFnt = nullptr );
+
+    void SetOmitPaint(bool bOmitPaint) { m_bOmitPaint = bOmitPaint; }
+    bool GetOmitPaint() const { return m_bOmitPaint; }
 };
 
 #endif

sw/source/core/text/inftxt.cxx

@@ -726,6 +726,11 @@ void SwTextPaintInfo::DrawText_( const OUString &rText, const SwLinePortion &rPo
                              rPor.GetNextPortion()->InFixMargGrp() ||
                              rPor.GetNextPortion()->IsHolePortion() );
 
+    if (m_bOmitPaint)
+    {
+        aDrawInf.SetOmitPaint(m_bOmitPaint);
+    }
+
     // Draw text next to the left border
     Point aFontPos(m_aPos);
     if( m_pFnt->GetLeftBorder() && rPor.InTextGrp() && !static_cast<const SwTextPortion&>(rPor).GetJoinBorderWithPrev() )

sw/source/core/text/inftxt.hxx

@@ -370,6 +370,7 @@ class SwTextPaintInfo : public SwTextSizeInfo
 
     sal_uInt16 m_nSpaceIdx;
     SwLineInfo const* m_pLineInfo{nullptr}; // hack: need this to get line props
+    bool m_bOmitPaint = false;
 
     void DrawText_(const OUString &rText, const SwLinePortion &rPor,
                    const TextFrameIndex nIdx, const TextFrameIndex nLen,
@@ -494,6 +495,7 @@ public:
     void SetSmartTags(sw::WrongListIterator *const pNew) { m_pSmartTags = pNew; }
     sw::WrongListIterator* GetSmartTags() const { return m_pSmartTags; }
     void SetLineInfo(SwLineInfo const*const pLineInfo) { m_pLineInfo = pLineInfo; }
+    void SetOmitPaint(bool bOmitPaint) { m_bOmitPaint = bOmitPaint; }
 };
 
 class SwTextFormatInfo : public SwTextPaintInfo

sw/source/core/text/itrpaint.cxx

@@ -42,6 +42,8 @@
 #include "pormulti.hxx"
 #include <doc.hxx>
 #include <fmturl.hxx>
+#include <IDocumentRedlineAccess.hxx>
+#include <redline.hxx>
 
 // Returns, if we have an underline breaking situation
 // Adding some more conditions here means you also have to change them
@@ -305,6 +307,9 @@ void SwTextPainter::DrawTextLine( const SwRect &rPaint, SwSaveClip &rClip,
     // Reference portion for the paragraph end portion
     SwLinePortion* pEndTempl = m_pCurr->GetFirstPortion();
 
+    const SwDoc& rDoc = GetInfo().GetTextFrame()->GetDoc();
+    const IDocumentRedlineAccess& rIDRA = rDoc.getIDocumentRedlineAccess();
+    const SwRedlineTable& rRedlineTable = rIDRA.GetRedlineTable();
     while( pPor )
     {
         bool bSeeked = true;
@@ -422,6 +427,36 @@ void SwTextPainter::DrawTextLine( const SwRect &rPaint, SwSaveClip &rClip,
             roTaggedLabel.emplace(nullptr, nullptr, &aPorInfo, *pOut);
         }
 
+        // See if the redline render mode requires to omit the paint of the text portion.
+        SwRedlineTable::size_type nRedline = SwRedlineTable::npos;
+        SwRedlineRenderMode eRedlineRenderMode = SwRedlineRenderMode::Standard;
+        if (GetRedln() && GetRedln()->IsOn())
+        {
+            nRedline = GetRedln()->GetAct();
+            eRedlineRenderMode = GetInfo().GetOpt().GetRedlineRenderMode();
+        }
+        bool bOmitPaint = false;
+        if (nRedline != SwRedlineTable::npos)
+        {
+            const SwRangeRedline* pRedline = rRedlineTable[nRedline];
+            RedlineType eType = pRedline->GetType();
+            if (eRedlineRenderMode == SwRedlineRenderMode::OmitInserts
+                && eType == RedlineType::Insert)
+            {
+                bOmitPaint = true;
+            }
+            else if (eRedlineRenderMode == SwRedlineRenderMode::OmitDeletes
+                     && eType == RedlineType::Delete)
+            {
+                bOmitPaint = true;
+            }
+        }
+
+        if (bOmitPaint)
+        {
+            GetInfo().SetOmitPaint(true);
+        }
+
         {
             // #i16816# tagged pdf support
             Por_Info aPorInfo(*pPor, *this, 0);
@@ -444,6 +479,11 @@ void SwTextPainter::DrawTextLine( const SwRect &rPaint, SwSaveClip &rClip,
             }
         }
 
+        if (bOmitPaint)
+        {
+            GetInfo().SetOmitPaint(false);
+        }
+
         // lazy open LBody and paragraph tag after num portions have been painted to Lbl
         if (pPor->InNumberGrp() // also footnote label
             // note: numbering portion may be split if it has multiple scripts

sw/source/core/txtnode/fntcache.cxx

@@ -1048,6 +1048,25 @@ void SwFntObj::DrawText( SwDrawTextInfo &rInf )
 
     Color aOldColor( pTmpFont->GetColor() );
     bool bChgColor = rInf.ApplyAutoColor( pTmpFont );
+
+    if (rInf.GetOmitPaint())
+    {
+        Color aColor = pTmpFont->GetColor();
+        sal_uInt16 nHue;
+        sal_uInt16 nSaturation;
+        sal_uInt16 nBrightness;
+        aColor.RGBtoHSB(nHue, nSaturation, nBrightness);
+        // 50% lightness: balance between completely omitting the paint and hard-to-notice small
+        // difference.
+        nBrightness = 50;
+        aColor = Color::HSBtoRGB(nHue, nSaturation, nBrightness);
+        if (aColor != pTmpFont->GetColor())
+        {
+            bChgColor = true;
+            pTmpFont->SetColor(aColor);
+        }
+    }
+
     if( !pTmpFont->IsSameInstance( rInf.GetOut().GetFont() ) )
         rInf.GetOut().SetFont( *pTmpFont );
     if ( bChgColor )

sw/source/uibase/app/swmodul1.cxx

@@ -484,23 +484,16 @@ std::size_t SwModule::InsertRedlineAuthor(const OUString& rAuthor)
 static void lcl_FillAuthorAttr( std::size_t nAuthor, SfxItemSet &rSet,
                         const AuthorCharAttr &rAttr, SwRedlineRenderMode eRenderMode = SwRedlineRenderMode::Standard )
 {
+    if (eRenderMode != SwRedlineRenderMode::Standard)
+    {
+        return;
+    }
+
     Color aCol( rAttr.m_nColor );
 
     if( rAttr.m_nColor == COL_TRANSPARENT )
     {
         aCol = lcl_GetAuthorColor(nAuthor);
-
-        // See if the redline render mode requires to de-saturize the color of the text portion.
-        if (eRenderMode != SwRedlineRenderMode::Standard)
-        {
-            sal_uInt16 nHue;
-            sal_uInt16 nSaturation;
-            sal_uInt16 nBrightness;
-            aCol.RGBtoHSB(nHue, nSaturation, nBrightness);
-            // 25% saturation: balance between complete gray and hard-to-notice small difference.
-            nSaturation = nSaturation / 4;
-            aCol = Color::HSBtoRGB(nHue, nSaturation, nBrightness);
-        }
     }
 
     bool bBackGr = rAttr.m_nColor == COL_NONE_COLOR;
@@ -556,22 +549,12 @@ static void lcl_FillAuthorAttr( std::size_t nAuthor, SfxItemSet &rSet,
 
 void SwModule::GetInsertAuthorAttr(std::size_t nAuthor, SfxItemSet &rSet, SwRedlineRenderMode eRenderMode)
 {
-    SwRedlineRenderMode eMode = SwRedlineRenderMode::Standard;
-    if (eRenderMode == SwRedlineRenderMode::OmitInserts)
-    {
-        eMode = eRenderMode;
-    }
-    lcl_FillAuthorAttr(nAuthor, rSet, m_pModuleConfig->GetInsertAuthorAttr(), eMode);
+    lcl_FillAuthorAttr(nAuthor, rSet, m_pModuleConfig->GetInsertAuthorAttr(), eRenderMode);
 }
 
 void SwModule::GetDeletedAuthorAttr(std::size_t nAuthor, SfxItemSet &rSet, SwRedlineRenderMode eRenderMode)
 {
-    SwRedlineRenderMode eMode = SwRedlineRenderMode::Standard;
-    if (eRenderMode == SwRedlineRenderMode::OmitDeletes)
-    {
-        eMode = eRenderMode;
-    }
-    lcl_FillAuthorAttr(nAuthor, rSet, m_pModuleConfig->GetDeletedAuthorAttr(), eMode);
+    lcl_FillAuthorAttr(nAuthor, rSet, m_pModuleConfig->GetDeletedAuthorAttr(), eRenderMode);
 }
 
 // For future extension: