Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct event dispatching for multiple simultaneous touch points #8232

Merged
merged 1 commit into from Nov 3, 2015

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented Oct 28, 2015

Instead of just converting the mouse into a single "touch" input, Servo can now listen for multi-touch events from Glutin, maintain a list of active touch points, and dispatch events for all of them.

r? @glennw (for the compositor changes) and @jdm (for the DOM changes)

Review on Reviewable

@jdm
Copy link
Member

jdm commented Oct 28, 2015

+S-needs-code-changes


Reviewed 3 of 7 files at r1, 1 of 1 files at r2.
Review status: 3 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


components/script/dom/document.rs, line 801 [r2] (raw file):
*t.Target() == target


Comments from the review on Reviewable.io

@mbrubeck mbrubeck force-pushed the mbrubeck:glutin-touch branch from be64b1c to 94d5d00 Oct 28, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Oct 28, 2015

Review status: 2 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.


components/script/dom/document.rs, line 801 [r2] (raw file):
How's this? It still uses Root for the comparison, but the code is cleaned up a bit...

An alternative we discussed on IRC would be to add a raw pointer comparison method that doesn't require Root.


Comments from the review on Reviewable.io

@mbrubeck mbrubeck force-pushed the mbrubeck:glutin-touch branch from 94d5d00 to f634e30 Oct 28, 2015
@mbrubeck mbrubeck removed the S-fails-tidy label Oct 28, 2015
@jdm
Copy link
Member

jdm commented Oct 28, 2015

Reviewed 1 of 2 files at r3.
Review status: 3 of 7 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Oct 28, 2015

The latest upstream changes (presumably #8229) made this pull request unmergeable. Please resolve the merge conflicts.

@mbrubeck mbrubeck force-pushed the mbrubeck:glutin-touch branch from f634e30 to 0175c61 Oct 28, 2015
@glennw
Copy link
Member

glennw commented Nov 1, 2015

Reviewed 2 of 7 files at r1.
Review status: 4 of 7 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


components/compositing/compositor.rs, line 84 [r1] (raw file):
Can we add some comments describing what this u32 represents?


components/compositing/windowing.rs, line 64 [r1] (raw file):
Should the i32 be a new type?


Comments from the review on Reviewable.io

@mbrubeck mbrubeck force-pushed the mbrubeck:glutin-touch branch from 0175c61 to c7612b1 Nov 2, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Nov 2, 2015

Rebased and addressed review comments.


Review status: 0 of 7 files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@glennw
Copy link
Member

glennw commented Nov 2, 2015

Reviewed 2 of 7 files at r1, 1 of 2 files at r3, 4 of 7 files at r4.
Review status: 4 of 7 files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@glennw
Copy link
Member

glennw commented Nov 2, 2015

@mbrubeck The compositor changes look good to me after those changes.

@mbrubeck
Copy link
Contributor Author

mbrubeck commented Nov 2, 2015

@bors-servo r=glennw

Carrying r=jdm for the script changes; aside from rebasing, the only change was that the identifier argument get wrapped in the TouchId newtype.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 2, 2015

📌 Commit c7612b1 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

Testing commit c7612b1 with merge 2142ec1...

bors-servo added a commit that referenced this pull request Nov 3, 2015
Correct event dispatching for multiple simultaneous touch points

Instead of just converting the mouse into a single "touch" input, Servo can now listen for multi-touch events from Glutin, maintain a list of active touch points, and dispatch events for all of them.

r? @glennw (for the compositor changes) and @jdm (for the DOM changes)

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8232)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

💔 Test failed - linux-dev

@jdm
Copy link
Member

jdm commented Nov 3, 2015

/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26:21: 26:34 error: unresolved import `script_traits::TouchEventType`. Maybe a missing `extern crate script_traits`? [E0432]
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26 use script_traits::{TouchEventType, TouchId};
                                                                                         ^~~~~~~~~~~~~
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26:21: 26:34 help: run `rustc --explain E0432` to see a detailed explanation
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26:37: 27:4 error: unresolved import `script_traits::TouchId`. Maybe a missing `extern crate script_traits`? [E0432]
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26 use script_traits::{TouchEventType, TouchId};
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:27 #[cfg(feature = "window")]
/home/servo/buildbot/slave/linux-dev/build/ports/glutin/window.rs:26:37: 27:4 help: run `rustc --explain E0432` to see a detailed explanation
error: aborting due to 2 previous errors
@mbrubeck mbrubeck force-pushed the mbrubeck:glutin-touch branch from c7612b1 to ef93650 Nov 3, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Nov 3, 2015

@bors-servo r=glennw

Moved an import to fix SERVO_HEADLESS builds.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

📌 Commit ef93650 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

Testing commit ef93650 with merge 3fdaa6e...

bors-servo added a commit that referenced this pull request Nov 3, 2015
Correct event dispatching for multiple simultaneous touch points

Instead of just converting the mouse into a single "touch" input, Servo can now listen for multi-touch events from Glutin, maintain a list of active touch points, and dispatch events for all of them.

r? @glennw (for the compositor changes) and @jdm (for the DOM changes)

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8232)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 3, 2015

@bors-servo bors-servo merged commit ef93650 into servo:master Nov 3, 2015
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable Review in progress: 3 of 7 files reviewed, all discussions resolved
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@mbrubeck mbrubeck deleted the mbrubeck:glutin-touch branch May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.