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

Implicitly enable EXT_float_blend and promote it to Community Approved. #2830

Merged
merged 1 commit into from
Mar 18, 2019

Conversation

kdashg
Copy link
Contributor

@kdashg kdashg commented Mar 16, 2019

Update tests to require implicit activation accordingly.

@MarkCallow
Copy link
Collaborator

This is going to get us into the same situation we got into with float rendering where apps will rely on the implicit enabling and then fail on platforms where this the feature is no available. In this case, that is every mobile platforms.

Why are we doing this? Since it is easy to update web applications why are we so keen not to break apps. They'll break anyway when run on mobile. We should force the authors to fix them by not implicitly enabling.

@kdashg
Copy link
Contributor Author

kdashg commented Mar 16, 2019

We can't update apps that are no longer maintained, so we do this dance where we enable it implicitly, but should warn in the console when it's used implicitly, and use that to convince authors to improve the portability of their content.

@kdashg
Copy link
Contributor Author

kdashg commented Mar 18, 2019

@kenrussell PTAL
I want to take this change in the tail end of Firefox 67 (which is merging to Beta soon) since we started doing the strict validation here in 67.

sdk/tests/conformance2/extensions/ext-float-blend.html Outdated Show resolved Hide resolved
@@ -29,7 +29,9 @@ void main()
}
`;

function testExtFloatBlend(shouldBlend, internalFormat) {
function testExtFloatBlend(internalFormat) {
const shouldBlend = gl.getSupportedExtensions().indexOf('EXT_float_blend') != -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

These mean the browser MUST expose EXT_float_blend if it supports float blend, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's very strict, but it is nice. Seems ok with me.

Copy link
Member

Choose a reason for hiding this comment

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

SGTM too.

@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<draft href="EXT_float_blend/">
<extension href="EXT_float_blend/">
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't there be a public_webgl post about this first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can post about it, but I don't think there's an ordering requirement.

Copy link
Member

@kenrussell kenrussell left a comment

Choose a reason for hiding this comment

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

LGTM with Kai's grammar find addressed. Please feel free to merge yourself after addressing.

@@ -29,7 +29,9 @@ void main()
}
`;

function testExtFloatBlend(shouldBlend, internalFormat) {
function testExtFloatBlend(internalFormat) {
const shouldBlend = gl.getSupportedExtensions().indexOf('EXT_float_blend') != -1;
Copy link
Member

Choose a reason for hiding this comment

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

SGTM too.

Update tests to require implicit activation accordingly.
@kdashg
Copy link
Contributor Author

kdashg commented Mar 18, 2019

Thanks!

@kdashg kdashg merged commit fae1d54 into KhronosGroup:master Mar 18, 2019
@kdashg kdashg deleted the promote-ext-float-blend branch March 18, 2019 22:30
kenrussell added a commit to kenrussell/WebGL that referenced this pull request Mar 18, 2019
Prevented building of the extension registry. Follow-up to KhronosGroup#2830.
kenrussell added a commit that referenced this pull request Mar 18, 2019
Prevented building of the extension registry. Follow-up to #2830.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants