[Libreoffice-commits] core.git: 2 commits - include/tools tools/qa tools/source

Tomaž Vajngerl (via logerrit) logerrit at kemper.freedesktop.org
Mon Apr 22 03:52:12 UTC 2019


 include/tools/color.hxx         |   23 ++++++++++++++++++-----
 tools/qa/cppunit/test_color.cxx |   20 --------------------
 tools/source/generic/color.cxx  |    8 ++++----
 3 files changed, 22 insertions(+), 29 deletions(-)

New commits:
commit d370b676be33c03b5d8e7bdbfe11cb7ea6423e79
Author:     Tomaž Vajngerl <tomaz.vajngerl at collabora.co.uk>
AuthorDate: Sat Apr 20 11:38:26 2019 +0900
Commit:     Tomaž Vajngerl <quikee at gmail.com>
CommitDate: Mon Apr 22 05:51:44 2019 +0200

    cleanup and rename COLORDATA_RGB to color::extractRGB
    
    Change-Id: I8d53c7010b4fea2a74e48f0649e944a498c211c7
    Reviewed-on: https://gerrit.libreoffice.org/71003
    Tested-by: Jenkins
    Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>

diff --git a/include/tools/color.hxx b/include/tools/color.hxx
index 9643f4028dc9..956b500ab752 100644
--- a/include/tools/color.hxx
+++ b/include/tools/color.hxx
@@ -26,7 +26,15 @@
 
 class SvStream;
 
-constexpr sal_uInt32 COLORDATA_RGB( sal_uInt32 n )          { return static_cast<sal_uInt32>(n & 0x00FFFFFF); }
+namespace color
+{
+
+constexpr sal_uInt32 extractRGB(sal_uInt32 nColorNumber)
+{
+    return nColorNumber & 0x00FFFFFF;
+}
+
+}
 
 // Color
 
@@ -115,7 +123,7 @@ public:
 
     Color GetRGBColor() const
     {
-        return COLORDATA_RGB(mValue);
+        return color::extractRGB(mValue);
     }
 
     sal_uInt8 GetColorError(const Color& rCompareColor) const;
@@ -202,7 +210,7 @@ inline void Color::SetTransparency( sal_uInt8 nTransparency )
 
 inline bool Color::IsRGBEqual( const Color& rColor ) const
 {
-    return COLORDATA_RGB(mValue) == COLORDATA_RGB(rColor.mValue);
+    return color::extractRGB(mValue) == color::extractRGB(rColor.mValue);
 }
 
 inline sal_uInt8 Color::GetLuminance() const
@@ -212,8 +220,8 @@ inline sal_uInt8 Color::GetLuminance() const
 
 constexpr sal_uInt8 ColorChannelMerge(sal_uInt8 nDst, sal_uInt8 nSrc, sal_uInt8 nSrcTrans)
 {
-    return static_cast<sal_uInt8>(((static_cast<sal_Int32>(nDst)-nSrc)*nSrcTrans+((nSrc<<8)|nDst))>>8);
-};
+    return sal_uInt8(((sal_Int32(nDst) - nSrc) * nSrcTrans + ((nSrc << 8) | nDst)) >> 8);
+}
 
 inline void Color::Merge( const Color& rMergeColor, sal_uInt8 cTransparency )
 {
diff --git a/tools/source/generic/color.cxx b/tools/source/generic/color.cxx
index 4bb5221c13d6..f79342d37862 100644
--- a/tools/source/generic/color.cxx
+++ b/tools/source/generic/color.cxx
@@ -35,7 +35,7 @@ sal_uInt8 Color::GetColorError( const Color& rCompareColor ) const
                          labs(long(rCompareColor.G) - G) +
                          labs(long(rCompareColor.B) - B);
 
-    return static_cast<sal_uInt8>(FRound( nErrAbs * 0.3333333333 ));
+    return sal_uInt8(FRound(double(nErrAbs) / 3.0));
 }
 
 void Color::IncreaseLuminance(sal_uInt8 cLumInc)
@@ -52,11 +52,11 @@ void Color::DecreaseLuminance(sal_uInt8 cLumDec)
     B = sal_uInt8(std::clamp(long(B) - cLumDec, 0L, 255L));
 }
 
