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.vajng...@collabora.co.uk> AuthorDate: Sat Apr 20 11:38:26 2019 +0900 Commit: Tomaž Vajngerl <qui...@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 <qui...@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.vajng...@collabora.co.uk> AuthorDate: Mon Apr 22 10:57:39 2019 +0900 Commit: Tomaž Vajngerl <qui...@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 <qui...@gmail.com> Tested-by: Tomaž Vajngerl <qui...@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); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits