Age | Commit message (Collapse) | Author |
|
towards making these immutable
Change-Id: I9f9ac17828018525194c34eadda66bbf863fd2f7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164795
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: Iaef46216dac6584f57b7933d658384f54d0a4544
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164772
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: If499e28e5ac69018b35b475a73ecb2dc4b78dad6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164786
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: I847e5de84f0636b5a169f383e319a6b8707cc31f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164773
Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: If7a31f8b3667d9a6b8719553567211071bd2d631
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164774
Tested-by: Jenkins
Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: Id229344a68925a1bde84f2b4aad46cfc5f01b797
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164769
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
sal_uLong to sal_uInt32 for the clipboard format.
Clipboard enum class uses sal_uInt32, so staying consistent with that.
Clipboard format doesn't exceed unsigned 32-bit int.
Change-Id: I1938f9ba877fc89c51415d9715a82e9b0c09d4e4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164604
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Jenkins
|
|
we don't need to store SfxStyleFamily::All in the mStyleSheetsByFamily
array, the call sites just iterate over the main vector for that
case.
Change-Id: I17fca2aa59e786d6dee13c884dedb9fde847b979
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164579
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
we can store pointers styleSheetsByFamily and avoid having
to access the vector
Change-Id: I36b5df981b6e53d9aa4193de419fc6a44f0ed2da
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164573
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Change-Id: I24a013d74bf8f9336496232d585ac1f0b069696c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164572
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
we can store the mStyleSheetPositionsByFamily inline, since it
is fixed size, and save some allocation overhead, and some pointer
chasing.
Change-Id: Id6ff02491e967b9fb145ba9752f4a52173692645
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164558
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
found by tweaking the stringview loplugin
Change-Id: I92203ba99642bef7951ffa146184c5562cb31d09
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163744
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
no need to de-allocate and then re-allocate the vectors inside
mStyleSheetPositionsByFamily
Change-Id: I3ad10173d9f3ba252619667afe13250045c943c5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164529
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Change-Id: If4a68433c57fdf3da56891fa0b4be6f8a991d929
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164528
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
only called by a friend from the same .so
Change-Id: I5f63e83325b291b95b0132089dc331f3b7e79362
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164538
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
and rename to ImpIsSpecialStandardFormat
Change-Id: Ie20c83906559f94e545f384807396ec8acf970f9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164537
Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
These are just locals, except for IniLnge which is set once in the
ctor and is then immutable
Change-Id: I0d8ac0c3ca729003a3575dea39b2746dfc53b4bc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164173
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: I09dea90e3e3f3fd0a4047b989329a027f788f695
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164148
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Change-Id: I2c8ad9999adc406dc850c59b48e49681099dc054
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164147
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
so fold it into the ctor
Change-Id: If063143ef47a8ab293edf3896fb51079d0e0284f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164144
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|
|
Like this since 2016 with 3a0abd3019ec3ca29b8f1378cdb32ebf741e6306
add SvxSearchItem::GetWildcard() SetWildcard()
Change-Id: Id988a6e58488af6b1f274a318e9d1f52c7a8b169
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163876
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
Tested-by: Jenkins
|
|
With ItemInfoPackages we now can have a buffered, static
global translation table from SlotIDs to WhichIDs since
the ItemInfoStatic used already contains all the needed
information.
Register that in registerItemInfoPackage at the Pool and
use it for lookup. That speeds up the lookup from O(n)
to O(1). Since that lookup is used in UI and UNO API
implementations this has positive effect on load/safe,
but also all interactive stuff in the whole office.
NOTE: I tried to use a merged version of that translation
table in the parent pool, but this shows double SlotIDs,
what is no wonder since that's what those are used for:
To get different WhichIDs for a SlotID in Item handling.
This *might* prevent getting rid of the chanined Pools
at all - sadly. The returned WhichID directly depends
on which Pool the function(s) GetWhichIDFromSlotID and
GetTrueWhichIDFromSlotID are called.
NOTE: Very strange is that the parameter 'bDeep' in that
functions is *not* passed down to the call to the
secondary Pool - probably an error, but risky to fix,
that may change already the behaviour :-(
Change-Id: Iea77ffad0f6a5401ab74fea0bbfc2589c66680ea
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163597
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: I51585f1c15984a066262023184f668662853d20f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163556
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
...instead of regular blank
Do not modify other blanks (thousand separator, fraction separator)
Change-Id: I82f8023a4e55d8091545191dee55a88aba25dbdd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161737
Tested-by: Jenkins
Reviewed-by: Laurent Balland <laurent.balland@mailo.fr>
|
|
Change-Id: Ie18f18655e4ee9ca70580b540678003a12b0465d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163458
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Secured usage of FontList in SvxFontListItem
usage (a test)
Change-Id: I412a7681b3ece4e5d3751165d4b566ccdf5b2da9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163257
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Driving forward the Item reworks I now take the
ItemPool in focus: It now does not hold any items
anymore and should be renamed to something like
ItemInfoProvider/ItemHelper, since it's main purpose
is to provide the Defaults for the Item functionality.
There is that SfxItemInfo, only a struct and bundling
SlotID and ItemFlags. There are also the DefaultItems,
just handled as ptrs in an array. It is/was always
error-prone to keep these in sync. Remember that it's
also necessary for the order to not only being sorted
but being increments of one with no gaps allowed in
the WhichIDs to which the Items are bound.
I now bundled that to a new class ItemInfo that joins
WhichID, SlotID, ItemFlags and the default Item. This
is a pure virtual base class, it comes in three
derivations:
(1) ItemInfoStatic:
This is supposed to be global static and hosts the
Item in a std::unique_ptr to ensure cleanup. It is
designed to be constructed once during runtime and
being shared globally. It allows the ItemPtr to be
nullptr to mark as non-static (if initial static is
not possible for some reason) but still offers the
needed data. Most cases (95%+) are of that case.
The contained Item is owned by that instance. The
flag isStaticDefault() is set at the Item.
(2) ItemInfoDynamic:
This is supposed to be used for cases where the Item
cannot be static: Mainly for SfxSetItem (that needs
a Pool itself in the contained SfxItemSet, so lifetime
is bound to that Pool), but other cases showed up in
the transition. These instances live while the Pool
lives and get destructed when the Pool goes down.
Also uses std::unique_ptr for the Item instance for
as much automated cleanup as possible, the contained
Item is owned by that instance, the instance by the
Pool. The flag isDynamicDefault() is set at the Item.
(3) ItemInfoUser:
This is used for UserDefaults that can be set for
every ItemInfo entry to 'overshadow' the default
from the 'outside'. It uses a regular Item and
the central access methods implCreateItemEntry/
implCleanupItemEntry to manage the Item instance,
thus works like a SfxPoolItemHolder. The Item
instance can be globally shared and re-used even
when the Pool goes down. Instances belong to the
Pool and are cleaned up when the Pool goes down.
This Item does not need any further flag to be
set.
The ItemInfos are organized using a class
called ItemInfoPackage:
This bundles groups of ItemInfoStatic to
functional instances. There are derivations/
implementations of this e.g. for Writer ItemPool
bundling all the needed defaults for Writer,
similar for draw/impress, Calc and other usages.
These ItemInfoPackage can be 'registered' at an
ItemPool using it's method registerItemInfoPackage.
This does all the needed stuff to setup that
group of ItemInfos at the Pool (It even sets
internal vars First/LastWhich, that info can just
be derived from the buildup ItemInfo Ptrs).
The ItemInfoPackage has methods 'size()' and
'getItemInfo(index) to allow looping over it
and deliver the infos the Pool needs. The
(forced, pure virtual) overloads of getItemInfo
in the specific implementations check for the
ItemPtr being nullptr and create a exclusive
incarnation of ItemInfoDynamic for the Pool if
needed, returning that. The Pool owns the
ItemInfoDynamic incarnations and uses the
ItemInfoStatic directly. On shutdown it cleans
up the ItemInfoDynamic as needed.
The ItemInfoUser is used by the Pool when a
UserDefault is set/used: for SetUserDefaultItem,
GetUserDefaultItem, ResetUserDefaultItem. It
is not held in a 2nd list, but directly in the
list of ItemInfo'ptrs: To keep track of this
an unordered_map is used that helds the original
ItemInfo associated with the WhichID. That way
no two lookups (as before) are needed to get the
current Pool's default for any WhichID.
The derivations of ItemInfoPackage are
encapsulated and just allow access to an
ItemInfoPackage& with a single method as
return value. All use a static local instance
of a std::array<ItemInfoStatic, FIXED_SIZE>
which constructs all ItemInfoStatic and the
static Item instances - if already possible.
Sometimes it is necessary to overload the
constructor to set some static instances
for Items later than the lib init. These are
also just marked with nullptr as Item instance.
Some need to overload getItemInfo to complete
instances of ItemInfoStatic, if needed, or
create and deliver instances of ItemInfoDynamic.
The registerItemInfoPackage also offers a
optional lambda callback: there were two cases
where local data from the Pool was needed to
incarnate the item - just add that to the
call to registerItemInfoPackage if needed,
see examples in the adapted code.
For the re-use of Items this means that now
in SfxItemSet/SfxPoolItemHolder *true* static
Items can and will be used without RefCount
directly and globally. This is also the case
for dynamic Items, with the exception of
differing Pools for SfxSetItems which cannot
be done.
Future:
That design is already prepared to allow
solving that Pool-chaining problem: currently
there are master/sub-pools and all accesses
have to traverse that structure before even
doing anything.
For the future the idea is more to 'compose'
a Pool by registering ItemInfoPackages, e.g.
for Writer pool you may start with SfxItemPool,
register the writer-specific ItemInfoPackage,
then the one for DrawingLayer (if needed) and
the one for EditEngine.
It should also be possible to get to smaller
granularities of that packages. Ideas for
new ones will emerge. We might also think
about composing Pools which can e.g. run Writer
and Chart, so allowing to use Chart *without*
OLE stuff in Writer - just ideas...
More changes:
- Adapted all stuff, cleaned up old stuff/
definitions
- Removed FreezeIdRanges, that can be done
once per Pool on-demand (and cannot be
forgotten to be called)
- Merged XOutdevItemPool with SdrItemPool
and offered a ItemInfoPackage which joins
both needed sets of Items
- All the cleanup hassle with Pools and
defaults cleaned up
- Adapted all access methods of the pool
to use that new stuff. Pool chaining
currently stays, but I use a central
method 'getTargetPool' instead of
recursive calling to get the correct
Pool for the action
Change-Id: I2b8d3d4c3cc80b1d0d0b3c0f4bd90d7656b4bab7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163157
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
... everywhere it is used to generate material for encryption.
Change-Id: Id3390376bb2f3a5fa1bbfd735850fce886ef7db2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162873
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
|
|
Sorted out some methods at ItemPool which process
Defaults to make more clear what is going on and
what which method is doing.
Change-Id: I2568d3e03d0a56a14b6fe4e04521e1a8e22c000b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162643
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
DER rules say that BOOLEAN values are either FALSE (0x00)
or TRUE (0xff)
Change-Id: I59f57557fbc4d6447e0d8e994b04adda1ee8c1a9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162597
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Obsoleted by commit 2484de6728bd11bb7949003d112f1ece2223c7a1 (Remove
non-const Sequence::begin()/end() in internal code, 2021-10-15) and
commit fb3c04bd1930eedacd406874e1a285d62bbf27d9 (Drop non-const
Sequence::operator[] in internal code, 2021-11-05).
Change-Id: Idbafef5d34c0d4771cbbf75b9db9712e504164cd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162640
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
|
|
Change-Id: Ibdfbb8c6492d18989d587230db674a7ff2546150
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162574
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Change-Id: I99f9730e573f06e53516d0d775df1b276ff83a90
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162573
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Change-Id: I49afb3957c5633f82431408402bead2a9b0328a7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162572
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Change-Id: I12f0cbbc1523a7451281ef501b51b56010105dfd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162570
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Change-Id: Idb09d903b97ca9697ac473b46ae85b2205be128a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162569
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Change-Id: Ia769e28fa1180c40f0e25763485a7b468d1f8f94
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162568
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Unfortunately I had overseen something with derived
classes, but it came now up on CI ASan/UBsan build
with a failing UnitTest - thanks to pointing me at
it.
The ItemInstanceManager at the SfxPoolItems are now
no longer static but constructed instances returned
on-demand.
Also added checks to really use an incarnated/
registered one *only* for that derivation and made
sure this is now correctly supported.
Had to change again, using createItemInstanceManager
to always create instances was less effective than
intended, back to getItemInstanceManager & static
instances in the Item implementations. Also added
some stuff to implCreateItemEntry/implCleanupItemEntry
to be more effective, e.g. direct handling of
slot stuff in latter one. Also some more asserts
and comments. Slot stuff is now handled without
RefCounting, takes some write accesses away...
Change-Id: I6cd69556b416510b5b23549dd042ff3ba155559d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162521
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
As addition to tdf#158605 I have added a
global default mechanism to support global
sharing of Item instances. It's automated
and complements the existing one, see one
of the last commits. All in all there are
now the following possibilities to support
this for individual Item derivations:
(1) Do nothing: In that case, if the Item
is shareable, the new mechanism will kick
in: It will start sharing the Item globally,
but not immediately: After a defined amount
of allowed non-shared ocurrences (look for
NUMBER_OF_UNSHARED_INSTANCES) an instance
of the default ItemInstanceManager, a
DefaultItemInstanceManager, will be incarnated
and used. NOTE: Mixing shared/unshared
instances is not a problem (we might even
implement a kind of 're-hash' when this
kicks in, but is not really needded).
(2) Overload getItemInstanceManager for
SfxPoolItem in a class derived from
SfxPoolItem and...
(2a) Return a static incarnation of
DefaultItemInstanceManager to immediately
start global sharing of that Item derivation.
(2b) Implement and return your own
implementation and static incarnation of
ItemInstanceManager to do something better/
faster that the default implementation can#
do. Example: SvxFontItem, uses hashing.
The NUMBER_OF_UNSHARED_INSTANCES is currently
at (50) which gives a decent relationship
bewteen no global sharing (speed) and memory
needs. Not sharing is faster (that access to
'find' an equal item is spared which might be
costly), sharing again needs less memory.
There are two supported ENVVARs to use:
(a) SVL_DISABLE_ITEM_INSTANCE_MANAGER:
This disables the mechanism of global Item
sharing completely. This can be used to test/
check speed/memory needs compared with using
it, but also may come in handy to check if
evtl. errors/resgrressions have to do with
it.
(b) SVL_SHARE_ITEMS_GLOBALLY_INSTANTLY:
This internally forces the
NUMBER_OF_UNSHARED_INSTANCES to be ignored
and start sharing ALL Item derivations
instantly.
Change-Id: I40d9c5f228f0bcbf239f2ce0a02d423054240570
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162478
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: Ieaba8db5df426ed30aca7366a86ee4fb11e03a44
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162480
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
The task shows that the commit including the Item
paradigm change has follow-ups: It now does no
longer try to share Items as much as possible
(detailed reasons in that commit). Mainly for speed
reasons since that sharing was done before by
(mostly) linearly searching in existing instances
registered at one Pool, using the operator== of the
SfxPoolItems. That costs runtime.
There is somewhere a sweet-spot between memory and
runtime: the number of Items allocated and the time
spent to share them more effectively. This task shows
- despite being a non-real-world document - that
for extremes like this putting work in sharing is
still needed.
But there are possibilities to combine both: If we
can implement solutions that do not need much time
to ideintify an aleady existing instance we will get
the best of both worlds.
As explained already in that change, if we would
need that again, then on a better base. Thus I drove
forward ITEM changes to a state where we are now able
to share Items globally in the office - not per pool
but for all ItemSets/ItemHolders and thus all Apps/
Models/opened documents.
NOTE: This currently needs to include the WhichID
that is included in the Item, so cannot share pure
Item-data (as the old usage did too). This does not
need to stay that way: If you think about it, the
association between WhichID and Pool/Holder is
defined in Pool/Holder, so theoretically the Item
does not need to contain the WhichID. This will
be hard to do due too many places in the code that
use the WhichID stored at the Item.
To support that I added an ItemInstanceManager with
a simple interface (find/add/remove) and it's usage
in the two central Item-existance managing methods
implCreateItemEntry/implCleanupItemEntry. The
interface is pure virtual and all methods private,
only the mentioned managing methods are allowed to
access these. Also added a virtual method to
SfxPoolItem called getItemInstanceManager() that
can be implemented by Items that want to support
that.
Also added a default implementation of
ItemInstanceManager called DefaultItemInstanceManager
that uses linear search using operator== from the
Item that can be used/added to every Item easily.
It works for all Items and does in principle what
the former implementation does. It is intended as
simple/fast fallback.
I also added a statistic element to measure the most
used non-RefCounted Items on an Office-run, this
will be printed at office shutdown using SAL_LOG
and the 'svl.items' flag.
I then checked all Items that were used in this
error/bug scenario that used an extensive number
of incarnations and added an ItemInstanceManager
for these.
For SvxFontItem I added one that creates a hash and
thus needs not to search for instances at all, with
the caveat that the WhichID needs to be included.
Thus the hash is not at the Item, but only in the
ItemInstanceManager implementation.
For SfxBoolItem I implemented one that hashes using
the WhichID and holding both possible states in an
associated std::pair, true and false, thus the
SfxBoolItem is identified fast and only two instances
per WhichID exist (when used in Pool/Holder).
For 11 other Items I just added using the standard
implementation, DefaultItemInstanceManager. Of
course the more we optimize the better it will get.
For all Items where I added that mechanism I also
added ASSERT_CHANGE_REFCOUNTED_ITEM to all write
calls/methods for that Item. It asserts when the
RefCounted Item is to be changed. This should be
done in all write accesses to Items, but we have
ca. 500 derivations.
NOTE: There was *one* place I found where that
was already done -> was alredy seen as problem,
but not really adressed.
Despite this example file is not representative,
it is still a start to init this new instance
re-use of Items.
I am already thinking about doing that globally,
depending on the usage number (maybe combined with
sizeof(item)). There is no argument to not even
change strategy at runtime when a specific number
of incarnations of one Item derivation is reached,
but this is not part of this fix.
Change-Id: Ie56cf40e0341f98dcbf29cf5c06368316829415e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162402
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
An instance of SfxVoidItem(0) was used to signal
the SfxItemState::DISABLED. This was done not only
using WhichID == 0, but using isVoidItem() at the
SfxPoolItem. Unfortunately this mixes up with usages
of SfxVoidItems, mostly for UI stuff/Slots.
This also means that all the time an SfxVoidItem
had to be cloned/delete when when added/removed
from ItemSet or ItemHolder. Much more action than
e.g. for INVALID_POOL_ITEM which we already use
by havong just a simple ptr to a single static
instance of an Item. Disabled should do the same
thing.
Unfortunately also the functionality was mixed
with non-SfxItemState::DISABLED purposes and these
were very hard to be separated.
But the current solution works now after some
quirks doing that. It even oes no more need the
isVoidItem() flag at the SfxPoolItem.
Change-Id: I99f03db144f541ae4ea35f3775b3b3d58a375a81
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162414
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
After some experiments I now can remove
DirectPutItemInPool and DirectRemoveItemFromPool.
With the changes done before it is now possible to
get rid of this 'compromize'. Now there are no
more Items held at the 'Pool' which now can be
developed in the direction of ssth like
'SfxItemSupport' - what it really is.
Some of the last usages of DirectPutItemInPool were
in SvxAreaTabDialog::SavePalettes(), so I tried to
trigger those cases with using LO and calling that
Dialog in all situations I could possibly think
about, but it was never used.
Then I added asserts and run a UnitTests in the apps,
also no luck. Thus I would guess these are not used.
These put changed stuff from the Dialog 'directly' to
the Pool (what is not really supported and is a hint
for a 'compromize' that some functionality did not
find/want to use the right spot in the model to save
and hold that data). Thus it *would* be accessible
using the SurrogateMechanism at the Pool (which is
also a 'compromize', see some of my other commits),
but I also found no hints at places where that is
done.
Thus I decided to create this change and let's see
if that asserts ever get triggered in the builds
or tests.
Indeed did not trigger. I checked what other places
do which use SfxObjectShell::Current() when they get
no DocShell: Most avoid doing something, but none
puts stuff to the Pool, so I go one step further
and do what other places do: warn and return. Also
simplified SvxAreaTabDialog::SavePalettes() as then
feasible.
Change-Id: I1c351eac4ada26288a56a69d57008a09f2d19121
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162340
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
It is used in SC, DirectPut* in Pool, fetched using
Surrogates, all bad. Only to transport data over the
Pool, may not even need to be an Item.
Trying to solve/losen that gordian knot, looks good.
Is now a normal data holder class, could find a good
parent for it that the Dialog and the instances
setting up/using that Dialog can use.
Forgot to reset that data instance in one place, but
also checked in-between a version that still used the
Item to excluse that the Poolis the same, but the
ScTabViewShell does change. FOund an error with
SfxPoolItemHolder when reseting, also changed.
Change-Id: I1c99d675d1cc3d21205c3e2df78d4b52a696e7ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162313
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: I26d745efecc29fe28e32dfc6b6a2ebe3c3bfd2e7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162329
Tested-by: Jenkins
Reviewed-by: Taichi Haradaguchi <20001722@ymail.ne.jp>
|
|
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 ::Put methods at SfxItemSet had an extra WhichID
parameter that was not really documented, but I would
guess often asked why it exists: An extra WhichID, just
called 'nWhich' (which makes things NOT clearer). That
is 'strange' since the Item given to be put already
internally has a WhichID, so why a 2nd one?
If you were really interested and read all that code
(no, no comments on that anywhere) you might know
that this a kind of 'Target-WhichID' under which
the Item shall be put to the ItemSet. Since this
is unclear for most people it is even dangerous and
explains why so many code places just hand over the
WhichID requsted from the Item that already gets
handed over.
To make it short: I removed that. For the 19 places
where this was really needed I added a new method
besides ::Put called ::PutAsTargetWhich that takes that
extra WhichID (now called TargetWhich) and takes the
needed actions. These are quite some because that may
be combined with the bPassingOwnership flag, see new
SfxItemSet::PutImplAsTargetWhich method.
This makes usage of ItemSets/Items less dangerous. It
also simplifies and thus makes safer the central helpers
implCreateItemEntry/implCleanupItemEntry which have some
less cases to handle.
Debugged the failing UnitTests showed that there is
an incarnate Item != SfxVoidItem that causes problems.
I checked for errors in the change, but no luck.
Afterr some time I found out that a ::Clone
implementation caused the problem: These need to
also copy the WichID of the original, but the
SfxFrameItem failed to do so. This did not cause
problems in the former version because
implCreateItemEntry was designed to set a missing/
different WhichID.
I corrected that in SfxFrameItem, also removed not
needed costructor that caused that. Also added a
SAL_WARN and a correction in implCreateItemEntry.
I could have added an assert (did so for running
local UnitTests), but should be enough.
NOTE: When hunting for Items except SfxVoidItem
that get crerated using a WhichID '0' i learned
that this indeed happens: There are some (5) calls
to SfxRequest::SetReturnValue that incarnate an
SfxBoolItem with WhichID '0' (ZERO). This is not
good and I think about how to change that...
Change-Id: I9854a14cdc42d1cc19c7b9df65ce74147d680825
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162124
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: Ic65f4cff636d67d94cb0cafc4f75f3bb57190c99
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161976
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
Tested-by: Jenkins
|
|
Currently a Pool-Attribute (in SfxItemInfo), but should
be a SfxPoolItem property. Originally 'moved' from old
'poolable' used as hint. I identified needs more general
and moved it to where it belongs.
Also reworked SfxItemInfo to no longer have/support
single bollean flags, but a FlagVariable and defined
SFX_ITEMINFOFLAG_* entries to access these, that will
make future changes easier without having to change
all palces where these get defined over and over again.
Added CheckItemInfoFlag for gereral access to that flag
and e.g. NeedsSurrogateSupport to directly check for
the SFX_ITEMINFOFLAG_SUPPORT_SURROGATE flag as syntactical
sugar, that makes the intention clear.
Change-Id: I09c238c7c5b7f721b657d7b0a44dbc8d14e02528
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161982
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: I49ab7a48e74055264037d6930692fdae8be6d3ee
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162022
Tested-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
|