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

Clear webrender image id when resizing a canvas. #17278

Conversation

asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jun 12, 2017

Webrender isn't very happy if images change size, so clear the webrender image key when resizing a canvas.



This change is Reviewable

@asajeffrey asajeffrey added A-content/canvas 2d canvas API A-content/script Related to the script thread A-webrender I-panic Servo encounters a panic. labels Jun 12, 2017
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 12, 2017
// Webrender doesn't let images change size, so we clear the webrender image key.
if let Some(image_key) = self.image_key.take() {
self.webrender_api.delete_image(image_key);
}
Copy link
Member

Choose a reason for hiding this comment

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

r=me, but could you do the same in the webgl_paint_thread? Right now the readback path suffers the same problem afaict.

Copy link
Member

Choose a reason for hiding this comment

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

(Or filing an issue is fine too)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do.

@asajeffrey asajeffrey force-pushed the canvas-clear-webrender-image-key-when-resizing branch from bfece4b to 4e04daa Compare June 13, 2017 14:30
@asajeffrey
Copy link
Member Author

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

📌 Commit 4e04daa has been approved by emilio

@highfive highfive assigned emilio and unassigned nox Jun 13, 2017
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 13, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 4e04daa with merge 8c2a7d6...

bors-servo pushed a commit that referenced this pull request Jun 13, 2017
…en-resizing, r=emilio

Clear webrender image id when resizing a canvas.

<!-- Please describe your changes on the following line: -->

Webrender isn't very happy if images change size, so clear the webrender image key when resizing a canvas.

---
<!-- 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 #17277
- [X] There are tests for these changes

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

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: emilio
Pushing 8c2a7d6 to master...

@bors-servo bors-servo merged commit 4e04daa into servo:master Jun 13, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/canvas 2d canvas API A-content/script Related to the script thread A-webrender I-panic Servo encounters a panic.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when setting canvas width or height
5 participants