Basically if a command is fully validated, it should no longer be called "unsafe". Instead, it should be guarded by if the context is ES3. If it's ES2 and there is no extension to enable the command, we should return InvalidCommand.
We still need a tag like es3api to replace 'unsafe' in gpu/command_buffer/build_gles2_cmd_buffer.py (In this file, we have a 'unsafe' tag for all es3 apis). Because it is not valid to call into es3 api when the caller is in WebGL1 or GLES2 context.
That's OK, Ken. I have investigated this issue these days, so I would like to summarize what we need to do for this issue:
1. Remove ‘unsafe’ tag in gpu/command_buffer/build_gles2_cmd_buffer.py, but we still need to use a tag like “es3api” to distinguish es3 api from es2 api. Then re-generate all auto-gen files.
2. Remove command line option –enable-unsafe-es3-apis from src/content/ and src/gpu/, see the original changes which add the command line option: https://crrev.com/894143002, https://crrev.com/911243002/, https://crrev.com/1571913002 , https://crrev.com/1753383003/
3. Workaround or fix the bug for gpu_unittests when es3 api is enabled by default. Once the command line option –enable-unsafe-es3-apis is removed, lots of gpu_unittests will fail. You can reproduce the bug by running gpu_unittests with –enable-unsafe-es3-apis command line option before it is removed. In addition, you can see a similar workaround at https://crrev.com/2291753002/ through IsES3Capable().
4. Remove the command line switch --enable-unsafe-es3-api in gpu-unittests and use appropriate context (For example, use ES3 context type). Some gpu_unittests added this cmd line option to test es3 api.
5. Remove the command line switch --enable-unsafe-es3-api from the gpu harness test in content/gpu/gpu_tests/, for example, these patches introduced the command line option: https://crrev.com/1385323002/ , https://crrev.com/2375323002/ , https://crrev.com/2363343002/.
6. Consider to expose es3 api to some other components like pepper3d api, etc.
I have upload my local patch to Chromium code review, feel free to pick up the code at https://codereview.chromium.org/2444813002/.
Thanks Yunchao for your detailed writeup and suggested patch set https://codereview.chromium.org/2444813002/ . We'll look carefully at it. Appreciate your continued help on these projects and sorry for stepping on your toes.
Ken, no problem. BTW, I will continue to investigate this issue. But if you think that the progress is slow, please feel free to assign it to anyone at your side and pick up my code or just start this issue from scratch at any time. I am OK, :).
some more sub-tasks about this issue except I listed at #4:
7) remove UnsafeES3APIs from ui/gpu/ and try to create ES3 context by default.
8) remove UnsafeES3APIs runtime flag from Blink WebGL, see the change at https://codereview.chromium.org/2451943002/
Zhenyao and Kai, as you are manually checking the validation code for every ES3 APIs. I want to take another sub-task. I have been aware of this task for a long time, but have no time to do it. I think it is time to do it.
The sub-task is: check all undefined behaviors in ES3 spec, make sure we have clearly defined the corresponding behavior in WebGL2 spec, and update the Chromium code if necessary, as well as update WebGL Spec/CTS if necessary.
yunchao: sounds good. I did a rough round on that and have already closed a bunch of holes. The one thing I didn't implement is the one you try to address right now, the texture rendering/sampling feedback loop (due to MRT). The reason was it requires some optimization because we don't want to add heavy-weighted validation in per draw call. I am sure I might miss more stuff, so a second pair of eyes definitely sound good.
Comment 1 by yunchao...@intel.com
, Oct 12 2016Owner: yunchao...@intel.com
Status: Assigned (was: Available)