chart2/qa/extras/chart2import.cxx | 23 ++++++++++
chart2/qa/extras/chart2import2.cxx | 4 -
chart2/qa/extras/data/pptx/tdf146756_bestFit.pptx |binary
chart2/qa/extras/xshape/data/reference/tdf90839-1.xml | 2
chart2/source/view/charttypes/PieChart.cxx | 38 ++++++++++++++----
5 files changed, 56 insertions(+), 11 deletions(-)
New commits:
commit 85f4395b6f40c0295a190cca09ecd51858fc3b31
Author: Justin Luth <justin.l...@collabora.com>
AuthorDate: Mon Jan 29 10:20:16 2024 -0500
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Tue Jan 30 10:42:48 2024 +0100
tdf#146756 pie chart2 import: improve response to bestFit failure
Fixes a 7.2 regression from
commit b0068342398786ca50304260434a18880dddf74d
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 <jl...@mail.com>
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/chart2/qa/extras/chart2import.cxx
b/chart2/qa/extras/chart2import.cxx
index b23e048c6949..8b5834df94c6 100644
--- a/chart2/qa/extras/chart2import.cxx
+++ b/chart2/qa/extras/chart2import.cxx
@@ -2009,6 +2009,29 @@ CPPUNIT_TEST_FIXTURE(Chart2ImportTest, testTdf146487)
CPPUNIT_ASSERT_EQUAL(OUString("371"), getXPath(pXmlDoc, aPath,
"sizeY"_ostr));
}
+CPPUNIT_TEST_FIXTURE(Chart2ImportTest, testTdf146756)
+{
+ // FIXME: the DPI check should be removed when either (1) the test is
fixed to work with
+ // non-default DPI; or (2) unit tests on Windows are made to use svp VCL
plugin.
+ if (!IsDefaultDPI())
+ return;
+
+ // given a chart on page 2
+ loadFromFile(u"pptx/tdf146756_bestFit.pptx");
+ Reference<chart::XChartDocument> xChartDoc = getChartDocFromDrawImpress(1,
0);
+ OString aXmlDump = OUStringToOString(getShapeDump(xChartDoc),
RTL_TEXTENCODING_UTF8);
+ xmlDocUniquePtr pXmlDoc(xmlParseDoc(reinterpret_cast<const
xmlChar*>(aXmlDump.getStr())));
+ OString aPath("//XShape[@text='New service request and approval;
18%']"_ostr);
+ assertXPath(pXmlDoc, aPath, 1);
+ // Expected something like 4 lines tall(1697), not 11 lines(3817).
+ CPPUNIT_ASSERT_EQUAL(OUString("1697"), getXPath(pXmlDoc, aPath,
"sizeY"_ostr));
+ // Expected some reasonable maximum text length for the label like 2350,
not 881.
+ sal_Int32 nTextLength = getXPath(pXmlDoc, aPath,
"textMaximumFrameWidth"_ostr).toInt32();
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(2350), nTextLength);
+ // MSO doesn't allow much more than 1/5 of the total chart width, so never
go higher than that
+ CPPUNIT_ASSERT_LESS(sal_Int32(2370.6), nTextLength);
+}
+
CPPUNIT_TEST_FIXTURE(Chart2ImportTest, testFixedSizeBarChartVeryLongLabel)
{
// Bar chart area size is fixed (not automatic) so we can't resize
diff --git a/chart2/qa/extras/chart2import2.cxx
b/chart2/qa/extras/chart2import2.cxx
index 863d425ae9a5..dc9a2cddae46 100644
--- a/chart2/qa/extras/chart2import2.cxx
+++ b/chart2/qa/extras/chart2import2.cxx
@@ -513,8 +513,8 @@ CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testTdf133376)
CPPUNIT_ASSERT(xDataPointLabel.is());
// Check the position of the 3rd data point label, which is out from the
pie slice
awt::Point aLabelPosition = xDataPointLabel->getPosition();
- CPPUNIT_ASSERT_DOUBLES_EQUAL(1208, aLabelPosition.X, 30);
- CPPUNIT_ASSERT_DOUBLES_EQUAL(5370, aLabelPosition.Y, 30);
+ CPPUNIT_ASSERT_DOUBLES_EQUAL(1624, aLabelPosition.X, 30);
+ CPPUNIT_ASSERT_DOUBLES_EQUAL(5635, aLabelPosition.Y, 30);
}
CPPUNIT_TEST_FIXTURE(Chart2ImportTest2, testTdf134225)
diff --git a/chart2/qa/extras/data/pptx/tdf146756_bestFit.pptx
b/chart2/qa/extras/data/pptx/tdf146756_bestFit.pptx
new file mode 100644
index 000000000000..628491079a52
Binary files /dev/null and b/chart2/qa/extras/data/pptx/tdf146756_bestFit.pptx
differ
diff --git a/chart2/qa/extras/xshape/data/reference/tdf90839-1.xml
b/chart2/qa/extras/xshape/data/reference/tdf90839-1.xml
index 222e595157fd..94258a1b351d 100644
--- a/chart2/qa/extras/xshape/data/reference/tdf90839-1.xml
+++ b/chart2/qa/extras/xshape/data/reference/tdf90839-1.xml
@@ -229,7 +229,7 @@
</XShape>
<XShape positionX="8412" positionY="7661" sizeX="1219" sizeY="1165"
type="com.sun.star.drawing.GroupShape"
name="CID/MultiClick/CID/D=0:CS=0:CT=0:Series=0:DataLabels=:DataLabel=2">
<XShapes>
- <XShape positionX="8412" positionY="7661" sizeX="1219"
sizeY="1165" type="com.sun.star.drawing.TextShape" text="Green 6%"
fontHeight="12.000000" fontColor="595959" textAutoGrowHeight="true"
textAutoGrowWidth="true" textContourFrame="false" textFitToSize="NONE"
textHorizontalAdjust="RIGHT" textVerticalAdjust="CENTER" textLeftDistance="0"
textRightDistance="0" textUpperDistance="0" textLowerDistance="0"
textMaximumFrameHeight="0" textMaximumFrameWidth="8466"
textMinimumFrameHeight="1" textMinimumFrameWidth="1" textAnimationAmount="0"
textAnimationCount="0" textAnimationDelay="0" textAnimationDirection="LEFT"
textAnimationKind="NONE" textAnimationStartInside="false"
textAnimationStopInside="false" textWritingMode="LR_TB" fillStyle="NONE"
fillColor="ffffff" fillTransparence="0" fillTransparenceGradientName="">
+ <XShape positionX="8412" positionY="7661" sizeX="1219"
sizeY="1165" type="com.sun.star.drawing.TextShape" text="Green 6%"
fontHeight="12.000000" fontColor="595959" textAutoGrowHeight="true"
textAutoGrowWidth="true" textContourFrame="false" textFitToSize="NONE"
textHorizontalAdjust="RIGHT" textVerticalAdjust="CENTER" textLeftDistance="0"
textRightDistance="0" textUpperDistance="0" textLowerDistance="0"
textMaximumFrameHeight="0" textMaximumFrameWidth="5929"
textMinimumFrameHeight="1" textMinimumFrameWidth="1" textAnimationAmount="0"
textAnimationCount="0" textAnimationDelay="0" textAnimationDirection="LEFT"
textAnimationKind="NONE" textAnimationStartInside="false"
textAnimationStopInside="false" textWritingMode="LR_TB" fillStyle="NONE"
fillColor="ffffff" fillTransparence="0" fillTransparenceGradientName="">
<FillTransparenceGradient style="LINEAR" startColor="000000"
endColor="000000" angle="0" border="0" xOffset="50" yOffset="50"
startIntensity="100" endIntensity="100" stepCount="0"/>
<FillGradient style="LINEAR" startColor="000000"
endColor="ffffff" angle="0" border="0" xOffset="50" yOffset="50"
startIntensity="100" endIntensity="100" stepCount="0"/>
<FillHatch style="SINGLE" color="3465a4" distance="20"
angle="0"/>
diff --git a/chart2/source/view/charttypes/PieChart.cxx
b/chart2/source/view/charttypes/PieChart.cxx
index 93221b468473..3635ea3fd3bc 100644
--- a/chart2/source/view/charttypes/PieChart.cxx
+++ b/chart2/source/view/charttypes/PieChart.cxx
@@ -402,6 +402,7 @@ void PieChart::createTextLabelShape(
// set the maximum text width to be used when text wrapping is enabled
double fTextMaximumFrameWidth = 0.8 * fPieRadius;
+ const double fCompatMaxTextLen = m_aAvailableOuterRect.getWidth() / 5.0;
if (m_aAvailableOuterRect.getWidth())
{
if (bHasCustomLabelPlacement)
@@ -412,7 +413,7 @@ void PieChart::createTextLabelShape(
if (aCustomSize.Width > 0)
fTextMaximumFrameWidth = aCustomSize.Width;
else
- fTextMaximumFrameWidth = m_aAvailableOuterRect.getWidth() /
5.0;
+ fTextMaximumFrameWidth = fCompatMaxTextLen;
}
else if (nLabelPlacement == css::chart::DataLabelPlacement::OUTSIDE)
{
@@ -450,13 +451,34 @@ void PieChart::createTextLabelShape(
{
if (m_aAvailableOuterRect.getWidth())
{
- if ((fAngleDegree >= 67.5 && fAngleDegree <= 112.5)
- || (fAngleDegree >= 247.5 && fAngleDegree <= 292.5))
- fTextMaximumFrameWidth = m_aAvailableOuterRect.getWidth()
/ 3.0;
- else
- fTextMaximumFrameWidth
- = 0.85 * (m_aAvailableOuterRect.getWidth() / 2.0 -
fPieRadius);
- nTextMaximumFrameWidth = ceil(fTextMaximumFrameWidth);
+ /* This was bestFit, but it didn't fit. So how best to handle
this?
+ *
+ * Two possible cases relating to compatibility
+ * 1.) It did fit for Microsoft, but our bestFit wasn't able
to do the same
+ * * In that case, the best response is to be as small as
possible
+ * (the distance from the chart edge to where the label
attaches to the slice)
+ * to avoid scaling the diagram with too long outside
labels,
+ * and to encourage fixing the bestFit algorithm.
+ * 2.) It didn't fit for Microsoft either (possible, but less
likely situation)
+ * * In that case, the compatible max length would be best
+ * * can expect the chart space has been properly sized to
handle the max length
+ *
+ * In the native LO case, it is also best to use as small as
possible,
+ * so that the user creating the diagram is annoyed and makes
the chart area larger.
+ *
+ * Therefore, handle this by making the label as small as
possible.
+ *
+ * Complication (tdf122765.pptx): it is possiible for the
aOuterPosition
+ * to be outside of the available outer rectangle (somehow),
+ * so in that bizarre case just try the positive value of the
result...
+ */
+ const sal_Int32 nOuterX = aPieLabelInfo.aOuterPosition.getX();
+ if (fAngleDegree < 90 || fAngleDegree > 270) // label is
placed on the right half
+ fTextMaximumFrameWidth = 0.8 *
abs(m_aAvailableOuterRect.getWidth() - nOuterX);
+ else // label is placed on the left half
+ fTextMaximumFrameWidth = 0.8 * nOuterX;
+
+ nTextMaximumFrameWidth = ceil(std::min(fTextMaximumFrameWidth,
fCompatMaxTextLen));
}
nScreenValueOffsetInRadiusDirection = (m_nDimension != 3) ? 150 :
0;
