<i18n dev> <AWT Dev> RFR: X11 default visual support for IM status window on VNC
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Sat Dec 1 00:49:13 UTC 2018
Looks fine, if there are no other comments I'll push the fix.
On 26/11/2018 05:02, Ichiroh Takiguchi wrote:
> Hello.
>
> Could you review the fix ?
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8212677
> Change: https://cr.openjdk.java.net/~itakiguchi/8212677/webrev.00/
>
> Screen shots are in JDK-8212677.
>
> I'd like to obtain a sponsor for this issue.
>
> Thanks,
> Ichiroh Takiguchi
> IBM Japan, Ltd.
>
> On 2018-06-21 21:58, Ichiroh Takiguchi wrote:
>> Hello Phil.
>>
>> I'm sorry, I forgot to put my comment against your question.
>>
>>> Is this changing the default visual for all WIndows, not just the IM status window?
>>> I think we need to understand the implications before this can be accepted.
>> I put following debug code:
>> =======
>> diff -r e1b3def12624
>> src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>> --- a/src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>> Wed Jun 13 06:35:04 2018 +0200
>> +++ b/src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>> Thu Jun 21 21:23:11 2018 +0900
>> @@ -667,6 +667,7 @@
>> InputOutput,
>> adata->awt_visInfo.visual,
>> attribmask, &attrib);
>> + fprintf(stderr, "status window id = 0x%X\n", status);
>> XSelectInput(dpy, status,
>> ExposureMask | StructureNotifyMask | EnterWindowMask |
>> LeaveWindowMask | VisibilityChangeMask);
>> @@ -680,6 +681,21 @@
>> statusWindow->fontset = XCreateFontSet(dpy,
>>
>> "-*-*-medium-r-normal-*-*-120-*-*-*-*",
>> &mclr, &mccr, &dsr);
>> + {
>> + int cntFonts;
>> + for(cntFonts = 0; cntFonts < mccr; cntFonts++) {
>> + fprintf(stderr, "[M][%d] %s\n", cntFonts, mclr[cntFonts]);
>> + }
>> + }
>> + {
>> + XFontStruct **font_struct_list;
>> + char **font_name_list;
>> + int cntFonts;
>> + int numFonts = XFontsOfFontSet(statusWindow->fontset,
>> &font_struct_list, &font_name_list);
>> + for(cntFonts = 0; cntFonts < numFonts; cntFonts++) {
>> + fprintf(stderr, "[L][%d] %s\n", cntFonts,
>> font_name_list[cntFonts]);
>> + }
>> + }
>> /* In case we didn't find the font set, release the list of
>> missing characters */
>> if (mccr > 0) {
>> XFreeStringList(mclr);
>> =======
>>
>> I tested it on RHEL7.
>> I thought since window id was assigned, but it was gone on current code.
>> =======
>> $ java -jar Notepad.jar
>> status window id = 0x4000055
>> ...
>> $ xwininfo -id 0x4000055
>> X Error: 9: Bad Drawable: 0x4000055
>> Request Major code: 14
>> Request serial number: 3
>> xwininfo: error: No such window with id 0x4000055.
>> =======
>>
>> =======
>> $ java -jar Notepad.jar
>> status window id = 0x40000CA
>> ...
>> $ xwininfo -id 0x40000CA
>>
>> xwininfo: Window id: 0x40000ca (has no name)
>>
>> Absolute upper-left X: 0
>> Absolute upper-left Y: 600
>> Relative upper-left X: 0
>> Relative upper-left Y: 600
>> Width: 80
>> Height: 22
>> Depth: 24
>> Visual: 0x21
>> Visual Class: TrueColor
>> Border width: 0
>> Class: InputOutput
>> Colormap: 0x20 (installed)
>> Bit Gravity State: ForgetGravity
>> Window Gravity State: NorthWestGravity
>> Backing Store State: NotUseful
>> Save Under State: no
>> Map State: IsUnMapped
>> Override Redirect State: yes
>> Corners: +0+600 -944+600 -944-146 +0-146
>> -geometry 80x22+0+600
>> =======
>>
>> According to main window: (Left side: without fix, right side: with fix)
>> =======
>> xwininfo: Window id: 0x4000009 " (failure | xwininfo: Window id:
>> 0x460007e " (failure
>>
>> Absolute upper-left X: 4 Absolute upper-left X: 4
>> Absolute upper-left Y: 25 Absolute upper-left Y: 25
>> Relative upper-left X: 0 Relative upper-left X: 0
>> Relative upper-left Y: 0 Relative upper-left Y: 0
>> Width: 492 Width: 492
>> Height: 571 Height: 571
>> Depth: 24 Depth: 24
>> Visual: 0x169 | Visual: 0x21
>> Visual Class: TrueColor Visual Class: TrueColor
>> Border width: 0 Border width: 0
>> Class: InputOutput Class: InputOutput
>> Colormap: 0x4000008 (not installed) | Colormap: 0x20 (installed)
>> Bit Gravity State: NorthWestGravity Bit Gravity State:
>> NorthWestGravity
>> Window Gravity State: NorthWestGravity Window Gravity
>> State: NorthWestGravity
>> Backing Store State: NotUseful Backing Store State: NotUseful
>> Save Under State: no Save Under State: no
>> Map State: IsViewable Map State: IsViewable
>> Override Redirect State: no Override Redirect State: no
>> Corners: +4+25 -528+25 -528-172 +4-1 Corners: +4+25
>> -528+25 -528-172 +4-1
>> -geometry 492x571+0+0 -geometry 492x571+0+0
>> =======
>>
>> So main window's visual also changed by this fix.
>>
>>> Similarly for the fontset change .. this might change what others get.
>>> The fontset spec. there seems very loose to me ..
>>
>> Without fix
>> =======
>> $ java -jar Notepad.jar
>> status window id = 0x4000055
>> [M][0] JISX0208.1983-0
>> [M][1] GB2312.1980-0
>> [L][0] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso8859-1
>> [L][1] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso8859-1
>> [L][2] -daewoo-gothic-medium-r-normal--16-120-100-100-c-160-ksc5601.1987-0
>> [L][3] -sony-fixed-medium-r-normal--16-120-100-100-c-80-jisx0201.1976-0
>> [L][4] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso10646-1
>> =======
>>
>> With fix
>> =======
>> $ java -jar Notepad.jar
>> status window id = 0x40000CA
>> [M][0] GB2312.1980-0
>> [L][0] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso8859-1
>> [L][1] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso8859-1
>> [L][2] -misc-fixed-medium-r-normal--14-130-75-75-c-140-jisx0208.1983-0
>> [L][3] -daewoo-gothic-medium-r-normal--16-120-100-100-c-160-ksc5601.1987-0
>> [L][4] -sony-fixed-medium-r-normal--16-120-100-100-c-80-jisx0201.1976-0
>> [L][5] -misc-fixed-medium-r-normal--13-120-75-75-c-70-iso10646-1
>> =======
>>
>> [M] means "Missing" font, [L] means "Loaded".
>>
>> On 2018-06-20 23:14, Philip Race wrote:
>>> My question has not been answered. I don't think this is ready to go in.
>>>
>>> -phil.
>>>
>>> On 6/20/18, 4:23 AM, Ichiroh Takiguchi wrote:
>>>> Hello.
>>>>
>>>> New fixed code is in:
>>>> http://cr.openjdk.java.net/~aleonard/defvis/webrev.02/
>>>>
>>>> Could you check fixed files again ?
>>>>
>>>> I only updated following part between webrev.01 and webrev.02
>>>> ======
>>>> diff -r 70a582d110a1 -r 6f04164a9d62 src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>>>> --- a/src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c Wed Jun 06 21:03:25 2018 +0900
>>>> +++ b/src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c Wed Jun 20 16:54:24 2018 +0900
>>>> @@ -677,7 +677,7 @@
>>>> return NULL;
>>>> }
>>>> statusWindow->w = status;
>>>> - //12-point font
>>>> + //12, 13-point fonts
>>>> statusWindow->fontset = XCreateFontSet(dpy,
>>>> "-*-*-medium-r-normal-*-*-120-*-*-*-*," \
>>>> "-*-*-medium-r-normal-*-*-130-*-*-*-*",
>>>> ======
>>>>
>>>>
>>>> Please create bugid and handle it.
>>>>
>>>> Thanks,
>>>> Ichiroh Takiguchi
>>>> IBM Japan, Ltd.
>>>>
>>>> On 2018-06-20 04:59, Naoto Sato wrote:
>>>>> Please change the comment in line 680, which should also mention 13 point font.
>>>>>
>>>>> Naoto
>>>>>
>>>>> On 6/19/18 12:54 PM, Naoto Sato wrote:
>>>>>> Looks OK wrt awt_InputMethod.c change.
>>>>>>
>>>>>> Naoto
>>>>>>
>>>>>> On 6/19/18 11:32 AM, Phil Race wrote:
>>>>>>> Where's the bug ID ?
>>>>>>>
>>>>>>> The review should have a bug ID in the subject line so we can all find it later !
>>>>>>>
>>>>>>> Is this changing the default visual for all WIndows, not just the IM status window?
>>>>>>> I think we need to understand the implications before this can be accepted.
>>>>>>>
>>>>>>> Similarly for the fontset change .. this might change what others get.
>>>>>>> The fontset spec. there seems very loose to me ..
>>>>>>>
>>>>>>> I think I18N-DEV should be asked about this too.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 06/19/2018 11:07 AM, Sergey Bylokhov wrote:
>>>>>>>> Looks fine, if there are no other comments I'll push the fix using the new bugid.
>>>>>>>>
>>>>>>>> On 06/06/2018 17:54, Ichiroh Takiguchi wrote:
>>>>>>>>> Hello Sergey.
>>>>>>>>> Thank you for your review.
>>>>>>>>>
>>>>>>>>> Could you review following patch ?
>>>>>>>>> http://cr.openjdk.java.net/~aleonard/defvis/webrev.01/
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Ichiroh Takiguchi
>>>>>>>>> IBM Japan, Ltd.
>>>>>>>>>
>>>>>>>>> On 2018-06-06 07:15, Sergey Bylokhov wrote:
>>>>>>>>>> Hi, Ichiroh.
>>>>>>>>>> The approach looks fine, but maybe it is possible to decrees code
>>>>>>>>>> duplication in findWithTemplate(). After the fix it will have two
>>>>>>>>>> similar loops.
>>>>>>>>>>
>>>>>>>>>> On 24/05/2018 22:24, Ichiroh Takiguchi wrote:
>>>>>>>>>>> Hello,
>>>>>>>>>>> IBM would like to contribute X11 default visual support for IM status window patch to OpenJDK project.
>>>>>>>>>>>
>>>>>>>>>>> Issue:
>>>>>>>>>>> Java's Native IM status window is not displayed even if it's there.
>>>>>>>>>>> Because of this issue, user cannot get proper visual feedback during key input operation.
>>>>>>>>>>> We found this issue on Tiger VNC.
>>>>>>>>>>>
>>>>>>>>>>> Reason:
>>>>>>>>>>> Java may pick up unexpected visual for Java's Native IM status window when Xserver supports multiple visual.
>>>>>>>>>>>
>>>>>>>>>>> Workaround:
>>>>>>>>>>> X11 default visual can be changed by FORCEDEFVIS environment variable, but it's not easy to find out default visual id.
>>>>>>>>>>>
>>>>>>>>>>> I'd like contribute following 2 files:
>>>>>>>>>>> M src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c
>>>>>>>>>>> (Change X11 visual setting)
>>>>>>>>>>> M src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>>>>>>>>>>> (Support 13 point X11 misc fonts (like k14 font for Japanese), since the fonts may defined for unscaled fonts.)
>>>>>>>>>>>
>>>>>>>>>>> webrev files are in
>>>>>>>>>>> http://cr.openjdk.java.net/~aleonard/defvis/
>>>>>>>>>>>
>>>>>>>>>>> I appreciate any feedback please, and how I would go about obtaining a sponsor and contributor?
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Ichiroh Takiguchi
>>>>>>>>>>> IBM Japan, Ltd.
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>
>
--
Best regards, Sergey.
More information about the i18n-dev
mailing list