On August 15, 2002 12:08 pm, Shachar Shemesh wrote:
configure.ac: Added a check for the existance of the fribidi include file. Nocheck for the library, as we do not statically link with it.
Like any programmer, I usually hate copying stuff around, but in this case isn't it better to simply have a copy of the fribidi heades around?
I mean, we do the runtime load and checks anyway, why do we need to have the header around? If we don't use much out of the header, I would say simply include the prototype of the functions we're calling into the dlls/gdi/fribidi.c file directly, and be done with it. No configure, no macros, no ifdefs, no mess.
Dimitrie O. Paun wrote:
On August 15, 2002 12:08 pm, Shachar Shemesh wrote:
configure.ac: Added a check for the existance of the fribidi include file. No check for the library, as we do not statically link with it.
Like any programmer, I usually hate copying stuff around, but in this case isn't it better to simply have a copy of the fribidi heades around?
I mean, we do the runtime load and checks anyway, why do we need to have the header around? If we don't use much out of the header, I would say simply include the prototype of the functions we're calling into the dlls/gdi/fribidi.c file directly, and be done with it. No configure, no macros, no ifdefs, no mess.
NOW you remeber to tell me about this? ;-)
But seriously, if I've already took the time to learn that tiny bit of autoconf necessary to implement this, and we already have the define, I think it is best to just leave it there. The argument, as I can see it, goes like this: Pro adding it:
1. Allows you to compile with or without, saving on runtime checks (not really an issue - MS does, as of Windows 2000, basically the same thing with a DLL called "LPK.DLL", Language Pack). 2. libfribidi is LGPL. If wine depends on it included to compile, we can't negotiate BiDi with rewine's code users for some other feature (unless the aprox. four libfribidi copyright owners give their consent, of course). 3. Remain in sync with the library with which we intend to link in runtime.
Con:
1. Need to write code (not an issue - code is already written). 2. No chance of packagers forgetting this library (not an issue - if packagers want this package removed, it's their own choice. That's what open source is all about). 3. A way to eliminate BiDi bugs (I don't think it's an issue - bugs need to be resolved anywhere they exist). 4. The library dependancy is expected to change if Behdad stands by his word of supplying a UTF-16 libfribidi (but then we will want the new library in the same way, won't we?) 5. Creating two variants of the code that, in theory, both need to be tested.
Of all these issues, I see Pro(3) and Con(5) as bearing any real relevance. In particular, Con(5) is the exact thing that made Windows 2000 with Hebrew so much better than any other MS Hebrew speaking OS. As I noted before, however, W2K really uses the runtime link methodology.
If the general public here votes to remove the ac related code, I'll go along with it (no question about it making my code simpler, albeit marginally).
Shachar
On August 16, 2002 12:43 pm, Shachar Shemesh wrote:
Dimitrie O. Paun wrote:
NOW you remeber to tell me about this? ;-)
yes.... :)))
If the general public here votes to remove the ac related code, I'll go along with it (no question about it making my code simpler, albeit marginally).
I haven't looked that closely over the patch, but for now, I still think it's simpler to avoid the config check, since the load can happen on other machine then where we compile anyway... Hmm, I have to leave in 1 min (for a camping trip), so the patch will have to wait until Monday... Alexandre decides, anyway!
-- Dimi.
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
If the general public here votes to remove the ac related code, I'll go along with it (no question about it making my code simpler, albeit marginally).
I suggest you write the real code first, and let us worry about the configure stuff later. There is no point in adding configure dependencies for non-existing code, and once the code is done we can have a better idea of what dependencies you actually need.
As mentioned already, I think including the fribidi algorithms directly in the code is a better approach, but the only way to really say which is the right way is to get something working.
Alexandre Julliard wrote:
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
If the general public here votes to remove the ac related code, I'll go along with it (no question about it making my code simpler, albeit marginally).
I suggest you write the real code first, and let us worry about the configure stuff later. There is no point in adding configure dependencies for non-existing code, and once the code is done we can have a better idea of what dependencies you actually need.
I have the creeping suspicion you don't realize just how much of a shortcut using fribidi is. All of the stuff we have there already that says "exteremely preliminary" is going to be replaced with approximately four to five lines of code.
Then again, since you will probably only have time to look at it on Monday, and since "the rest of the code" is hopefully going to be ready by then, let's just postpone the discussion a bit :-)
As mentioned already, I think including the fribidi algorithms directly in the code is a better approach, but the only way to really say which is the right way is to get something working.
I talked to Behdad (one of the libfribidi maintainers) about this some time ago. Basically, the Unicode BiDirectional code is fully Unicode 3.2 complient, and no changes are expected there (assuming Unicode don't change the algorithm itself again, like they did between 3.0 and 3.2, 2.1 and 3.0, 2.0 and 2.1, and that's just the versions I have on record. Everything before 2.0 is a total blank to me).
The thing that is not yet complete, and which I have neither the resources nor the knowledge to handle, is the glyphing support (read - Arabic). Since it is interesting to me to have the working (but not interesting enough to try and dust off my extremely rusty Arabic), and since no Arabic speaking programmer vulonteered to step up to the task on either this list, the Israel Linux mailing list, or the Ivrix mailing list (where libfribidi was originally brewed), and since the only Arabic representative who did contact me disappeared with no trace after two emails, if we don't use an external library for this, we won't have any support for this at all.
At the moment, I think it is particularily counter-useful to have mandatory BiDi support in Wine, as this means (among mere applying the BiDi algorithm) translating each string printed to UTF-32 and back (unless we can say for sure that no BiDi is going to be required - havn't found a really good algorithm for that yet, but I'm working on it). Not having the proper library on your machine is a pretty good way of indicating that BiDi is not interesting to you (and is, more or less, the way MS tackled the exact same dillema).
The only thing that can cause me to change my mind about this is if porting libfribidi to UTF-16 will, indeed, be as easy as Behdad suggested. Personally, I have my doubts where sarrogates are used in right-to-left context (hi-lo order may get reveresed, and the characters hopelessly mangled). If, however, we can invest minimal effort and get a UTF-16 fork of libfribidi, I will see no sense in non-Wine distribution of this fork. Please understand, however, that this is a fork, and it will need to remain synchronized with the main branch.
Personally, I think my time would be better spent trying to get the edit control to work BiDi (a problem not solved in mozilla, for example), hunt down why DrawText doesn't always use the correct charset, add the charset selection to the "ChooseFont" dialog, make sure that switching keyboards send out the WM_KEYCHANGE message, or any other of the endless list of tasks still waiting to be done before we can truely say that Wine supports BiDi. There is still a long long way to go, and this is really just the begining.
Shachar
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
I have the creeping suspicion you don't realize just how much of a shortcut using fribidi is. All of the stuff we have there already that says "exteremely preliminary" is going to be replaced with approximately four to five lines of code.
Knowing Microsoft, I have the creeping suspicion that it's not going to be that easy. I'd be very happy to be proven wrong, but there's only one way to do that, and it's to show us the code. Not the configure checks, the actual code.
At the moment, I think it is particularily counter-useful to have mandatory BiDi support in Wine, as this means (among mere applying the BiDi algorithm) translating each string printed to UTF-32 and back (unless we can say for sure that no BiDi is going to be required - havn't found a really good algorithm for that yet, but I'm working on it). Not having the proper library on your machine is a pretty good way of indicating that BiDi is not interesting to you (and is, more or less, the way MS tackled the exact same dillema).
Well, obviously the number one reason for having that code inside Wine is that we can make it work with UTF-16 and skip the conversion step. And if there's really a need to disable BiDi for performance reasons, a config option is a much better way than trying to guess depending on the existence of an external library.
The only interesting question is whether using the external library allows us to do everything we need with reasonable performance, and whether it makes things easier. The only way to answer that is to try it one way or the other and see what you come up with.
If you think using the external library is the way to go, fine, do it that way and submit a patch. Then we can start really discussing it. And don't worry about configure checks or packaging issues; we'll deal with that later on.
Alexandre Julliard wrote:
If you think using the external library is the way to go, fine, do it that way and submit a patch. Then we can start really discussing it. And don't worry about configure checks or packaging issues; we'll deal with that later on.
No actual patch, yet, as I still have a few heap corruptions in certain cases. Attached, however, is the preliminary "Wine with external libfribidi". Do your worst, come back with an opinion. This is not, yet, cleaned up, but the interface is a pretty good representation of what the final version is going to be.
As a bynote, I will add that it seems that libfribidi may not be as mature as I have hoped, and if problems keep popping up, I will put it into our code myself (or, more preciseley, try). It will not be an easy task, as it currently carries it's own unicode tables, configure scripts, and so on. Since you vulenteered to help with this.... ;-).
Shachar
Index: configure.ac =================================================================== RCS file: /home/sun/sources/wine/cvs/wine/configure.ac,v retrieving revision 1.70 diff -u -r1.70 configure.ac --- configure.ac 15 Aug 2002 22:08:41 -0000 1.70 +++ configure.ac 16 Aug 2002 06:44:27 -0000 @@ -434,6 +434,18 @@ fi AC_SUBST(FREETYPEINCL)
+dnl **** Check for fribidi **** +# AC_CHECK_LIB(libfribidi,fribidi_log2vis,fribidi_lib=yes,fribidi_lib=no,) +AC_CHECK_HEADER([fribidi/fribidi.h],fbd_lib=yes,fbd_lib=no) +if test "$fbd_lib"="yes" +then + AC_DEFINE(HAVE_FRIBIDI,1) + AC_DEFINE(HAVE_FRIBIDI_H,1) +else + AC_DEFINE(HAVE_FRIBIDI,0) + AC_DEFINE(HAVE_FRIBIDI_H,0) +fi + dnl **** Check for parport (currently Linux only) **** AC_CACHE_CHECK([for parport header/ppdev.h], ac_cv_c_ppdev, AC_TRY_COMPILE( Index: dlls/gdi/Makefile.in =================================================================== RCS file: /home/sun/sources/wine/cvs/wine/dlls/gdi/Makefile.in,v retrieving revision 1.32 diff -u -r1.32 Makefile.in --- dlls/gdi/Makefile.in 16 Aug 2002 00:42:06 -0000 1.32 +++ dlls/gdi/Makefile.in 16 Aug 2002 06:44:59 -0000 @@ -42,6 +42,7 @@ enhmfdrv/mapping.c \ enhmfdrv/objects.c \ freetype.c \ + fribidi.c \ gdi16.c \ gdi_main.c \ mfdrv/bitblt.c \ Index: include/config.h.in =================================================================== RCS file: /home/sun/sources/wine/cvs/wine/include/config.h.in,v retrieving revision 1.127 diff -u -r1.127 config.h.in --- include/config.h.in 9 Aug 2002 19:49:32 -0000 1.127 +++ include/config.h.in 14 Aug 2002 18:48:23 -0000 @@ -113,6 +113,12 @@ /* Define to 1 if you have the <freetype/tttables.h> header file. */ #undef HAVE_FREETYPE_TTTABLES_H
+/* Define to 1 if fribidi is available during compilation */ +#undef HAVE_FRIBIDI + +/* Define to 1 if you have the <fribidi/fribidi.h> header file. */ +#undef HAVE_FRIBIDI_H + /* Define to 1 if you have the `ftruncate' function. */ #undef HAVE_FTRUNCATE
Index: objects/gdiobj.c =================================================================== RCS file: /home/sun/sources/wine/cvs/wine/objects/gdiobj.c,v retrieving revision 1.78 diff -u -r1.78 gdiobj.c --- objects/gdiobj.c 16 Aug 2002 00:42:07 -0000 1.78 +++ objects/gdiobj.c 16 Aug 2002 06:44:28 -0000 @@ -633,6 +633,10 @@
WineEngInit();
+#if HAVE_FRIBIDI + WineBidiInit(); +#endif + return TRUE; }
Index: objects/text.c =================================================================== RCS file: /home/sun/sources/wine/cvs/wine/objects/text.c,v retrieving revision 1.49 diff -u -r1.49 text.c --- objects/text.c 16 Aug 2002 00:42:07 -0000 1.49 +++ objects/text.c 18 Aug 2002 17:55:07 -0000 @@ -28,6 +28,7 @@ #include "gdi.h" #include "wine/debug.h" #include "winnls.h" +#include "bidi.h"
WINE_DEFAULT_DEBUG_CHANNEL(text);
@@ -158,29 +159,20 @@ else if(dc->funcs->pExtTextOut) { DWORD fontLangInfo=0; - if( !(flags&(ETO_GLYPH_INDEX|ETO_IGNORELANGUAGE)) && - ((fontLangInfo=GetFontLanguageInfo( hdc ))&(GCP_REORDER|GCP_GLYPHSHAPE)) ) + if( !bidi_available && (!(flags&(ETO_GLYPH_INDEX|ETO_IGNORELANGUAGE)) && + ((fontLangInfo=GetFontLanguageInfo( hdc ))&(GCP_REORDER|GCP_GLYPHSHAPE))) ) { /* The caller did not specify that language processing was already done, * and the font idetifies iteself as requiring language processing. */ - GCP_RESULTSW gcp; + WCHAR *lpOutString=HeapAlloc(GetProcessHeap(), 0, count*sizeof(WCHAR));
- gcp.lStructSize=sizeof(gcp); - gcp.lpOutString=HeapAlloc(GetProcessHeap(), 0, count*sizeof(WCHAR)); - gcp.lpOrder=NULL; - gcp.lpDx=NULL; - gcp.lpCaretPos=NULL; - gcp.lpClass=NULL; - gcp.lpGlyphs=NULL; - gcp.nGlyphs=0; - gcp.nMaxFit=0; - - GetCharacterPlacementW(hdc, str, count, 0, &gcp, GCP_REORDER ); + /* XXX need to support explicit requests for LTR or RTL */ + BIDI_log2vis(str, count, 0, lpOutString, NULL, NULL, NULL);
ret = dc->funcs->pExtTextOut(dc->physDev,x,y,flags|ETO_IGNORELANGUAGE, - lprect,gcp.lpOutString,count,lpDx); - HeapFree(GetProcessHeap(), 0, gcp.lpOutString); + lprect,lpOutString,count,lpDx); + HeapFree(GetProcessHeap(), 0, lpOutString); } else ret = dc->funcs->pExtTextOut(dc->physDev,x,y,flags,lprect,str,count,lpDx); } --- /dev/null 2002-07-08 21:54:59.000000000 +0300 +++ dlls/gdi/fribidi.c 2002-08-18 20:49:35.000000000 +0300 @@ -0,0 +1,372 @@ +/* + * The main BiDirectional functions, and interface to FriBiDi + * + * Copyright 2002 Shachar Shemesh. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include "config.h" + +#if HAVE_FRIBIDI + +#include "windef.h" +#include "winerror.h" +#include "winreg.h" +#include "wingdi.h" +#include "wine/unicode.h" +#include "wine/port.h" +#include "wine/debug.h" +#include "gdi.h" +#include "font.h" + +#include "bidi.h" +#if HAVE_FRIBIDI_H +#include <fribidi/fribidi.h> +#endif /* HAVE_FRIBIDI_H */ + +#include <limits.h> + +#ifndef SONAME_LIBFRIBIDI +#define SONAME_LIBFRIBIDI "libfribidi.so" +#endif + +WINE_DEFAULT_DEBUG_CHANNEL(text); + +/* + * A global variable stating whether runtime support for BiDi + * is available + */ +BOOL bidi_available=FALSE; + +static void *fbd_handle = NULL; + +#define MAKE_FUNCPTR(f) static typeof(f) * p##f = NULL; + +MAKE_FUNCPTR(fribidi_log2vis) +MAKE_FUNCPTR(fribidi_remove_bidi_marks) +MAKE_FUNCPTR(fribidi_mirroring_status) +MAKE_FUNCPTR(fribidi_set_mirroring) + + +/* + * Load the fribidi library, and initialize the "bidi_available" var. + */ +BOOL WineBidiInit(void) +{ + if( bidi_available || fbd_handle!=NULL ) + { + WARN("WineBidiInit called after already being initialized\n"); + + return TRUE; + } + + fbd_handle = wine_dlopen(SONAME_LIBFRIBIDI, RTLD_NOW, NULL, 0 ); + if( !fbd_handle ) { + TRACE("Wine cannot find the FriBiDi library. Wine will not have Bidirectional\n" + "support. To gain BiDirectional support, install libfribidi (available\n" + "as a package to your system, or from http://fribidi.sourceforge.net)%5Cn" + ); + + return FALSE; + } + +#define LOAD_FUNCPTR(f) if((p##f = wine_dlsym(fbd_handle, #f, NULL, 0)) == NULL){WARN("Can't find symbol %s\n", #f); goto sym_not_found;} + + LOAD_FUNCPTR(fribidi_log2vis) + LOAD_FUNCPTR(fribidi_remove_bidi_marks) + LOAD_FUNCPTR(fribidi_mirroring_status) + LOAD_FUNCPTR(fribidi_set_mirroring) + +#undef LOAD_FUNCPTR + + bidi_available=TRUE; + + return TRUE; + +sym_not_found: + WINE_MESSAGE( + "Wine cannot find some of the libfribidi function necessary for proper BiDirectional\n" + "support. Please make sure you are using at least version 0.10.4 of libfribidi. If\n" + "it is not available as a package for your system, you can download it from\n" + "http://fribidi.sf.net/%5Cn" + ); + + wine_dlclose(fbd_handle, NULL, 0); + fbd_handle=NULL; + + return FALSE; +} + +#define UTF16_SARROGATE_MASK (0xfc00) +#define UTF16_SARROGATE_HI_VAL (0xd800) +#define UTF16_SARROGATE_LO_VAL (0xdc00) +#define UTF16_SARROGATE_MASK_NOT (0x03ff) +#define UTF16_SARROGATE_ADJUST (0x10000) + +#define UTF16_IS_HI(val) (((val)&UTF16_SARROGATE_MASK)==UTF16_SARROGATE_HI_VAL) +#define UTF16_IS_LO(val) (((val)&UTF16_SARROGATE_MASK)==UTF16_SARROGATE_LO_VAL) + +#define UTF16_HIGH_MARK (0xffff) /* If a value is higher than + this, it needs to be sarrogated */ + +#define UTF32_SARROGATE_COMPOSE(hi,lo) ((((FriBidiChar)((hi)&(UTF16_SARROGATE_MASK_NOT)))<<10)+\ + ((lo)&(UTF16_SARROGATE_MASK_NOT))+UTF16_SARROGATE_ADJUST) + +#define UTF32_SARROGATE_DECOMPOSE_HI(w) ((((w)-UTF16_SARROGATE_ADJUST)>>10)|UTF16_SARROGATE_HI_VAL) +#define UTF32_SARROGATE_DECOMPOSE_LO(w) ((((w)-UTF16_SARROGATE_ADJUST)&UTF16_SARROGATE_MASK_NOT)|UTF16_SARROGATE_LO_VAL) + +#define UNICODE_LRM 0x200e +#define UNICODE_RLM 0x200f + +/*********************************************************************** + * BIDI_wctoFriBidiChar + * + * RETURNS + * + * String provided in WCHARs (UTF-16) in str as FriBidiChar (UTF-32), + * for easy use with the FriBidi library. String will always be NULL + * terminated. + * + * Will return NULL on error + * + * It is up to the caller to free the returned buffer from the process' + * heap. + * + * ERRORS + * + * ERROR_BAD_LENGTH - count (or string length) is so big that + * calculating number of bytes required results + * in an integer overflow + * All errors returnable by HeapAlloc + * + */ +FriBidiChar *BIDI_wctoFribidiChar( + LPCWSTR str, /* [in] Source UTF-16 string */ + INT count, /* [in] Number of characters in str, or -1 if str is NULL + terminated */ + INT *plen, /* [out] If non-NULL, on return it will point to the number + of UTF-32 characters in the string, including nPrePad + and nPostPad */ + INT nPrePad, /* [in] Number of characters to pad at begining of string. */ + INT nPostPad /* [in] Number of characters to pad at end of string. */ + ) +{ + int nChars=count; + + FriBidiChar *ret=NULL; + + if( count==-1 ) + { + int i; + + for( nChars=0,i=0; str[i]!=0; ++nChars ) + { + if( UTF16_IS_HI(str[i]) ) + { + /* We have the start of a sarrogate pair here. */ + if( UTF16_IS_LO(str[i+1]) ) + { + ++i; + } + } + } + } + + /* If we were passed "-1" in count, it will now be the real characters. If not, we + * will just assume that UTF-32 will have the same number of characters as UTF-16. + * We may be allocating more than necessary, but it's cheaper than counting + * sarrogates and then allocating + */ + + /* Make sure we won't copy more than we allocate */ + if( ((DWORD)(nChars+nPrePad+nPostPad+1))>(UINT_MAX/sizeof(FriBidiChar)) ) + { + WARN("BIDI_wctoFribidiChar: Someone tried an interget overflow attack on us\n"); + + SetLastError( ERROR_BAD_LENGTH ); + + return NULL; + } + + ret=(FriBidiChar *)HeapAlloc( GetProcessHeap(), 0, sizeof(FriBidiChar)*(nChars+nPrePad+nPostPad+1)); + + if(ret!=NULL) + { + int len; + int i; + + if( plen==NULL ) + plen=&len; + + for( *plen=0; *plen<nPrePad; ++*plen ) + ret[*plen]=0; + + if( count==-1 ) + { + for( i=0; str[i]!=0; ++i, ++*plen ) + { + if( !UTF16_IS_HI(str[i]) || !UTF16_IS_LO(str[i+1]) ) + { + ret[*plen]=str[i]; + } else + { + ret[*plen]=UTF32_SARROGATE_COMPOSE(str[i], str[i+1]); + ++i; + } + } + } else /* number of chars given in advance */ + { + for( i=0; i<count; ++i, ++*plen ) + { + if( !UTF16_IS_HI(str[i]) || (i+1)>=count || !UTF16_IS_LO(str[i+1]) ) + { + ret[*plen]=str[i]; + } else + { + ret[*plen]=UTF32_SARROGATE_COMPOSE(str[i], str[i+1]); + ++i; + } + } + } + + for( i=0; i<=nPostPad; ++i, ++*plen ) + ret[*plen]=0; + } + + return ret; +} + +/*********************************************************************** + * BIDI_FribidiChartow + * + * RETURNS + * + * On success - number of characters copied. + * On failure - zero. + * + * BUGS + * + * None known. + * + */ +DWORD BIDI_FribidiChartow( + FriBidiChar *src, /* [in] Source UTF-32 string, NULL terminated */ + LPWSTR dst, /* [out] Buffer to receive the UTF-16 string */ + INT dstlen /* [in] Number of characters in "dst" */ + ) +{ + DWORD len=0; + int i; + + for( i=0; len<dstlen && src[i]!=0; ++i,++len ) + { + if( src[i]<=UTF16_HIGH_MARK ) + { + dst[len]=src[i]; + } else /* We need to place a sarrogate here */ + { + dst[len]=UTF32_SARROGATE_DECOMPOSE_HI(src[i]); + if( (++len)<dstlen ) + { + dst[len]=UTF32_SARROGATE_DECOMPOSE_LO(src[i]); + } + } + } + + if( src[i]!=0 ) + { + SetLastError( ERROR_MORE_DATA ); + len=0; + } + + return len; +} + +/*********************************************************************** + * BIDI_log2vis + * + * RETURNS + * + * TRUE if succesful, FALSE if not. + * + * BUGS + * + * Only handles what libfribidi knows how to handle, which is not, yet, + * everything. Hopefully, through the magic of dynamic linking, this + * will improve. + * + * lpClass is unused at this time. + */ +BOOL BIDI_log2vis( + LPCWSTR str, /* [in] Logical order string (wide chars) */ + INT len, /* [in] Length of src */ + DWORD flags, /* [in] Flags for modifying behaviour */ + LPWSTR visual_str, /* [out] The visual order string */ + UINT *position_L_to_V_list, /* [out] Array matching logical index + (array index) with the visual location */ + UINT *position_V_to_L_list, /* [out] Array matching visual index + with the logical location */ + BYTE *embedding_level_list /* [out] Array to be filled with the embedding + levels as defined in the BiDi algorithm. + Even levels mean left-to-right, odd right-to-left */ + ) +{ + int len32; + int ret=FALSE; + FriBidiChar *src32=BIDI_wctoFribidiChar( str, len, &len32, 0, 0 ); + + if( src32!=NULL ) + { + FriBidiChar *dst32=(FriBidiChar *)HeapAlloc(GetProcessHeap(), 0, len32*sizeof(FriBidiChar) ); + FriBidiChar dir; + + switch( flags&BIDI_FLAGS_CONTEXT_MASK ) + { + case BIDI_FLAGS_LTR_CONTEXT: + dir=FRIBIDI_TYPE_L; + break; + case BIDI_FLAGS_RTL_CONTEXT: + dir=FRIBIDI_TYPE_R; + break; + default: + dir=FRIBIDI_TYPE_N; + break; + } + + if( pfribidi_log2vis( src32, len32, &dir, dst32, + position_L_to_V_list, position_V_to_L_list, + embedding_level_list ) ) + { + BIDI_FribidiChartow( dst32, visual_str, len ); + + ret=TRUE; + } else + { + ERR("fribidi_log2vis failed\n"); + } + + HeapFree( GetProcessHeap(), 0, dst32 ); + HeapFree( GetProcessHeap(), 0, src32 ); + } else + { + ERR("libfribidi_log2vis failed\n"); + } + + return ret; +} + +#endif /* HAVE_FRIBIDI */ + --- /dev/null 2002-07-08 21:54:59.000000000 +0300 +++ include/bidi.h 2002-08-17 21:50:42.000000000 +0300 @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2002 Shachar Shemesh + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifdef HAVE_FRIBIDI + +/* + * A global variable stating whether runtime support for BiDi + * is available + */ +extern BOOL bidi_available; + +/* + * Load the fribidi library, and initialize the "bidi_available" var. + */ +BOOL WineBidiInit(void); + +/* + * Convert logical to visual order + */ +BOOL BIDI_log2vis( LPCWSTR str, INT len, DWORD flags, LPWSTR visual_str, + UINT *position_L_to_V_list, UINT *position_V_to_L_list, BYTE *embedding_level_list ); + +/* Don't leave the paragraph context to be guessed */ +#define BIDI_FLAGS_LTR_CONTEXT 0x00000001 +#define BIDI_FLAGS_RTL_CONTEXT 0x00000002 +#define BIDI_FLAGS_CONTEXT_MASK 0x00000003 + +#else +/* The optimizer should be able to figure out that an "if(FALSE)" will never + * take place. This saves callers from checking the "HAVE_FRIBIDI" macro. + */ +#define bidi_available FALSE +#endif /* HAVE_FRIBIDI */ +
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
No actual patch, yet, as I still have a few heap corruptions in certain cases. Attached, however, is the preliminary "Wine with external libfribidi". Do your worst, come back with an opinion. This is not, yet, cleaned up, but the interface is a pretty good representation of what the final version is going to be.
What I don't see is how you are going to implement all of the GetCharacterPlacement features, like the various override flags, the lpClass array, etc. Is fribidi going to support all of that the way we need it?
As a bynote, I will add that it seems that libfribidi may not be as mature as I have hoped, and if problems keep popping up, I will put it into our code myself (or, more preciseley, try). It will not be an easy task, as it currently carries it's own unicode tables, configure scripts, and so on. Since you vulenteered to help with this.... ;-).
Sure, I can do the unicode tables if you tell me exactly what information you need.
Alexandre Julliard wrote:
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
No actual patch, yet, as I still have a few heap corruptions in certain cases. Attached, however, is the preliminary "Wine with external libfribidi". Do your worst, come back with an opinion. This is not, yet, cleaned up, but the interface is a pretty good representation of what the final version is going to be.
What I don't see is how you are going to implement all of the GetCharacterPlacement features, like the various override flags, the lpClass array, etc.
The only overrides in "GetCharacterPlacement" are the lpClass parameter. The few weeks of silence from me were spent trying to investigate every aspect of GetCharacterPlacement. The results are, roughly:
1. This function is impossible. It has overlapping arguments, docs are extremely unclear as to what arguments you can avoid supplying, and what arguments are mandatory. 2. The function is supposed to let you do all reordering once, and then repeat the actual printing as many times as necessary. In practice, I find it hard to believe that this is a high priority issue for anyone. 3. Things are further complicated by the fact that it has no out of bounds way of specifying what the base direction is (and, MS being MS, this means that a base direction of LTR is chosen, and not according to the Unicode algorithm).
In short, I find it hard to believe that ANYONE would really use that function at all. The lpClass were particularily annoying, as they obviously did SOMETHING, but defenitely not what you would expect. Why is that relevant? Read a little further.
Is fribidi going to support all of that the way we need it?
Is fribidi going to support all the GetCharacterPlacement functionality? No, classes are not going to be supported by it. Everything else should not pose a problem.
Is fribidi going to support all the functionality we are likely to need? Yes, I believe it will. That is why I tried to stress the amount of shortcutting this library is going to get us. Implementing the various unicode algorithms is a pain, and one to be avoided. If, at some stage in the future, it turns out that some programs really do need classes from GetCharacterPlacement, well, we'll have this discussion again.
As a bynote, I will add that it seems that libfribidi may not be as mature as I have hoped, and if problems keep popping up, I will put it into our code myself (or, more preciseley, try). It will not be an easy task, as it currently carries it's own unicode tables, configure scripts, and so on. Since you vulenteered to help with this.... ;-).
Sure, I can do the unicode tables if you tell me exactly what information you need.
Wouldn't that create strange dependancies between modules? Who currently "owns" the unicode tables?
Shachar
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
- Things are further complicated by the fact that it has no out of bounds way of specifying what the base direction is (and, MS being MS, this means that a base direction of LTR is chosen, and not according to the Unicode algorithm).
Isn't that what the GCPCLASS_PREBOUNDLTR etc. flags are about? Or is this something different?
Is fribidi going to support all the functionality we are likely to need? Yes, I believe it will. That is why I tried to stress the amount of shortcutting this library is going to get us. Implementing the various unicode algorithms is a pain, and one to be avoided. If, at some stage in the future, it turns out that some programs really do need classes from GetCharacterPlacement, well, we'll have this discussion again.
That's not acceptable. It's perfectly OK to say that we won't implement some things until we find something that needs them, but it's not OK to pick a design that will prevent us from implementing them once they are needed (and they *will* become needed someday).
Wouldn't that create strange dependancies between modules? Who currently "owns" the unicode tables?
The Unicode tables are part of libwine_unicode, so that they can be shared between all the modules that need them. So that's not a problem.
Alexandre Julliard wrote:
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
- Things are further complicated by the fact that it has no out of bounds way of specifying what the base direction is (and, MS being MS, this means that a base direction of LTR is chosen, and not according to the Unicode algorithm).
Isn't that what the GCPCLASS_PREBOUNDLTR etc. flags are about? Or is this something different?
You would think so, wouldn't you? I mean, that would be the logical thing to do. When reading the manual, it is obvious that this is what's its there for. It does seem a bit strange that you would have to allocate an array the size of the string just for this single byte specifying direction, but still....
While on the subject, using the unicode standard of upercase means Hebrew and lowercase means English, you'd expect the following string: english HEBREW when used with clases saying all characters are Hebrew, to be reordered to: WERBEH hsilgne What you WOULDNT expect, is to get eWERBEH hsilgn
The sad truth of the matter is that a LOT more investigation will be required until we realize what lpClass is doing, and I don't think any commercial or other needed that function badly enough to go through this investigation. The end result is that I don't believe we will EVER really have to do that research.
Is fribidi going to support all the functionality we are likely to need? Yes, I believe it will. That is why I tried to stress the amount of shortcutting this library is going to get us. Implementing the various unicode algorithms is a pain, and one to be avoided. If, at some stage in the future, it turns out that some programs really do need classes from GetCharacterPlacement, well, we'll have this discussion again.
That's not acceptable. It's perfectly OK to say that we won't implement some things until we find something that needs them, but it's not OK to pick a design that will prevent us from implementing them once they are needed (and they *will* become needed someday).
I may have came across wrong. I am not suggesting we stick to libfribidi forever, whatever it can do is fine, and what it can't won't be done. To emphasize this point, you will notice that my patch does not export any of libfribidi's functions. In retrospect, I think I'll rename the .c file to "bidi" - will be more apropriate.
I am saying that it is covering all of our current needs, and thus we should go for it as it saves us somewhere between a month of work and half a year (calendaric time, estimates based on assumption that I'm the only one working on it). If at some future time we come to the conclusion that libfribidi is not enough, we can either add the required functionality to it, integrate it into Wine or replace it altogether. I am hoping that, by that time, interest in wine will be high enough for more people to be involved.
In any case, I tried to create the interfaces so that "fribidi" is not a dominant part of them.
Shachar
Shachar Shemesh wine-devel@sun.consumer.org.il writes:
I may have came across wrong. I am not suggesting we stick to libfribidi forever, whatever it can do is fine, and what it can't won't be done. To emphasize this point, you will notice that my patch does not export any of libfribidi's functions. In retrospect, I think I'll rename the .c file to "bidi" - will be more apropriate.
I am saying that it is covering all of our current needs, and thus we should go for it as it saves us somewhere between a month of work and half a year (calendaric time, estimates based on assumption that I'm the only one working on it). If at some future time we come to the conclusion that libfribidi is not enough, we can either add the required functionality to it, integrate it into Wine or replace it altogether. I am hoping that, by that time, interest in wine will be high enough for more people to be involved.
What I'm saying is that it's not a good idea to start using fribidi, create dependencies and problems for packagers, etc. if we know that it's a wrong design and that we will need to replace it. The truth is that bidi is not really a priority feature (as shown by the number of people interested in making it work), and so it doesn't really matter if it works tomorrow or only in 3 months. What matters is to pick a correct design so that if more people start to care they can build on it, instead of having to throw it away and restart from scratch.
Alexandre Julliard wrote:
What I'm saying is that it's not a good idea to start using fribidi, create dependencies and problems for packagers, etc. if we know that it's a wrong design and that we will need to replace it.
Ok, accepted. Not only by me, but also by Bidi's maintainers.
I'm currently shifting my efforts to creating the required infrastructure in fribidi for supporting all of our current and future needs (UTF-16 included). Once that's in fribidi, I trust depending on it will not be considered a problem, right?
The truth is that bidi is not really a priority feature (as shown by the number of people interested in making it work),
Ouch :-(. Actually, there is a lot of interest in people wanting Wine to support Bidi (or at least, Hebrew). This can be seen from the comments on any of the many articles in recent press about Wine, Lindows etc. Sadly, this did not create a movement of developers, true (unless you count yours truely as a "movement").
and so it doesn't really matter if it works tomorrow or only in 3 months. What matters is to pick a correct design so that if more people start to care they can build on it, instead of having to throw it away and restart from scratch.
Point taken and observed. I'll make sure fribidi has, at least, the final package dependancy (I don't know, yet, whether packagers will choose to package fribidi-utf16 in a different package), and I'll resubmit. I have, pretty much, a guarantee from Behdad that the required changes will be supported.
Time to direct efforts torwards a better fribidi design.
Shachar
-
Alexandre Julliard -
Dimitrie O. Paun -
Dimitrie O. Paun -
Shachar Shemesh