Re: [Libreoffice-commits] core.git: tdf#89870: Add config option for layout engine

On 11/03/2016 06:23 PM, Khaled Hosny wrote:
> commit 6324efd70dfa8c6be84cba1fa29658e3373cbbe3
> Author: Khaled Hosny <khaledhosny@eglug.org>
> Date:   Thu Nov 3 19:21:01 2016 +0200
>
>     tdf#89870: Add config option for layout engine
>
>     The env variable takes precedence.

Do we really need this?  I've read through 
<https://bugs.documentfoundation.org/show_bug.cgi?id=89870#c28> and 
following, but I'm not too convinced.

* How long will we keep supporting the old code?  "Forever", or do we 
have a clear idea when to drop it?

* Who exactly is the perceived audience of setting that configuration 
option, vs. setting the environment variable?

We shouldn't light-heartedly fill the configuration with unnecessary 
entries.

And the use of a string ("new" vs. "old", instead of a bool or an 
integer enumeration), presumably intended to make manually editing it in 
the advanced configuration options pane easier, solves that usability 
issue in the wrong way IMO.

>     Change-Id: I273ec867725dd012b005d49fbfaa1dc27203c021
>
> diff --git a/officecfg/registry/schema/org/openoffice/Office/Common.xcs 
> b/officecfg/registry/schema/org/openoffice/Office/Common.xcs
> index e8a9fae..0e645ad 100644
> --- a/officecfg/registry/schema/org/openoffice/Office/Common.xcs
> +++ b/officecfg/registry/schema/org/openoffice/Office/Common.xcs
> @@ -851,6 +851,13 @@
>        <info>
>          <desc>Contains settings for VCL.</desc>
>        </info>
> +      <prop oor:name="TextLayoutEngine" oor:type="xs:string" oor:nillable="false">
> +        <info>
> +          <desc>Specifies which text layout engine should be use. Possible
> +                values are ("old", "new").</desc>
> +        </info>
> +        <value>new</value>
> +      </prop>
>        <prop oor:name="UseOpenGL" oor:type="xs:boolean" oor:nillable="false">
>          <info>
>             <desc>Specifies if OpenGL rendering should be used in VCL backends
> diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
> index 2e09aaf..ef9bbc0 100644
> --- a/vcl/source/gdi/sallayout.cxx
> +++ b/vcl/source/gdi/sallayout.cxx
> @@ -34,6 +34,8 @@
>
>  #include <i18nlangtag/lang.h>
>
> +#include <officecfg/Office/Common.hxx>
> +
>  #include <tools/debug.hxx>
>  #include <vcl/svapp.hxx>
>
> @@ -770,7 +772,8 @@ bool SalLayout::IsSpacingGlyph( sal_GlyphId nGlyph )
>
>  bool SalLayout::UseCommonLayout()
>  {
> -    static bool bUse = getenv("SAL_NO_COMMON_LAYOUT") == nullptr;
> +    static bool bUse = (getenv("SAL_NO_COMMON_LAYOUT") == nullptr) &&
> +                       (officecfg::Office::Common::VCL::TextLayoutEngine::get() == "new");
>      return bUse;
>  }