summaryrefslogtreecommitdiff
path: root/svl
AgeCommit message (Collapse)Author
2024-03-14Related: tdf#160056 pass 'StarFormat' flag in instead of changing stateCaolán McNamara
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>
2024-03-13These methods can be const tooCaolán McNamara
Change-Id: Iaef46216dac6584f57b7933d658384f54d0a4544 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164772 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-03-13SvNumberFormatter::GetUserDefColor can be constCaolán McNamara
Change-Id: If499e28e5ac69018b35b475a73ecb2dc4b78dad6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164786 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-03-13SvNumberformat::ImpGetLogicalOutput can be constCaolán McNamara
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>
2024-03-13SvNumberformat::ImpGetFractionOfSecondString can be constCaolán McNamara
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>
2024-03-13ImpIsEntry can be constCaolán McNamara
Change-Id: Id229344a68925a1bde84f2b4aad46cfc5f01b797 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164769 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-03-11tdf#114441 use sal_uInt32 instead of sal_uLongKeldin Maldonado (KNM)
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
2024-03-08tdf#158773 reduce size of IndexedStyleSheetsNoel Grandin
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>
2024-03-08tdf#158773 reduce cost of stylesheet lookupNoel Grandin
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>
2024-03-08tdf#158773 GetNumberOfStyleSheets can be an inline methodNoel Grandin
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>
2024-03-08tdf#158773 flatten data of IndexedStyleSheetsNoel Grandin
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>
2024-03-08use more string_viewNoel Grandin
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>
2024-03-07tdf#158773 reduce time spent in IndexedStyleSheets::ReindexNoel Grandin
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>
2024-03-07tdf#158773 reduce dynamic_cast'ing in TextProperties::NotifyNoel Grandin
Change-Id: If4a68433c57fdf3da56891fa0b4be6f8a991d929 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164528 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-03-07ResetDefaultSystemCurrency can be privateCaolán McNamara
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>
2024-03-07IsSpecialStandardFormat can be private, only used internallyCaolán McNamara
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>
2024-03-01scope of MutexGuard can be reducedCaolán McNamara
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>
2024-02-29use member init listCaolán McNamara
Change-Id: I09dea90e3e3f3fd0a4047b989329a027f788f695 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164148 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-02-29if we rearrange, we don't need to create maLanguageTag twiceCaolán McNamara
Change-Id: I2c8ad9999adc406dc850c59b48e49681099dc054 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/164147 Tested-by: Jenkins Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
2024-02-29SvNumberFormatter::ImpConstruct is only used once by the single ctorCaolán McNamara
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>
2024-02-24tdf#159862: set SearchWildcard to false changes SearchRegularExpression valueJulien Nabet
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
2024-02-19ITEM: Speedup SlotIDToWhichID translationsArmin Le Grand (allotropia)
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>
2024-02-18ITEM: Rename for more control over SlotID usagesArmin Le Grand (allotropia)
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>
2024-02-17tdf#158890 Replace '?' with figure blankLaurent Balland
...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>
2024-02-16ITEM: Better test for DynamicDefaultItemsArmin Le Grand (allotropia)
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>
2024-02-13ITEM: corrected error for SfxSetItemArmin Le Grand (allotropia)
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>
2024-02-12ITEM: ItemPool Rework (I)Armin Le Grand (allotropia)
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>
2024-02-01check that rtl_random_getBytes() was successfulMichael Stahl
... 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>
2024-01-27ITEM: Cleanup some Pool stuff with DefaultsArmin Le Grand (allotropia)
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>
2024-01-27tdf#159381 TimeStamp(RFC3161) create problem by asn1 format error.Noel Grandin
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>
2024-01-27Drop std::as_const from css::uno::Sequence iterationsMike Kaganski
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25Fix typoAndrea Gelmini
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>
2024-01-25ITEM: Slight re-design of global Item-ReusageArmin Le Grand (allotropia)
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>
2024-01-24ITEM: Add some default global sharingArmin Le Grand (allotropia)
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>
2024-01-24add #include unordered_mapCaolán McNamara
Change-Id: Ieaba8db5df426ed30aca7366a86ee4fb11e03a44 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162480 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
2024-01-23tdf#158605 Add global SfxPoolItem re-useArmin Le Grand (allotropia)
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>
2024-01-23ITEM: Solve SfxVoidItem(0) situationArmin Le Grand (allotropia)
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>
2024-01-21ITEM: Remove Direct(Put|Remove)Item(In|From)PoolArmin Le Grand (allotropia)
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>
2024-01-21ITEM: solve ScCondFormatDlgItem situationArmin Le Grand (allotropia)
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>
2024-01-20Fix typoAndrea Gelmini
Change-Id: I26d745efecc29fe28e32dfc6b6a2ebe3c3bfd2e7 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162329 Tested-by: Jenkins Reviewed-by: Taichi Haradaguchi <20001722@ymail.ne.jp>
2024-01-19ITEM: Needed reworks on ItemSurrogate mechanismArmin Le Grand (allotropia)
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>
2024-01-17ITEM: Remove suspicious extra-Which in ::PutArmin Le Grand (allotropia)
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>
2024-01-15Fix typoAndrea Gelmini
Change-Id: Ic65f4cff636d67d94cb0cafc4f75f3bb57190c99 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161976 Reviewed-by: Julien Nabet <serval2412@yahoo.fr> Tested-by: Jenkins
2024-01-14ITEM: Move Shareable ItemFlag to SfxPolItemArmin Le Grand (allotropia)
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>
2024-01-13cid#1585309 Dereference before null checkCaolán McNamara
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>