Page MenuHome

Input Method Editor Support for Windows
AbandonedPublic

Authored by Julian Eisel (Severin) on Aug 27 2014, 8:31 AM.
Tokens
"Like" token, awarded by campbellbarton."Like" token, awarded by nirenyang."Yellow Medal" token, awarded by leon_cheung."Like" token, awarded by yuzukyo.

Details

Summary

Input method editor is used for typing complex characters,
It has been widely used in East Asia.

support in uibutton, console, text editor

short explanation for each file:

  • intern/ghost/GHOST_C-api.h enable/disable ime function used for wm,
  • intern/ghost/GHOST_IWindow.h enable/disable ime ghost interface
  • intern/ghost/GHOST_Types.h difine ghost event enum and data type
  • intern/ghost/intern/GHOST_ImeWin32.h wrap Windows IME api, modify from Chromium.
  • intern/ghost/intern/GHOST_SystemWin32.h create ime event
  • source/blender/editors/interface/interface_handlers.c handle wm ime event,
  • source/blender/windowmanager/intern/wm_event_system.c change ghost event to wm event
  • source/blender/windowmanager/wm_event_system.h get wm ime event data

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Made just first iteration of review.

CMakeLists.txt
1055

Is it something all platforms has or some special version of windows/libraries installed are needed?

intern/ghost/intern/GHOST_ImeWin32.cpp
95

Is it correct that only this languages needs IME? What about guys from Korea i.e.?

intern/ghost/intern/GHOST_SystemWin32.cpp
922

Skip empty lines?

source/blender/editors/include/UI_interface.h
1000

Think we don't usually use extern explicitly

source/blender/editors/interface/interface_handlers.c
2423

Shouldn't it be DPI aware? Same applies to other pixel offset arithmetic in the patch.

2616

Use bool.

2619

Same as above.

source/blender/editors/interface/interface_widgets.c
1479

It's a bit difficult to follow the code without having an access to patch hunks context, but does this mean you're allocating string for every draw?

