Re: proposition for patch in CommonSalLayout.cxx
Stanisław Jeśmanowicz wrote >> > This is not always the case, because if a platform didn't include > graphite2 shaper in its HarfBuzz > implementaion, you won't have it anyway ( like ./configure > --with-graphite2=no ) > Even if you set it the shapers list (as in > https://cgit.freedesktop.org/libreoffice/core/commit/?id=3cee50476e422e3ed84169cdcbe6bd9883fc9316 > ) > And if graphite2 shaper is implemented, then it will be first (as you can > see in the harfbuzz code: > https://github.com/harfbuzz/harfbuzz/blob/master/src/hb-shaper-list.hh ) > And generally speaking, hard-coded list in such a case disables possible > new shapers. Perhaps the goal was indeed to avoid non tested new shapers in LO that could bring some havoc. So by hard coding shapers, you're sure you won't have problems. Now of course, if nobody ever test new shapers, LO will be stuck with old ones. Anyway, now you got an account, once you had already submitted your license (see https://wiki.documentfoundation.org/Development/GetInvolved#License_statement), I'll put Khaled in cc, he certainly will bring interesting thoughts here. Julien -- Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
Stanisław Jeśmanowicz wrote > ... > Thank you for your advice, but getting familiar with gerrit on top of git > would take some time for > me now. > I hope, that some of your developers could commit this patch for me in vcl > module. > It is simple and won't harm anything. > But people could benefit from it on all platforms that use system's native > HarfBuzz implementation. > ... I can understand you don't have time for gerrit but would it be possible you submit a license statement? About this line, I retrieved the initial patches which put this line: 1) earliest https://cgit.freedesktop.org/libreoffice/core/commit/?id=3cee50476e422e3ed84169cdcbe6bd9883fc9316: Author: Khaled Hosny Date: Fri Mar 10 16:53:08 2017 +0200 tdf#106466: Use graphite2 shaper first We want to always prefer Graphite shaping when supported by the font, which is also what HarfBuzz does by default. Change-Id: I6670fc03b8e6b3d7e07e1b8e0062880524da1655 Reviewed-on: https://gerrit.libreoffice.org/35046 Tested-by: Jenkins Reviewed-by: Khaled Hosny 2) oldest https://cgit.freedesktop.org/libreoffice/core/commit/?id=7854d35cd8172b201f1f3ad247860f242e5cb06b Author: Khaled Hosny Date: Thu Oct 6 04:15:41 2016 +0200 Use HarfBuzz shape plan for a bit more control This way we control exactly what shapers we use in what order, and as an extra we can now tell which shaper HarfBuzz ends up using. Julien -- Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
On 9.3.2021 20.51, Stanisław Jeśmanowicz wrote: On 3/9/21 6:34 PM, julien2412 wrote: Stanisław Jeśmanowicz wrote ... Thank you for your advice, but getting familiar with gerrit on top of git would take some time for me now. I hope, that some of your developers could commit this patch for me in vcl module. It is simple and won't harm anything. But people could benefit from it on all platforms that use system's native HarfBuzz implementation. ... I can understand you don't have time for gerrit but would it be possible you submit a license statement? I have created an account and can log in to https://gerrit.libreoffice.org/ but it is not clear for me where to submit a license statement. If you already created an account, you might as well submit the patch using the web interface: https://gerrit.libreoffice.org/Documentation/user-inline-edit.html Re: license statement: https://wiki.documentfoundation.org/Development/GetInvolved#License_statement Ilmari ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
On 3/9/21 6:34 PM, julien2412 wrote: Stanisław Jeśmanowicz wrote ... Thank you for your advice, but getting familiar with gerrit on top of git would take some time for me now. I hope, that some of your developers could commit this patch for me in vcl module. It is simple and won't harm anything. But people could benefit from it on all platforms that use system's native HarfBuzz implementation. ... I can understand you don't have time for gerrit but would it be possible you submit a license statement? I have created an account and can log in to https://gerrit.libreoffice.org/ but it is not clear for me where to submit a license statement. About this line, I retrieved the initial patches which put this line: 1) earliest https://cgit.freedesktop.org/libreoffice/core/commit/?id=3cee50476e422e3ed84169cdcbe6bd9883fc9316: Author: Khaled Hosny Date: Fri Mar 10 16:53:08 2017 +0200 tdf#106466: Use graphite2 shaper first We want to always prefer Graphite shaping when supported by the font, which is also what HarfBuzz does by default. This is not always the case, because if a platform didn't include graphite2 shaper in its HarfBuzz implementaion, you won't have it anyway ( like ./configure --with-graphite2=no ) Even if you set it the shapers list (as in https://cgit.freedesktop.org/libreoffice/core/commit/?id=3cee50476e422e3ed84169cdcbe6bd9883fc9316 ) And if graphite2 shaper is implemented, then it will be first (as you can see in the harfbuzz code: https://github.com/harfbuzz/harfbuzz/blob/master/src/hb-shaper-list.hh ) And generally speaking, hard-coded list in such a case disables possible new shapers. Change-Id: I6670fc03b8e6b3d7e07e1b8e0062880524da1655 Reviewed-on: https://gerrit.libreoffice.org/35046 Tested-by: Jenkins Reviewed-by: Khaled Hosny 2) oldest https://cgit.freedesktop.org/libreoffice/core/commit/?id=7854d35cd8172b201f1f3ad247860f242e5cb06b Author: Khaled Hosny Date: Thu Oct 6 04:15:41 2016 +0200 Use HarfBuzz shape plan for a bit more control This way we control exactly what shapers we use in what order, and as an extra we can now tell which shaper HarfBuzz ends up using. Julien -- Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice Stan -- __ Stanislaw Jesmanowicz stan mail2 jesmanowicz com Amsterdam voice : + 31 20 6126193 The Netherlandsmobile: + 31 653380520 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
Hello Julien, On 3/9/21 9:18 AM, julien2412 wrote: Hello, In addition to what Miklos indicated, I think you may be interested in https://wiki.documentfoundation.org/Development/GetInvolved. Indeed, it seems you've already downloaded sources, built and tried to fix a problem with a patch. So you only need now a gerrit account and submit your license statement (not the most difficult). About the patch itself, I got no expertise so can't tell at all. In general a patch can be discussed precisely in gerrit. Thank you for your advice, but getting familiar with gerrit on top of git would take some time for me now. I hope, that some of your developers could commit this patch for me in vcl module. It is simple and won't harm anything. But people could benefit from it on all platforms that use system's native HarfBuzz implementation. Julien. -- Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice Regards, Stan -- __ Stanislaw Jesmanowicz stan mail2 jesmanowicz com Amsterdam voice : + 31 20 6126193 The Netherlandsmobile: + 31 653380520 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
Hello Miklos,
On 3/9/21 9:14 AM, Miklos Vajna wrote:
Hi Stan,
On Mon, Mar 08, 2021 at 03:50:21PM +0100, Stanisław Jeśmanowicz
wrote:
Dear all,
I noticed that LibreOffice is using the hard coded harfbuzz shapers list in
CommonSalLayout.cxx file (line 470).
I recommend the patch attached to be more flexible.
This take all possible shapers in a given HarfBuzz implementation.
This also allows the use of newly created shapers.
Just repleces line:
- const char*const pHbShapers[] = { "dt", "graphite2", "coretext_aat", "ot",
"fallback", nullptr };
with:
+ const char **pHbShapers = hb_shape_list_shapers();
Are you more or less reverting 7854d35cd8172b201f1f3ad247860f242e5cb06b?
Can you describe what is the practical benefit of doing so?
The benefits of applying this patch are:
1. HarfBuzz allows the development of external shapers, like graphite2.
This way, if a LibreOffice is build with system HarfBuzz
(--with-system-harfbuzz),
then any new developed shaper would work without recompiling LibreOffice.
2. Also in case of using system HarfBuzz, a particular HarfBuzz implementation
can decide
which shaper to use out of currently existing list: graphite2, uniscribe,
directwrite (so far).
3. Last, but not least, I have developed a HarfBuzz shaper and I would like
that it work on systems
that have implemented it.
I have send a similar patch request to Scribus people, because thy use also
hard coded shapers list.
But open source distribution of Chromium works with new shapers out of box.
Regards,
Miklos
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Regards,
Stan
--
__
Stanislaw Jesmanowicz stan mail2 jesmanowicz com
Amsterdam voice : + 31 20 6126193
The Netherlandsmobile: + 31 653380520
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
Hello, In addition to what Miklos indicated, I think you may be interested in https://wiki.documentfoundation.org/Development/GetInvolved. Indeed, it seems you've already downloaded sources, built and tried to fix a problem with a patch. So you only need now a gerrit account and submit your license statement (not the most difficult). About the patch itself, I got no expertise so can't tell at all. In general a patch can be discussed precisely in gerrit. Julien. -- Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: proposition for patch in CommonSalLayout.cxx
Hi Stan,
On Mon, Mar 08, 2021 at 03:50:21PM +0100, Stanisław Jeśmanowicz
wrote:
> Dear all,
>
> I noticed that LibreOffice is using the hard coded harfbuzz shapers list in
> CommonSalLayout.cxx file (line 470).
> I recommend the patch attached to be more flexible.
> This take all possible shapers in a given HarfBuzz implementation.
> This also allows the use of newly created shapers.
>
> Just repleces line:
> - const char*const pHbShapers[] = { "dt", "graphite2", "coretext_aat", "ot",
> "fallback", nullptr };
>
> with:
> + const char **pHbShapers = hb_shape_list_shapers();
Are you more or less reverting 7854d35cd8172b201f1f3ad247860f242e5cb06b?
Can you describe what is the practical benefit of doing so?
Regards,
Miklos
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
proposition for patch in CommonSalLayout.cxx
Dear all,
I noticed that LibreOffice is using the hard coded harfbuzz shapers list in CommonSalLayout.cxx file
(line 470).
I recommend the patch attached to be more flexible.
This take all possible shapers in a given HarfBuzz implementation.
This also allows the use of newly created shapers.
Just repleces line:
- const char*const pHbShapers[] = { "dt", "graphite2", "coretext_aat", "ot",
"fallback", nullptr };
with:
+ const char **pHbShapers = hb_shape_list_shapers();
Best regards,
Stan
= patch
--- libreoffice-7.0.4.9/vcl/source/gdi/CommonSalLayout.cxx 2021-03-07
18:38:41.339770315 +0100
+++ libreoffice-7.0.4.x/vcl/source/gdi/CommonSalLayout.cxx 2021-03-08
12:29:37.045184836 +0100
@@ -463,11 +463,8 @@
nMinRunPos, nRunLen);
hb_buffer_set_cluster_level(pHbBuffer,
HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS);
-// The shapers that we want HarfBuzz to use, in the order of
-// preference. The coretext_aat shaper is available only on macOS,
-// but there is no harm in always including it, HarfBuzz will
-// ignore unavailable shapers.
-const char*const pHbShapers[] = { "dt", "graphite2", "coretext_aat", "ot", "fallback",
nullptr };
+// Take all possible shapers in a given HarfBuzz implementation.
+const char **pHbShapers = hb_shape_list_shapers();
bool ok = hb_shape_full(pHbFont, pHbBuffer, maFeatures.data(), maFeatures.size(),
pHbShapers);
assert(ok);
(void) ok;
--
__
Stanislaw Jesmanowicz stan mail2 jesmanowicz com
Amsterdam voice : + 31 20 6126193
The Netherlandsmobile: + 31 653380520
--- libreoffice-7.0.4.9/vcl/source/gdi/CommonSalLayout.cxx 2021-03-07 18:38:41.339770315 +0100
+++ libreoffice-7.0.4.x/vcl/source/gdi/CommonSalLayout.cxx 2021-03-08 12:29:37.045184836 +0100
@@ -463,11 +463,8 @@
nMinRunPos, nRunLen);
hb_buffer_set_cluster_level(pHbBuffer, HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS);
-// The shapers that we want HarfBuzz to use, in the order of
-// preference. The coretext_aat shaper is available only on macOS,
-// but there is no harm in always including it, HarfBuzz will
-// ignore unavailable shapers.
-const char*const pHbShapers[] = { "dt", "graphite2", "coretext_aat", "ot", "fallback", nullptr };
+// Take all possible shapers in a given HarfBuzz implementation.
+const char **pHbShapers = hb_shape_list_shapers();
bool ok = hb_shape_full(pHbFont, pHbBuffer, maFeatures.data(), maFeatures.size(), pHbShapers);
assert(ok);
(void) ok;
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
