| Age | Commit message (Collapse) | Author |
|---|
|
based on the commit message from 7186541219599e1b51ad35601c2cd015a329f360
"Resolves: tdf#131562 decimal separator may not be surrounded by blanks"
Change-Id: I19c2a687663304003566e9d93504f0baf33f1d83
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169111
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
|
|
Change-Id: Ia57006f88c8d0d0658178edb4d645b9d63de37c7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169089
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
|
|
Number format with no integer digit, such as "+.##;-.##"
was placing string with the decimal separator.
This change insert string at the begining of the number string as
expected.
This format code is saved as "+#.##;-#.##" in ODF, but display is the
same; so no change is made on this side.
Add unit test for this format
Change-Id: I74fbe0e9a5303672ac7927d37922c06a762feba6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/168577
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
|
|
With all the changes done for Items we can now do deeper
basic changes to the ItemSet itself with manageable risk.
I already did https://gerrit.libreoffice.org/c/core/+/166455
aka 'ITEM: Add measurements for SfxItemSet usages' to
get some statistical information about the fill/usage grade
of the ItemSet's fixed PtrArray to SfxPoolItems, check
that out to get an own picture.
Those results show that an average usage is between some
extremes ranging from 0% to 50%, but when using more checks
and using multiple files/interactions/edits in all
applications we end up with around typical 12%-19% of that
array used, the rest is nullptr's.
Thus I thought about one of the initial ideas of this
series of changes (ITEM), to use a std::unordered_map
(A) instead of that fixed array of SfxPoolItem Ptr's (B).
Tthat again was for a complete type-based rewrite, which
I once did a POC, but the code cannot be adapted to that,
just too much work.
Those are very different in architecture, (B) is done
since a long time (since ever), but as pointed out above,
(A) is now possible. There are many aspects to it, let's
grep some:
Speed (iterate): (A) and (B) are both linear. (A) has
less entries, but may touch different mem areas
(buckets). (B) is linear, but many empty spaces which
are usually uselessly iterated.
Speed (access Item by WhichID): (A) is hashed by
WhichID, so mostly linear for unordered_set/hash.
(B) is in a linear array, but has to calculate the
offset for each WhichID access.
So I guess speed will mostly equal out.
Memory: (A) will be dynamically allocated (buckets),
but stl is highly optimized and may even re-use areas,
has to provide some extra info but will need less
places for Items since it's dynamic and can start empty.
(B) will be allocated once (except AllItemSet) and may
even be 'derived' to the ItemSet (SfxItemSetFixed), but
has to allocate all space at once.
I can go in lots of more detail here, but due to the
results of the statistics I just made a test now,
including measuring some results (will include in
gerrit, not here). I used two pro versions for that.
That way I have some data now.
Result is:
- It is now possible to do this, it runs stable :-)
- Speed: As expected, mostly no change
- Memory: Depending on usage, 0% to 25% less,
usually around 8-10%
Side effects:
- SfxAllItemSet could be done without WhichRanges,
thus without expensive 'merges'
- SfxItemSetFixed is not needed. While the idea is
good, it needs a lot of extra stuff
- Access to Items is linear if set
- WhichRanges: Still needed, but for vaildity
checking/filtering of ItemSet content
- WhichRanges: Worth to think about if these are
needed at all, probably just exist for historical
reasons since allocation/number of added Items
was never ever dynamic -> just not allocatable
Putting the current version on gerrit, may still
trigger some UTs (checked SW/SC/SD...)
I did not like that functionality at ItemSet that
hands out a vector of the set items for cases where
to avoid iterating and deleting items at the same
time at an ItemSet, so changed to using a local
vector of remembered WhichIDs and deleting after
the iterator is done. I also saw some strange
usages of SfxItemIter in sw which i will have to
check.
Since there are still problems with UTs which I can
not reproduce locally I have now added asserts to
the case that an ItemSet gets changed with still
having active SfxItemIter(s). That is always an
error, but with the architecture of that fixed
array of ItemPtrs did not have devastating effects
(purely by coincidence).
With that asserts, UTs run well in SC and SD, but
in SW I get 11 (eleven!) asserts from the UTs, all
of them from 'ITEM: SfxItemSet ClearItem' BTW.
I guess these have to be fixed before thinking
about this change...
Good news is that all 11 cases were the same in SW,
in SwHistorySetAttrSet::SwHistorySetAttrSet which
does some strange things using two SfxItemIter in
parallel. Thus SW UTs are also clear and I see no
more errors caused by ItemSets being changed while
SfxItemIters are alive.
Bad news is that I still have errors to hunt...
NOTE: Have now cleaned all UTs, this showed that
there are some unexpected side-effects of the Items
being processed in another order when SfxItemIter
is used, also found one case where a
WhichRangesContainer is constructed for a
SfxItemSet using the set items from another ItemSet
and SfxItemIter to do so. There *might* be more
cases not covered by UTs.
NOTE: While speed stays the same and mem is reduced
up to 25% (see measurements in 1st comment) another
*important* aspect is that this frees the way for
using ItemSets *without* WhichRanges - these are
necessary mainly to create that fixed array of
pointers to the Items in a *manageable* size. With
a dynamic structure like unordered_set there is
in principle *no need* anymore to use WhichRanges
to pre-define the Items a Set could hold. There
is one exception: We have cases where one ItemSet
is set at another one with defined WhichRanges to
*filter* the contained Items - these would have to
be identified. This is a rare case and we would have
to check cases where an ItemSet gets set at another
ItemSet. This would be as if all ItemSets would be
AllItemSets in principle - much easier for everyone.
NOTE: Waited for 24.8 split just to not take
unnecessary risks.
Change-Id: I75b0ac1d8a4495a3ee93c1117bdf618702785990
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166972
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
Tested-by: Jenkins
|
|
Change-Id: Idae670a53d6d9aab0ec7132077f3e7b7f6fa5287
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167595
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
I had done these a while ago, when I looked into extending loplugin:ostr to do
more automatic rewriting, and these were places where I needed to do something
manually, for one reason or another, because the automatic rewriting would not
pick it up correctly.
However, I got distracted, and a wholesale automatic rewrite would still run
into cases where an _ostr/_ustr instance from a library's .rodata would still be
referenced after the library has already been dlcose'd. So I never came around
to finishing all that.
But there appears to be renewed interest in (automatic) rewritings here now, so
it probably makes sense if I share this part of my work anyway.
Change-Id: I3da9d38398e4bca373cb0000a9d34b49a36ad58a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166792
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
Tested-by: Jenkins
|
|
Some of the exclusions were too aggressive. Restrict them to only the
important classes, which exposes some more places this plugin applies.
Change-Id: I1b2d1fb24391adc71ed0984f94168f61a149479f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165154
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
After commit 9eb9083ff2fdaeb96399a0830a4394de4e29ef64 (Use Dragonbox
to implement doubleTo*String*, 2022-02-18), the rounding that is used
in SvNumberFormatter became strictly more correct; however, it now
differed from what ROUND spreadsheet function returned, because the
latter uses rtl_math_round, which calls rtl::math::approxFloor.
To make the visual number representation consistent, this change uses
rtl_math_round in SvNumberformat::ImpGetNumberOutput.
Change-Id: I05b0bed7d3a6c73584a77adbae2835c95be249fa
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/165142
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
|
|
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>
|
|
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>
|
|
...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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
which flushes out the fact that, previously, when
SfxItemSetFixed was being copied, we were not
actually taking advantage of the "internal" memory,
and were actually allocating a separate block of memory,
like a "normal" SfxItemSet.
Change-Id: I6a8073c58b464d53bfd2a54cf1dd27a3f2cb3df7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160377
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Removed some special stuff in the ItemSet/Item tooling regarding
ScPatternAttr. There are still quite a view (grep for
isExceptionalSCItem), but all these are identified and isolated.
Only for that Item (of over 500) are these exceptions, so that raelly
does not fit into the Item schemata in any way. Not even the pool
default of ScPatternAttr is without trouble. It is the only and last
Item that needs to be 'pooled' in the sense to avoid copies on any
price. That is OK, but should not be done in the ItemSet schemata.
In short: It should not be an Item at all. It should be changed
and renamed to something describing it's role (CellAttributes..?)
and get helped/assisted by something called CellAttributeHelper at
SC's model or Pool. It should not even be derived from SetItem, it
could just contain an shared_ptr of SfxItemSet (allows more and better
optimizations - think about Clone() and op==).
In parallel, all these hacks in the ItemSet/Item stuff could be
removed, making that faster and easier. Also quite some usages of
DirectPutItemInPool could be cleaned-up, this only exists since
there *is* no defined place to hold that data (coud be
CellAttributeHelper in the future). Putting Items directly to the
pool (and in most cases never remove them again) is just nonsense
and another hint that all this does not fit to the Item/ItemSet
schema at all.
This is now - after hard isolation of problems and have all working -
doable. It may be one of the next things to do, but there are
other candidates, too. Doing this would mostly only help SC...
Found another hack that uses DirectPutItemInPool and *never* removes
any Item again, see comments framelinkarray.cxx
And another one: PoolItemTest::testPool() explicitely tests
DirectPutItemInPool stuff - which makes no sense long-term,
but for now keep it and make it work by marking the slots used
as _bNeedsPoolRegistration == true
Have now overhauled the framelinkarray stuff to work without
DirectPutItemInPool and Cell not being a SfxPoolItem. That will
be much less complex and use much less calls/checks. Since this
is the data structure created for every calc repaint that should
get faster, too.
Also for now and memory loss security I added code to
DirectPutItemInPool to behave the same as with the unchanged
implCreateItemEntry: register Items so that the garbage collection
still is used. This will/can be removed when all usages of
DirectPutItemInPool is cleaned up.
Directly registering in DirectPutItemInPoolImpl is more tricky
than thought, but a good thing to do. Looking at that I saw that
tryRegisterSfxPoolItem is not used anymore, so rearranged some stuff.
Change-Id: If07762b0a25e2739027f81872441772dc82a25d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159685
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
Change-Id: Ia74b15213a05da36f48932811d70d231ec7ee164
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/159673
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
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>
|
|
Exponent in scientific number may use '?' as blank like in format "0.00E+?0"
This change:
- adds interpreatation of '0' and '?' in exponent
- adds "blank-exponent-digits" attribute to scientific number for import
and export to ODF
- prevents using exponent with only '?'. There must be at least one '0'
in exponent
- adds QA test of such format and test import/export/import to ODF and OOXML
- corrects one basic test
Change-Id: If52edc632a161f842270bb2fd77af535e2b978d4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154986
Tested-by: Jenkins
Reviewed-by: Laurent Balland <laurent.balland@mailo.fr>
|
|
Change-Id: I31d46c2b75888474136ecd630fd3f817db189fb4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158223
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
...now that warning about O[U]String vars that could be O[U]StringLiteral is no
longer useful
Change-Id: I389e72038171f28482049b41f6224257dd11f452
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/157992
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
Change-Id: I2d09b2b83e1b50493ec88d0b2c323a83c0c86395
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/157647
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Tested-by: Jenkins
|
|
This change is not about speed improvements but diverse
preparations to make changes/reading/understanding easier.
It does not change speed AFAIK.
Added a global static debug-only counter to allow getting
an overview over number of all allocated SfxPoolItem's
and the still alloated ones at office shutdown. The values
are used in Application::~Application to make a short info
statement. It allows to be able to quickly detect if an
error in future changes may lead to memory losses - these
would show in dramaitically higher numbers then (hopefully)
immediately.
Moved SfxVoidItem to own source/header.
Added container library interface support to SfxItemSet,
adapted already some methods to use it - not all possible,
I will commit & get status from gerrit 1st if all still works
and then continue.
Changed INVALID_POOL_ITEM from -1 to use a global unique
incarnation of an isolated derivation from SfxPoolItem. It
allows to avoid the (-1) pointer hack. Since still just
pointers are compared it's not worse. NOTE: That way, more
'special' SfxPoolItem's may be used for more States - a
candidate is e.g. SfxVoidItem(0) which represents ::DISABLED
state -- unfortunately not only, it is also used (mainly for
UI stuff) with 'real' WhichIDs - hard to sort out, will have
to stay that way for now AFAIK.
Changed INVALID_POOL_ITEM stuff to use a static extern
incarnated item in combination with a inline method
to return it, called GetGlobalStaticInvalidItemInstance().
Isolated create/cleanup of a SfxPoolItem entry in
SfxItemSet to further modularize/simplify that. It is
currently from constructor & destructor but already shows
that PoolDefaults are handled differently - probably an
error. Still, for now, do no change in behaviour (yet).
Got regular 'killed by the Kill-Wrapper' messages from
gerrit, seems to have to do with UITest_sw_findReplace.
That python/c++ scripting stuff is hard to debug, but
finally I identified the problem has to do with
the INVALID_POOL_ITEM change. It was in
SfxItemSet::InvalidateAllItems() where still a (-1)
was used -> chaos in detecting invalid items.
Change-Id: I595e1f25ab660c35c4f2d19c233d1dfadfe25214
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/155675
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <Armin.Le.Grand@me.com>
|
|
that can be initialised at compile-time instead of runtime
Change-Id: I08d516fdc13a3a79f93c079f89ac44cbc7a1ed71
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153620
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
A bunch of range based loop changes in various qa
sections that also take out about 1% of SAL_N_ELEMENTS
Change-Id: I8ef000e9aa400cd8363b48f6175f6ab258cefbd9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152422
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
|
|
Change-Id: I99ef22d0062b3e9573abd3c60078e72472681c32
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152339
Tested-by: Jenkins
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Only decimal separator after S or SS was treated
This change adds [S] and [SS] to treat formats like
[SS].00
Update: correct export to XML
Add QA unit tests
Change-Id: I97ce4084d3caab2fcd18f1c70cd4221596290563
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151823
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
|
|
Change-Id: If92643e3e8aad5a3367b9ee20afa5a1fef7ee8ca
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151317
Tested-by: Julien Nabet <serval2412@yahoo.fr>
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
|
|
Sometimes, the first character of the doi string is auto capitalized, which isn't recognized as DOI.
Now, the doi detection is able to recognize doi string with the first character capitalized,
like what is done in url recognition.
Change-Id: I95334941dc4cda3095f1750fea927640dea55e23
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151142
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
Detect DOI string in the form of "doi:10.*" and add hyperlink to it.
It works the same way as url recognition.
Change-Id: I3c4e78a110fd81ad7e727d5e9acee7e51127466a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150954
Tested-by: Jenkins
Reviewed-by: Heiko Tietze <heiko.tietze@documentfoundation.org>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
If round value is an integer and there is no integer part in the number
format, then nDiv was wrongly forced to 0.
Add corresponding unit tests.
Change-Id: Ib69393eca8f6c2bdda0eacfc83637ab0c971ff2d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149118
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
|
|
when applying my upcoming patch to also consider O[U]StringBuffer
Change-Id: Id8f229c3a5223dee8d2710caf15d4612594fc763
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/149748
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
Change-Id: I707f1a5402189a2453f5d11beb4e1c493699e28b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148909
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
|
|
Change-Id: Ic2990ebc2e4a9a36dcd3f90c5f634ca7dd225d52
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147491
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
it's already defined in include/tools/color.hxx
Change-Id: I26eaba4a1279fadd8669e9702b695e02871052d9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142512
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
|
|
Without this, it may not actually be there, so interning "" would
use a different string instance, and then comparing with
SharedString::getEmptyString() would actually compare non-equal.
Change-Id: I22660f63aa321e3a8f72cfb96df1db56e08fbb84
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140402
Tested-by: Jenkins
Reviewed-by: Eike Rathke <erack@redhat.com>
|
|
Change-Id: Iae7facef72ad17b29b49ea5b52aab77c16357da8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/137031
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
|
|
There is this number format:
<number:date-style style:name="N36" number:automatic-order="true">
<number:day number:style="long"/>
<number:text>.</number:text>
<number:month number:style="long"/>
<number:text>.</number:text>
<number:year number:style="long"/>
</number:date-style>
in a paragraph which has fo:language="zxx", so the field has
LANGUAGE_NONE.
MSWordExportBase::GetNumberFormat() exports as: DATE \@"dd/MM/yyyy"
But should be: DATE \@"dd.MM.yyyy"
Follow Eike's suggestion to use the number format's language in case the
field doesn't have one.
Change-Id: I596bea5daa75c717931b3c5d5506103b87b8ee08
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134638
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
|
|
Change-Id: Iefa570476bf0c881e36679ae9511ff63162e05d6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/133771
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
and once we remove that self mapping, no need to call GetWhich for those
IDs either
Change-Id: Ia881328a29bb022dace8d5f25c57279a381e0377
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129321
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
The task presents an URL that ends with a closing bracket. If
pasted to LO, the closing bracket got interpreted as not being part
of the URL due to the heuristical interpretation of URLs in
urihelper.
Adapted this to handle matching brackets, so that an closing and
ending bracket will be added to the uri text when there is a
matching pair.
Added unit test to testFindFirstURLInText with simplified uri
example.
Change-Id: I58dd460a37d0066ff46845832eabd2a790e4ccd1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126832
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
...during CppunitTest_svl_qa_cppunit, after
df42cb6552b20372f62b5a361709670db80e4ed4 "Optimize assignment from
OUStringLiteral to OUString",
> ==918==ERROR: AddressSanitizer: stack-use-after-scope on address 0x2b2e203b5900 at pc 0x2b2e1b9004f8 bp 0x7ffc06726270 sp 0x7ffc06726268
> READ of size 4 at 0x2b2e203b5900 thread T0
> #0 0x2b2e1b9004f7 in void rtl::str::release<_rtl_uString>(_rtl_uString*) /sal/rtl/strtmpl.hxx:1064:9
> #1 0x2b2e1b8d916c in rtl_uString_release /sal/rtl/ustring.cxx:1785:12
> #2 0x2b2e36263ec5 in rtl::OUString::~OUString() /include/rtl/ustring.hxx:493:9
> #3 0x2b2e3622c5ff in (anonymous namespace)::Test::testTdf103060() /svl/qa/unit/svl.cxx:553:1
[...]
> [2304, 2320) 'EXPECTED_G3' (line 550) <== Memory access at offset 2304 is inside this variable
(<https://ci.libreoffice.org/job/lo_ubsan/2176/>).
(aa2064c5c5f23f6f4b7bc44e12345b37f66995bc "Improve loplugin:stringliteralvar"
had failed to introduce those OUStringLiteral variables as static.)
Change-Id: I59168979fcc4b055d17d1d4f315577eef1027505
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124134
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
|
|
... to avoid hidden cost of multiple COW checks, because they
call getArray() internally.
This obsoletes [loplugin:sequenceloop].
Also rename toNonConstRange to asNonConstRange, to reflect that
the result is a view of the sequence, not an independent object.
TODO: also drop non-const operator[], but introduce operator[]
in SequenceRange.
Change-Id: Idd5fd7a3400fe65274d2a6343025e2ef8911635d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123518
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
|
|
Change-Id: I7a62f7c5c8e6fceebcb9671fa28ec98dd7a312a7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122878
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
|
|
NF_DATETIME_ISO_YYYYMMDD_HHMMSS000
YYYY-MM-DD HH:MM:SS.000
Users may expect to see that if they enter such, instead of the
real ISO 8601 "T" format.
Change-Id: Iad81750d1c74eedd8d4360163b29ecac98ef6824
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122502
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
|
|
NF_DATETIME_ISO_YYYYMMDDTHHMMSS000
YYYY-MM-DD"T"HH:MM:SS,000
using either ',' or '.' separator,
'.' if Time100SecSep is '.'
',' else
A prerequisite for tdf#88359 to not lose data when importing such,
especially without "Detect special numbers" on.
Change-Id: I02ab682636e6ddbcc4537183a3625ea61662f016
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122400
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
|
|
we already have GetCharClass and we never return a nullptr
Change-Id: I3cb79bc60be614c0474ecfdaad17991f2ecb6368
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122208
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
|
so I can make changes without running into cyclic dependencies
between header files
Change-Id: I98a91c7cc66002ba745cdb8239e5cc267922a45c
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120412
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
|