source/blender/editors/space_console/space_console.c
235 ↗(On Diff #2730)

bool.

256 ↗(On Diff #2730)

Wait, can't we just have int xy[2] in ime?

source/blender/editors/space_info/textview.c
160 ↗(On Diff #2730)

I'm still not a fan of doing per-draw allocations.

source/blender/editors/space_info/textview.h
59 ↗(On Diff #2730)

Seems ime is rather global, so we can skip adding it all over the place?

source/blender/editors/space_text/text_draw.c
377 ↗(On Diff #2730)

Can't underline become some utility function?

1374 ↗(On Diff #2730)

That's a bit weird looking. Wouldn't it be better if the one who sets tmp to null to make sure there's no ime?

1385 ↗(On Diff #2730)

That is again per-draw alloc..

CMakeLists.txt
1055

The imm32 library is a generic library. For instance the English version of 64-bit Windows 7 on my laptop has it in C:/Windows/system32/imm32.dll.

We're very appreciate you can give a hands to help and change these codes to conforms the Blender fundation's architecture request and code style,

I've contact Dun Liang already, he was struggling with his school stuff,and haven't time to touch this patch for very long periods. And others of us didnt know how to code,we are all arits and end users. So, Thanks a lot for help us make a better implementation and force this change go to master. Thnaks very much.

After request from Ton, I'll handle review of this (starting this week). @Bastien Montagne (mont29) will supervise me a bit.

Julian Eisel (Severin) set the repository for this revision to rB Blender.

Review and further development will happen via the input_editor_method branch.

Diff's are handy for review still, updating the patch for a final check before going into master isn't really a problem.

@Campbell Barton (campbellbarton), Ah, yeah, just wanted to point to the branch so people know we're working on it. The diff can still be updated regularly.

Julian Eisel (Severin) retitled this revision from Blender input method editor support for windows to Input Method Editor Support for Windows.Nov 26 2014, 1:04 AM
Julian Eisel (Severin) edited edge metadata.
Julian Eisel (Severin) updated this revision to Diff 2939.

First Review Update

Update includes:

  • make IME a build option by adding a CMake flag "WITH_INPUT_IME"
  • avoid using MEM_mallocN on every redraw
  • proper licencing for code from external sources
  • lots of code style cleanup (some may seem picky, but we should try to be consistent as much as possible)

Call for Testing

To test this patch for it's functionality, I need some people, mainly from East Asia, to test it. I will provide testbuilds soon.

Campbell Barton (campbellbarton) requested changes to this revision.

notes on latest patch.

CMakeLists.txt
20

-limm32 should be conditionally appended too. (following scons configs also)

intern/ghost/GHOST_ImeWin32.h
351 ↗(On Diff #2939)

*picky*, ghost convention is to use:

m_somename instead of somename_.

intern/ghost/intern/GHOST_ImeWin32.cpp
274

prefer braces around indented blocks like this.

350

if its a block of code you don't want to run (and not a descriptive comment), prefer just #if 0

intern/ghost/intern/GHOST_ImeWin32.h
22

wouldn't bother with these lines. people can add themselves to contributors if they really want later.

source/blender/editors/interface/interface_widgets.c
20

would perfer this stay const, rule of thumb - drawing functions shouldn't manipulate buttons.

This revision now requires changes to proceed.Nov 29 2014, 4:39 PM

Added some inline comments, to justify some of the changes from the latest update

intern/ghost/intern/GHOST_ImeWin32.cpp
95

This doesn't restrict the support for IME to Chinese and Japanese, it's only a workaround for some special cases. The code comment above describes this a bit more detailed.

source/blender/editors/interface/interface_handlers.c
2423

I'm not sure if this code is needed, at all. Disabling it doesn't have any effect. Will investigate more.

source/blender/editors/space_info/textview.c
160 ↗(On Diff #2730)

I got rid of all the other cases where strings were allocated per draw, but in this case, I'm afraid this isn't possible, as the texteditor shouldn't limit the number of characters per line. In the console I created char [1024] though, but for the console that should be enough.

source/blender/editors/space_text/text_draw.c
1374 ↗(On Diff #2730)

Seems to me that this is only done to disable the if(...) below. I've addressed that in the latest update.

Answering some of Campbell's inline comments.

intern/ghost/GHOST_ImeWin32.h
351 ↗(On Diff #2939)

What would you suggest here? "ime_is_compositing"?

intern/ghost/intern/GHOST_ImeWin32.h
22

Not sure what you mean here, but most of this file is copied and modified code from chromium. That's why I did some modifications to the license block.

I've compiled the latest code form severin and distribute the 64bit binary blender to user and almost 50 users give the feedback. users from blendercn.org. I record these feedback in chinese http://www.blendercn.org/963.html.
Let me explain feedback record above:
There are two main windows platform when user use blender : windows 7 and windows 8
There are so many solutions for chinese version IME: PINYIN , WUBI ,SHUANGPIN, voice distinguish, handwriting distinguish etc.
There are so many companies develop IME: google bing sougou baidu etc.

the feedback from users are there part:
1,sougou PINYIN and WUBI are OK in both platform and the four-Fifths user use this version IME
2,every transform input will get a index character and the right part,(like input tianxia via IME the right part is 天下,but from IME will be t天下 ,the first 't' in the input) like bing IME in windows 8 little user use this version IME
3,double or repeat input with IME (like this: input tianxia via IME the right part is 天下,will be 天下天下 two times character)there appears that IME will remember latest input chinese charactor and when input next lation index, the will first release the last remember character and then the input one.

Some studio in blendercn have use 1 part's IME,so it's good for us, we can't encounter any problems.
If 2 and 3 can't be solved, we'll recommand user install 1 part's IME product, when user base grow up, these user who in situation 2 and 3 should find someone who can fix these thing. url http://www.blendercn.org/963.html will be the best reference to choose IME for Blender.

And for japanese, we test google japanese IME, win7 and win8 are OK, don't know people in japanese encounter what problems.

intern/ghost/intern/GHOST_ImeWin32.cpp
95

Obviously, there are no korean user tigger a future requests or something else, search from the whole blender site and the d.b.o site there are no one.
Like Blender fudation's rule : no stockholder, no codes, make things more simple and safe.

intern/ghost/GHOST_ImeWin32.h
351 ↗(On Diff #2939)

m_is_composing;

however if this file is from Chrome, it may be better to make minimal changes - and note at the top of the file that this is taken from chrome and not matching existing ghost conventions. (better revert some changes made in that case)... then its not so much hassle to keep in sync.

@xueke pei (yuzukyo), its hard to understand your last message, probably also because I'm not a user of IME.

Can you confirm the limitations (2 & 3) are also limitations in google-chrome? (where our IME code comes from).

We should at least be sure that issues were not introduced by having a bad IME port of chrome's code.

@Campbell Barton (campbellbarton)
I can confirm that chrome code not intruduce these limitations.
Because when we use chrome programme all IME can perfect input character.
If you need more evidence,please tell me the method how to get these thing to provide.

@xueke pei (yuzukyo), Could you explain in detail exactly what fails?

Could you give steps for someone (an English speaker), to redo the error, once the IME patch is applied?

If IME is not perfect, we may not be able to solve easily, but at least try to define the problem, it may also be some simple mistake we can fix for the release.

(a video showing the difference between Chrome and Blender for eg)

Hi all, I've uploaded some test videos, in order to show what @xueke pei (yuzukyo) talked about, presenting different issues with different IMEs.

I've tested with 14 IMEs on my Windows 7 / 8 / 8.1 64bit OS, solutions based on either PINYIN or WUBI.

Chinese IME test1 (with Blender)

Chinese IME Test (with Chrome)

Detailed test result see here: Feedback - Asian IME Support in Blender

I'm not so sure why more problems exposed on Windows 8 / 8.1. Probably the new Metro UI?

Let me know what we can do to present further information that may be helpful here. :)

Btw, After saving startup file with some Chinese characters typed in and then close Blender, it failed to start again (Blender tried itself to open it, but it shuts down in a flash), but can be opened in other builds. I'm not sure if it is caused by the build that supports IME.

It looks like this before saving:

Campbell Barton (campbellbarton) accepted this revision.

@Julian Eisel (Severin), made commits to the branch:

It builds on Linux & Windows (WITH_INPUT_IME on/off), but I didn't test IME its self.

@Julian Eisel (Severin) / @Tamito Kajiyama (kjym3), would you be able to do a final test?

@Julian Eisel (Severin) - if all runs well, I think the Branch is OK to commit to master: input_method_editor_partial_support
But would be happy if others check on the branch too.

update to sync with input_method_editor_partial_support

@Campbell Barton (campbellbarton), yeah, I followed it, thanks a lot! I'm currently adding WITH_INPUT_IME support for Scons, which isn't there, yet. Other than that, LGTM :)

last update missed new files

Julian Eisel (Severin) updated this revision to Diff 2975.

WITH_BF_IME build option for Scons

I tested IME and build flags for their functionality, everything works as expected (Win7 64Bit, MSVC, SCons and CMake)

@Leon Cheung (leon_cheung), @xueke pei (yuzukyo) or anyone else, would you mind compiling a list of fully funcional IMEs, which can be added to the Release Logs?

There are two minor bugs that I'm aware of:

  • text offset for text larger than button doesn't take IME input into account
  • text from IME is not inserted at the correct location (this only appears when changing cursor location with mouse click, not with arrow keys)

Will look into them soon, should at least be fixed for release.

Ready to land from here! @Campbell Barton (campbellbarton), may I commit :)?

xueke pei (yuzukyo) added a comment.EditedDec 6 2014, 7:52 PM

@Julian Eisel (Severin)

Can this table ok? I copy from Leon's sheet: Feedback - Asian IME Support in Blender

IME NAMEwin7win8,win8.1url
Sougou PINYIN chinese IMEfull supportfull supporthttp://pinyin.sogou.com/
Bing PINYIN chinese IMEfull supportfull supporthttp://bing.msn.cn/pinyin/
QQ PINYIN chinese IMEfull supportfull supporthttp://qq.pinyin.cn/
Microsoft Japanese IME(integrated)full supportfull support

@Julian Eisel (Severin)

Hi

The last commit to master didn't include two file: GHOST_ImeWin32.h and GHOST_ImeWin32.cpp.  When I use Cmake,it tells me some file miss. Is this right? or should I do something. Thanks.

ps, when I copy two file above, and compiled. we test the building, And It work fine. If something we should take care to test more, let us know, thanks.

Again for all, IME support has been committed (rBe81d077c852f43b63). Even if this is only a part of this patch and we still have to work on Text Editor, Console and font objects support, I'd like to close this diff, as it starts to get quite unorganized and confused. Stay tuned for moar...

Thanks all!