Improve the clipping and scrolling API #1428

mrobinson · 2017-06-24T10:57:29Z

This branch includes two improvements to the clipping and scrolling API:

Create DisplayListBuilder::define_scroll_frame API which avoids having to guess when a Clip item should create a ScrollFrame and when it shouldn't. Additionally, this removes a lot of weirdness around whether a ScrollFrame's clip starts at the origin of the ScrollFrame or not. It doesn't matter any longer, because the Clip created for the ScrollFrame is entirely independent.
Make all clipping coordinates relative to the StackingContext. This was whirlpool of weirdness in the API that only caused confusion. We now hide this detail, making clip coordinates the same as any other coordinates.

There are a few other small cleanups included with this, such as remove the push_clip_node and pop_clip_node APIs.

This change is

glennw · 2017-06-24T20:24:03Z

@mrobinson Do you have Servo branch with API updates for this and the previous PR? Would be good to be able to do some testing with Servo and land a WR update once this is reviewed and merged.

mrobinson · 2017-06-29T22:40:42Z

I've pushed a new commit to this branch and the Servo part of this PR is available here: https://github.com/mrobinson/servo/tree/webrender-update.

bors-servo · 2017-06-30T18:45:26Z

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


        Add a define_scroll_frame API

This makes it explicit whether or not you are creating a ScrollFrame when using the API. Before this was based on the content size of the clip, which was awkward and could lead to weird situations where you created a ScrollFrame when you didn't mean to or the reverse. Fixes #1335.

staktrace · 2017-07-07T12:30:59Z

