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

Fix tiling and dirty rect calculations for external images #2847

Merged
merged 1 commit into from Jun 26, 2018

Conversation

aosmond
Copy link
Contributor

@aosmond aosmond commented Jun 26, 2018

  • When we have a dirty rect, we should cull all tiled requests which do
    not overlap with the dirty rect, not just blob image requests.

  • We should not take the dirty rect from the image template until we
    have processed all of the requests. This will allow each tile to
    re-upload only the modified segment.


This change is Reviewable

- When we have a dirty rect, we should cull all tiled requests which do
not overlap with the dirty rect, not just blob image requests.

- We should not take the dirty rect from the image template until we
have processed all of the requests. This will allow each tile to
re-upload only the modified segment.
@aosmond aosmond force-pushed the ao_dirty_rect_tiling_fixes branch from e94ef98 to 413bbd2 Compare June 26, 2018 12:26
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you for addressing this!
Note that the dirty rect invalidation is still wrong - we may receive updates to different tiles of the the same template across multiple frames, in which case erasing it on the first frame would make it misbehave... The new logic is better than the old one though, so I think we should proceed regardless.

@kvark
Copy link
Member

kvark commented Jun 26, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 413bbd2 has been approved by kvark

@bors-servo
Copy link
Contributor

⌛ Testing commit 413bbd2 with merge 46a967a...

bors-servo pushed a commit that referenced this pull request Jun 26, 2018
Fix tiling and dirty rect calculations for external images

- When we have a dirty rect, we should cull all tiled requests which do
not overlap with the dirty rect, not just blob image requests.

- We should not take the dirty rect from the image template until we
have processed all of the requests. This will allow each tile to
re-upload only the modified segment.

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

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing 46a967a to master...

@bors-servo bors-servo merged commit 413bbd2 into servo:master Jun 26, 2018
@staktrace
Copy link
Contributor

@aosmond This PR causes crashtest failures in gecko, see try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=e989ff39da6b5c43911c03c9c1efa133a7e15adf

@aosmond
Copy link
Contributor Author

aosmond commented Jun 27, 2018

The backtraces don't spell out the offending line number, but I suspect the logic for unwrapping request.tile vs template.tiling was incorrectly inverted, but was fine before because it only applied to blob images. update_texture_cache does it the other way.

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.

None yet

4 participants