Bug 111967 - Vertical text in table formatted incorrectly
Summary: Vertical text in table formatted incorrectly
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0 target:5.4.4
Keywords: bibisected, bisected, regression
: 105178 107272 112066 113170 (view as bug list)
Depends on:
Blocks: CJK Vertical-Text
  Show dependency treegraph
 
Reported: 2017-08-22 13:08 UTC by Mark Hung
Modified: 2017-12-08 21:14 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample text file. (9.38 KB, application/vnd.oasis.opendocument.text)
2017-08-22 13:08 UTC, Mark Hung
Details
Writer format correctly in 5.4.0.3 (Windows) (28.37 KB, image/png)
2017-08-22 13:10 UTC, Mark Hung
Details
Writer format incorrectly in 6.0.0.0alpha (69.26 KB, image/png)
2017-08-22 13:11 UTC, Mark Hung
Details
assertion message (4.75 KB, text/plain)
2017-08-22 13:11 UTC, Mark Hung
Details
Testcase in Writer 6.0.0.0alpha1 (2017-10-20) (91.90 KB, image/png)
2017-10-20 09:35 UTC, Volga
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hung 2017-08-22 13:08:53 UTC
Created attachment 135726 [details]
Sample text file.

The sample file is formatted incorrectly.
It crashed when placing cursor on the text.
Comment 1 Mark Hung 2017-08-22 13:10:42 UTC
Created attachment 135727 [details]
Writer format correctly in 5.4.0.3 (Windows)
Comment 2 Mark Hung 2017-08-22 13:11:16 UTC
Created attachment 135728 [details]
Writer format incorrectly in 6.0.0.0alpha
Comment 3 Mark Hung 2017-08-22 13:11:51 UTC
Created attachment 135729 [details]
assertion message
Comment 4 Jacques Guilleron 2017-08-22 17:00:51 UTC
Hi Mark,

I don't reproduce with
LO  6.0.0.0.alpha0+ Build ID: 88179e3de8865ea07d5017ca0723afd10ad44ba7
CPU threads: 2; OS: Windows 6.1; UI render: default; 
TinderBox: Win-x86@39, Branch:master, Time: 2017-08-06_23:49:25
Locale: fr-FR (fr_FR); Calc: group
Linux only?
Comment 5 Buovjaga 2017-09-02 16:54:50 UTC
I confirm the incorrect position, but not the crash.

Does it still crash with latest build?