-void Color::DecreaseContrast( sal_uInt8 cContDec )
+void Color::DecreaseContrast(sal_uInt8 nContDec)
 {
-    if( cContDec )
+    if (nContDec)
     {
-        const double fM = ( 128.0 - 0.4985 * cContDec ) / 128.0;
+        const double fM = (128.0 - 0.4985 * nContDec) / 128.0;
         const double fOff = 128.0 - fM * 128.0;
 
         R = sal_uInt8(std::clamp(FRound(R * fM + fOff), 0L, 255L));
commit 60de1cb01986ff6c9aba58dc05dec6b3dc2402d4
Author:     Tomaž Vajngerl <tomaz.vajngerl at collabora.co.uk>
AuthorDate: Mon Apr 22 10:57:39 2019 +0900
Commit:     Tomaž Vajngerl <quikee at gmail.com>
CommitDate: Mon Apr 22 05:51:35 2019 +0200

    Replace compile time test with a static_assert
    
    There is no need to check this in a test when we can do it
    directly in the header. static_assert will complain when compiling
    something that uses the header file instead of waiting that it
    finds a piece of code, where it is actually needed. The purpose of
    the test was the same (fail early).
    
    The main problem was that Color can be created and converted to
    sal_uInt32 string completely in compile time, so it can be used for
    "case" in a "switch" statement, which requires that in "case" it
    uses a constant. Normally this isn't possible unless we resolve
    and convert a Color to sal_uInt32 in compile time.
    This use-case is used somewhere in the code, but it takes a lot
    of (re)compiling to get to that piece of code, where it would
    eventually fail.
    
    Change-Id: I1f1f9b77c19a0e61f78ce703b380d98a569da833
    Reviewed-on: https://gerrit.libreoffice.org/71060
    Reviewed-by: Tomaž Vajngerl <quikee at gmail.com>
    Tested-by: Tomaž Vajngerl <quikee at gmail.com>

diff --git a/include/tools/color.hxx b/include/tools/color.hxx
index 25032c5d29b9..9643f4028dc9 100644
--- a/include/tools/color.hxx
+++ b/include/tools/color.hxx
@@ -243,6 +243,11 @@ namespace com { namespace sun { namespace star { namespace uno {
     }
 } } } }
 
+// Test compile time conversion of Color to sal_uInt32
+
+static_assert (sal_uInt32(Color(0x00, 0x12, 0x34, 0x56)) == 0x00123456);
+static_assert (sal_uInt32(Color(0x12, 0x34, 0x56)) == 0x00123456);
+
 // Color types
 
 constexpr ::Color COL_BLACK                   ( 0x00, 0x00, 0x00 );
diff --git a/tools/qa/cppunit/test_color.cxx b/tools/qa/cppunit/test_color.cxx
index 571a1f37291d..8ecf03572295 100644
--- a/tools/qa/cppunit/test_color.cxx
+++ b/tools/qa/cppunit/test_color.cxx
@@ -21,7 +21,6 @@ namespace
 class Test: public CppUnit::TestFixture
 {
 public:
-    void testConstruction();
     void testVariables();
     void test_asRGBColor();
     void test_readAndWriteStream();
@@ -31,7 +30,6 @@ public:
     void testBColor();
 
     CPPUNIT_TEST_SUITE(Test);
-    CPPUNIT_TEST(testConstruction);
     CPPUNIT_TEST(testVariables);
     CPPUNIT_TEST(test_asRGBColor);
     CPPUNIT_TEST(test_readAndWriteStream);
@@ -42,24 +40,6 @@ public:
     CPPUNIT_TEST_SUITE_END();
 };
 
-void Test::testConstruction()
-{
-    // Compile time construction of the Color and representation as a sal_uInt32
-
-    Color aColor = Color(0xFF, 0xFF, 0x00);
-
-    switch (sal_uInt32(aColor))
-    {
-        case sal_uInt32(Color(0xFF, 0xFF, 0x00)):
-            break;
-        case sal_uInt32(Color(0x00, 0x00, 0xFF, 0xFF)):
-            break;
-        default:
-            CPPUNIT_ASSERT(false);
-            break;
-    }
-}
-
 void Test::testVariables()
 {
     Color aColor(0x44, 0x88, 0xAA);


More information about the Libreoffice-commits mailing list