Skip to content

Conversation

@gterzian
Copy link
Member

@gterzian gterzian commented Jan 28, 2017

Fix for servo/servo#15251


This change is Reviewable

@gterzian gterzian force-pushed the fix_current_scroll_layer_caching branch 3 times, most recently from 435b776 to f954c94 Compare January 28, 2017 09:50
@gterzian gterzian force-pushed the fix_current_scroll_layer_caching branch from f954c94 to 41c7e3f Compare January 28, 2017 09:53
@gterzian
Copy link
Member Author

@jdm So I am still unable to reproduce the panic. If you can do so consistently, could I please ask you to test if this fixes it? This one requires building with servo/servo#15111

@glennw
Copy link
Member

glennw commented Jan 29, 2017

This seems OK to me - but let's double check with @mrobinson first.

@gterzian
Copy link
Member Author

@glennw Thank you. I changed the variables names a bit for clarity...

@mrobinson
Copy link
Member

I think that we will need to change the API in order to let WebRender know when a batch of scroll layers doesn't have any correspondence with the previous one. I need that for a PR that I'm working on, but a decent fallback here makes sense as well. Thanks!

@mrobinson
Copy link
Member

The new API is here: #806

This patch should work fine in tandem with that.

@glennw
Copy link
Member

glennw commented Jan 30, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 4a5403c has been approved by glennw

@bors-servo
Copy link
Contributor

⌛ Testing commit 4a5403c with merge c32413c...

bors-servo pushed a commit that referenced this pull request Jan 31, 2017
Fix currently scrolling layer id  caching

Fix for servo/servo#15251

<!-- 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/800)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 4a5403c into servo:master Jan 31, 2017
@gterzian
Copy link
Member Author

@glennw @mrobinson Thanks for your help, I will also take a look at the new API, and the scroll testing capabilities of Wrench. Sorry I didn't squash that one extra commit, I guess it's not the end of the world...

@glennw
Copy link
Member

glennw commented Jan 31, 2017

No worries, thanks for the fix!

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