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 upDon't delete webrender image keys immediately. #17606
Conversation
|
r? @glennw |
|
@asajeffrey Could you do the same for the webgl thread as well? #13035 (comment) suggests that it is needed there too. |
| if let Some(image_key) = self.old_image_key.take() { | ||
| self.webrender_api.delete_image(image_key); | ||
| } | ||
| if let Some(image_key) = self.old_image_key.take() { |
This comment has been minimized.
This comment has been minimized.
MortimerGoro
Jul 10, 2017
Contributor
shouldn't this be if let Some(image_key) = self.very_old_image_key.take()?
This comment has been minimized.
This comment has been minimized.
|
@jdm I'm working on a major WebGL refactor. See servo/webrender#1353 and servo/webrender#607. I'll submit a PR soon. It will be better to wait for that because WebGL thread and WR code are going to be changed (to use WR external image API) |
|
OK, we can hold off if the WebGL refactor will land soon. This bug is causing some intermittents, so it would be nice to merge it. |
|
|
asajeffrey
force-pushed the
asajeffrey:canvas-delay-deleting-image-keys
branch
from
e039da7
to
953e8a0
|
@glennw @MortimerGoro I rebased against #17694, can this land now, or should we continue to hold off? |
|
@asajeffrey yes, this can land now |
|
@jdm, do the changes for WebGL look good to you? |
| image_key: Option<webrender_api::ImageKey>, | ||
| // An old webrender image key that can be deleted when the next epoch ends. | ||
| old_image_key: Option<webrender_api::ImageKey>, | ||
| // An old webrender image key that can be deleted when the current epoch ends. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
asajeffrey
force-pushed the
asajeffrey:canvas-delay-deleting-image-keys
branch
from
8630251
to
3002c45
|
@bors-servo r=jdm |
|
|
…=jdm Don't delete webrender image keys immediately. <!-- Please describe your changes on the following line: --> We currently delete webrender image id's immediately on resizing a canvas, which can cause panics. This PR delays deleting the image id until an epoch has passed. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #17534 - [X] These changes do not require tests because the intermittent failure is already triggered by the css-paint-api tests. <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17606) <!-- Reviewable:end -->
|
|
asajeffrey commentedJul 5, 2017
•
edited by larsbergstrom
We currently delete webrender image id's immediately on resizing a canvas, which can cause panics. This PR delays deleting the image id until an epoch has passed.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is