Skip to content

Conversation

@glennw
Copy link
Member

@glennw glennw commented Sep 6, 2017

Instead of maintaining a copy of a ClipRegion etc, flatten
any input clip structures into a list of simple clips.

This simplifies some of the mask cache code, and is also
prep work for some further simplifications and optimizations
to how we handle the mask cache structures.


This change is Reviewable

Instead of maintaining a copy of a ClipRegion etc, flatten
any input clip structures into a list of simple clips.

This simplifies some of the mask cache code, and is also
prep work for some further simplifications and optimizations
to how we handle the mask cache structures.
@glennw
Copy link
Member Author

glennw commented Sep 6, 2017

r? @kvark

I think this is a net improvement, but it may not be super clear what the benefits are until the follow ups are completed. So we can leave this sit until the rest is done, if you prefer.

The main bit to check is the mask_cache update method. I think it's correct, and tests seem OK, but that code is a bit subtle so I might have missed something.

@glennw
Copy link
Member Author

glennw commented Sep 6, 2017

pub enum ClipSource {
Complex(LayerRect, f32, ClipMode),
Region(ClipRegion),
Rectangle(LayerRect),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad we are getting rid of the complex clips :)

match *clip {
ClipSource::Complex(..) => {
ClipSource::RoundedRectangle(..) => {
complex_clip_count += 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we need to rework the complex_clip_count, at least rename it

for source in sources {
match *source {
ClipSource::Complex(rect, radius, mode) => {
ClipSource::Image(ref mask) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that we have 1:1 relationship between local_rect/local_inner modification and a source item, we may go fancy and use fold() here ;)

@kvark
Copy link
Member

kvark commented Sep 6, 2017

@glennw

it may not be super clear what the benefits are until the follow ups are completed

oh no, sir, it's totally clear that this is a good simplification to me
Thank you!

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 94b76a2 has been approved by kvark

@bors-servo
Copy link
Contributor

⌛ Testing commit 94b76a2 with merge eee9a36...

bors-servo pushed a commit that referenced this pull request Sep 7, 2017
Change internal clip sources to be a simple array of clips.

Instead of maintaining a copy of a ClipRegion etc, flatten
any input clip structures into a list of simple clips.

This simplifies some of the mask cache code, and is also
prep work for some further simplifications and optimizations
to how we handle the mask cache structures.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1667)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-travis

@kvark
Copy link
Member

kvark commented Sep 7, 2017

No output has been received in the last 10m0s

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 94b76a2 with merge 01c38a2...

bors-servo pushed a commit that referenced this pull request Sep 7, 2017
Change internal clip sources to be a simple array of clips.

Instead of maintaining a copy of a ClipRegion etc, flatten
any input clip structures into a list of simple clips.

This simplifies some of the mask cache code, and is also
prep work for some further simplifications and optimizations
to how we handle the mask cache structures.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1667)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: kvark
Pushing 01c38a2 to master...

@bors-servo bors-servo merged commit 94b76a2 into servo:master Sep 7, 2017
@bors-servo bors-servo mentioned this pull request Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants