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 upfix(webgl): fix issue https://github.com/servo/servo/issues/21352 #21353
Conversation
If we have a bounded framebuffer previously, which is not the one to delete. we would not change the !binding, but we would change the GLSide with Command::BindFramebuffer(Default).
highfive
commented
Aug 7, 2018
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon. |
highfive
commented
Aug 7, 2018
|
Heads up! This PR modifies the following files:
|
highfive
commented
Aug 7, 2018
|
|
|
@bors-servo try |
fix(webgl): fix issue #21352 If we have a bounded framebuffer previously, which is not the one to delete. we would not change the !binding, but we would change the GLSide with Command::BindFramebuffer(Default). change it to: check the id before sending the Command --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [x] These changes fix #21352 - [ ] There are tests for these changes OR - [x] These changes do not require tests because `it's obviously simple.` <!-- 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/21353) <!-- Reviewable:end -->
|
|
|
Hmm, the fact that the webgl conformance tests don't report any difference is a bit worrying. |
|
I will add a test, afterwards. Maybe in this Saturday. |
|
@jdm I don't think this is worrisome, there is no way to write a test that we actually deleted the thing on the GL side, given we don't ever ask it if something was deleted (we keep track of deletion status on the DOM side independently). Let's land this. |
|
@bors-servo r+ |
|
|
fix(webgl): fix issue #21352 If we have a bounded framebuffer previously, which is not the one to delete. we would not change the !binding, but we would change the GLSide with Command::BindFramebuffer(Default). change it to: check the id before sending the Command --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [x] These changes fix #21352 - [ ] There are tests for these changes OR - [x] These changes do not require tests because `it's obviously simple.` <!-- 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/21353) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
fix(webgl): fix issue #21352 If we have a bounded framebuffer previously, which is not the one to delete. we would not change the !binding, but we would change the GLSide with Command::BindFramebuffer(Default). change it to: check the id before sending the Command --- - [ ] `./mach build -d` does not report any errors - [ ] `./mach test-tidy` does not report any errors - [x] These changes fix #21352 - [ ] There are tests for these changes OR - [x] These changes do not require tests because `it's obviously simple.` <!-- 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/21353) <!-- Reviewable:end -->
|
|
DanxiongLei commentedAug 7, 2018
•
edited by SimonSapin
If we have a bounded framebuffer previously, which is not the one to delete.
we would not change the !binding, but we would change the GLSide with Command::BindFramebuffer(Default).
change it to:
check the id before sending the Command
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThese changes fix #21352
There are tests for these changes OR
These changes do not require tests because
it's obviously simple.This change is