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

Allow array length() method to be called on arbitrary expressions #2493

Merged

Conversation

Oletus
Copy link
Contributor

@Oletus Oletus commented Aug 24, 2017

Reverse the expectations on a test that was based on an incorrect
interpretation of the spec. While the GLSL ES 3.00 spec says that
length is only allowed on "array names", it was always the intent to
allow it on arbitrary expressions that return an array. This has been
clarified in GLSL ES 3.20.

Since the revised test adds new requirements, it makes sense to
remove the test entirely from the 2.0.0 conformance suite.

Reverse the expectations on a test that was based on an incorrect
interpretation of the spec. While the GLSL ES 3.00 spec says that
length is only allowed on "array names", it was always the intent to
allow it on arbitrary expressions that return an array. This has been
clarified in GLSL ES 3.20.

Since the revised test adds new requirements, it makes sense to
remove the test entirely from the 2.0.0 conformance suite.
@kenrussell
Copy link
Member

Thanks Olli for researching and clarifying this. What is needed to make conformance2/glsl3/array-length-side-effects.html start passing again after this change?

@kenrussell kenrussell merged commit 9024c57 into KhronosGroup:master Aug 24, 2017
@Oletus Oletus deleted the reverse-incorrect-array-length-test branch August 25, 2017 07:44
@Oletus
Copy link
Contributor Author

Oletus commented Aug 25, 2017

A fix in ANGLE's shader translator is needed. Since HLSL doesn't have an array length operation, any expressions with side effects that have .length() called on them need to be transformed so that the expression with side effects is separated, and length() is replaced with a constant. It can probably be done something like this:

(foo()).length()

becomes

(foo()[0], 5)

where foo() is a function returning an array of length 5.

@kenrussell
Copy link
Member

Is an ANGLE bug already filed about this? What's the timeframe for getting the test passing again? It would be good to not leave tests failing for an indefinite period of time. Thanks for your help.

@Oletus
Copy link
Contributor Author

Oletus commented Aug 29, 2017

I filed an ANGLE bug now: https://bugs.chromium.org/p/angleproject/issues/detail?id=2142

It should not be that much work to fix, I'll try to take care of it during the next few weeks.

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

2 participants