Re: [Libreoffice-commits] core.git: fix fdo#60534 : use DefaultCharMapper::mapChar() to map RTL string unicodes

The below breaks on MSVC/Windows, against our internal icu 51.  Its 
DefaultCharMapper (icu/source/layout/DefaultCharMapper.h) and 
LECharMapper (icu/source/layout/LEFontInstance.h) it is derived from are 
not marked as U_LAYOUT_API, so their non-inline function definitions 
(esp., LECharMapper::~LECharMapper and DefaultMapper::mapChar) are not 
exported on Windows.  They are virtual, so this wouldn't be a problem if 
DefaultCharMapper were used in a way that function calls would 
necessarily be resolved via vtable, but that's not the case in the below 
code.

I wonder whether that is a known issue with ICU (it appears to be still 
this way in its SVN trunk, though), or whether we use DefaultCharMapper 
in an unsupported way here.

Any insight?

Stephan

On 04/08/2013 10:10 AM, navin patidar wrote:
> commit 83d9c5562c27b5f766157eba70bebd320463a0af
> Author: navin patidar <patidar@kacst.edu.sa>
> Date:   Sun Apr 7 11:47:01 2013 +0300
>
>      fix fdo#60534 : use DefaultCharMapper::mapChar() to map RTL string unicodes.
>
>      Use DefaultCharMapper::mapChar() to map RTL string unicodes
>      to their mirror compatible  unicodes.
>
>      Change-Id: I5bd2fd18bf96c49bbdf2be521a9cf28c311c7a09
>      Reviewed-on: https://gerrit.libreoffice.org/3221
>      Reviewed-by: Miklos Vajna <vmiklos@suse.cz>
>      Tested-by: Miklos Vajna <vmiklos@suse.cz>
>
> diff --git a/vcl/inc/graphite_layout.hxx b/vcl/inc/graphite_layout.hxx
> index bbf6f15..fa6d25c 100644
> --- a/vcl/inc/graphite_layout.hxx
> +++ b/vcl/inc/graphite_layout.hxx
> @@ -97,6 +97,7 @@ public:
>   private:
>       const gr_face *         mpFace; // not owned by layout
>       gr_font *               mpFont; // not owned by layout
> +    sal_Unicode *           mpStr;
>       int                     mnSegCharOffset; // relative to ImplLayoutArgs::mpStr
>       long                    mnWidth;
>       std::vector<int>        mvChar2BaseGlyph;
> diff --git a/vcl/source/glyphs/graphite_layout.cxx b/vcl/source/glyphs/graphite_layout.cxx
> index 8b19c31..3e3c329 100644
> --- a/vcl/source/glyphs/graphite_layout.cxx
> +++ b/vcl/source/glyphs/graphite_layout.cxx
> @@ -48,6 +48,7 @@
>   #include <unicode/ubidi.h>
>   #include <unicode/uscript.h>
>
> +#include <layout/DefaultCharMapper.h>
>   // Graphite Libraries (must be after vcl headers on windows)
>   #include <graphite2/Segment.h>
>
> @@ -490,6 +491,7 @@ GraphiteLayout::GraphiteLayout(const gr_face * face, gr_font * font,
>                                  const grutils::GrFeatureParser * pFeatures) throw()
>     : mpFace(face),
>       mpFont(font),
> +    mpStr(NULL),
>       mnWidth(0),
>       mfScaling(1.0),
>       mpFeatures(pFeatures)
> @@ -501,6 +503,7 @@ GraphiteLayout::~GraphiteLayout() throw()
>   {
>       clear();
>       // the features and font are owned by the platform layers
> +    delete[] mpStr;
>       mpFeatures = NULL;
>       mpFont = NULL;
>   }
> @@ -591,12 +594,28 @@ gr_segment * GraphiteLayout::CreateSegment(ImplLayoutArgs& rArgs)
>           }
>           size_t numchars = gr_count_unicode_characters(gr_utf16, rArgs.mpStr + mnSegCharOffset,
>                   rArgs.mpStr + (rArgs.mnLength > limit + 64 ? limit + 64 : rArgs.mnLength), NULL);
> +
> +        mpStr = new sal_Unicode[rArgs.mnLength+1];
> +        if(mpStr)
> +            memcpy(mpStr, rArgs.mpStr, (rArgs.mnLength+1)*2);
> +
> +        if (bRtl && mpStr)
> +        {
> +            DefaultCharMapper cmap(true, bRtl);
> +            int i=0;
> +            while(rArgs.mpStr[i])
> +            {
> +                mpStr[i]=(sal_Unicode) cmap.mapChar((sal_uInt32)rArgs.mpStr[i]);
> +                i++;
> +            }
> +        }
> +
>           if (mpFeatures)
>               pSegment = gr_make_seg(mpFont, mpFace, 0, mpFeatures->values(), gr_utf16,
> -                                        rArgs.mpStr + mnSegCharOffset, numchars, bRtl);
> +                                        mpStr + mnSegCharOffset, numchars, bRtl);
>           else
>               pSegment = gr_make_seg(mpFont, mpFace, 0, NULL, gr_utf16,
> -                                        rArgs.mpStr + mnSegCharOffset, numchars, bRtl);
> +                                        mpStr + mnSegCharOffset, numchars, bRtl);
>
>           //pSegment = new gr::RangeSegment((gr::Font *)&mrFont, mpTextSrc, &maLayout, 
> mnMinCharPos, limit);
>           if (pSegment != NULL)
> _______________________________________________
> Libreoffice-commits mailing list
> Libreoffice-commits@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits