From 8bfccd3a71d911b6da6c108a1312b712431f99d8 Mon Sep 17 00:00:00 2001 From: Chris Sherlock Date: Wed, 13 Jan 2016 03:05:29 +1100 Subject: 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 Tested-by: Chris Sherlock --- 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(-) create mode 100644 vcl/CppunitTest_vcl_fontmetric.mk create mode 100644 vcl/qa/cppunit/fontmetric.cxx diff --git a/include/vcl/metric.hxx b/include/vcl/metric.hxx index 28b582f63929..e289b722cbb1 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 000000000000..5206db23d20b --- /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 1480504c72bb..0be5a4bb18bc 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 6a23c44afe31..400eb196f6ca 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 000000000000..b7db7dfafe11 --- /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 + +#include +#include + +#include + +#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 1a9d05fbdb8a..a3d5a679722d 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 ee4c204eb172..00a5f73326cd 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(); -- cgit v1.2.3