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

Investigate if GetTexParameter should fail if 0 is returned. #20372

Closed
gootorov opened this issue Mar 21, 2018 · 2 comments
Closed

Investigate if GetTexParameter should fail if 0 is returned. #20372

gootorov opened this issue Mar 21, 2018 · 2 comments

Comments

@gootorov
Copy link
Contributor

@gootorov gootorov commented Mar 21, 2018

Currently GetTexParameter fails if 0 is returned, but none of the specs specify that.
Specs:
WebGL
GLES

let parameter = gl.get_tex_parameter_iv(target, pname);
if parameter == 0 {
Ok(WebGLParameter::Invalid)

WebGLParameter::Invalid => {
self.webgl_error(InvalidEnum);
NullValue()

@nox nox added the A-content/webgl label Mar 21, 2018
@fnune
Copy link
Contributor

@fnune fnune commented Apr 4, 2018

This is not the case anymore:

    fn get_tex_parameter(gl: &gl::Gl,
                       target: u32,
                       pname: u32,
                       chan: WebGLSender<i32> ) {
        let result = gl.get_tex_parameter_iv(target, pname);
        chan.send(result).unwrap();
    }

https://github.com/servo/servo/blob/master/components/canvas/webgl_thread.rs#L1084-L1090
I think?

@gootorov
Copy link
Contributor Author

@gootorov gootorov commented Apr 4, 2018

It was refactored, but the check was left intact:

match receiver.recv().unwrap() {
value if value != 0 => Int32Value(value),
_ => {
self.webgl_error(InvalidEnum);
NullValue()
}
}

If it's not needed, the whole thing can be simplified to Int32Value(receiver.recv().unwrap()).

@anholt anholt mentioned this issue May 6, 2018
6 of 7 tasks complete
bors-servo added a commit that referenced this issue May 7, 2018
Webgl fixes

<!-- Please describe your changes on the following line: -->
Add gl.getParameter() support for webgl unpack settings, and remove some unreachable and incorrect error-generation code

---
<!-- 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 #20372
- [x] These changes fix #20553
- [x] These changes fix #20554

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/20754)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 7, 2018
Webgl fixes

<!-- Please describe your changes on the following line: -->
Add gl.getParameter() support for webgl unpack settings, and remove some unreachable and incorrect error-generation code

---
<!-- 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 #20372
- [x] These changes fix #20553
- [x] These changes fix #20554

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/20754)
<!-- Reviewable:end -->
@nox nox added the C-assigned label May 9, 2018
bors-servo added a commit that referenced this issue May 18, 2018
Webgl fixes

<!-- Please describe your changes on the following line: -->
Add gl.getParameter() support for webgl unpack settings, and remove some unreachable and incorrect error-generation code

---
<!-- 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 #20372
- [x] These changes fix #20553
- [x] These changes fix #20554

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/20754)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 18, 2018
Webgl fixes

<!-- Please describe your changes on the following line: -->
Add gl.getParameter() support for webgl unpack settings, and remove some unreachable and incorrect error-generation code

---
<!-- 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 #20372
- [x] These changes fix #20553
- [x] These changes fix #20554

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/20754)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 19, 2018
Webgl fixes

<!-- Please describe your changes on the following line: -->
Add gl.getParameter() support for webgl unpack settings, and remove some unreachable and incorrect error-generation code

---
<!-- 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 #20372
- [x] These changes fix #20553
- [x] These changes fix #20554

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/20754)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.