Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upStore tile dirty rects in tile space. #3272
Merged
Conversation
Before this PR each tile stores its own dirty rect relative to the entire image. This is somewhat confusing because this field is used to track partial uploads correctly and uploads reason about dirty rects relative to the tiles so it has to do some conversions. Storing relative to the tile is also truer to the "device space" the rects are tagged with. Last but not least the image-relative coordinates will make even less sense for tiled blob images as we start trating them as virtually infinite planes.
|
try looks good, r? anyone |
| // It is important to never assume an empty dirty rect implies a full reupload here, | ||
| // although we are able to do so elsewhere. We store the descriptor's full rect instead | ||
| // It is important to never assume a dirty rect equal to None implies a full reupload here, | ||
| // although we are able to do so elsewhere. We store the image or tile's full rect instead | ||
| // There are update sequences which could cause us to forget the correct dirty regions | ||
| // regions if we cleared the dirty rect when we received None, e.g.: | ||
| // 1) Update with no dirty rect. We want to reupload everything. | ||
| // 2) Update with dirty rect B. We still want to reupload everything, not just B. | ||
| // 3) Perform the upload some time later. |
This comment has been minimized.
This comment has been minimized.
|
r=me with docs updated |
|
Ah, I see you delete the commented code in #3277 so should be fine..? |
|
@bors-servo r=Gankro |
|
|
bors-servo
added a commit
that referenced
this pull request
Nov 7, 2018
Store tile dirty rects in tile space. Before this PR each tile stores its own dirty rect relative to the entire image. This is somewhat confusing because this field is used to track partial uploads correctly and uploads reason about dirty rects relative to the tiles so it has to do some conversions. Storing relative to the tile is also truer to the "device space" the rects are tagged with. Last but not least the image-relative coordinates will make even less sense for tiled blob images as we start treating them as virtually infinite planes. <!-- 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/3272) <!-- Reviewable:end -->
|
|
|
@nical This PR is not happy-making in Gecko: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=f39162e828cb0a91e51504ef813201573bebd918 |
bors-servo
added a commit
that referenced
this pull request
Nov 7, 2018
Revert "Store tile dirty rects in tile space." Reverts #3272. It had a green try but the rebase apparently didn't go well as the webrender update is on fire. <!-- 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/3284) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Nov 7, 2018
Revert "Store tile dirty rects in tile space." Reverts #3272. It had a green try but the rebase apparently didn't go well as the webrender update is on fire. <!-- 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/3284) <!-- Reviewable:end -->
bors-servo
added a commit
that referenced
this pull request
Nov 23, 2018
Introduce the DirtyRect type This is Alexis' [DirtyRect patch](https://phabricator.services.mozilla.com/D9559) completed and rebased on top of #3272. It turns `Option<Rect>` into an explicit type which is a lot easier to understand as `None` previously meant different things in different areas of the code. <!-- 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/3277) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
nical commentedNov 5, 2018
•
edited by larsbergstrom
Before this PR each tile stores its own dirty rect relative to the entire image. This is somewhat confusing because this field is used to track partial uploads correctly and uploads reason about dirty rects relative to the tiles so it has to do some conversions. Storing relative to the tile is also truer to the "device space" the rects are tagged with.
Last but not least the image-relative coordinates will make even less sense for tiled blob images as we start treating them as virtually infinite planes.
This change is