Implement WebGLRenderingContext::isEnabled

[ofekd]

Implemented WebGLRenderingContext::isEnabled

• Please note the return value of the implementation, not sure about returning false on InvalidEnum.
• the webgl/conformance-1.0.3/conformance/state/gl-enable-enum-test.html wpt test is disabled on linux but was enabled on my machine and run to validate expectations.
• the /webgl/conformance-1.0.3/conformance/context/methods.html test is disabled everywhere. Enabling and running on my machine showed incorrect expectations beyond isEnabled. The test was temporarily edited to test just for isEnabled and it ran successfully.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #13040
• There are tests for these changes (please read details above)
• These changes do not require tests because _____

---

This change is

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @emilio (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/script/dom/webglrenderingcontext.rs, components/script/dom/webidls/WebGLRenderingContext.webidl
• @emilio: components/script/dom/webglrenderingcontext.rs

[jdm]

@bors-servo: try

[bors-servo]

⌛ Trying commit d379683 with merge bcc9508...

[bors-servo]

💔 Test failed - mac-rel-wpt

[highfive]

  ▶ TIMEOUT [expected OK] /webgl/conformance-1.0.3/conformance/textures/origin-clean-conformance.html

[jdm]

@bors-servo: retry

• #13341

[bors-servo]

⌛ Trying commit d379683 with merge bc4189f...

[bors-servo]

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

[bors-servo]

⌛ Trying commit d379683 with merge 0891ab1...

[bors-servo]

💔 Test failed - linux-rel

[jdm]

Filed #13342 for that crash.

[emilio]

Can you write a comment here in the lines of:

// TODO: We could write this without the need of IPC if needed, recording the calls to `enable` and `disable`.

This is more than fine for now though, thanks for doing this!

r=me with that.

[emilio]

Oh, also, bonus points would be factoring out the constants with enable and disable, but that can also be a later refactoring :)

[ofekd]

Hi @emilio, Can you elaborate a bit more about factoring out the constants? I'll try and do that.

Thanks

[emilio]

It would be something like making a method called validate_feature_enum,
for example, that checks the enum and returns true if the constant is one
of the valid ones, or false (setting an invalid enum error) in case it's
not.

You would call this function from the three sites instead of doing the
match in all three.

Thank you! :)

On Sep 21, 2016 1:58 PM, "ofekd" notifications@github.com wrote:

@ofekd commented on this pull request.

In components/script/dom/webglrenderingcontext.rs
#13337:

@@ -1399,6 +1399,25 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
buffer.map_or(false, |buf| buf.target().is_some() && !buf.is_deleted())
}

• // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
• fn IsEnabled(&self, cap: u32) -> bool {

•    match cap {
  

•        constants::BLEND | constants::CULL_FACE | constants::DEPTH_TEST | constants::DITHER |
  

•        constants::POLYGON_OFFSET_FILL | constants::SAMPLE_ALPHA_TO_COVERAGE | constants::SAMPLE_COVERAGE |
  

•        constants::SAMPLE_COVERAGE_INVERT | constants::SCISSOR_TEST => {
  

•            let (sender, receiver) = ipc::channel().unwrap();
  

Hi @emilio https://github.com/emilio, Can you elaborate a bit more
about factoring the the constants? I'll try and do that.

Thanks

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#13337, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABQwulwAckV-P4DhVC15VI3CKWKD9JWEks5qsRvdgaJpZM4KB3Lh
.

[ofekd]

@bors-servo r=emilio

[KiChjang]

@ofekd Sorry, bors-servo only listens to reviewers.

@bors-servo r=emilio

[bors-servo]

📌 Commit ee149cd has been approved by emilio

[bors-servo]

⌛ Testing commit ee149cd with merge f09595e...

[bors-servo]

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev