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>
The "final" was removed in commit
e06f2bb00137655dbf6f0a8c8c2fc555720f4d3f (tdf#142467 crash on calling
'getInfoHelper' in final class, 2021-05-26)
and commit
6cb1745c24 (tdf#142467: Update comments
with GCC bug ID, 2021-05-31)
due to a bug in gcc <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100797>.
Now it has been amost 3 years past and that gcc bug has long been fixed,
so it is time to restore the "final".
Change-Id: I7db9fe59209cfbae4c04bb3a91cd764db9a38d98
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163447
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.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>
...so recent LLVM 19 trunk libc++ in debug mode complained during
CppunitTest_chart2_export2 about
> ~/llvm/inst/bin/../include/c++/v1/__debug_utils/strict_weak_ordering_check.h:59: assertion __comp(*(__first + __a), *(__first + __b)) failed: Your comparator is not a valid strict-weak ordering
at
> 5 libsystem_c.dylib 0x0000000183279a40 abort + 180
> 6 libc++.1.0.dylib 0x00000001030f9d98 _ZNSt3__123__cxx_atomic_notify_oneEPVKv + 0
> 7 libchartcorelo.dylib 0x00000002f817f0ec _ZNSt3__135__check_strict_weak_ordering_sortedB8de190000INS_11__wrap_iterIPNS_6vectorIdNS_9allocatorIdEEEEEEN5chart12_GLOBAL__N_116lcl_LessXOfPointEEEvT_SB_RT0_ + 960
> 8 libchartcorelo.dylib 0x00000002f817e6cc _ZNSt3__118__stable_sort_implB8de190000INS_17_ClassicAlgPolicyENS_11__wrap_iterIPNS_6vectorIdNS_9allocatorIdEEEEEEN5chart12_GLOBAL__N_116lcl_LessXOfPointEEEvT0_SC_RT1_ + 268
> 9 libchartcorelo.dylib 0x00000002f8172a90 _ZNSt3__111stable_sortB8de190000INS_11__wrap_iterIPNS_6vectorIdNS_9allocatorIdEEEEEEN5chart12_GLOBAL__N_116lcl_LessXOfPointEEEvT_SB_T0_ + 68
> 10 libchartcorelo.dylib 0x00000002f8172820 _ZN5chart11VDataSeries15doSortByXValuesEv + 508
> 11 libchartcorelo.dylib 0x00000002f8064c44 _ZN5chart9AreaChart12createShapesEv + 1528
> 12 libchartcorelo.dylib 0x00000002f80f2ae0 _ZN5chart9ChartView28impl_createDiagramAndContentERKNS_18CreateShapeParam2DERKN3com3sun4star3awt4SizeE + 4440
> 13 libchartcorelo.dylib 0x00000002f80f77ac _ZN5chart9ChartView14createShapes2DERKN3com3sun4star3awt4SizeE + 2728
> 14 libchartcorelo.dylib 0x00000002f80f58ec _ZN5chart9ChartView12createShapesEv + 692
> 15 libchartcorelo.dylib 0x00000002f80f4598 _ZN5chart9ChartView15impl_updateViewEb + 288
But the introduced use of `std::strong_order(first[0], second[0]) < 0` then
triggered a false
> lo/core/chart2/source/view/main/VDataSeries.cxx:105:61: error: NullToMemberPointer ValueDependentIsNotNull ZeroLiteral -> nullptr [loplugin:nullptr]
> 105 | return std::strong_order(first[0], second[0]) < 0;
> | ^
so needed some hack in loplugin:nullptr.
And old versions of libc++, still used at least on Android, do not have any
implementations of std::strong_order. So detect those cases in configure.ac
(checking for std::strong_order for double, which is what is actually being used
in the code) and fall back to operator <=> for now, even if that will not
provide a strict weak ordering and will thus continue to violate the
requirements of std::sort.
And then our venerable clang-format 5.0.0 would have broken the token `<=>` into
`<= >`, so exclude include/o3tl/compare.hxx from its mis-treatment.
Change-Id: I7a64a630eb5f560dce59f3ff9d51ca3d1adc70be
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163075
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <stephan.bergmann@allotropia.de>
Updated the chart properties dialogs to work asynchronously. The changes affect the following dialogs:
Chart Type: Accessible by double-clicking on a chart to select it, then right-clicking to open the context menu. Selecting 'Chart type...' opens the dialog for modifying chart types.
Insert Axes: Triggered by double-clicking on a chart to select it, then right-clicking and choosing 'Insert/Delete Axes' from the context menu. This dialog allows users to add or remove chart axes.
Insert Titles: Opens when you double-click on a chart, right-click, and select 'Insert Titles...' from the context menu. It's used for adding titles to the chart and its axes.
Insert Trendline: For column charts, you can double-click on a column, right-click, and choose 'Insert trendline..' to open this dialog.
Insert Error Bars: In a column chart, after selecting a column, right-clicking and choosing 'Insert X Error Bars..' or 'Insert Y Error Bars..' opens the dialog for adding error bars to the chart.
Signed-off-by: codewithvk <vivek.javiya@collabora.com>
Change-Id: Id7bb8682150f2e3115420d9259c6afd41682641e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162655
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
(cherry picked from commit 71a3cfb71dddba8098d74d7bb1dd414e4696914b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162824
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
This is a followup to commit 85f4395b6f.
Although there is no pressing need for this patch in my opinion,
it DOES fix a 7.1 regression in importing MSO charts with long labels.
MSO wraps text at 1/5 the width of the chart.
7.1 Regression commit 75a8b367f2
Author: Balazs Varga on Tue Aug 25 12:32:02 2020 +0200
tdf#136105 tdf#134883 pie chart: improve data label position
Before Balazs' commit, the text width for everything was simply
fTextMaximumFrameWidth = 0.8 * fPieRadius.
I personally think Balazs' no wrapping looks better
(for outside labels, when there is enough space)
but in order to be consistent with how we handle
wrapping for bestFit-that-didn't-fit labels,
and to have our charts be as interoperable with OOXML as possible,
it makes good sense to use the same logic as the previous patch here.
Interestingly, Balazs broke some unit tests that specifically
were testing to make sure that text wrapping existed.
Fixed: // text wrap: wrap all text labels except Yellow one
make CppunitTest_chart2_xshape CPPUNIT_TEST_NAME=testPieChartLabels2
Fixed: // text wrap: wrap no text label except Yellow one
make CppunitTest_chart2_xshape CPPUNIT_TEST_NAME=testPieChartLabels3
Interestingly, I couldn't just copy/paste Ctrl-F12 dump to fix
make CppunitTest_chart2_xshape CPPUNIT_TEST_NAME=testPieChartLabels4
so I instead did a copy/paste of SAL_WARN("DUH",getXShapeDumpString());
Change-Id: I19f2ce2ce9c7653ae92dd596f0aaca1ed83f41bb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162764
Tested-by: Justin Luth <jluth@mail.com>
Reviewed-by: Justin Luth <jluth@mail.com>
Fixes a 7.2 regression from
commit b006834239
author Tünde Tóth on Wed Dec 16 18:26:26 2020 +0100
tdf#138777 pie chart: improve long data label width
When a label fails to bestFit inside the pie slice,
it will be placed outside of the pie of course.
However, we can't assume that there is any chart space
available to place a label outside.
Tünde got that part right. He limited the space available
based on the chart edge. But there are some optimizations
that can improve that.
1.) Every little bit can help. As we go away from the
X-axis, we gain a little bit of space, so use that...
2.) Don't assume that the pie chart is in the middle of the page.
3.) Use a consistent algorithm for all degrees - much simpler.
make CppunitTest_chart2_import CPPUNIT_TEST_NAME=testTdf146756
Change-Id: I0d8528bc227768f91237cda6b74bf9365820bfa7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162704
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
... and use a compatible 1/5 width if not specified.
This patch depends on the previous oox patch
(commit 301e27cbeb)
which actually reads the width into the model.
Fixes a 7.2 regression from
commit b006834239
author Tünde Tóth on Wed Dec 16 18:26:26 2020 +0100
tdf#138777 pie chart: improve long data label width
and is basically a re-write of 7.1's
commit 20da1a5dd3
author Tünde Tóth <toth.tunde@nisz.hu> on Fri Oct 09 09:24:18 2020 +0200
tdf#134978 Chart OOXML Import: fix pie chart label custom position
This is very risky, but then ANYTHING changing chart2 is risky.
There were a lot of changes made in 7.1,
and they all invited regressions.
However, our chart implementation is not in a good state,
and certainly is not very interoperable,
so it is worth taking the risk.
Anything dealing with manualLayout at this point
should have originated as a pptx,
so forcing a compatible max width should be fairly safe.
It probably isn't actually all that risky after all.
largely copied code from
commit 4223ff2be6
Author: Balazs Varga on Wed Jan 15 16:31:35 2020 +0100
tdf#48436 Chart: add CustomLabelPosition UNO API property
Fortunately this all goes away after a round-trip
since custom label placement is lost on export to OOXML,
and that really helps to reduce the risk.
make CppunitTest_chart2_import CPPUNIT_TEST_NAME=testTdf146487
Change-Id: I9722fc6c759c15ac3924780e6fc124f02fba07e1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162590
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
In various places, the degrees refer to an area/width,
and in those cases 360 means everything, so mk correctly
removed NormAngle for those situations.
But now copy/paste has added that to areas where a simple
angle is being described, and in those cases
using NormAngle360 is better.
Change-Id: I4aa722c4a635f862dae5405f2ee929215a42faf9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162585
Reviewed-by: Justin Luth <jluth@mail.com>
Tested-by: Jenkins
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>
Yeah, it took me a while to clue in that this is no longer
a circle, so we can't apply circle logic to it any more.
Change-Id: I00f8b95cc75311342b9abd8c522ed06ca343a566
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162630
Reviewed-by: Justin Luth <jluth@mail.com>
Tested-by: Jenkins
Caolán McNamara