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

Use a faster scroll speed under X11 #6373

Merged
merged 1 commit into from Jun 22, 2015

Conversation

@robertknight
Copy link

robertknight commented Jun 13, 2015

Platforms may report scroll deltas either in
chunks/lines/rows or pixels, depending on the
platform API and device capabilities.

If the platform reports a line/chunk-based delta
then the application needs to convert the delta
into a suitable number of pixels. Apple's documentation for example states
that the app should interpret the delta as a number of lines or rows to scroll,
depending on the type of view.

This commit just hardcodes it to 57 as
a starting point which matches the value that
Firefox calculates as the max char height
for the root frame on my system.

This depends on this Glutin PR: rust-windowing/glutin#483

Fixes #5660

Review on Reviewable

@highfive
Copy link

highfive commented Jun 13, 2015

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Ms2ger (or someone else) soon.

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jun 13, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5275

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@pcwalton
Copy link
Contributor

pcwalton commented Jun 17, 2015

Great! I'm currently working on cherry-picking over your Glutin change.

@robertknight
Copy link
Author

robertknight commented Jun 17, 2015

Thanks @pcwalton! - A related question - what's the current thinking on CEF vs glutin for the Servo goal of an alpha browser this year? I saw there was discussion about whether to support multiple ports or just CEF but I couldn't see what the final outcome was.

@pcwalton
Copy link
Contributor

pcwalton commented Jun 17, 2015

There hasn't been a final decision AFAIK—thanks for reminding me, since I should bring it up again. I'm still in favor of going CEF-only and recreating the Glutin shell in terms of CEF.

@pcwalton
Copy link
Contributor

pcwalton commented Jun 17, 2015

@robertknight We've merged the change to Glutin in servo/glutin. Can you update to that commit in this PR? r=me with that.

@pcwalton
Copy link
Contributor

pcwalton commented Jun 17, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jun 17, 2015

📌 Commit 89e9aa0 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jun 17, 2015

Testing commit 89e9aa0 with merge aeff2eb...

bors-servo pushed a commit that referenced this pull request Jun 17, 2015
…alton

Use a faster scroll speed under X11

Platforms may report scroll deltas either in
chunks/lines/rows or pixels, depending on the
platform API and device capabilities.

If the platform reports a line/chunk-based delta
then the application needs to convert the delta
into a suitable number of pixels. Apple's documentation for example states
that the app should interpret the delta as a number of lines or rows to scroll,
depending on the type of view.

This commit just hardcodes it to 57 as
a starting point which matches the value that
Firefox calculates as the max char height
for the root frame on my system.

This depends on this Glutin PR: rust-windowing/glutin#483

Fixes #5660

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

bors-servo commented Jun 17, 2015

💔 Test failed - mac2

@pcwalton
Copy link
Contributor

pcwalton commented Jun 17, 2015

@robertknight Looks like Mac didn't compile.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 20, 2015

There appears to be a merge commit on your PR branch; please rebase on master to get rid of it.

@robertknight robertknight force-pushed the robertknight:gh5660-rob-x11_scroll_speed branch 2 times, most recently from 87b2927 to 6f24583 Jun 20, 2015
@pcwalton
Copy link
Contributor

pcwalton commented Jun 22, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jun 22, 2015

📌 Commit 6f24583 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jun 22, 2015

Testing commit 6f24583 with merge e0956e4...

bors-servo pushed a commit that referenced this pull request Jun 22, 2015
…alton

Use a faster scroll speed under X11

Platforms may report scroll deltas either in
chunks/lines/rows or pixels, depending on the
platform API and device capabilities.

If the platform reports a line/chunk-based delta
then the application needs to convert the delta
into a suitable number of pixels. Apple's documentation for example states
that the app should interpret the delta as a number of lines or rows to scroll,
depending on the type of view.

This commit just hardcodes it to 57 as
a starting point which matches the value that
Firefox calculates as the max char height
for the root frame on my system.

This depends on this Glutin PR: rust-windowing/glutin#483

Fixes #5660

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

bors-servo commented Jun 22, 2015

💔 Test failed - mac2

@pcwalton
Copy link
Contributor

pcwalton commented Jun 22, 2015

This doesn't build due to a missing glutin import

Robert Knight
Platforms may report scroll deltas either in
chunks/lines/rows or pixels, depending on the
platform API and device capabilities.

If the platform reports a line/chunk-based delta
then the application needs to convert the delta
into a suitable number of pixels.

This commit just hardcodes it to 57 as
a starting point which matches the value that
Firefox calculates as the max char height
for the root frame on my system.

Fixes #5660
@robertknight robertknight force-pushed the robertknight:gh5660-rob-x11_scroll_speed branch from 6f24583 to 658df60 Jun 22, 2015
@robertknight
Copy link
Author

robertknight commented Jun 22, 2015

Sorry @pcwalton - I hadn't updated the glutin dependency in Cargo.lock for the CEF port. It hadn't occurred to me that used glutin as well. I've just pushed a fix for that.

Is there a better way to keep the various Cargo.lock files in sync?

@pcwalton
Copy link
Contributor

pcwalton commented Jun 22, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jun 22, 2015

📌 Commit 658df60 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jun 22, 2015

Testing commit 658df60 with merge 7e0f186...

bors-servo pushed a commit that referenced this pull request Jun 22, 2015
…alton

Use a faster scroll speed under X11

Platforms may report scroll deltas either in
chunks/lines/rows or pixels, depending on the
platform API and device capabilities.

If the platform reports a line/chunk-based delta
then the application needs to convert the delta
into a suitable number of pixels. Apple's documentation for example states
that the app should interpret the delta as a number of lines or rows to scroll,
depending on the type of view.

This commit just hardcodes it to 57 as
a starting point which matches the value that
Firefox calculates as the max char height
for the root frame on my system.

This depends on this Glutin PR: rust-windowing/glutin#483

Fixes #5660

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

bors-servo commented Jun 22, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 658df60 into servo:master Jun 22, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@pcwalton
Copy link
Contributor

pcwalton commented Jun 22, 2015

\o/ Thanks for your hard work on this!

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

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