Bug 146756 - FILEOPEN PPTX: labels of pie chart wrap much too aggressively (compared to 365) as well as too conservatively
Summary: FILEOPEN PPTX: labels of pie chart wrap much too aggressively (compared to 36...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Chart (show other bugs)
Version:
(earliest affected)
7.2.0.4 release
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:24.8.0
Keywords: bibisected, bisected, regression
Depends on:
Blocks: OOXML-Chart Pie-and-Donut
  Show dependency treegraph
 
Reported: 2022-01-14 09:34 UTC by Gerald Pfeifer
Modified: 2024-10-15 21:29 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
The example file in PP 2013 and current master (120.76 KB, image/png)
2022-01-20 12:34 UTC, Gabor Kelemen (allotropia)
Details
Visual comparision for alternate example from comment #4 (370.33 KB, image/png)
2022-08-06 07:42 UTC, Gerald Pfeifer
Details
piechartformat_mso2010.pdf: not particularly great in earlier versions of MS either. (3.78 KB, application/pdf)
2024-01-26 18:45 UTC, Justin L
Details
my piechart-manualLayoutB.pptx: testing zero offset, and 60% offset (42.11 KB, application/vnd.openxmlformats-officedocument.presentationml.presentation)
2024-01-27 17:22 UTC, Justin L
Details
tdf146756_maxLabelWidth.docx: seems to be 1/5 of total chart width (33.94 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2024-01-27 20:49 UTC, Justin L
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gerald Pfeifer 2022-01-14 09:34:06 UTC
How to reproduce:

1. Open the sample slide https://bugs.documentfoundation.org/attachment.cgi?id=177210 from bug #146487 in Impress and 365

2. Observe labels wrap very aggressively, even breaking the numbers across multiple lines, cf. screenshot https://bugs.documentfoundation.org/attachment.cgi?id=177211 from bug https://bugs.documentfoundation.org/attachment.cgi?id=177211
Comment 1 Gerald Pfeifer 2022-01-14 21:06:26 UTC
Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: f1f2daefbe921d223079a50358527e1d35470850
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Comment 2 Gabor Kelemen (allotropia) 2022-01-20 12:34:04 UTC
Created attachment 177669 [details]
The example file in PP 2013 and current master

Confirming the issue in 

Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 3b48e20b0101584a5e7ef48ba82238e735f0772b
CPU threads: 13; OS: Windows 10.0 Build 19042; UI render: Skia/Raster; VCL: win
Locale: hu-HU (hu_HU); UI: en-US
Calc: threaded

"Best fit" label alignments are difficult :(.
Comment 3 Gabor Kelemen (allotropia) 2022-01-20 23:30:51 UTC
Seems to be a side effect since 7.2 from:

https://git.libreoffice.org/core/+/b0068342398786ca50304260434a18880dddf74d

author	Tünde Tóth <toth.tunde@nisz.hu>	Fri Dec 11 09:13:46 2020 +0100
committer	László Németh <nemeth@numbertext.org>	Wed Dec 16 18:26:26 2020 +0100

tdf#138777 pie chart: improve long data label width

Adding CC to: Tünde Tóth
Comment 4 Gerald Pfeifer 2022-07-27 23:34:08 UTC
The chart on the second slide of attachment #181458 [details] of bug #150176 is
an even stronger example; cf. attachment #181459 [details] for visuals.

Tünde, do you have any plans (maybe even timeline) to look into this
regression?
Comment 5 Gerald Pfeifer 2022-08-06 07:42:16 UTC
Created attachment 181635 [details]
Visual comparision for alternate example from comment #4
Comment 6 Justin L 2024-01-26 18:02:04 UTC
The two document here are quite different beasts.

piechartformat.pptx (comment 0) defines a custom (relative-to-what?) X/Y position as well as a size. The W/H were not imported or considered. https://gerrit.libreoffice.org/c/core/+/162590

aroundboxlines.pptx (comment 4) defines custom (relative) X/Y positions, but no size. Well, that certainly isn't very helpful. What do we do with that? Is it supposed to still try to fit inside a slice? (We are left to guess - which seems to be standard fare for all things chart-related.)

Unfortunately, there doesn't seem to be a "correct answer" for anything here.
Comment 7 Justin L 2024-01-26 18:45:16 UTC
Created attachment 192188 [details]
piechartformat_mso2010.pdf: not particularly great in earlier versions of MS either.
Comment 8 Justin L 2024-01-27 17:22:36 UTC
Created attachment 192191 [details]
my piechart-manualLayoutB.pptx: testing zero offset, and 60% offset

(In reply to Justin L from comment #6)
> defines a custom (relative-to-what?) X/Y position
Seems to be "relative to the mid-point on the circumference + a small padding amount. In other words, relative to the start of an outside label.

Our code seems to be handling that pretty much correct in terms of basic placement - no obvious logical errors regarding positioning. Our pie is larger, so getting exact identical placement isn't likely.

The key take-away for me is that having manual layout likely holds no meaning whatsoever about whether the text should try to fit inside the slice or not.
Comment 9 Justin L 2024-01-27 20:49:20 UTC
Created attachment 192193 [details]
tdf146756_maxLabelWidth.docx: seems to be 1/5 of total chart width
Comment 10 Commit Notification 2024-01-29 14:48:22 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/301e27cbebf7d6e4c9b82290d7cd555c43f0c999

tdf#146756 pie chart2: import extLst manualLayout W and H

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2024-01-30 09:41:10 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a77403d11a60ebe6aeb33d9b6ae611412d9ab1cc

tdf#146756 pie chart2 import: use manualLayout Width for pie chart labels

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2024-01-30 09:43:13 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/85f4395b6f40c0295a190cca09ecd51858fc3b31

tdf#146756 pie chart2 import: improve response to bestFit failure

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Justin L 2024-01-30 12:53:36 UTC
[One way to "fix" these document would have been to simply round-trip the files in LO - which would have solved most of the problems. Widening the chart area would take care of the rest.]

The main focus of this bug report (maximum text length) has been fixed. One key was identifying what appears to be Microsoft's magic number (1/5 of the width).
So I'm going to mark this bug as fixed. (No intention of backporting regression-inviting patches - this should be for 24.8 only.)

(In reply to Justin L from comment #6)
> The two document here are quite different beasts.
> 
> piechartformat.pptx (comment 0) defines a custom ... size. 
Fixed by comment 10 and comment 11's patches

> aroundboxlines.pptx (comment 4) defines custom (relative) X/Y positions, but
> no size. What do we do with that?
This is where the 1/5th limit comes in. Comment 11 fixes the wrapping size of the two gray-ish slices that defined relative x/y positions.

Comment 12 fixes the wrapping of the orange "bestFit" slice, which fits in MSO, but doesn't fit in LO - so LO had to move the label outside of the circle.


Additional work could still be done for comment 4's document.
A.) Improve our best-fit algorithm to that it can auto-adjust the width if that helps it to fit (MSO is able to do that, although sometimes our algorithm works when MSO doesn't --- so I wouldn't say that ours is wrong.)

B.) The two manually-positioned labels aren't fully inside their slice. LO has scaled down the size of the pie chart (probably because of A., which required some space below the chart to place an outside label). If the chart was full sized, then the manual placement of the two gray-ish slice's labels would look better.

C.) "Monitor, trouleshoot, and remediate" is not in bold, even though the properties say it is? All the others are bold, and any manual attempt to change it fails (but works on the others).
Comment 14 Commit Notification 2024-01-31 09:47:22 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9833ac64c43b140233eca4ebb98e445a462425fc

tdf#146756 pie chart2 import: use consistent outside-label max text len

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Gerald Pfeifer 2024-01-31 09:53:03 UTC
Verifying with today's nightly build

  Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
  Build ID: dfcdef6d94993131b5f150d00d08cc81a987eebb
  CPU threads: 12; OS: Linux 6.6; UI render: default; VCL: gtk3
  Locale: en-US (en_US.UTF-8); UI: en-US

> (No intention of backporting regression-inviting patches - this should
> be for 24.8 only.)

Totally agreed, yes.


As for the other, open items, I believe bug #159462 that you filed
addressed item A?

Shall I go ahead and file a bug for item C (text not shown bold)?
Comment 16 Commit Notification 2024-01-31 13:54:12 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/19eeee25e13993806545552cc49da79a7bd6b990

related tdf#146756 chart2: code-readers documentation

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Justin L 2024-01-31 14:40:54 UTC
(In reply to Gerald Pfeifer from comment #15)
> I believe bug #159462 that you filed addressed item A?
Correct

> Shall I go ahead and file a bug for item C (text not shown bold)?
I'll do it. Bug 159475.
Comment 18 Commit Notification 2024-06-06 17:19:39 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ec73b2894009a4cbb3446f8370b3de7bbbd5a35f

tdf#146756 unit test font: substitute Carlito for Calibri font

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.