Created attachment 150007 [details] Document showing issue In the attached document there should be kashida (long horizontal lines) inside justified Arabic words, but instead there are gaps. This is a regression as kashida was showing correctly in 6.1.x
Created attachment 150008 [details] 6.2.x defective output
Created attachment 150010 [details] 6.1.x correct output
I’m trying to debug this but getting nowhere, so hopefully someone can bisect this and find the commit that broke it. (When testing, sometimes it is necessary to change the text when opening the document, e.g., by adding a space, for the kashida justification to work).
I noticed that lcl_CheckKashidaPositions returned false at first test since rInf.GetOut()->GetMinKashida() returns 0 With gdb, I found that LogicalFontInstance::GetKashidaWidth could return 0 because in some cases, we don't enter if block with hb_font_get_glyph I wonder if https://cgit.freedesktop.org/libreoffice/core/commit/?id=23c5125148a8110d88385b29570bf0b7d4400458 may be the cause. Difficult to test since it can't be reverted without conflicts.
On pc Debian x86-64 with master sources updated today, I could reproduce this.
(In reply to Julien Nabet from comment #4) > I noticed that lcl_CheckKashidaPositions returned false at first test since > rInf.GetOut()->GetMinKashida() returns 0 > > With gdb, I found that LogicalFontInstance::GetKashidaWidth could return 0 > because in some cases, we don't enter if block with hb_font_get_glyph > > I wonder if > https://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=23c5125148a8110d88385b29570bf0b7d4400458 may be the cause. > Difficult to test since it can't be reverted without conflicts. I don’t think so, the kashida positions are calculated and they are inserted in the glyph items vector, but they are not drawn, so much later than this code.
bisected with 6.1 to this commit. But I have not Noto fonts on my computer, so I'm not sure if it's correct. Can you check it? e7b772a9c8a1c688c438e0f72ffff0a9b4263783 is the first bad commit commit e7b772a9c8a1c688c438e0f72ffff0a9b4263783 Author: Norbert Thiebaud <nthiebaud@gmail.com> Date: Mon Dec 18 14:36:58 2017 -0800 source e45e2c4897933f14c90a65fa74d0ad2a0b620ede author Yousuf Philips <philipz85@hotmail.com> 2017-12-16 16:18:18 +0400 committer Yousuf Philips <philipz85@hotmail.com> 2017-12-18 23:26:39 +0100 commit e45e2c4897933f14c90a65fa74d0ad2a0b620ede (patch) tree 6c3fbff8304e52d01a66b04e29163d27a14ee117 parent bdcbf601410f29642aa27e2b8fab312f929d569f (diff) tdf#103080 October 2017 update to Noto fonts
(In reply to raal from comment #7) > bisected with 6.1 to this commit. But I have not Noto fonts on my computer, > so I'm not sure if it's correct. Can you check it? > > e7b772a9c8a1c688c438e0f72ffff0a9b4263783 is the first bad commit > commit e7b772a9c8a1c688c438e0f72ffff0a9b4263783 > Author: Norbert Thiebaud <nthiebaud@gmail.com> > Date: Mon Dec 18 14:36:58 2017 -0800 > > source e45e2c4897933f14c90a65fa74d0ad2a0b620ede > > author Yousuf Philips <philipz85@hotmail.com> 2017-12-16 16:18:18 +0400 > committer Yousuf Philips <philipz85@hotmail.com> 2017-12-18 23:26:39 +0100 > commit e45e2c4897933f14c90a65fa74d0ad2a0b620ede (patch) > tree 6c3fbff8304e52d01a66b04e29163d27a14ee117 > parent bdcbf601410f29642aa27e2b8fab312f929d569f (diff) > tdf#103080 October 2017 update to Noto fonts This only adds the fonts, but the issue should be reproducible with any Arabic font. Try changing the font to an Arabic font you have on your system (avoid non-Arabic fonts as font fallabcks breaks kashida in a different, unrelated bug.
Hi Khaled, my bisection points to https://cgit.freedesktop.org/libreoffice/core/commit/?id=c45b23377bb2fe44c26f1287ff38495344e4ca50, however, the commit is from LibreOffice 6.0 and you can't reproduce it in 6.1... this is weird
(In reply to Xisco Faulí from comment #9) > Hi Khaled, > my bisection points to > https://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=c45b23377bb2fe44c26f1287ff38495344e4ca50, however, the commit is from > LibreOffice 6.0 and you can't reproduce it in 6.1... this is weird I reverted this commit locally and it didn’t fix the gap *inside* words issue, however it fixed the issue I had with kashida justfication not being done when the file is first open (gap *between* words). While bisecting, try to always add a space somewhere in the line then remove it, you might need this to activate kashida justification.
Khaled Hosny committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/0a8e9cc5c1782f1cd50ef338ec2aa4f6776a4c0e%5E%21 tdf#124109: Revert "Only do kashida insertion with fonts that have non-zero width kashidas" It will be available in 6.3.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.
Hi Khaled, second try pressing the space bar -> https://cgit.freedesktop.org/libreoffice/core/commit/?id=436b829f5b904d76039db0818cff5dedf1ae89f1. Give it a try and let me know if this bisection is the right one...
(In reply to Xisco Faulí from comment #12) > Hi Khaled, > second try pressing the space bar -> > https://cgit.freedesktop.org/libreoffice/core/commit/ > ?id=436b829f5b904d76039db0818cff5dedf1ae89f1. > Give it a try and let me know if this bisection is the right one... Thanks Xisco, that is the commit indeed.
Adding Cc: to Miklos Vajna
Khaled Hosny committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/f8ca6e0a59bff51fcb09af4fa6d9cd458b32f223%5E%21 tdf#124109: Fix missing kashida glyphs It will be available in 6.3.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.
This is resolved now, thanks Khaled!
Khaled Hosny committed a patch related to this issue. It has been pushed to "libreoffice-6-1": https://git.libreoffice.org/core/+/be189bfc1709dce48111eed49d847c05ef82164e%5E%21 tdf#124109: Revert "Only do kashida insertion with fonts that have non-zero width kashidas" It will be available in 6.1.6. 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.
Khaled Hosny committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/76848756909cb969b4689ebd2dd3b5fa27b0dc23%5E%21 tdf#124109: Revert "Only do kashida insertion with fonts that have non-zero width kashidas" It will be available in 6.2.3. 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.
Verified in Version: 6.3.0.0.alpha0+ Build ID: 4c5d0e4822dcd0c6c9397a45e3afb66d53ebaafc CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US Calc: threaded @Khaled, thanks for fixing this issue! Any chance we could have a unittest for this issue ?
Khaled Hosny committed a patch related to this issue. It has been pushed to "libreoffice-6-2": https://git.libreoffice.org/core/+/b2e81e11e154b660172356969252f00ba63f3a2b%5E%21 tdf#124109: Fix missing kashida glyphs It will be available in 6.2.3. 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.
Miklos Vajna committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/bb4fcc3d968367d682d6da057083378e9d171a22%5E%21 Related: tdf#124109 sw: save one vcl layout call in SwFntObj::DrawText() It will be available in 6.3.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.
> Any chance we could have a unittest for this issue ? Good question, certainly not trivial. I guess what the test has to do is pre-compute a glyph list, then call DrawTextArray() with the pre-computed glyph list and somehow assert that GenericSalLayout::ApplyDXArray() inserts the kashida glyphs. I'll give it a try, but I can't promise it. (From my point of view, usually VCL "just works" and then we can assert what VCL calls Writer does, but here the root cause is in VCL itself.)
Miklos Vajna committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/+/8308d6a38ff283a2feebf84a95198159887cd685%5E%21 Related: tdf#124109 vcl: make glyph item caching work with kashida glyphs It will be available in 6.3.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.
Great! it works for my in: Version: 6.2.4.0.0+ Build ID: ca0ac3938020e8a544071fc6b08eb6b2830b5cc1 CPU threads: 1; OS: Linux 5.0; UI render: default; VCL: gtk2; TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:libreoffice-6-2, Time: 2019-04-19_06:33:21 Locale: en-US (en_US.UTF-8); UI-Language: en-US Calc: threaded