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

async scene building: cache collisions from reused picture ids #2769

Closed
Gankra opened this issue May 23, 2018 · 3 comments
Closed

async scene building: cache collisions from reused picture ids #2769

Gankra opened this issue May 23, 2018 · 3 comments

Comments

@Gankra
Copy link
Contributor

@Gankra Gankra commented May 23, 2018

On my linux machine with async-scene-building, if I open this in one tab:

data:text/html,<div style="width: 400px;text-shadow: 1px 1px 2px green;">Hello there my name is alexis and this is my amazing test-case for shadows being broken under async scene building

and then open this in a second tab (identical except the shadow is red):

data:text/html,<div style="width: 400px;text-shadow: 1px 1px 2px red;">Hello there my name is alexis and this is my amazing test-case for shadows being broken under async scene building

the second tab has a green shadow, as it is using the cached shadow picture (really the final blur picture) from the other tab.

The issue is that async scene building repeatedly creates new FrameBuilder::empty()'s, resetting the picture id to 0 and that the picture cache doesn't factor in scene/document id's.

@kvark
Copy link
Member

@kvark kvark commented May 23, 2018

To clarify, our render task caching is susceptible to the following key collisions:

  • between different documents
  • between different epochs (counts of the scene rebuilds) of the same document

Could be trivially solved by tracking the document epoch and adding it as well as the document ID itself to the cache task key.

@Gankra
Copy link
Contributor Author

@Gankra Gankra commented May 23, 2018

@kvark kvark added the bugzilled label May 23, 2018
@gw3583
Copy link
Collaborator

@gw3583 gw3583 commented May 24, 2018

Ah, I had not realized there was a frame builder per document. A simple solution might be to just make it a global atomic u64 that we use to get the next picture id?

bors-servo added a commit that referenced this issue Jun 4, 2018
pipe through a scene_id to distinguish cached pictures

fixes #2769

<!-- 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/2776)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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