KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

readonly writeonly buffer variable #7

Closed qjia7 closed 6 years ago

qjia7 commented 7 years ago

In GLSL ES 3.10 session 4.9 [Memory Access Qualifiers], it has the following description:'A variable could be qualified as both readonly and writeonly, disallowing both read and write, but still be passed to imageSize() to have the size queried. '. That is for image variable. However, for buffer variable, it seems that there is no such scenario for both readonly and writeonly. And I tried the native deqp in linux. The result is that native dEQP treat a buffer that has both readonly and writeonly memory qualifiers as an compile error. I think GLSL ES spec should clearly specify whether readonly writeonly buffer variable is allowed. For example: const std::string &source = "#version 310 es\n" "layout(binding = 3) buffer buf {\n" " readonly writeonly int b1;\n" "};\n" "void main()\n" "{\n" "}\n"; Above shader should be compiled successfully or produce a compile-time error?

johnkslang commented 7 years ago

Can you provide more information about the test that expects readonly writeonly to be an error? This appears to be an inconsistency between the specification (which allows it, because it does not say it is an error) and the test, which we will look at internally in Khronos. Having more information about the test will help.

qjia7 commented 7 years ago

@johnkslang Sorry for the late response. The description has been updated. Please take a look.

johnkslang commented 7 years ago

We will look at Khronos.

The general rule is that things are allowed by the grammar, and then disallowed by specific statements in the spec body. So, unless there is a rule disallowing a combination (that the grammar allows), it is legal.

gnl21 commented 7 years ago

@qjia7 I can't find a dEQP test that expects this to be a compile error. I believe that it is allowed by the spec and testing suggests that at least some implementations allow it.

There is a possible use for this kind of qualification (analogous to image size), for example: buffer B { readonly writeonly arr[]; };

void main() { arr.size(); }

Perhaps the GLSL spec text could be updated to make it clear that the mention of imageSize is just an example.

qjia7 commented 7 years ago

@gnl21 Sorry that I forgot to say that I modified the deqp case and added the readonly writeonly buffer variable to test. After that, the compile failed. If I only add readonly buffer variable or writeonly buffer variable, the compile passed. It's only to verify whether the driver follows the spec. However, it seems that the driver implementation treats it as illegal. Thanks to provide the example. Maybe that case can be added to the deqp test.

johnkslang commented 7 years ago

This sounds like a defect in your driver, as the Khronos intent is to allow readonly writeonly. Khronos will update the GLSL specification to make this more clear.

ghost commented 6 years ago

The WG agreed that a spec clarification is in order, leaving assigned to @johnkslang to propose a change. The driver bug is a separate issue, and a CTS test is now written which should catch that in future.

johnkslang commented 6 years ago

The specification is internally modified now to be explicit about allowing readonly writeonly to images, block members, and block level. Should see it at next release.

pdaniell-nv commented 6 years ago

The GLSL/ESSL specs have been updated and released. Closing.