ab7c81f556
To understand this, some look back in history will be needed to see why it is as it is today. In some (reworked) comments 'poolable' is described as flag to hold Items in the ItemPool, also always having only one incarnation of each possible Item. This is not the original intention, but a side-effect. The reason is what the binary format in the office did: To save a document, the Objects & the Pool were saved, *not* individual Items *together* with the objects. The Pool was completely (binary) saved (and loaded) in one run. Temporary IDs were used to represent at the objects in file which Items were referenced. This *required* to have only one incarnation per item to have a minimal binary file size, thus this high effort was put into this. At doc load, the pool was loaded, all Items were set to RefCount 5000, the references from the objects were restored and then for each Item the RefCount was lowered by 5000 again and - if being zero - deleted. Items for UI were marked 'non-poolable' to *not* safe them with the document, so poolable was a flag to decide if that Info/Item was to be saved with the document - or more direct: if it is Model Data. Items are small, so if we prefer runtime it is okay to no longer being strict with this, anyways does not happen often and has only marginal memory effects - compared to runtime effects/savings. Other problems which this caused: One example is that objects in the UNDO stack were still in the pool, so e.g. deleted pictures were saved with the document despite no longer being used (!). That is the reason we have an UndoItemPool and a method MigrateItemPool to move stuff to that Pool when objects go to the UNDO stack - all of this is also no longer needed. Cleaning this up means to ideally have all items in the SfxItemSet, no longer at the Pool. The Pool should be reduced to a 'Default-Item- Holder' and a 'Slot-to-whichId-mapper'. This needs thorough cleanups/removals, but will be worth it because that massive simplification(s) will increase safety an runtime and make migrating to the goal of completely type-based ItemSet stuff easier for the future. Hopefully only view code in the office working with items will have to be changed for this. In this 1st step I already found that some 'compromizes' will be needed: - There are still Items that have to be at the pool to make the Surrogate-stuff working. This gives back all Items in a Pool of a type and is used in ca. 80 cases. Each one looks at these Items *without* context (e.g. a SfxItemSet at an Object would be a context), so if e.g. a dialog is open that temporarily uses Items of that type you would also get these - without knowing about it... To make that work there is still a mechanism to have Items at the Pool, but now just *registering* (and un-reg) them without any sort/search/ remove needs. Also only for Items that need that, so I evaluated the GetItemSurrogates calls and added some asserts when GetItemSurrogates tries to access an unregistered item type which needs to be added. - Another caveat is that there are about 250 places that directly put Items to the Pool (not all remove these, that is done at pool deletion, so some kind of silent 'garbage-collection' is in place). To have an overview I renamed the accessing methods to separate them from the same functionality at the SfxItemSet, which had the same names. An implementation does still add these directly to the pool, there is no way to cleanup those usages for now. In principle all these should be changed to hold the data at an SfxItemSet. I am still hunting problems. But you can build the office, all apps work (including chart) and you can do speed comparisons already. There are test throwing errors, so I hunt these now. It is hard to give an estimation about how much more changes/corrections will be needed. Completed adaptions to new registered Items at Pool, that reduces the failing tests. Still many that I need to hunt. Added stuff to work around that 'compromize' in ScDocumentPool: It overloads ::PutImpl of the pool to implement special handling for a single Item in SC, the ScPatternAttr. In former code that method was used from SfxItemSet and ::PutImpl at the pool directly, so it was only used in one place. I am not sure if it was used from the SfxItemSet functionality, but better offer it for now. To not waste too much runtime the callbacks depend on the boolean 'NewItemCallback' at the SfxPoolItem, it gets set for that single Item in SC and only then the callbacks trigger. I hope to get rid of those again, e.g. newItem_UseDirect is only needed since we have no 'real' StaticPoolDefaults currently - another thing that needs to be cleaned up in a next step. Since usages of impl(Create|Cleanup)ItemEntry and Direct(Put|Remove)ItemInPoolImpl got more and more similar I decided to unify that: move impl(Create|Cleanup)ItemEntry to tooling, make it globally available in svl and use it also directly for Direct(Put|Remove)ItemInPoolImpl. This slightly increases the failing tests again, but only since in Direct(Put|Remove)ItemInPoolImpl that fallback (e.g. tryToGetEqualItem) was used before, thus this is the same class of errors (SfxPoolItem ptr-compare) as the others which I will need to find anyways. Also fixed some missing stuff. Have now idenified and redirected all SfxPoolItem ptr-compares to be able to debug these - one cause for the remaining errors is probably that before with bPoolable those often were sufficient, but are no longer. Used the [loplugin:itemcompare] and a local clang build to do so, see https://gerrit.libreoffice.org/c/core/+/157172 Stabilized Direct(Put|Remove)ItemInPoolImpl forwards, added parameter to implCreateItemEntry to signal that it gets called from DirectPool stuff - currently needed. Hopefully when getting rid of that DirectPool stuff we can remove that again Added two more debug functionalities: - Added a SerialNumber to allow targeted debugging for deterministic cases - Added registering & listing of still-allocated SfxPoolItems at office shutdown Found PtrComp error in thints.cxx - POC, thanks to areSfxPoolItemPtrsEqual. Will hopefully help more with other tests Found some wrong asserts/warnings where I was too careful and not finding something/succeeding is OK, fixes some UnitTests for SC For SC I now just tried to replace all areSfxPoolItemPtrsEqual with the full-ptr-content compare SfxPoolItem::areSame. I also needed to experiment/adapt the newItem_Callback solution but got it working. Did that replacement now for SW too, found some places where the direct ptr compare is OK. Continued for the rest of occurrences, now all 160 places evaluated. Also done some cleanups. Massive cleanups of stuff no longer needed with this paradigm change. Also decided to keep tryToGetEqualItem/ITEM_CLASSIC_MODE for now. It is used for *one* Item (ScPatternAttr/ATTR_PATTERN) in SC that already needs many exceptions. Also useful for testing if errors come up on this change to test if it is related to this. Added forwarding of target Pool for ::Clone in SvxSetItem and SvxSetItem, simplified SfxStateCache::SetState_Impl and returned to simple ptr compares in SfxPoolItem::areSame to not do the test in areSfxPoolItemPtrsEqual. Debugged through UITest_calc_tests9 and found that in tdf133629 where BoxStyle is applied to fully selected empty calc the Item- reuse fallback has to be used not only for ATTR_PATTERN, see comment @implCreateItemEntry. Maybe more... Problem with test_tdf156611_insert_hyperlink_like_excel. Found that in ScEditShell::GetFirstURLFieldFromCell the correct SvxURLField is found and returned as ptr, but it's usage crashes. That is due to the SfxItemSet aEditSet used there gets destroyed at function return what again deletes the SvxFieldItem that is holding the SvxURLField that gets returned. This shows a more general problem: There is no 'SfxPoolItemHolder' that safely holds a single SfxPoolItem - like a SfxItemSet for a single Item (if Items would be shared_ptrs, that would be a safe return value). That will be needed in the future, but for now use another solution: Since I see no reason why EE_FEATURE_FIELD should not be shareable I wil change this for ow in the SfxItemInfo for EditCharAttribField. That way the Item returned will be shared (RefCnt > 1) and thus not be deleted. I changed the return value for GetURLField() and GetFirstURLFieldFromCell() in ScEditShell: At least for GetFirstURLFieldFromCell the return type/value was not safe: The SvxFieldItem accessed there and held in the local temporary SfxItemSet may be deleted with it, so return value can be corrupted/deleted. To avoid that, return a Clone of SvxFieldData as a unique_ptr. With all that UnitTest debugging and hunting and to get the paradigm change working to no longer rely on shared/pooled items I lost a little bit focus on speed, so I made an optimization round for the two central methods implCreateItemEntry/implCleanupItemEntry to get back to the speed improvements that I detected when starting this change. It was mainly lost due to that 'strange' chained pool stuff we have, so I added to detect the target pool (the one at which the WhichID is registered) directly and only once. Next thing to cleanup will/should be the pool and it's concept, all this is not needed and really costs runtime. Since implCreateItemEntry/implCleanupItemEntry are executed millions of times, each cycle counts here. Had an error in the last changes: pool::*_Impl methods use index instead of WhichID - most of them. Another bad trap, I really need to cleanup pool stuff next. Change-Id: I6295f332325b33268ec396ed46f8d0a1026e2d69 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/157559 Tested-by: Jenkins Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
112 lines
3.4 KiB
C++
112 lines
3.4 KiB
C++
/* -*- 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/.
|
|
*
|
|
* This file incorporates work covered by the following license notice:
|
|
*
|
|
* Licensed to the Apache Software Foundation (ASF) under one or more
|
|
* contributor license agreements. See the NOTICE file distributed
|
|
* with this work for additional information regarding copyright
|
|
* ownership. The ASF licenses this file to you under the Apache
|
|
* License, Version 2.0 (the "License"); you may not use this file
|
|
* except in compliance with the License. You may obtain a copy of
|
|
* the License at http://www.apache.org/licenses/LICENSE-2.0 .
|
|
*/
|
|
#ifndef INCLUDED_EDITENG_FONTITEM_HXX
|
|
#define INCLUDED_EDITENG_FONTITEM_HXX
|
|
|
|
#include <editeng/editengdllapi.h>
|
|
#include <rtl/ustring.hxx>
|
|
#include <tools/fontenum.hxx>
|
|
#include <svl/poolitem.hxx>
|
|
|
|
/** This item describes a Font.
|
|
*/
|
|
class EDITENG_DLLPUBLIC SvxFontItem final : public SfxPoolItem
|
|
{
|
|
OUString aFamilyName;
|
|
OUString aStyleName;
|
|
FontFamily eFamily;
|
|
FontPitch ePitch;
|
|
rtl_TextEncoding eTextEncoding;
|
|
|
|
public:
|
|
static SfxPoolItem* CreateDefault();
|
|
|
|
explicit SvxFontItem(const sal_uInt16 nId);
|
|
SvxFontItem(const FontFamily eFam, OUString aFamilyName,
|
|
OUString aStyleName,
|
|
const FontPitch eFontPitch /*= PITCH_DONTKNOW*/,
|
|
const rtl_TextEncoding eFontTextEncoding /*= RTL_TEXTENCODING_DONTKNOW*/,
|
|
const sal_uInt16 nId);
|
|
|
|
// "pure virtual Methods" from SfxPoolItem
|
|
virtual bool operator==(const SfxPoolItem& rItem) const override;
|
|
virtual SvxFontItem* Clone(SfxItemPool *pPool = nullptr) const override;
|
|
virtual bool QueryValue(css::uno::Any& rVal, sal_uInt8 nMemberId = 0) const override;
|
|
virtual bool PutValue(const css::uno::Any& rVal, sal_uInt8 nMemberId) override;
|
|
|
|
virtual bool GetPresentation(SfxItemPresentation ePres,
|
|
MapUnit eCoreMetric, MapUnit ePresMetric,
|
|
OUString &rText, const IntlWrapper&) const override;
|
|
|
|
// Access methods:
|
|
void SetFamilyName(const OUString& rFamilyName)
|
|
{
|
|
aFamilyName = rFamilyName;
|
|
}
|
|
const OUString &GetFamilyName() const
|
|
{
|
|
return aFamilyName;
|
|
}
|
|
|
|
void SetStyleName(const OUString &rStyleName)
|
|
{
|
|
aStyleName = rStyleName;
|
|
}
|
|
const OUString &GetStyleName() const
|
|
{
|
|
return aStyleName;
|
|
}
|
|
|
|
void SetFamily(FontFamily _eFamily)
|
|
{
|
|
eFamily = _eFamily;
|
|
}
|
|
FontFamily GetFamily() const
|
|
{
|
|
return eFamily;
|
|
}
|
|
|
|
void SetPitch(FontPitch _ePitch)
|
|
{
|
|
ePitch = _ePitch;
|
|
}
|
|
FontPitch GetPitch() const
|
|
{
|
|
return ePitch;
|
|
}
|
|
|
|
void SetCharSet(rtl_TextEncoding _eEncoding)
|
|
{
|
|
eTextEncoding = _eEncoding;
|
|
}
|
|
rtl_TextEncoding GetCharSet() const
|
|
{
|
|
return eTextEncoding;
|
|
}
|
|
|
|
void dumpAsXml(xmlTextWriterPtr pWriter) const override;
|
|
};
|
|
|
|
EDITENG_DLLPUBLIC void GetDefaultFonts(SvxFontItem& rLatin,
|
|
SvxFontItem& rAsian,
|
|
SvxFontItem& rComplex);
|
|
|
|
#endif // INCLUDED_EDITENG_FONTITEM_HXX
|
|
|
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|