Closed Bug 84380 Opened 24 years ago Closed 23 years ago

Need a component that generates thai presentation forms

Categories

(Core :: Internationalization, defect, P3)

Product:
Core
Component:
Internationalization
Platform:
Sun
Solaris
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.8

People

(Reporter: prabhat.hegde, Assigned: roland.mainz)

References

Details

(Keywords: intl)

Attachments

(2 files, 7 obsolete files)

Sample CTLized output
97.22 KB, image/gif
Details
Patch for 2002-01-07-08-trunk
7.49 KB, patch
Details | Diff | Splinter Review
To be able to view thai web-pages in *nix, we need to be able to generate presentation forms (context-sensitive shaping). The approach adopted here is to use the module and shaper api from pango (www.pango.org) alongwith the implementation of the thai shaper and plug it into mozilla/intl/ctl. Another approach would be to write gfx-wrappers for pango api, but that will involve significant work since there are multiple areas of pango api/functionality that is duplicated in gfx. Also, the pango implementation is based on latest versions of glib and gmodule which means that mozilla code needs to depend on the development version of glib/gmodule. The patch attached here provides a lightweight version of pango api that eliminates the dependencies mentioned above while maintaining as much source-level compatibility as possible. It also provides implementation of an XPCOM object that calls the pango api. Hooking up this module with gfx or nsIUnicodeEncoder/nsICharRepresentible should enable thai rendering support on *nix platforms.
diff file follows below: This should go with new sources (ctl.zip) that i will be attaching in *.zip format ? config/final-link-libs ? config/mkdepend/mkdepend ? intl/ctl Index: configure =================================================================== RCS file: /cvsroot/mozilla/configure,v retrieving revision 1.765 diff -u -r1.765 configure --- configure 2001/05/24 02:14:42 1.765 +++ configure 2001/06/06 23:12:24 @@ -114,6 +114,8 @@ ac_help="$ac_help --disable-bidi Disable bi-directional support" ac_help="$ac_help + --enable-ctl Enable Complex Text Support" +ac_help="$ac_help --enable-monolithic-toolkit Link the toolkit into the app" ac_help="$ac_help @@ -11115,6 +11117,24 @@ fi +# Check whether --enable-ctl or --disable-ctl was given. +if test "${enable_ctl+set}" = set; then + enableval="$enable_ctl" + if test "$enableval" = "yes"; then + SUNCTL=1 + cat >> confdefs.h <<\EOF +#define SUNCTL 1 +EOF + + elif test "$enableval" = "no"; then + : + else + { echo "configure: error: Option, ctl, does not take an argument ($enableval). " 1>&2; exit 1; } + fi + +fi + + MOZ_MONOLITHIC_TOOLKIT=1 case "$target" in @@ -13621,6 +13641,7 @@ s%@MOZ_STATIC_COMPONENTS@%$MOZ_STATIC_COMPONENTS%g s%@ENABLE_TESTS@%$ENABLE_TESTS%g s%@IBMBIDI@%$IBMBIDI%g +s%@SUNCTL@%$SUNCTL%g s%@MOZ_USER_DIR@%$MOZ_USER_DIR%g s%@FULL_STATIC_BUILD@%$FULL_STATIC_BUILD%g s%@BUILD_IDLC@%$BUILD_IDLC%g Index: configure.in =================================================================== RCS file: /cvsroot/mozilla/configure.in,v retrieving revision 1.864 diff -u -r1.864 configure.in --- configure.in 2001/05/24 02:07:36 1.864 +++ configure.in 2001/06/06 23:12:25 @@ -3060,6 +3060,16 @@ AC_DEFINE(IBMBIDI) fi + +dnl =============================================== +dnl = --enable-ctl +dnl = Enable Complex Text Support. OFF by default. +dnl =============================================== +MOZ_ARG_ENABLE_BOOL(ctl, +[ --enable-ctl Enable Complex Text Support ], + [ SUNCTL=1 + AC_DEFINE(SUNCTL)] ) + dnl ======================================================== dnl = dnl = --enable-monolithic-toolkit @@ -4073,6 +4083,7 @@ AC_SUBST(MOZ_STATIC_COMPONENTS) AC_SUBST(ENABLE_TESTS) AC_SUBST(IBMBIDI) +AC_SUBST(SUNCTL) AC_SUBST(MOZ_USER_DIR) AC_SUBST(FULL_STATIC_BUILD) AC_SUBST(BUILD_IDLC) Index: config/autoconf.mk.in =================================================================== RCS file: /cvsroot/mozilla/config/autoconf.mk.in,v retrieving revision 3.189 diff -u -r3.189 autoconf.mk.in --- config/autoconf.mk.in 2001/05/22 07:52:25 3.189 +++ config/autoconf.mk.in 2001/06/06 23:12:25 @@ -77,6 +77,7 @@ MOZ_STATIC_COMPONENTS = @MOZ_STATIC_COMPONENTS@ ENABLE_TESTS = @ENABLE_TESTS@ IBMBIDI = @IBMBIDI@ +SUNCTL = @SUNCTL@ BUILD_IDLC = @BUILD_IDLC@ MOZ_EDITOR_API_LOG = @MOZ_EDITOR_API_LOG@ MOZ_ENDER_LITE = @MOZ_ENDER_LITE@ Index: intl/Makefile.in =================================================================== RCS file: /cvsroot/mozilla/intl/Makefile.in,v retrieving revision 1.7 diff -u -r1.7 Makefile.in --- intl/Makefile.in 1999/11/06 03:22:48 1.7 +++ intl/Makefile.in 2001/06/06 23:12:33 @@ -28,5 +28,9 @@ DIRS = uconv unicharutil locale strres lwbrk chardet +ifdef SUNCTL +DIRS += ctl +endif + include $(topsrcdir)/config/rules.mk
Blocks: 79284
Created an attachment in *.zip format. (for lack of understanding of a better type, i've chosen binary for attachment). Also, is there a way to avoid making multiple updates to same bug and do it all in one operation? thanks,
Blocks: thai
Reassign to ftang, cc to bstell.
Assignee: nhotta → ftang
Keywords: intl
mark assign to ftang and target moz1.0.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
the patch (06/12/01 07:46 above) minimizes/eliminates any risk whatsoever of thai ctl rendering code affecting any non-unix, non-intended platforms. It is expected to be enabled using configure option : --with-extensions=default,ctl. In addition the hook that uses this piece of code will be enabled only under --enable-ctl. This means that a *nix build that needs to generate thai presentation forms before rendering needs to : use : configure --with-extensions=default,ctl --enable-ctl <other options> Unix builds that do not use the above mentioned flags are not expected to be affected in any way. frank, pl let me know how to proceed furthur on this - thanks,
Changing QA contact to katakai@japan.sun.com. Please re-assign further as appropriate.
QA Contact: andreasb → katakai
I assume you want to check in the http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38075 into mozilla/extensions/ctl since extensions/ctl is not part of the browser, you could check in without suprereview.
hi masaki, Could you please review this latest patch that will need to get into extensions/ctl and checkin? Frank has created a directory ctl under extensions and also added a readme file there. Note this patch contains: pango code and wrapper. nsIUnicodeEncoder implementation for tis620. The way this is expected to be called in gfx is to do a XPCOM query for this interface and use it if it exists. thanks, prabhat.
Frank, Could you assign to me for this bug? I'd like to check in the files for Prabhat. I understand the check in does not need sr= because the files will be put under extansions/. But is r= required? If so, can you ask someone of i18n team for review? I have verified the files could be compiled under Linux and Solaris platform without errors.
reassign to katakai
Assignee: ftang → katakai
Status: ASSIGNED → NEW
Frank, can I check in the files without any r= ?
masaki, this *underdevelopment* components is not yet build by default. check it in w/o r/sr for now. (Don't change anything outside /extensions/ctl) Then, we will ask Owen Taylor to review the Pango code and I will review the rest.
files checked in uner extensions/ctl/
r=ftang for the following files: extensions/ctl/Makefile.in extensions/ctl/public/Makefile.in extensions/ctl/public/nsCtlCIID.h extensions/ctl/src/Makefile.in extensions/ctl/src/nsCtlLEModule.cpp Issue about extensions/ctl/src/nsUnicodeToTIS620.h 83 PRUint8 state; 84 PRInt32 byteOff; 85 PRInt32 charOff; please use mState, mByteOff, mCharOff etc 89 int Itemize(const PRUnichar* aSrcBuf, PRInt32 aSrcLen, textRunList *aRunList); Please use PRInt32 Itemize about extensions/ctl/src/nsUnicodeToTIS620.cpp: 143 nsCOMPtr<nsILE> mCtlObj; 144 static NS_DEFINE_CID(kLECID, NS_ULE_CID); 145 nsresult rv; 146 textRunList txtRuns; 147 textRun *aPtr, *aTmpPtr; 148 149 // No question of starting the conversion from an offset 150 charOff = byteOff = 0; 151 152 mCtlObj = do_CreateInstance(kLECID, &rv); 1. should "nsCOMPtr<nsILE> mCtlObj;" be a private member data of class nsUnicodeToTIS620 and create in the constructor instead? 2. How can do_CreateInstance work for nsILE ? I see no code implement the factor for kLECID. Since nsULE is implemented inside the same component, should we create it by calling new nsULE() instead ? I really want to hide nsILE inside this compoenent and make it a private interface inside the component for now. 3. 153 if (NS_FAILED(rv)) { 154 printf("ERROR: Cannot create instance of component " NS_ULE_PROGID " [%x].\n", 155 rv); 156 printf("Thai Text Layout Will Not Be Supported\n"); 157 } I see no error handling code which prevent crashing while you cannot create mCtlObj . Please add such code. Please wrap any printf inside #ifdef DEBUG code 4. any reason to have the following two lines around? 25 #include <stdio.h> 26 #include <ctype.h> /* isspace */ Issue about extensions/ctl/public/nsILE.h it include "pango-types.h" and "pango-glyph.h" and it also does not meet XPCOM interface guideline. Since we do not need nsILE interface for now, I suggest we move this into extensions/ctl/src as private interface for now. Please get owen to review extensions/ctl/src/pangoLite/* and extensions/ctl/src/thaiShaper/* . Thanks
hi frank, Have incorporated your r= suggestions. Can you r= once more and i will request masaki to checkin. thanks, prabhat.
r=ftang for (the current checked in source + the latest diff) mozilla/extensions/ctl/public/nsILE.h mozilla/extensions/ctl/src/nsCtlLEModule.cpp mozilla/extensions/ctl/src/nsULE.cpp mozilla/extensions/ctl/src/nsULE.h mozilla/extensions/ctl/src/nsUnicodeToTIS620.cpp mozilla/extensions/ctl/src/nsUnicodeToTIS620.h mozilla/gfx/src/gtk/nsFontMetricsGTK.cpp However, I see you use InitGlobals and FreeGlobals incorrectly in your code. a. InitGlobals currently return PRBool, but your return code return nsresult code ( return NS_ERROR_FAILURE or return NS_OK ). Please change it to make it consistent. Also, make sure the checking of InitGlobals return is consistent. b. You seems init and free globals every conversion. Is that what you really want to do ? I see no benefit from this code, you should do EITHER (not both) (i) move gCharSetManager and gDefaultTISConverter from file global into functional stack variable and create destroy every time, or (ii) implement object counting ( +1 in nsUnicodeToTIS620::nsUnicodeToTIS620 and -1 in nsUnicodeToTIS620::~nsUnicodeToTIS620) and call FreeGlobals while the count is equal to 0 in nsUnicodeToTIS620::~nsUnicodeToTIS620 since this is not turn on by default, why don't you check in what you have now and make a seperate patch to address my comment .
for the change in mozilla/gfx/src/gtk/nsFontMetricsGTK.cpp, read my comment in 84394
hi masaki, Have attached a zip file which contain both the diff and the actual sources. The changes w.r.t current ctl extensions are the following: A> Incorporate frank's review fixes (08/07) B> Bugfix in TIS620Encoder C> Fix memory corruption D> Makefile changes in pangoLite directory to install pango.modules Could you please checkin the same for me, thanks? prabhat.
patch checked-in. Makefile.in and pango.modules in pangoLite/ are not in diff but I think those are needed so I've checked in them. It seems that current configure script was broken when we specify --enable-ctl option. I'll ask leaf to fix the problem.
filed bug as bug 100275 for configure problem, assigned to leaf.
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 (you can query for this string to delete spam or retrieve the list of bugs I've moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
pango-util.c has an ifdef HAVE_FLOCKFILE yet there is no way to turn this on... shouldn't something be put in configure & configure.in? On AIX I can't built because getc_unlocked is already defined (in stdio.h) and yet because there is no test to set the define HAVE_FLOCKFILE we try to do it twice. someone needs to write the configure.in test to turn this on (IMHO) if we are going to keep pangoLite as part of the build... so if this is indeed targeted for 1.0.1 then we should either remove it from extensions=all and have another enable technique
jdunn: Enabling CTL for all platforms was bug 115840. Can you please open a new bug, component "build config" and CC: me, please ?
Prabhat/katakai: Can you please file a new all-in-one patch (no ZIP file, only plain gdiff -u, please) for the CTL-stuff which is not yet in the "trunk" ? I'd like to _push_ this bug a little bit if possible...
hi roland, Here the diff of stuff left to be checked in. In my opinion very small (< 100 lines for both rendering and text edit operations) and localized with only dependency i can think of being glib.h. I also built and tested pangolite (ie, the extensions/ctl stuff) on windows a long while ago. Will try to find the makefile.win patches to test build on windows. Note - I've never tried to test rendering using pangolite on windows. Just that the platform independant source (pangolite) is buildable on windows) with just the glib.h dependency. Am also attaching gif files for output without and with ctl support. Thanks to masaki-san for XPrint related diffs. Also, arthit (who's ccd on this bug) is the thai user who's been testing this. Index: gfx/src/gtk/Makefile.in =================================================================== RCS file: /cvsroot/mozilla/gfx/src/gtk/Makefile.in,v retrieving revision 1.66 diff -u -r1.66 Makefile.in --- gfx/src/gtk/Makefile.in 2001/11/22 11:13:20 1.66 +++ gfx/src/gtk/Makefile.in 2002/01/09 01:27:05 @@ -117,6 +117,12 @@ EXTRA_DSO_LDOPTS += $(MOZ_XPRINT_LDFLAGS) endif +ifdef SUNCTL +INCLUDES += \ + -I$(DIST)/include/ctl \ + $(NULL) +endif + LOCAL_INCLUDES = \ -I$(srcdir)/. \ -I$(srcdir)/.. \ Index: gfx/src/gtk/nsFontMetricsGTK.cpp =================================================================== RCS file: /cvsroot/mozilla/gfx/src/gtk/nsFontMetricsGTK.cpp,v retrieving revision 1.189 diff -u -r1.189 nsFontMetricsGTK.cpp --- gfx/src/gtk/nsFontMetricsGTK.cpp 2001/12/04 00:47:36 1.189 +++ gfx/src/gtk/nsFontMetricsGTK.cpp 2002/01/09 01:27:06 @@ -307,8 +307,13 @@ static nsFontCharSetInfo KOI8U = { "KOI8-U", SingleByteConvert, 0 }; static nsFontCharSetInfo TIS620 = +// Added to support thai context sensitive shaping if +// --enable-ctl is in force +#ifdef SUNCTL + { "tis620-2", SingleByteConvert, 0 }; +#else { "TIS-620", SingleByteConvert, 0 }; - +#endif static nsFontCharSetInfo Big5 = { "x-x-big5", DoubleByteConvert, 1 }; static nsFontCharSetInfo CNS116431 = Index: layout/html/base/src/Makefile.in =================================================================== RCS file: /cvsroot/mozilla/layout/html/base/src/Makefile.in,v retrieving revision 1.74 diff -u -r1.74 Makefile.in --- layout/html/base/src/Makefile.in 2001/12/06 05:45:01 1.74 +++ layout/html/base/src/Makefile.in 2002/01/09 01:27:10 @@ -145,6 +145,12 @@ DEFINES += -D_IMPL_NS_HTML +ifdef SUNCTL +INCLUDES += \ + -I$(DIST)/include/ctl \ + $(NULL) +endif + INCLUDES += \ -I$(srcdir)/../../../xul/base/src \ -I$(srcdir)/../../../xul/content/src \ Index: layout/html/base/src/nsTextFrame.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/base/src/nsTextFrame.cpp,v retrieving revision 1.342 diff -u -r1.342 nsTextFrame.cpp --- layout/html/base/src/nsTextFrame.cpp 2001/12/07 14:50:43 1.342 +++ layout/html/base/src/nsTextFrame.cpp 2002/01/09 01:27:11 @@ -100,6 +100,10 @@ //ahmed end #endif // IBMBIDI +#ifdef SUNCTL +#include "nsILE.h" +#endif + #ifndef PR_ABS #define PR_ABS(x) ((x) < 0 ? -(x) : (x)) #endif @@ -2297,6 +2301,35 @@ while (sdptr){ sdptr->mStart = ip[sdptr->mStart] - mContentOffset; sdptr->mEnd = ip[sdptr->mEnd] - mContentOffset; +#ifdef SUNCTL + static NS_DEFINE_CID(kLECID, NS_ULE_CID); + + nsCOMPtr<nsILE> mCtlObj; + mCtlObj = do_CreateInstance(kLECID, &rv); + if (NS_FAILED(rv)) { + NS_WARNING("Cell based Cursor Movement Will Not Be Supported\n"); + mCtlObj = nsnull; + } + else { + PRInt32 mStart, mEnd; + + if (sdptr->mEnd < textLength) { + mCtlObj->GetRangeOfCluster(text, PRInt32(textLength), sdptr->mEnd, + &mStart, &mEnd); + if (sdptr->mStart > sdptr->mEnd) /* Left Edge */ + sdptr->mEnd = mStart; + else + sdptr->mEnd = mEnd; + } + + /* Always start selection from a Right Edge */ + if (sdptr->mStart > 0) { + mCtlObj->GetRangeOfCluster(text, PRInt32(textLength), + sdptr->mStart, &mStart, &mEnd); + sdptr->mStart = mEnd; + } + } +#endif #ifdef IBMBIDI AdjustSelectionPointsForBidi(sdptr, textLength, CHARTYPE_IS_RTL(charTyp e), level & 1, isBidiSystem); #endif @@ -3909,12 +3942,31 @@ #endif aPos->mContentOffset = 0; PRInt32 i; +#ifdef SUNCTL + static NS_DEFINE_CID(kLECID, NS_ULE_CID); + + nsCOMPtr<nsILE> mCtlObj; + mCtlObj = do_CreateInstance(kLECID, &rv); + if (NS_FAILED(rv)) { + NS_WARNING("Cell based Cursor Movement Will Not Be Supported\n"); + mCtlObj = nsnull; +#endif for (i = aPos->mStartOffset -1 - mContentOffset; i >=0; i--){ if (ip[i] < ip[aPos->mStartOffset - mContentOffset]){ aPos->mContentOffset = i + mContentOffset; break; } } +#ifdef SUNCTL + } + else { + PRInt32 mPreviousOffset; + mCtlObj->PrevCluster((const PRUnichar*)paintBuffer.mBuffer, + textLength,aPos->mStartOffset,&mPreviousOffset); + aPos->mContentOffset = i = mPreviousOffset; + } +#endif + if (i <0){ found = PR_FALSE; GetPrevInFlow(&frameUsed); @@ -3930,12 +3982,32 @@ #endif PRInt32 i; aPos->mContentOffset = mContentLength; +#ifdef SUNCTL + static NS_DEFINE_CID(kLECID, NS_ULE_CID); + + nsCOMPtr<nsILE> mCtlObj; + mCtlObj = do_CreateInstance(kLECID, &rv); + if (NS_FAILED(rv)) { + NS_WARNING("Cell based Cursor Movement Will Not Be Supported\n"); + mCtlObj = nsnull; +#endif + for (i = aPos->mStartOffset +1 - mContentOffset; i <= mContentLength; i++){ if (ip[i] > ip[aPos->mStartOffset - mContentOffset]){ aPos->mContentOffset = i + mContentOffset; break; } } +#ifdef SUNCTL + } + else { + PRInt32 mNextOffset; + mCtlObj->NextCluster((const PRUnichar*)paintBuffer.mBuffer, + textLength, aPos->mStartOffset, &mNextOffset); + aPos->mContentOffset = i = mNextOffset; + } +#endif + /* if (aStartOffset == 0 && (mState & TEXT_SKIP_LEADING_WS)) i--; //back up because we just skipped over some white space. why skip over the char also? */ Index: nsFontMetricsXlib.cpp =================================================================== RCS file: /cvsroot/mozilla/gfx/src/xlib/nsFontMetricsXlib.cpp,v retrieving revision 1.88 diff -u -r1.88 nsFontMetricsXlib.cpp --- nsFontMetricsXlib.cpp 2001/12/04 00:47:38 1.88 +++ nsFontMetricsXlib.cpp 2001/12/14 02:28:10 @@ -278,7 +278,12 @@ static nsFontCharSetXlibInfo KOI8U = { "KOI8-U", SingleByteConvert, 0 }; static nsFontCharSetXlibInfo TIS620 = +// --enable-ctl is in force +#ifdef SUNCTL + { "tis620-2", SingleByteConvert, 0 }; +#else { "TIS-620", SingleByteConvert, 0 }; +#endif static nsFontCharSetXlibInfo Big5 = { "x-x-big5", DoubleByteConvert, 1 }; Pl let me know if you have any issues. thanks for your help, prabhat.
Stealing from katakai ...
Assignee: katakai → Roland.Mainz
Attached image Sample CTLized output —
BTW, i am trying a bidi-enabled build, pl disregard orientation.
Status: NEW → ASSIGNED
Can we other CLOSE or DUPLICATE (of this one) all other CTL-extension-related bugs (assuming they are fixed by my upcoming patch...), please ?
Priority: -- → P3
Target Milestone: mozilla1.0.1 → mozilla0.9.8
Patch for GTK+ gfx, Xlib gfx, Xprint gfx and layout I've added /usr/openwin/lib/locale/th_TH/X11/fonts/TrueType and /usr/openwin/lib/locale/th_TH/X11/fonts/75dpi to my X font path (xset +fp path_to_font) and started Xprt with |Xprt -fp $(xset q | fgrep "/X11/fonts/misc/") -audit 4 :1| After that procudure I can view and print (via Xprint module) Thai (minor issue: I can't speak/read thai :)
Attachment #37444 - Attachment is obsolete: true
Attachment #38075 - Attachment is obsolete: true
Attachment #40322 - Attachment is obsolete: true
Attachment #41037 - Attachment is obsolete: true
Attachment #41038 - Attachment is obsolete: true
Attachment #44719 - Attachment is obsolete: true
Attachment #49640 - Attachment is obsolete: true
We need reviews from the following people: - katakai (or prabhat) for the patch in general - dbaron for the changes in layout/ - dauphin for the Makefile.in changes After that I'll request superreview...
Keywords: patch, review
Is this a Solaris only feature? If not, we might change SUNCTL to something less Solaris sounding?
r=cls on the Makefile.in changes. I believe SUNCTL is used in the same vein as IBMBIDI, meaning that the company provided the implementation not that the feature is platform specific.
Brian Stell wrote: > Is this a Solaris only feature? Not really. > If not, we might change SUNCTL to something less Solaris sounding? Uhm... Sun people conntributed the code - like IBM people contributed their BIDI code - which is #ifdef'ed with the IBMBIDI symbol. Let's stick with the symbol... :)
This is okay with me.
r=prabhat.hegde@sun.com for non Makefile portion of Roland's patch. One issue i can think of, is that Makefiles to build extensions/ctl on windows is needed before ctl is ready to be turned on by default in layout. Regarding SUNCTL, pl feel free to change it as fit - not a problem for me.
sr=attinasi for the layout changes
Patch checked in (http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=day&who=timeless%25mac.com), marking bug as FIXED.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verified on Solaris with --enable-ctl flag, www.thai.net can be browsed properly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: