Closed bansan85 closed 1 year ago
I finally took to some time to understand what's the problem is.
At https://github.com/mosra/magnum/commit/1847c7201df44bbc78a4bbf2b41838f080890580#diff-c0ea44662863be913517b9e9a9ce2befe9c9eaadf01e4e36283ff9e97e0164d5R147, you say that supportedVersion
is GLES310
/ 300 / 200.
But at https://github.com/mosra/magnum/commit/1847c7201df44bbc78a4bbf2b41838f080890580#diff-c0ea44662863be913517b9e9a9ce2befe9c9eaadf01e4e36283ff9e97e0164d5R344, you exclude GLES310.
Do you think you should replace state._version < GL::Version::GLES310
by state._version <= GL::Version::GLES310
?
Hey, sorry for completely neglecting this issue, and your PRs.
This particular commit is enabling a GLES 3.1+ feature where texture, input and output locations can be specified directly in the shader using layout()
qualifiers instead of having to specify them externally from the C++ code. Thus, the checks should indeed be version < GLES310
, i.e. calling bindAttribute()
etc. only if GLES 3.1 isn't supported.
That it renders a gray image seems to be some sort of a driver bug, which I would most probably need to work around in some way. I ran through the shader tests on a bunch of targets that have GLES 3.2 and it all passes for me. You're saying that if you use <=
instead, it works? Do you need to make that change in both branches or just in one?
Can you share more details about the hardware you test on? Ideally the output of the magnum-gl-info
executable if you can run it there. Thank you!
No problem for the delay. Plus, hardware specific problem could be difficult to fix.
The phone:
https://rog.asus.com/phones/rog-phone-6-model/
The log:
Yes, replacing <
by <=
solves the problem.
Branch 1 and 2 enabled : color Branch 1 disabled, 2 enabled : gray Branch 1 enabled, 2 disabled : color Branch 1 and 2 disabled : gray
Magnum/configure.h
:
#define MAGNUM_TARGET_GL
#define MAGNUM_TARGET_GLES
/* #undef MAGNUM_TARGET_GLES2 */
#define MAGNUM_TARGET_GLES3
/* #undef MAGNUM_TARGET_WEBGL */
#define MAGNUM_TARGET_EGL
/* #undef MAGNUM_TARGET_VK */
For branch 1, if I force it :
#ifndef MAGNUM_TARGET_GLES2
: true
_flags
: 0
_jointCount
: false
Tell me if you need for information.
Thank you!
So that seems like explicit uniform location isn't working on Adreno. Or a subset of it, or the shader is triggering some unfortunate code path in the driver but the rest of the feature works. (I remember having an issue with explicit uniform location on Intel drivers on Windows, where the explicit uniform location only worked if the uniforms were numbered in an order exactly as they appeared in the code, with no gaps, and that was just impossible to workaround in a robust way so there I had to disable them completely.)
I looked around and didn't find any bugreports related to this. But since it's an ES3.1 feature, it's possible that very few projects actually opted in to use it and stayed on the classic way for more compatibility (which also is why it may be buggy). The list at https://chromium.googlesource.com/chromium/src/gpu/+/refs/heads/main/config/gpu_driver_bug_list.json contains a ton of Adreno-related bugs, but this one not, probably for a similar reason, that Chrome just doesn't make use of explicit uniform location.
I have extensive tests for the Flat shader, could you run them on the device? Assuming you have adb
already set up, it should be about doing this:
master
yet, you may want to update to include commit 38b6c0fee899929f833eb1977090ec1f1f3b400f that adds graceful handling for certain driver limits that I discovered when running the tests myself yesterdayMAGNUM_BUILD_TESTS
and MAGNUM_BUILD_GL_TESTS
enabled, and MAGNUM_WITH_TGAIMPORTER
and MAGNUM_WITH_ANYIMAGEIMPORTER
as well so the tests can load imagesCORRADE_TEST_COLOR=ON ctest -R ShadersFlatGLTest -V
-- CMake's ctest
uploads the test executable together with all input and ground truth images to /data/local/tmp
and runs it, the colors enabled are just to make the output look clearer in the terminal but otherwise are non-essentialI'm interested in an output both with vanilla unchanged code, and with the "Branch 1 enabled, 2 disabled" changes from above. The output will show some SKIP
cases for multi-draw tests (as those work only on ANGLE and WebGL) and possibly for SSBO tests, but other than that it should run through all, printing various detailed error reports if something fails. I still half-hope this problem is related just to some corner case and not the whole feature, so hopefully even the unchanged build would show at least some of the render*()
cases passing and then I could possibly figure out a smaller workaround. If not, then I'd have to add an Adreno-specific workaround to disable explicit uniform location altogether.
Thanks again!
I wasn't able to run ctest. So I copy all built files and all tga files to device. I hope it's fine.
false_false.log : first branch disabled / second branch disabled (vanilla)
true_false.log : first branch forced (if(true)
) / second branch disabled (if(false)
)
true_true.log : first branch forced (if(true)
) / second branch forced (if(true)
)
Both branch definitely must be forced in my case.
Thanks, nice, so it can be convinced to render everything properly :)
The GL errors in the first case are suspicious, and might point to something. Could you re-run the first case with --magnum-gpu-validation on
passed on the command line? That should print more detailed info for the GL errors happening.
In the second case it looks like it only affects UBO usage and not classic uniforms, which reduces the scope of the needed workaround quite a lot. On the other hand, SSBO binding works, even though it's the same layout(binding=N)
syntax. Heh.
Sorry that you couldn't get ctest to run, forgot to mention the runner script was only ever tested on Linux. (I wanted to eventually ask you to run all tests there to uncover potential other driver bugs, but given there's about 300 test executables, without the ctest automation it'd be way too painful.)
Oh, and one more thing so I have the full information, can you get me the output of magnum-gl-info --limits
as well? Thank you!
I launched a find . -name "*Test" -exec {} \; &> all_tests.log
on vanilla. There is a lots of failures :/ But I have a working software so I can live with that. But if you want I do more tests, I can. all_tests.log
The result of vanilla for ShadersFlatGLTest --magnum-gpu-validation on
. vanilla_gpu_validation.log
And magnum-gl-info --limits
. magnum-gl-info--limits.log
It doesn't seem that bad, actually :)
GL::Buffer::bind()
error, which is likely my fault. This call is used heavily in the shader tests, and you managed to make all those pass, so that's just a broken test case, most likely due to not respecting alignment restrictions there.Text::Renderer
test that aborts due to an assertion that shouln't be there on a GLES3 build at all. I assume that's just some stale binary, so also nothing to be concerned with.The debug / GPU validation log unfortunately didn't enlighten me much. It seems like the driver just completely ignores the uniform(binding=N)
and uniform(location=N)
qualifiers in the shader code.
I got two "last hope" ideas for fixing the Flat shader -- can you try this patch with no other change? Maybe the driver just has some version checks messed up and compiling the shader as GLSL ES 3.20 instead of 3.10 would make it recognize everything properly?
diff --git a/src/Magnum/Shaders/FlatGL.cpp b/src/Magnum/Shaders/FlatGL.cpp
index fe5a5cbe9..412c34931 100644
--- a/src/Magnum/Shaders/FlatGL.cpp
+++ b/src/Magnum/Shaders/FlatGL.cpp
@@ -162,7 +162,7 @@ template<UnsignedInt dimensions> typename FlatGL<dimensions>::CompileState FlatG
#else
const GL::Version version = context.supportedVersion({
#ifndef MAGNUM_TARGET_WEBGL
- GL::Version::GLES310,
+ GL::Version::GLES320,
#endif
GL::Version::GLES300, GL::Version::GLES200});
#endif
The other idea would be this -- again try this change alone, not even with the above. This makes the EXPLICIT_BINDING
and EXPLICIT_UNIFORM_LOCATION
defines present always, regardless of the __VERSION__
used. These guard the layout(binding=N)
and layout(location=N)
qualifiers. I'm thinking that maybe the driver just reports some strange __VERSION__
value which caused these to get ignored. If this change helps, I'd be interested in knowing what's the __VERSION__
actually reported.
diff --git a/src/Magnum/Shaders/compatibility.glsl b/src/Magnum/Shaders/compatibility.glsl
index 7f5b9eb24..d8bde772c 100644
--- a/src/Magnum/Shaders/compatibility.glsl
+++ b/src/Magnum/Shaders/compatibility.glsl
@@ -47,12 +47,10 @@
#if __VERSION__ >= 300
#define EXPLICIT_ATTRIB_LOCATION
#endif
- #if __VERSION__ >= 310
- #define EXPLICIT_BINDING
- #define EXPLICIT_UNIFORM_LOCATION
- #endif
/* RUNTIME_CONST is not available in OpenGL ES */
#endif
+#define EXPLICIT_BINDING
+#define EXPLICIT_UNIFORM_LOCATION
/* Precision qualifiers are not supported in GLSL 1.20 */
#if !defined(GL_ES) && __VERSION__ == 120
If neither of the two make any difference, I'll proceed with creating the full workaround. Thanks a lot for being this responsive!
Thanks for your diagnostic. It's annoying that GPU driver are so buggy. LibreOffice also maintains a (very) small blacklist GPU list for OpenGL / Skia for computer but blocks very widely.
For vanilly + GL::Version::GLES310
=> GL::Version::GLES320
, it doesn't solve anything. ShadersFlatGLTest --magnum-gpu-validation on
randomly crash at random tests. After 10 runs, I success to have a full log. gles320.log
For vanilly + change to force EXPLICIT_BINDING
/ EXPLICIT_UNIFORM_LOCATION
. Nice :) Everything works. force_explicit.log
__VERSION__
in compatibility.glsl
equals 300.
Thank you!
__VERSION__
incompatibility.glsl
equals 300.
Well, that's just stupid :/ Such a silly bug. So basically the driver supports everything and has no bugs whatsoever, it's just that when they added ES3.1+ support they forgot to update the __VERSION__
builtin? :sweat_smile:
So the fix would be to supply my own version define and ignore __VERSION__
on this driver. I'm figuring out some nice way to do this, will get back to you later today with a patch to test.
I searched a little on https://rog-forum.asus.com without success. My Rog 6 phone is up to date (33.0610.2810.104) and not rooted.
Alright, if I did everything correctly, the a00270cd6ccea390000ed9b1030c67b8dc9bd409 and 9b160e95c391d95dbbc09cce367f3a2df4255cd6 commits in the next
branch should detect the Adreno driver, apply a adreno-glsl-version-stuck-at-300
workaround (which will be listed in the startup log), the ShadersFlatGLTest
should now pass without errors, and so should work your own code as well.
Once you confirm, I'll merge to master
.
I checkout next. I have color without any modification! Thanks,
log for all tests. I added some missing data files. all_tests.log
Wonderful :) The above is now pushed to master
.
I went through the log and everything seems fine now, the only remaining errors I see in ShadersMeshVisualizerGLTest
. The GL errors happen only in some test cases, so it's likely something minor that I could fix quickly.
If you still have time, could you re-run this single test with --magnum-gpu-validation on
and give me the output? It also asserts on a shader compilation error due to the test not checking limits thoroughly enough, hopefully this patch fixes all of those asserts and it's just the GL errors left:
diff --git a/src/Magnum/Shaders/Test/MeshVisualizerGLTest.cpp b/src/Magnum/Shaders/Test/MeshVisualizerGLTest.cpp
index b579a4d35..45fd6e710 100644
--- a/src/Magnum/Shaders/Test/MeshVisualizerGLTest.cpp
+++ b/src/Magnum/Shaders/Test/MeshVisualizerGLTest.cpp
@@ -4581,8 +4581,9 @@ template<MeshVisualizerGL2D::Flag flag> void MeshVisualizerGLTest::renderObjectV
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags2D & MeshVisualizerGL2D::Flag::TextureTransformation ? 4 : 3))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -4796,8 +4797,9 @@ template<MeshVisualizerGL3D::Flag flag> void MeshVisualizerGLTest::renderObjectV
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags3D & MeshVisualizerGL3D::Flag::TextureTransformation ? 5 : 4))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -5290,8 +5292,9 @@ template<MeshVisualizerGL2D::Flag flag> void MeshVisualizerGLTest::renderSkinnin
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 4)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.jointCount ? 4 : 3))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -5451,8 +5454,9 @@ template<MeshVisualizerGL3D::Flag flag> void MeshVisualizerGLTest::renderSkinnin
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 4)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.jointCount ? 5 : 4))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -5613,8 +5617,9 @@ template<MeshVisualizerGL2D::Flag flag> void MeshVisualizerGLTest::renderInstanc
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags & MeshVisualizerGL2D::Flag::TextureTransformation ? 3 : 4))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -5911,7 +5916,7 @@ template<MeshVisualizerGL3D::Flag flag> void MeshVisualizerGLTest::renderInstanc
#endif
/* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags & MeshVisualizerGL3D::Flag::TextureTransformation ? 5 : 4))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -6239,8 +6244,9 @@ template<MeshVisualizerGL2D::Flag flag> void MeshVisualizerGLTest::renderInstanc
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 4)
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -6396,8 +6402,9 @@ template<MeshVisualizerGL3D::Flag flag> void MeshVisualizerGLTest::renderInstanc
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 5)
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
} else
#endif
@@ -6571,8 +6578,9 @@ void MeshVisualizerGLTest::renderMulti2D() {
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags & MeshVisualizerGL2D::Flag::TextureTransformation ? 4 : 3))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
}
#endif
@@ -6910,8 +6918,9 @@ void MeshVisualizerGLTest::renderMulti3D() {
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 3)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < (data.flags & MeshVisualizerGL3D::Flag::TextureTransformation ? 5 : 4))
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
}
#endif
@@ -7253,7 +7262,8 @@ void MeshVisualizerGLTest::renderMultiSkinningWireframe2D() {
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 4)
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
}
@@ -7503,8 +7513,9 @@ void MeshVisualizerGLTest::renderMultiSkinningWireframe3D() {
CORRADE_SKIP(GL::Version::GLES310 << "is not supported.");
#endif
- /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders */
- if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 4)
+ /* Some drivers (ARM Mali-G71) don't support SSBOs in vertex shaders,
+ some others (Qualcomm Adreno 730) support just 4 blocks */
+ if(GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) < 5)
CORRADE_SKIP("Only" << GL::Shader::maxShaderStorageBlocks(GL::Shader::Type::Vertex) << "shader storage blocks supported in vertex shaders.");
}
#endif
Thanks!
I patched MeshVisualizerGLTest.cpp then ran all tests with and without gpu-validation. all_tests_rog6.zip
And if you are interested in, the same tests ran in a ROG5, rooted and not up to date 18.0840.2112.210. rog5.zip
Ahh, finally it clicked for me. The remaining errors were my fault, geometry-shader-less wireframe rendering in the MeshVisualizer has some unused uniforms (in particular, they get DCE'd by the shader compiler), and turns out the Adreno driver doesn't like that I'm attempting to set them even though they're not there anymore. Not sure if it's valid or invalid according to the GL spec, nevertheless it's something I probably shouldn't be doing.
So I patched the shader in 65669be25c46934e45fb7e9587009509240956a6 to not do that anymore, and that should fix the remaining errors. Which makes this issue fully resolved and I can finally close it :tada:
Thanks for all the help and diagnostic here, much appreciated! I looked at the ROG5 test output, it seems that the older drivers work equally well, which is very nice.
Thanks for spending time on my problem :) I don't have any remaining problem.
I tried to update from corrade/magnum 2020.06 to master bc1859f653e4dde58df1a1291cd75ce0325bcf30.
I now have my image in gray instead of color. I made a bisect and git gave me commit 1847c7201df44bbc78a4bbf2b41838f080890580.
I used corrade mosra/corrade@68d0216383edaf2c8f03da9c404e2158c4790c27
I only maintains the application. I didn't develop this part. What information do you need to diagnostic the problem ?