| Age | Commit message (Collapse) | Author |
|---|
|
ItemType is useful and faster than RTTI. Until now it was
implemented by a 16-bit member in the base class, plus
(potentially) all constructors having to hand a value
in at item construction type (of type SfxItemType) to
get that member set correctly.
This works, but there is no reliable way to guarantee
coverage, and there have already been cases with missing
SfxItemType - these fallback to '0' and thus all Items
with ItemType() == 0 are assumed equal and might be
static_cast'ed to the wrong classes. Note that I
identified *35* Items that had no correct ItemType
set/implemented actually. It also uses 16-bit per
incarnated Item at runtime.
I thought and realized now a more systematic approach
to do that with a pure virtual function at the Item
itself. That can also be secured by a clang compiler
plugin in the future to keep it working. It uses one
virtual function per derived class, no longer space
in incarnated Items. Also the constructors will get
more simple again.
But the main aspect is security - we cannot afford
Items potentially being held as equal if they are not.
Unfortunately C++ does not offer something like a
'strict pure virtual function' that would force to
be overloaded in every derivation, but the used
methotology and adding a clang test is reasonably
safe.
Have now done the cleanup of previous method.
Change-Id: I04768285f1e9b73d64b0bb87df401944b5d35678
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/180017
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: I7dcb9768a8cd63200b8f8c50d8170e78ff5aeec2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171068
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Jenkins
|
|
ItemSurrogates are a possibility to iterate over
all SfxPoolItems associated with a WhichID at a
ItemPool to collect or change data.
It is in general not a good idea: the correct action
would be to iterate over the model data and change/
adapt/collect data on the Items of the type
in question. This is because the *assumtion* that you
iteate over all the Items associated with a document
model is *not* correct:
The ItemPool of the model is used for various ItemSets,
e.g. Dialogs/Sidebar and others, so you might get Items
not only from the DocumentModel but from elsewhere.
You might even get Items from *other* models, so
changing these might have unpredictable effects (!)
It is clear to me that this mechanism is more convenient
that iterating over the document models, and it might
have been invented due to this and then used in more
and maore cases. There should be a lambda/callback-based
mechanism in every document model to do this. Until then
we have to live with this 'compromize'. There are over
100 usages currrently, so no way to easily replace this.
For those reasons I changed this to be more safe: There
are two methods to do this now:
1: iterateItemSurrogates to allow read/write access. I
identified six places where that mechanism was used
to change SfxPoolItems, with the use of const_cast.
This now offers a lambda/callback mechanism and the
needed data in a helper (SurrogateData). Internally
it iterates over ItemSets and ItmHolders registered
and thus associated with the Pool. Changing an Item
means to set a changed Item at the Pool/replace the
holder.
2: GetItemSurrogates/FindItemSurrogate to allow
read-only iteration (2nd one with filter). This
collects the Items in a vector that you provide over
which you can then iterate. Do *not* use const_cast
and change the Item when using this (!)
Note that mechanism (2) pe-filters the Items so that
you get each only once: Of couse one Item can be set
in more than one ItemSet/Holder (but also in more than
one model). This filtering is not possible for (1), you
have to evtl. do the same replace action for the same
item, but this is the only way to not change Items that
are associated wth another model.
Note that (2) could also be changed to a lambda/callback
mechanism similar to (1), but there are too many places
that would beed to be adapted. That would have the
advantage to not need to pre-collect the candidates in a
first run.
Also removed/replaced FindItemSurrogate with using
GetItemSurrogates and locally filtering with that needle.
That also made me remove/cleanup CollectSurrogates, it's
only used in one place now.
Change-Id: I0fe2f51f4fca45e1e5aea032cb96bb77b4567f4d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162254
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
The issue is that the flag RegisteredAtPool
at the SfxPoolItem is Pool-dependent: It marks that
the Item is registeres at *one* Pool/Model. This
makes it Pool-dependent. Due to this there is no way
to share Items that need to be registered globally/
in multiple Pools/Models what is one of the goals
for optimal sharing.
We can also not live without having access to all
Items associated with the Pool, due to mechanisms
in place like the Surrogate stuff.
This again is used for two purposes:
(1) Access all Items associated with one Pool/Model,
at least that is the assumption. This is not valid
since it gets corrupted with a single ItemSet/Holder
used that does not host model data, e.g. an open
Dialog or the Sidebar (or...). But works in
principle.
(2) Access extra-Items that are held nowhere and
are created using DirectPutItemInPool, e.g. infos
for a Dialog. These would need a instance/place to
host them, the Pool is (ab)used for that.
Both are 'compromizes' (to not use a more bad word)
and should not exist. (1) should iterate over the
Model and do actions. There are even places that
use (1) to *change* Items, by casting them to
non-const, even RefCounted ones, so having no control
over what all might be changed doing so. Since we
talk about ca. 100+ places there is no way to get
away from this - I can imagine how this happened:
The 'poolable' attr traditionally needed for the old
binary format was one day 'used' to not need to
iterate over the Model, an API was added to access
and this usage was copied. Sigh..
It is even used when ODF is loaded: E.g. the
FillStyle is imported from XML, interpreted, and
put into an ItemSet. Then it gets set at the
XShape using UNO API and a *name* -> that name and
the Surrogate mechanism is used to find and set the
FillStyle at the Model Object. The FillStyle could
probably just be set using UNO API and the data
directly.
The association between Model/Pool and Item is
created by the object hosting the Item, these are
ItemSets and ItemHolders. Thus it is possible to
register these at the Pool. This allows to iterate
and collect the Items associated with the Pool
and keep the Surrogate mechanism alive.
This is the main change done here. It limits
the registrations to Items for which (at the Pool)
the NeedsPoolRegistration is set, also
Item-independent. Speed is okay, I saw no big
changes in my tests here. The registration is
just pointers, no ownership or RefCounting needed
here.
The advantage is that Items get closer to be
shared office-wide, they can be referenced by
multiple ItemSets (RefCnt) associated with
different Pools/Models.
NOTE: This is not true for SfxSetItems, these are
and will stay Pool-dependent due to their need
to a Pool in the contained ItemSet.
Note that we have ca. six deivations of SfxSetItem,
but ca. 500+ Item derivations, so not too bad.
For the usages of Surrogates to change existing,
RefCounted Items: These can now at least be
changed - if they show up to be problematic - to
iterate over the registered ItemSets and change
Items there the correct way: Set a changed one
at the ItemSet. That also allows Objects to
*react* on ItemChanges, there is no way to do
that with the existing 'compromize'...
UnitTests show that this already works well for
SC and SD, but SW has still some issues. I will
put this to gerrit now, but there will be
additional work.
A involved problem is the current DefaultItem
handling and the state the Pool implementation
is in. E.g. StaticDefaults are not really static,
Pools hard-delete the DefaultItems (forcing the
RefCnt to zero to not have the destructor
complain) and other quirks. Looking at that
right now, hoping to get this change done without
having to change that too much.
I thought about adapting PoolItemTest to this,
but it is only related to DirectPutItemInPool
which is mostly gone and hopefully completely
soon.
Nonetheless I adapted that mechanism to use a
list of SfxPoolItemHolder at the Pool. That makes
it safe and abandons the need for indirect
garbage collection removal at the Pool.
Change-Id: Ib47f21dafa989202930919eace5f7e9c5632ce96
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161896
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
found with the help of a temporary loplugin (which i have put into the
store/ folder)
Change-Id: Ide40d09bef6993ace50039a8fd0439b7e29c09a6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/135288
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
to act as an extra check that we have the association of Item and
TypedWhichId annotations correct.
(*) requires that I add an upcasting constructor to TypedWhichId
(*) Make the field dialog stuff in writer use a new item id
FN_FIELD_DIALOG_DOC_PROPS instead of abusing the
existing SID_DOCINFO
Change-Id: Ica4aea930c80124609a063768c9af5a189df1c27
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129098
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
With CWS impress64 a partial fix for this was implemented to drop
unreferenced named items including all non-color fills after ODF import,
but this is only done in sd so move the code that does that to svx and
call it from sc and sw as well.
Implement some UNO interface for this, it's at least better than a magic
string, and not obvious how a better solution would look like since it's
known only at the end of the import if a bitmap is used or not.
Another problem: when the Area tab is used to change to a different kind
of fill, the items with the details for the previous fill aren't
cleared, and so they are written to ODF files. Hence bitmaps in the
file can be referenced even if they aren't actually used, and bloat up
the files.
Fix this by dropping all unused draw:fill-image-name attributes in ODF
import.
Also do the same for Gradient and Hatch fills; Transparency gradients
can be combined with anything so leave them as they are.
Change-Id: I0b591fd9f963d974d0c3e7208b99621ad61dd93c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118950
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
|
|
just so we have one fewer odd constructors here
Change-Id: I81278e9436747a4eb46a33da9bfec7a8b30079b9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118982
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
which exposed a bug in SvxUnoNameItemTable::replaceByName
Change-Id: If3207df6d46a2185b78ea5c3e1c145527b42d7cd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/115126
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Change-Id: I076f16d0536b534abf0ced4d76051eadb4c0e033
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114949
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
...subclasses, so we always call the SfxPoolItem::operatar==
which will ensure that we are always comparing two objects that
belong to the same class.
And fix the fallout in SvxUnoNameItemTable.
Change-Id: I5ec7fedaa914a328897b54c016ad13e505a15937
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/108599
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Found with bin/find-unneeded-includes
Only removal proposals are dealt with here.
Change-Id: I8d8a3e13932b004678b305f9a6883062854f9201
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/90140
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
|