Arch Linux 64-bit, KDE Plasma 5
Version: 6.0.0.0.alpha0+
Build ID: 4082b5874adddedf8332fe977b6bb47b6949b302
CPU threads: 8; OS: Linux 4.12; UI render: default; VCL: kde4; 
Locale: fi-FI (fi_FI.UTF-8); Calc: group
Built on August 31st 2017
Comment 6 Mark Hung 2017-09-03 04:57:21 UTC
(In reply to Buovjaga from comment #5)
> I confirm the incorrect position, but not the crash.
> 
> Does it still crash with latest build?
> 

Yes. It still crashes with latest build (2017-9-2 ).

Version: 6.0.0.0.alpha0+
Build ID: cb14008337e7b7ebbc0b5724f98ac9633e1b9331
CPU threads: 4; OS: Linux 4.10; UI render: default; VCL: gtk3; 
Locale: zh-TW (zh_TW.UTF-8); Calc: group
Comment 7 Buovjaga 2017-09-03 13:23:21 UTC
Ok, let's set to new for the incorrect position at least..
Comment 8 Xisco Faulí 2017-09-07 13:45:10 UTC
I can't reproduce the crash neither.

Regarding the incorrect position, regression introduced by:

author	Khaled Hosny <khaledhosny@eglug.org>	2017-08-13 21:03:43 (GMT)
committer	Michael Stahl <mstahl@redhat.com>	2017-08-15 11:02:17 (GMT)
commit	5aab2900dfdc9f12adda378470149670a2a069df (patch)
tree	62f9fb087a3771086e7af0e061e7f3bd2977b0ae
parent	a69850bc0bb8ff90d5676259c42c5e98a7f11150 (diff)
tdf#109142: Update to HarfBuzz 1.4.8

Bisected with: bibisect-linux64-6.0

Adding Cc: to Khaled Hosny

@Khaled: should it be reported to the HarfBuzz project instead?
Comment 9 Khaled Hosny 2017-09-08 23:22:40 UTC
I guess something changed in HarfBuzz since the last version we used that broke the vertical glyph positioning heuristic. No idea what is going on nor time to check.
Comment 10 Mark Hung 2017-09-09 17:45:45 UTC
Hi Khaled,

I tested Harfbuzz from 1.3.2 to 1.4.8. It seems that the issue was introduced in Harfbuzz 1.3.4. I hope this would be helpful.
Comment 11 Buovjaga 2017-09-09 18:25:46 UTC
(In reply to Mark Hung from comment #10)
> Hi Khaled,
> 
> I tested Harfbuzz from 1.3.2 to 1.4.8. It seems that the issue was
> introduced in Harfbuzz 1.3.4. I hope this would be helpful.

Did you test 1.5.0 and 1.5.1 btw.? :)
Comment 12 Xisco Faulí 2017-09-09 19:10:19 UTC
*** Bug 112066 has been marked as a duplicate of this bug. ***
Comment 13 Khaled Hosny 2017-09-09 19:10:56 UTC
(In reply to Mark Hung from comment #10)
> Hi Khaled,
> 
> I tested Harfbuzz from 1.3.2 to 1.4.8. It seems that the issue was
> introduced in Harfbuzz 1.3.4. I hope this would be helpful.

Yes, I remember now. I was actually setting next to Behdad when he was hacking this code for that release and I remember wondering if these changes are going to affect the brittle way we (I) position vertical glyphs. Unfortunately all the details escape me know and I need to dig both the HarfBuzz changes and the related LibreOffice code to have anything meaningful :( Actually I think that is why I didn’t update our bundled HarfBuzz all that time.
Comment 14 cyanshrike 2017-09-11 05:48:28 UTC
One major change in HarfBuzz 1.3.4 is "Fix vertical glyph origin in hb-ot-font".

I can use hb-view and hb-shape to see the different result between 1.3.3 and 1.3.4.

harfbuzz-1.3.3-win32\hb-shape --font-funcs=ot --shaper=ot --font-size=72 --direction=ltl --font-file="C:\Windows\Fonts\mingliu.ttc" --text=測試
[gid15828=0+72|gid23054=1+72]

harfbuzz-1.3.3-win32\hb-shape --font-funcs=ot --shaper=ot --font-size=72 --direction=ttb --font-file="C:\Windows\Fonts\mingliu.ttc" --text=測試
[gid15828=0+0,-72|gid23054=1+0,-72]

harfbuzz-1.3.4-win32\hb-shape --font-funcs=ot --shaper=ot --font-size=72 --direction=ltl --font-file="C:\Windows\Fonts\mingliu.ttc" --text=測試
[gid15828=0+72|gid23054=1+72]

harfbuzz-1.3.4-win32\hb-shape --font-funcs=ot --shaper=ot --font-size=72 --direction=ttb --font-file="C:\Windows\Fonts\mingliu.ttc" --text=測試
[gid15828=0@-36,-58+0,-72|gid23054=1@-36,-58+0,-72]
Comment 15 Mark Hung 2017-09-15 10:48:55 UTC
> 
> Yes, I remember now. I was actually setting next to Behdad when he was
> hacking this code for that release and I remember wondering if these changes
> are going to affect the brittle way we (I) position vertical glyphs.
> Unfortunately all the details escape me know and I need to dig both the
> HarfBuzz changes and the related LibreOffice code to have anything
> meaningful :( Actually I think that is why I didn’t update our bundled
> HarfBuzz all that time.

Hi Khaled,

I checked more detail today.

Inside hb-font-private.hh, fallback version of get_glyph_h_origin() and get_glyph_v_origin() were introduced in 1.3.4. It worked if I replace those back to non-fallback version in subtract_glyph_h_origin() , subtract_glyph_v_origin(), get_glyph_h_origin_with_fallback(), get_glyph_v_origin_with_fallback(). 

The test might be naive, but guess_v_origin_minus_h_origin() looks suspicious from my point of view. I don't know how it might related to "how you position vertical glyphs". I'll try to check that if you have some code pointers.

BTW, 1.5.0 and 1.5.1 don't work too.
Comment 16 Mark Hung 2017-09-15 10:53:12 UTC
> Inside hb-font-private.hh, fallback version of get_glyph_h_origin() and
> get_glyph_v_origin() were introduced in 1.3.4. It worked if I replace those
> back to non-fallback version in subtract_glyph_h_origin() ,
> subtract_glyph_v_origin(), get_glyph_h_origin_with_fallback(),
> get_glyph_v_origin_with_fallback(). 
> 

Oops, make mistake while copying. 

I mean replacing fallback version with non-fallback one in  
subtract_glyph_h_origin()
subtract_glyph_v_origin()
add_glyph_h_origin()
add_glyph_v_origin()
Comment 17 Khaled Hosny 2017-09-15 12:27:15 UTC
I think this should be reported to HarfBuzz and get the developers input on this.
Comment 18 Xisco Faulí 2017-09-17 21:29:40 UTC
I guess we can delete the crash part from the summary
Comment 19 Commit Notification 2017-09-18 09:18:59 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=79ceef1dd0970055a8de0f0676e1cd992a484ee1

related: tdf#111967 fix assertions about silly column positions

It will be available in 6.0.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 20 Xisco Faulí 2017-10-19 08:38:12 UTC
A polite ping to Michael Stahl: is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Thanks
Comment 21 Mark Hung 2017-10-19 10:37:07 UTC
@Xisco Faulí:

the crashing part is fixed but vertical writing isn't. 
We still need upstream ( harfbuzz ) to fix the issue.
Comment 22 Volga 2017-10-20 09:35:06 UTC
Created attachment 137133 [details]
Testcase in Writer 6.0.0.0alpha1 (2017-10-20)

This is still unresolved.

Version: 6.0.0.0.alpha1+ (x64)
Build ID: 8743a4172e5a168992f36e408c5db688a959000c
CPU threads: 4; OS: Windows 10.0; UI render: default; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2017-10-20_01:29:13
Locale: zh-CN (zh_CN); Calc: group
Comment 23 Khaled Hosny 2017-11-04 22:31:34 UTC
*** Bug 107272 has been marked as a duplicate of this bug. ***
Comment 24 Volga 2017-11-08 14:37:11 UTC Comment hidden (no-value)
Comment 25 Commit Notification 2017-11-17 21:34:53 UTC
Mark Hung committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=58af692e28146a1ecb4fbeef89e839baa1a1b39d

tdf#111967 translate offsets so it is relative to v origin

It will be available in 6.0.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 26 Volga 2017-11-19 05:03:34 UTC
OK, this is fixed. Confirmed in the following build:

Version: 6.0.0.0.alpha1+ (x64)
Build ID: b904d639a801d6d610d8e53ba23cae9781ab9569
CPU threads: 4; OS: Windows 10.0; UI render: default; 
TinderBox: Win-x86_64@42, Branch:master, Time: 2017-11-19_02:10:19
Locale: zh-CN (zh_CN); Calc: group

And the bug 103969 is also fixed by above commit. So is it possible to backport to 5.4/5.3 branches?
Comment 27 Volga 2017-11-27 04:13:03 UTC
Hi Mark,

LibO 5.4.4 will coming, it it possible to backport to that?
Comment 28 Commit Notification 2017-11-30 12:22:17 UTC
Mark Hung committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2ed722a7a49e9e90480e49e1ef0cec8e8c79b79d&h=libreoffice-5-4

tdf#111967 translate offsets so it is relative to v origin

It will be available in 5.4.4.

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

Affected users are encouraged to test the fix and report feedback.
Comment 29 Mark Hung 2017-11-30 13:42:21 UTC
The solution seems to work fine for Windows but still not so good for Linux. But I will close this issue.
Comment 30 Mark Hung 2017-12-08 21:14:12 UTC
*** Bug 113170 has been marked as a duplicate of this bug. ***
Comment 31 Mark Hung 2017-12-08 21:14:35 UTC
*** Bug 105178 has been marked as a duplicate of this bug. ***