@glennw or @kvark , can we get this one reviewed and landed? It is needed in order to do a WR update in gecko. (Well, we either need this or #1459, but this is the correct solution and #1459 is just a temp workaround).

mrobinson · 2017-07-07T12:44:34Z

I just noticed the reftest failure here. I'm looking at this now.


        Make clip coordinates relative to stacking context

Clip coordinates previously used a weird coordinate space relative to the origin of the display item. This was different than every other coordinate specified in the display list, so we smooth over this wrinkle. To do this we need to copy out the complex clips from the auxiliary list and adjust them so that they are relative to the scroll clip node. Fixes #1090. Fixes #1408.


        Add rounded rectangles for local clip

This is a slight revert of the removal of per-item complex clips, but these clips are important for Servo.


        Update the WebRender version

This is necessary because of the changes to the API.

mrobinson · 2017-07-07T17:09:57Z

Okay. The latest version of the PR fixes the typo in the ref test, but one of the jobs seemed to fail due to an error on the bot. I'll rerun that particular configuration.

glennw · 2017-07-09T22:09:49Z

Reviewed 16 of 27 files at r1, 15 of 15 files at r2, 3 of 3 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

glennw

This seems like a much cleaner API, nice!

glennw · 2017-07-09T22:11:27Z

Let's get this merged, and try to get both Servo and Gecko updated to the API changes this week :)

@bors-servo r+

bors-servo · 2017-07-09T22:11:28Z

📌 Commit 957ded1 has been approved by glennw

bors-servo · 2017-07-09T22:11:30Z

⌛ Testing commit 957ded1 with merge b9cfcc5...


        Auto merge of #1428 - mrobinson:scroll-clip-api, r=glennw

Improve the clipping and scrolling API This branch includes two improvements to the clipping and scrolling API: 1. Create `DisplayListBuilder::define_scroll_frame` API which avoids having to guess when a Clip item should create a ScrollFrame and when it shouldn't. Additionally, this removes a lot of weirdness around whether a ScrollFrame's clip starts at the origin of the ScrollFrame or not. It doesn't matter any longer, because the Clip created for the ScrollFrame is entirely independent. 2. Make all clipping coordinates relative to the StackingContext. This was whirlpool of weirdness in the API that only caused confusion. We now hide this detail, making clip coordinates the same as any other coordinates. There are a few other small cleanups included with this, such as remove the `push_clip_node` and `pop_clip_node` APIs.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1428)

bors-servo · 2017-07-09T22:39:10Z

💔 Test failed - status-travis

glennw · 2017-07-09T23:30:07Z

@bors-servo retry

Looks like a nightly-specific compile error in a dependency? Let's just confirm it's consistent...

bors-servo · 2017-07-09T23:30:10Z

⌛ Testing commit 957ded1 with merge 84d1044...


        Auto merge of #1428 - mrobinson:scroll-clip-api, r=glennw

Improve the clipping and scrolling API This branch includes two improvements to the clipping and scrolling API: 1. Create `DisplayListBuilder::define_scroll_frame` API which avoids having to guess when a Clip item should create a ScrollFrame and when it shouldn't. Additionally, this removes a lot of weirdness around whether a ScrollFrame's clip starts at the origin of the ScrollFrame or not. It doesn't matter any longer, because the Clip created for the ScrollFrame is entirely independent. 2. Make all clipping coordinates relative to the StackingContext. This was whirlpool of weirdness in the API that only caused confusion. We now hide this detail, making clip coordinates the same as any other coordinates. There are a few other small cleanups included with this, such as remove the `push_clip_node` and `pop_clip_node` APIs.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1428)

bors-servo · 2017-07-09T23:54:43Z

💔 Test failed - status-travis

glennw · 2017-07-10T00:33:51Z

This appears to be broken on nightly due to this change rust-lang/rust#34537 which is causing a compile error in core-text.

A change has been made in core-text to fix this servo/core-text-rs@6847e69 but it hasn't been version bumped / published yet.

If it requires a major version bump in core-text, that could take quite a while to get all deps updated. Perhaps we could disable building WR on nightly temporarily, since we really need to get this PR (and a few others) landed soon.

Thoughts @nox @jdm @kvark ?

glennw · 2017-07-10T02:34:28Z

#1462 has landed now, which should fix the CI error here.

@bors-servo retry

bors-servo · 2017-07-10T02:34:31Z

⌛ Testing commit 957ded1 with merge 519e519...


        Auto merge of #1428 - mrobinson:scroll-clip-api, r=glennw

Improve the clipping and scrolling API This branch includes two improvements to the clipping and scrolling API: 1. Create `DisplayListBuilder::define_scroll_frame` API which avoids having to guess when a Clip item should create a ScrollFrame and when it shouldn't. Additionally, this removes a lot of weirdness around whether a ScrollFrame's clip starts at the origin of the ScrollFrame or not. It doesn't matter any longer, because the Clip created for the ScrollFrame is entirely independent. 2. Make all clipping coordinates relative to the StackingContext. This was whirlpool of weirdness in the API that only caused confusion. We now hide this detail, making clip coordinates the same as any other coordinates. There are a few other small cleanups included with this, such as remove the `push_clip_node` and `pop_clip_node` APIs.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1428)

bors-servo · 2017-07-10T03:14:12Z

☀️ Test successful - status-travis
Approved by: glennw
Pushing 519e519 to master...

mrobinson requested review from kvark and glennw Jun 24, 2017

mrobinson force-pushed the mrobinson:scroll-clip-api branch from 6371218 to 6b5a3a1 Jun 29, 2017

mrobinson force-pushed the mrobinson:scroll-clip-api branch 2 times, most recently from c3a09cd to 82a4153 Jun 30, 2017

matprec mentioned this pull request Jul 4, 2017

Update to latest webrender thinklibs/fungui#18

Closed

mrobinson force-pushed the mrobinson:scroll-clip-api branch from 82a4153 to e5f754a Jul 6, 2017

mrobinson mentioned this pull request Jul 7, 2017

Treat all clips as scroll frames (partial revert of #1412) #1459

Closed

staktrace mentioned this pull request Jul 7, 2017

Remove the per-item complex clipping and masking from the API #1412

Merged

mrobinson added 3 commits Jun 24, 2017

Add rounded rectangles for local clip

5066a13

This is a slight revert of the removal of per-item complex clips, but these clips are important for Servo.

Update the WebRender version

Loading status checks…

957ded1

This is necessary because of the changes to the API.

mrobinson force-pushed the mrobinson:scroll-clip-api branch from e5f754a to 957ded1 Jul 7, 2017

glennw approved these changes Jul 9, 2017

View changes

This was referenced Jul 10, 2017

WIP: Replace Text Blur with PushTextShadow and PopTextShadow DisplayItems #1454

Merged

Add an experimental geometry api #1461

Closed

mrobinson deleted the mrobinson:scroll-clip-api branch Jul 17, 2017

servo / webrender

Improve the clipping and scrolling API #1428

Improve the clipping and scrolling API #1428

mrobinson commented Jun 24, 2017 •

edited by larsbergstrom

glennw commented Jun 24, 2017

mrobinson commented Jun 29, 2017

bors-servo commented Jun 30, 2017

staktrace commented Jul 7, 2017

mrobinson commented Jul 7, 2017

mrobinson commented Jul 7, 2017

glennw commented Jul 9, 2017

glennw left a comment

glennw commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

glennw commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

glennw commented Jul 10, 2017

glennw commented Jul 10, 2017

bors-servo commented Jul 10, 2017

bors-servo commented Jul 10, 2017

servo / webrender

Join GitHub today

Improve the clipping and scrolling API #1428

Improve the clipping and scrolling API #1428

Conversation

mrobinson commented Jun 24, 2017 • edited by larsbergstrom

glennw commented Jun 24, 2017

mrobinson commented Jun 29, 2017

bors-servo commented Jun 30, 2017

staktrace commented Jul 7, 2017

mrobinson commented Jul 7, 2017

mrobinson commented Jul 7, 2017

glennw commented Jul 9, 2017

glennw left a comment

glennw commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

glennw commented Jul 9, 2017

bors-servo commented Jul 9, 2017

bors-servo commented Jul 9, 2017

glennw commented Jul 10, 2017

glennw commented Jul 10, 2017

bors-servo commented Jul 10, 2017

bors-servo commented Jul 10, 2017

mrobinson commented Jun 24, 2017 •

edited by larsbergstrom