From 81b487a13cf938a65bb309aa132f60db67de7d27 Mon Sep 17 00:00:00 2001 From: jenglish Date: Wed, 26 Mar 2008 19:04:08 +0000 Subject: XIM reorganization and cleanup; see [Patch 1919791] for details. --- ChangeLog | 6 +++ generic/tkEvent.c | 128 ++++++++++++++++++++--------------------------------- generic/tkInt.h | 18 +------- unix/tcl.m4 | 4 -- unix/tkConfig.h.in | 3 -- unix/tkUnixEvent.c | 118 ++++++++++++++---------------------------------- unix/tkUnixKey.c | 73 +++++++++++++++--------------- 7 files changed, 124 insertions(+), 226 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4e64393..c4ec21f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2008-03-26 Joe English + + * generic/tkInt.h, generic/tkEvent.c, unix/tkUnixEvent.c, + unix/tkUnixKey.c: XIM reorganization and cleanup; see + [Patch 1919791] for details. + 2008-03-21 Joe English * generic/tk.decls, generic/ttk/ttkStubLib.c, unix/Makefile.in: diff --git a/generic/tkEvent.c b/generic/tkEvent.c index fda8ac9..ffc8484 100644 --- a/generic/tkEvent.c +++ b/generic/tkEvent.c @@ -12,7 +12,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkEvent.c,v 1.34 2006/11/24 18:04:14 jenglish Exp $ + * RCS: @(#) $Id: tkEvent.c,v 1.35 2008/03/26 19:04:09 jenglish Exp $ */ #include "tkInt.h" @@ -210,9 +210,7 @@ static void UpdateButtonEventState(XEvent *eventPtr); static int WindowEventProc(Tcl_Event *evPtr, int flags); #ifdef TK_USE_INPUT_METHODS static int InvokeInputMethods(TkWindow *winPtr, XEvent *eventPtr); -#if TK_XIM_SPOT -static void CreateXIMSpotMethods(TkWindow *winPtr); -#endif /* TK_XIM_SPOT */ +static void CreateXIC(TkWindow *winPtr); #endif /* TK_USE_INPUT_METHODS */ /* @@ -319,69 +317,52 @@ InvokeMouseHandlers( /* *---------------------------------------------------------------------- * - * CreateXIMSpotMethods -- + * CreateXIC -- * - * Create the X input methods for our winPtr. XIM is only ever enabled on - * Unix. - * - * Results: - * None. - * - * Side effects: - * An input context is created or we Tcl_Panic. + * Create the X input context for our winPtr. + * XIM is only ever enabled on Unix. * *---------------------------------------------------------------------- */ -#if defined(TK_USE_INPUT_METHODS) && TK_XIM_SPOT +#if defined(TK_USE_INPUT_METHODS) static void -CreateXIMSpotMethods( +CreateXIC( TkWindow *winPtr) { TkDisplay *dispPtr = winPtr->dispPtr; + long im_event_mask = 0L; + const char *preedit_attname = NULL; + XVaNestedList preedit_attlist = NULL; - if (dispPtr->flags & TK_DISPLAY_XIM_SPOT) { - XVaNestedList preedit_attr; + if (dispPtr->inputStyle & XIMPreeditPosition) { XPoint spot = {0, 0}; - if (dispPtr->inputXfs == NULL) { - /* - * We only need to create one XFontSet - */ + preedit_attname = XNPreeditAttributes; + preedit_attlist = XVaCreateNestedList(0, + XNSpotLocation, &spot, + XNFontSet, dispPtr->inputXfs, + NULL); + } - char **missing_list; - int missing_count; - char *def_string; + winPtr->inputContext = XCreateIC(dispPtr->inputMethod, + XNInputStyle, dispPtr->inputStyle, + XNClientWindow, winPtr->window, + XNFocusWindow, winPtr->window, + preedit_attname, preedit_attlist, + NULL); - dispPtr->inputXfs = XCreateFontSet(dispPtr->display, - "-*-*-*-R-Normal--14-130-75-75-*-*", - &missing_list, &missing_count, &def_string); - if (missing_count > 0) { - XFreeStringList(missing_list); - } - } + if (preedit_attlist) { + XFree(preedit_attlist); + } - preedit_attr = XVaCreateNestedList(0, XNSpotLocation, - &spot, XNFontSet, dispPtr->inputXfs, NULL); - if (winPtr->inputContext != NULL) { - Tcl_Panic("inputContext not NULL"); - } - winPtr->inputContext = XCreateIC(dispPtr->inputMethod, - XNInputStyle, XIMPreeditPosition|XIMStatusNothing, - XNClientWindow, winPtr->window, - XNFocusWindow, winPtr->window, - XNPreeditAttributes, preedit_attr, - NULL); - XFree(preedit_attr); - } else { - if (winPtr->inputContext != NULL) { - Tcl_Panic("inputContext not NULL"); - } - winPtr->inputContext = XCreateIC(dispPtr->inputMethod, - XNInputStyle, XIMPreeditNothing|XIMStatusNothing, - XNClientWindow, winPtr->window, - XNFocusWindow, winPtr->window, - NULL); + /* + * Adjust the window's event mask if the IM requires it. + */ + XGetICValues(winPtr->inputContext, XNFilterEvents, &im_event_mask, NULL); + if ((winPtr->atts.event_mask & im_event_mask) != im_event_mask) { + winPtr->atts.event_mask |= im_event_mask; + XSelectInput(winPtr->display, winPtr->window, winPtr->atts.event_mask); } } #endif @@ -397,8 +378,7 @@ CreateXIMSpotMethods( * context). * * When the event is a FocusIn event, set the input context focus to the - * receiving window. This is needed for certain versions of Solaris, but - * we are still not sure whether it is being done in the right way. + * receiving window. * * Results: * 1 when we are done with the event. @@ -419,38 +399,24 @@ InvokeInputMethods( TkDisplay *dispPtr = winPtr->dispPtr; if ((dispPtr->flags & TK_DISPLAY_USE_IM)) { - long im_event_mask = 0L; if (!(winPtr->flags & (TK_CHECKED_IC|TK_ALREADY_DEAD))) { winPtr->flags |= TK_CHECKED_IC; if (dispPtr->inputMethod != NULL) { -#if TK_XIM_SPOT - CreateXIMSpotMethods(winPtr); -#else - if (winPtr->inputContext != NULL) { - Tcl_Panic("inputContext not NULL"); - } - winPtr->inputContext = XCreateIC(dispPtr->inputMethod, - XNInputStyle, XIMPreeditNothing|XIMStatusNothing, - XNClientWindow, winPtr->window, - XNFocusWindow, winPtr->window, - NULL); -#endif - } - } - if (winPtr->inputContext != NULL && - (eventPtr->xany.type == FocusIn)) { - XGetICValues(winPtr->inputContext, - XNFilterEvents, &im_event_mask, NULL); - if (im_event_mask != 0L) { - XSelectInput(winPtr->display, winPtr->window, - winPtr->atts.event_mask | im_event_mask); - XSetICFocus(winPtr->inputContext); + CreateXIC(winPtr); } } - if (eventPtr->type == KeyPress || eventPtr->type == KeyRelease) { - if (XFilterEvent(eventPtr, None)) { - return 1; - } + switch (eventPtr->type) { + case FocusIn: + if (winPtr->inputContext != NULL) { + XSetICFocus(winPtr->inputContext); + } + break; + case KeyPress: + case KeyRelease: + if (XFilterEvent(eventPtr, None)) { + return 1; + } + break; } } return 0; diff --git a/generic/tkInt.h b/generic/tkInt.h index 2186089..7ae4bee 100644 --- a/generic/tkInt.h +++ b/generic/tkInt.h @@ -11,7 +11,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: $Id: tkInt.h,v 1.81 2007/12/13 15:24:14 dgp Exp $ + * RCS: $Id: tkInt.h,v 1.82 2008/03/26 19:04:09 jenglish Exp $ */ #ifndef _TKINT @@ -145,16 +145,6 @@ typedef struct TkCursor { } TkCursor; /* - * This defines whether we should try to use XIM over-the-spot style input. - * Allow users to override it. It is a much more elegant use of XIM, but uses - * a bit more memory. - */ - -#ifndef TK_XIM_SPOT -#define TK_XIM_SPOT 1 -#endif - -/* * The following structure is kept one-per-TkDisplay to maintain information * about the caret (cursor location) on this display. This is used to dictate * global focus location (Windows Accessibility guidelines) and to position @@ -530,9 +520,8 @@ typedef struct TkDisplay { #ifdef TK_USE_INPUT_METHODS XIM inputMethod; /* Input method for this display. */ -#if TK_XIM_SPOT + XIMStyle inputStyle; /* Input style selected for this display. */ XFontSet inputXfs; /* XFontSet cached for over-the-spot XIM. */ -#endif #endif /* TK_USE_INPUT_METHODS */ Tcl_HashTable winTable; /* Maps from X window ids to TkWindow ptrs. */ @@ -572,8 +561,6 @@ typedef struct TkDisplay { * Indicates that we should collapse motion events on this display * TK_DISPLAY_USE_IM: (default on, set via tk.tcl) * Whether to use input methods for this display - * TK_DISPLAY_XIM_SPOT: (default off) - * Indicates that we should use over-the-spot XIM on this display * TK_DISPLAY_WM_TRACING: (default off) * Whether we should do wm tracing on this display. * TK_DISPLAY_IN_WARP: (default off) @@ -582,7 +569,6 @@ typedef struct TkDisplay { #define TK_DISPLAY_COLLAPSE_MOTION_EVENTS (1 << 0) #define TK_DISPLAY_USE_IM (1 << 1) -#define TK_DISPLAY_XIM_SPOT (1 << 2) #define TK_DISPLAY_WM_TRACING (1 << 3) #define TK_DISPLAY_IN_WARP (1 << 4) diff --git a/unix/tcl.m4 b/unix/tcl.m4 index 3a13ea3..9476cae 100644 --- a/unix/tcl.m4 +++ b/unix/tcl.m4 @@ -1454,10 +1454,6 @@ dnl AC_CHECK_TOOL(AR, ar) # files in compat/*.c is being linked in. AS_IF([test x"${USE_COMPAT}" != x],[CFLAGS="$CFLAGS -fno-inline"]) - - # XIM peeking works under XFree86. - AC_DEFINE(PEEK_XCLOSEIM, 1, [May we use XIM peeking safely?]) - ;; GNU*) SHLIB_CFLAGS="-fPIC" diff --git a/unix/tkConfig.h.in b/unix/tkConfig.h.in index 819efff..285bb38 100644 --- a/unix/tkConfig.h.in +++ b/unix/tkConfig.h.in @@ -130,9 +130,6 @@ /* Define to the version of this package. */ #undef PACKAGE_VERSION -/* May we use XIM peeking safely? */ -#undef PEEK_XCLOSEIM - /* Is this a static build? */ #undef STATIC_BUILD diff --git a/unix/tkUnixEvent.c b/unix/tkUnixEvent.c index ab95290..bb66552 100644 --- a/unix/tkUnixEvent.c +++ b/unix/tkUnixEvent.c @@ -9,7 +9,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkUnixEvent.c,v 1.26 2007/12/13 15:28:50 dgp Exp $ + * RCS: @(#) $Id: tkUnixEvent.c,v 1.27 2008/03/26 19:04:10 jenglish Exp $ */ #include "tkUnixInt.h" @@ -25,18 +25,6 @@ typedef struct ThreadSpecificData { } ThreadSpecificData; static Tcl_ThreadDataKey dataKey; -#if defined(TK_USE_INPUT_METHODS) && defined(PEEK_XCLOSEIM) -/* - * Structure used to peek into internal XIM data structure. This is only known - * to work with XFree86. - */ - -struct XIMPeek { - void *junk1, *junk2; - XIC ic_chain; -}; -#endif - /* * Prototypes for functions that are referenced only in this file: */ @@ -173,54 +161,10 @@ TkpCloseDisplay( TkWmCleanup(dispPtr); #ifdef TK_USE_INPUT_METHODS -#if TK_XIM_SPOT if (dispPtr->inputXfs) { XFreeFontSet(dispPtr->display, dispPtr->inputXfs); } -#endif if (dispPtr->inputMethod) { - /* - * Calling XCloseIM with an input context that has not been freed can - * cause a crash. This crash has been reproduced under Linux systems - * with XFree86 3.3 and may have also been seen under Solaris 2.3. The - * crash is caused by a double free of memory inside the X library. - * Memory that was already deallocated may be accessed again inside - * XCloseIM. This bug can be avoided by making sure that a call to - * XDestroyIC is made for each XCreateIC call. This bug has been fixed - * in XFree86 4.2.99.2. The internal layout of the XIM structure - * changed in the XFree86 4.2 release so the test should not be run - * for with these new releases. - */ - -#if defined(TK_USE_INPUT_METHODS) && defined(PEEK_XCLOSEIM) - int do_peek = 0; - struct XIMPeek *peek; - - if (strstr(ServerVendor(dispPtr->display), "XFree86")) { - int vendrel = VendorRelease(dispPtr->display); - - if (vendrel < 336) { - /* 3.3.4 and 3.3.5 */ - do_peek = 1; - } else if (vendrel < 3900) { - /* Other 3.3.x versions */ - do_peek = 1; - } else if (vendrel < 40000000) { - /* 4.0.x versions */ - do_peek = 1; - } else { - /* Newer than 4.0 */ - do_peek = 0; - } - } - - if (do_peek) { - peek = (struct XIMPeek *) dispPtr->inputMethod; - if (peek->ic_chain != NULL) { - Tcl_Panic("input contexts not freed before XCloseIM"); - } - } -#endif XCloseIM(dispPtr->inputMethod); } #endif @@ -621,9 +565,7 @@ TkpSync( * * OpenIM -- * - * Tries to open an X input method, associated with the given display. - * Right now we can only deal with a bare-bones input style: no preedit, - * and no status. + * Tries to open an X input method associated with the given display. * * Results: * Stores the input method in dispPtr->inputMethod; if there isn't a @@ -639,11 +581,12 @@ static void OpenIM( TkDisplay *dispPtr) /* Tk's structure for the display. */ { - unsigned short i; + int i; XIMStyles *stylePtr; + XIMStyle bestStyle = 0; if (XSetLocaleModifiers("") == NULL) { - goto error; + return; } dispPtr->inputMethod = XOpenIM(dispPtr->display, NULL, NULL, NULL); @@ -656,38 +599,45 @@ OpenIM( goto error; } -#if TK_XIM_SPOT /* - * If we want to do over-the-spot XIM, we have to check that this mode is - * supported. If not we will fall-through to the check below. + * Select the best input style supported by both the IM and Tk. */ - for (i = 0; i < stylePtr->count_styles; i++) { - if (stylePtr->supported_styles[i] - == (XIMPreeditPosition | XIMStatusNothing)) { - dispPtr->flags |= TK_DISPLAY_XIM_SPOT; - XFree(stylePtr); - return; + XIMStyle thisStyle = stylePtr->supported_styles[i]; + if (thisStyle == (XIMPreeditPosition | XIMStatusNothing)) { + bestStyle = thisStyle; + break; + } else if (thisStyle == (XIMPreeditNothing | XIMStatusNothing)) { + bestStyle = thisStyle; } } -#endif /* TK_XIM_SPOT */ + XFree(stylePtr); + if (bestStyle == 0) { + goto error; + } - for (i = 0; i < stylePtr->count_styles; i++) { - if (stylePtr->supported_styles[i] - == (XIMPreeditNothing | XIMStatusNothing)) { - XFree(stylePtr); - return; + dispPtr->inputStyle = bestStyle; + + /* + * Create an XFontSet for preedit area. + */ + if (dispPtr->inputStyle & XIMPreeditPosition) { + char **missing_list; + int missing_count; + char *def_string; + + dispPtr->inputXfs = XCreateFontSet(dispPtr->display, + "-*-*-*-R-Normal--14-130-75-75-*-*", + &missing_list, &missing_count, &def_string); + if (missing_count > 0) { + XFreeStringList(missing_list); } } - XFree(stylePtr); - error: - if (dispPtr->inputMethod) { - /* - * This call should not suffer from any core dumping problems since we - * have not allocated any input contexts. - */ + return; +error: + if (dispPtr->inputMethod) { XCloseIM(dispPtr->inputMethod); dispPtr->inputMethod = NULL; } diff --git a/unix/tkUnixKey.c b/unix/tkUnixKey.c index 7dc91da..4d422cf 100644 --- a/unix/tkUnixKey.c +++ b/unix/tkUnixKey.c @@ -9,7 +9,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tkUnixKey.c,v 1.12 2007/02/27 14:52:57 dkf Exp $ + * RCS: @(#) $Id: tkUnixKey.c,v 1.13 2008/03/26 19:04:10 jenglish Exp $ */ #include "tkInt.h" @@ -24,16 +24,9 @@ * Tk_SetCaretPos -- * * This enables correct placement of the XIM caret. This is called by - * widgets to indicate their cursor placement, and the caret location is - * used by TkpGetString to place the XIM caret. This is currently only + * widgets to indicate their cursor placement. This is currently only * used for over-the-spot XIM. * - * Results: - * None - * - * Side effects: - * None - * *---------------------------------------------------------------------- */ @@ -44,16 +37,42 @@ Tk_SetCaretPos( int y, int height) { - TkCaret *caretPtr = &(((TkWindow *) tkwin)->dispPtr->caret); + TkWindow *winPtr = (TkWindow *) tkwin; + TkDisplay *dispPtr = winPtr->dispPtr; + + if ( dispPtr->caret.winPtr == winPtr + && dispPtr->caret.x == x + && dispPtr->caret.y == y + && dispPtr->caret.height == height) + { + return; + } + + dispPtr->caret.winPtr = winPtr; + dispPtr->caret.x = x; + dispPtr->caret.y = y; + dispPtr->caret.height = height; +#ifdef TK_USE_INPUT_METHODS /* - * Use height for best placement of the XIM over-the-spot box. + * Adjust the XIM caret position. */ + if ( (dispPtr->flags & TK_DISPLAY_USE_IM) + && (dispPtr->inputStyle & XIMPreeditPosition) + && (winPtr->inputContext != NULL) ) + { + XVaNestedList preedit_attr; + XPoint spot; - caretPtr->winPtr = ((TkWindow *) tkwin); - caretPtr->x = x; - caretPtr->y = y; - caretPtr->height = height; + spot.x = dispPtr->caret.x; + spot.y = dispPtr->caret.y + dispPtr->caret.height; + preedit_attr = XVaCreateNestedList(0, XNSpotLocation, &spot, NULL); + XSetICValues(winPtr->inputContext, + XNPreeditAttributes, preedit_attr, + NULL); + XFree(preedit_attr); + } +#endif } /* @@ -85,9 +104,6 @@ TkpGetString( int len; Tcl_DString buf; Status status; -#ifdef TK_USE_INPUT_METHODS - TkDisplay *dispPtr = winPtr->dispPtr; -#endif /* * Overallocate the dstring to the maximum stack amount. @@ -97,13 +113,9 @@ TkpGetString( Tcl_DStringSetLength(&buf, TCL_DSTRING_STATIC_SIZE-1); #ifdef TK_USE_INPUT_METHODS - if ((dispPtr->flags & TK_DISPLAY_USE_IM) + if ((winPtr->dispPtr->flags & TK_DISPLAY_USE_IM) && (winPtr->inputContext != NULL) && (eventPtr->type == KeyPress)) { -#if TK_XIM_SPOT - XVaNestedList preedit_attr; - XPoint spot; -#endif len = XmbLookupString(winPtr->inputContext, &eventPtr->xkey, Tcl_DStringValue(&buf), Tcl_DStringLength(&buf), NULL, @@ -121,21 +133,6 @@ TkpGetString( len = 0; } -#if TK_XIM_SPOT - /* - * Adjust the XIM caret position. We might want to check that this is - * the right caret.winPtr as well. - */ - - if (dispPtr->flags & TK_DISPLAY_XIM_SPOT) { - spot.x = dispPtr->caret.x; - spot.y = dispPtr->caret.y + dispPtr->caret.height; - preedit_attr = XVaCreateNestedList(0, XNSpotLocation, &spot, NULL); - XSetICValues(winPtr->inputContext, - XNPreeditAttributes, preedit_attr, NULL); - XFree(preedit_attr); - } -#endif } else { len = XLookupString(&eventPtr->xkey, Tcl_DStringValue(&buf), Tcl_DStringLength(&buf), NULL, NULL); -- cgit v0.12