[Libreoffice-commits] core.git: include/vcl vcl/CppunitTest_vcl_fontmetric.mk vcl/inc vcl/Module_vcl.mk vcl/qa vcl/source

Chris Sherlock chris.sherlock79 at gmail.com
Tue Jan 12 17:09:10 PST 2016


 include/vcl/metric.hxx            |    3 +
 vcl/CppunitTest_vcl_fontmetric.mk |   53 +++++++++++++++++++++++++++++++++
 vcl/Module_vcl.mk                 |    1 
 vcl/inc/impfont.hxx               |   29 +++++++++++-------
 vcl/qa/cppunit/fontmetric.cxx     |   61 ++++++++++++++++++++++++++++++++++++++
 vcl/source/gdi/metric.cxx         |   15 ++++++++-
 vcl/source/outdev/font.cxx        |    6 +--
 7 files changed, 152 insertions(+), 16 deletions(-)

New commits:
commit 8bfccd3a71d911b6da6c108a1312b712431f99d8
Author: Chris Sherlock <chris.sherlock79 at gmail.com>
Date:   Wed Jan 13 03:05:29 2016 +1100

    vcl: Create accessor and mutator for font scaling in FontMetric
    
    This is fragile code! There are actually *two* classes that do almost
    precisely the same thing, they are:
    
    - ImplFontMetric, and
    - ImplFontMetricData
    
    They both have much in common, including their class name, and even
    most of their functionality. In fact, they both have common accessor
    functions. When I look at the code, it looks like OutputDevice is
    actually given an ImplFontMetricData object, which it then uses to
    populate an ImplFontMetric object...
    
    Basically, I'm going to merge these classes. To do so, I'm going to
    do the following:
    
    Step 1: Implement accessor functions for ImplFontMetric and FontMetric
            (then remove the friendship of this class to OutputDevice!)
    Step 2: Write a unit test for each accessor function in ImplFontMetric
    Step 3: Ensure that ImplFontMetric and ImplFontMetricData use some
            sort of smart pointer (probably an intrusive_ptr like I did
            ages ago with FontCharMap)
    Step 4: Merge the two classes together once their class interfaces
            are the same and I am satisfied they do the same thing
    Step 5: Find all instances of inefficient usage - for instance, I can
            do away with the code that copies the ImplFontMetricData
            attributes into an ImplFontMetric object.
    
    Change-Id: I07c1cb848774b130fa2ca60b51da53e07754dd00
    Reviewed-on: https://gerrit.libreoffice.org/21399
    Reviewed-by: Chris Sherlock <chris.sherlock79 at gmail.com>
    Tested-by: Chris Sherlock <chris.sherlock79 at gmail.com>

diff --git a/include/vcl/metric.hxx b/include/vcl/metric.hxx
index 28b582f..e289b72 100644
--- a/include/vcl/metric.hxx
+++ b/include/vcl/metric.hxx
@@ -52,9 +52,12 @@ public:
     long                GetExtLeading() const;
     long                GetLineHeight() const;
     long                GetSlant() const;
+    bool                IsScalable() const;
     bool                IsFullstopCentered() const;
     long                GetBulletOffset() const;
 
+    void                SetScalableFlag(bool);
+
     FontMetric&         operator=( const FontMetric& rMetric );
     bool                operator==( const FontMetric& rMetric ) const;
     bool                operator!=( const FontMetric& rMetric ) const
diff --git a/vcl/CppunitTest_vcl_fontmetric.mk b/vcl/CppunitTest_vcl_fontmetric.mk
new file mode 100644
index 0000000..5206db2
--- /dev/null
+++ b/vcl/CppunitTest_vcl_fontmetric.mk
@@ -0,0 +1,53 @@
+# -*- Mode: makefile-gmake; tab-width: 4; indent-tabs-mode: t -*-
+#
+# This file is part of the LibreOffice project.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+
+$(eval $(call gb_CppunitTest_CppunitTest,vcl_fontmetric))
+
+$(eval $(call gb_CppunitTest_set_include,vcl_fontmetric,\
+    $$(INCLUDE) \
+    -I$(SRCDIR)/vcl/inc \
+))
+
+$(eval $(call gb_CppunitTest_add_exception_objects,vcl_fontmetric, \
+	vcl/qa/cppunit/fontmetric \
+))
+
+$(eval $(call gb_CppunitTest_use_externals,vcl_fontmetric,boost_headers))
+
+$(eval $(call gb_CppunitTest_use_libraries,vcl_fontmetric, \
+	comphelper \
+	cppu \
+	cppuhelper \
+	sal \
+	svt \
+	test \
+	tl \
+	tk \
+	unotest \
+	vcl \
+	$(gb_UWINAPI) \
+))
+
+$(eval $(call gb_CppunitTest_use_api,vcl_fontmetric,\
+	udkapi \
+	offapi \
+))
+
+$(eval $(call gb_CppunitTest_use_ure,vcl_fontmetric))
+$(eval $(call gb_CppunitTest_use_vcl,vcl_fontmetric))
+
+$(eval $(call gb_CppunitTest_use_components,vcl_fontmetric,\
+	configmgr/source/configmgr \
+	i18npool/util/i18npool \
+	ucb/source/core/ucb1 \
+))
+
+$(eval $(call gb_CppunitTest_use_configuration,vcl_fontmetric))
+
+# vim: set noet sw=4 ts=4:
diff --git a/vcl/Module_vcl.mk b/vcl/Module_vcl.mk
index 1480504..0be5a4b 100644
--- a/vcl/Module_vcl.mk
+++ b/vcl/Module_vcl.mk
@@ -97,6 +97,7 @@ $(eval $(call gb_Module_add_check_targets,vcl,\
 	CppunitTest_vcl_lifecycle \
 	CppunitTest_vcl_bitmap_test \
 	CppunitTest_vcl_fontcharmap \
+	CppunitTest_vcl_fontmetric \
 	CppunitTest_vcl_complextext \
 	CppunitTest_vcl_filters_test \
 	CppunitTest_vcl_outdev \
diff --git a/vcl/inc/impfont.hxx b/vcl/inc/impfont.hxx
index 6a23c44..400eb19 100644
--- a/vcl/inc/impfont.hxx
+++ b/vcl/inc/impfont.hxx
@@ -104,25 +104,32 @@ private:
     sal_uInt16          mnMiscFlags;   // Misc Flags
     sal_uInt32          mnRefCount;    // Reference Counter
 
-    enum { DEVICE_FLAG=1, SCALABLE_FLAG=2, LATIN_FLAG=4, CJK_FLAG=8, CTL_FLAG=16, FULLSTOP_CENTERED_FLAG=32 };
+    bool                mbScalableFont;
+
+    // TODO: As these are progressively moved from bit fields into boolean variables, comment them out.
+    // Eventually this enum will not be needed and we can remove it.
+    enum { DEVICE_FLAG=1, /* SCALABLE_FLAG=2, */ LATIN_FLAG=4, CJK_FLAG=8, CTL_FLAG=16, FULLSTOP_CENTERED_FLAG=32 };
 
 public:
+
+    bool                operator==( const ImplFontMetric& ) const;
+
                         ImplFontMetric();
     void                AddReference();
     void                DeReference();
 
-    long                GetAscent() const       { return mnAscent; }
-    long                GetDescent() const      { return mnDescent; }
-    long                GetIntLeading() const   { return mnIntLeading; }
-    long                GetExtLeading() const   { return mnExtLeading; }
-    long                GetLineHeight() const   { return mnLineHeight; }
-    long                GetSlant() const        { return mnSlant; }
-    bool                IsFullstopCentered() const { return  ((mnMiscFlags & FULLSTOP_CENTERED_FLAG ) != 0); }
+    long                GetAscent() const                           { return mnAscent; }
+    long                GetDescent() const                          { return mnDescent; }
+    long                GetIntLeading() const                       { return mnIntLeading; }
+    long                GetExtLeading() const                       { return mnExtLeading; }
+    long                GetLineHeight() const                       { return mnLineHeight; }
+    long                GetSlant() const                            { return mnSlant; }
 
-    long                GetBulletOffset() const { return mnBulletOffset; }
-    bool                IsScalable() const      { return ((mnMiscFlags & SCALABLE_FLAG) != 0); }
+    bool                IsScalable() const                          { return mbScalableFont; }
+    bool                IsFullstopCentered() const                  { return  ((mnMiscFlags & FULLSTOP_CENTERED_FLAG ) != 0); }
+    long                GetBulletOffset() const                     { return mnBulletOffset; }
 
-    bool                operator==( const ImplFontMetric& ) const;
+    void                SetScalableFlag(bool bScalable)             { mbScalableFont = bScalable; }
 };
 
 
diff --git a/vcl/qa/cppunit/fontmetric.cxx b/vcl/qa/cppunit/fontmetric.cxx
new file mode 100644
index 0000000..b7db7df
--- /dev/null
+++ b/vcl/qa/cppunit/fontmetric.cxx
@@ -0,0 +1,61 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <test/bootstrapfixture.hxx>
+
+#include <osl/file.hxx>
+#include <osl/process.h>
+
+#include <vcl/metric.hxx>
+
+#include "impfont.hxx"
+
+class VclFontMetricTest : public test::BootstrapFixture
+{
+public:
+    VclFontMetricTest() : BootstrapFixture(true, false) {}
+
+    void testScalableFlag();
+    void testEqualityOperator();
+
+    CPPUNIT_TEST_SUITE(VclFontMetricTest);
+    CPPUNIT_TEST(testScalableFlag);
+    CPPUNIT_TEST(testEqualityOperator);
+    CPPUNIT_TEST_SUITE_END();
+};
+
+void VclFontMetricTest::testScalableFlag()
+{
+    // default constructor should set scalable flag to false
+    FontMetric aFontMetric;
+
+    CPPUNIT_ASSERT_MESSAGE( "Scalable flag should be false after default constructor called", !aFontMetric.IsScalable() );
+
+    aFontMetric.SetScalableFlag(true);
+
+    CPPUNIT_ASSERT_MESSAGE( "Scalable flag should be true", aFontMetric.IsScalable() );
+}
+
+void VclFontMetricTest::testEqualityOperator()
+{
+    // default constructor should set scalable flag to false
+    FontMetric aLhs, aRhs;
+
+    aLhs.SetScalableFlag(true);
+    aRhs.SetScalableFlag(true);
+
+    CPPUNIT_ASSERT_MESSAGE( "Scalable flag set same", aLhs == aRhs );
+}
+
+
+CPPUNIT_TEST_SUITE_REGISTRATION(VclFontMetricTest);
+
+CPPUNIT_PLUGIN_IMPLEMENT();
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/gdi/metric.cxx b/vcl/source/gdi/metric.cxx
index 1a9d05f..a3d5a67 100644
--- a/vcl/source/gdi/metric.cxx
+++ b/vcl/source/gdi/metric.cxx
@@ -34,7 +34,8 @@ ImplFontMetric::ImplFontMetric()
     mnSlant( 0 ),
     mnBulletOffset( 0 ),
     mnMiscFlags( 0 ),
-    mnRefCount( 1 )
+    mnRefCount( 1 ),
+    mbScalableFont( false )
 {}
 
 inline void ImplFontMetric::AddReference()
@@ -52,6 +53,8 @@ inline void ImplFontMetric::DeReference()
 
 bool ImplFontMetric::operator==( const ImplFontMetric& r ) const
 {
+    if( mbScalableFont != r.mbScalableFont )
+        return false;
     if( mnMiscFlags  != r.mnMiscFlags )
         return false;
     if( mnAscent     != r.mnAscent )
@@ -154,4 +157,14 @@ long FontMetric::GetBulletOffset() const
     return mpImplMetric->GetBulletOffset();
 }
 
+bool FontMetric::IsScalable() const
+{
+    return mpImplMetric->IsScalable();
+}
+
+void FontMetric::SetScalableFlag(bool bScalable)
+{
+    mpImplMetric->SetScalableFlag( bScalable );
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx
index ee4c204..00a5f73 100644
--- a/vcl/source/outdev/font.cxx
+++ b/vcl/source/outdev/font.cxx
@@ -82,8 +82,7 @@ FontMetric OutputDevice::GetDevFont( int nDevFontIndex ) const
         aFontMetric.SetWeight( rData.GetWeight() );
         aFontMetric.SetItalic( rData.GetSlantType() );
         aFontMetric.SetWidthType( rData.GetWidthType() );
-        if( rData.IsScalable() )
-            aFontMetric.mpImplMetric->mnMiscFlags |= ImplFontMetric::SCALABLE_FLAG;
+        aFontMetric.SetScalableFlag( rData.IsScalable() );
         if( rData.IsBuiltInFont() )
             aFontMetric.mpImplMetric->mnMiscFlags |= ImplFontMetric::DEVICE_FLAG;
     }
@@ -217,8 +216,7 @@ FontMetric OutputDevice::GetFontMetric() const
     aMetric.mpImplMetric->mnMiscFlags   = 0;
     if( pFontAttributes->IsBuiltInFont() )
             aMetric.mpImplMetric->mnMiscFlags |= ImplFontMetric::DEVICE_FLAG;
-    if( pFontAttributes->IsScalable() )
-            aMetric.mpImplMetric->mnMiscFlags |= ImplFontMetric::SCALABLE_FLAG;
+    aMetric.SetScalableFlag( pFontAttributes->IsScalable() );
     if( pFontAttributes->IsFullstopCentered())
             aMetric.mpImplMetric->mnMiscFlags |= ImplFontMetric::FULLSTOP_CENTERED_FLAG;
     aMetric.mpImplMetric->mnBulletOffset = pFontAttributes->GetBulletOffset();


More information about the Libreoffice-commits mailing list