From b3759b6a981e75f0045ab06d3e3cb8f83c44d067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 21 Oct 2019 00:18:32 +0200 Subject: [PATCH] GL: ensure gl_PointCoord works correctly also on compatibility profile. --- doc/changelog.dox | 5 + .../GL/Implementation/RendererState.cpp | 16 +- src/Magnum/GL/Implementation/RendererState.h | 4 +- src/Magnum/GL/Implementation/State.cpp | 2 +- src/Magnum/GL/Test/CMakeLists.txt | 41 ++++- src/Magnum/GL/Test/RendererGLTest.cpp | 164 +++++++++++++++++- .../Test/RendererGLTestFiles/pointcoord.tga | Bin 0 -> 786 bytes src/Magnum/GL/Test/configure.h.cmake | 3 + 8 files changed, 227 insertions(+), 8 deletions(-) create mode 100644 src/Magnum/GL/Test/RendererGLTestFiles/pointcoord.tga diff --git a/doc/changelog.dox b/doc/changelog.dox index 5ad6e3cb6..26a439869 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -210,6 +210,11 @@ See also: @ref opengl-workarounds, @ref platforms-html5-webgl-timer-queries and @ref platforms-html5-webgl-queries-zero for more information about various caveats. +- Explicitly calling @cpp glEnable(GL_POINT_SPRITE) @ce on compatibility + contexts in order to ensure the GLSL @cpp gl_PointCoord @ce variable has + a defined output. On core and ES/WebGL contexts this is enabled implicitly, + but for example NVidia drivers have @cpp gl_PointCoord @ce undefined when + `GL_POINT_SPRITE` is not enabled on compatibility contexts. - New `--magnum-gpu-validation` @ref GL-Context-command-line "command-line option" and a corresponding environment variable to conveniently enable @gl_extension{KHR,debug} debug output. This flag also causes diff --git a/src/Magnum/GL/Implementation/RendererState.cpp b/src/Magnum/GL/Implementation/RendererState.cpp index 95b1fb732..6bd57608d 100644 --- a/src/Magnum/GL/Implementation/RendererState.cpp +++ b/src/Magnum/GL/Implementation/RendererState.cpp @@ -31,7 +31,7 @@ namespace Magnum { namespace GL { namespace Implementation { -RendererState::RendererState(Context& context, std::vector& extensions) +RendererState::RendererState(Context& context, ContextState& contextState, std::vector& extensions) #ifndef MAGNUM_TARGET_WEBGL : resetNotificationStrategy() #endif @@ -106,6 +106,20 @@ RendererState::RendererState(Context& context, std::vector& extensi else minSampleShadingImplementation = nullptr; #endif + + #ifndef MAGNUM_TARGET_GLES + /* On compatibility profile we need to explicitly enable GL_POINT_SPRITE + in order to have gl_PointCoord working (on NVidia at least, Mesa behaves + as if it was always enabled). On core profile this is enabled + implicitly, thus GL_POINT_SPRITE is not even in headers and calling + glEnable(GL_POINT_SPRITE) would cause a GL error. See + RendererGLTest::pointCoord() for more information. */ + if(!context.isCoreProfileInternal(contextState)) { + glEnable(0x8861 /*GL_POINT_SPRITE*/); + } + #else + static_cast(contextState); + #endif } RendererState::PixelStorage::PixelStorage(): diff --git a/src/Magnum/GL/Implementation/RendererState.h b/src/Magnum/GL/Implementation/RendererState.h index 98e3af47d..76a045e06 100644 --- a/src/Magnum/GL/Implementation/RendererState.h +++ b/src/Magnum/GL/Implementation/RendererState.h @@ -33,8 +33,10 @@ namespace Magnum { namespace GL { namespace Implementation { +struct ContextState; + struct RendererState { - explicit RendererState(Context& context, std::vector& extensions); + explicit RendererState(Context& context, ContextState& contextState, std::vector& extensions); Range1D(*lineWidthRangeImplementation)(); void(*clearDepthfImplementation)(GLfloat); diff --git a/src/Magnum/GL/Implementation/State.cpp b/src/Magnum/GL/Implementation/State.cpp index 5e45b8942..c3c98478c 100644 --- a/src/Magnum/GL/Implementation/State.cpp +++ b/src/Magnum/GL/Implementation/State.cpp @@ -66,7 +66,7 @@ State::State(Context& context, std::ostream* const out) { framebuffer.reset(new FramebufferState{context, extensions}); mesh.reset(new MeshState{context, *this->context, extensions}); query.reset(new QueryState{context, extensions}); - renderer.reset(new RendererState{context, extensions}); + renderer.reset(new RendererState{context, *this->context, extensions}); shader.reset(new ShaderState(context, extensions)); shaderProgram.reset(new ShaderProgramState{context, extensions}); texture.reset(new TextureState{context, extensions}); diff --git a/src/Magnum/GL/Test/CMakeLists.txt b/src/Magnum/GL/Test/CMakeLists.txt index 4a620cec2..2233b0711 100644 --- a/src/Magnum/GL/Test/CMakeLists.txt +++ b/src/Magnum/GL/Test/CMakeLists.txt @@ -101,12 +101,14 @@ if(NOT MAGNUM_TARGET_GLES) endif() if(BUILD_GL_TESTS) + # Otherwise CMake complains that Corrade::PluginManager is not found, wtf + find_package(Corrade REQUIRED PluginManager) + corrade_add_test(GLAbstractTextureGLTest AbstractTextureGLTest.cpp LIBRARIES MagnumOpenGLTesterTestLib) corrade_add_test(GLBufferGLTest BufferGLTest.cpp LIBRARIES MagnumOpenGLTester) corrade_add_test(GLCubeMapTextureGLTest CubeMapTextureGLTest.cpp LIBRARIES MagnumOpenGLTesterTestLib) corrade_add_test(GLFramebufferGLTest FramebufferGLTest.cpp LIBRARIES MagnumOpenGLTesterTestLib) corrade_add_test(GLMeshGLTest MeshGLTest.cpp LIBRARIES MagnumOpenGLTesterTestLib) - corrade_add_test(GLRendererGLTest RendererGLTest.cpp LIBRARIES MagnumOpenGLTester) corrade_add_test(GLRenderbufferGLTest RenderbufferGLTest.cpp LIBRARIES MagnumOpenGLTester) corrade_add_test(GLTextureGLTest TextureGLTest.cpp LIBRARIES MagnumOpenGLTesterTestLib) corrade_add_test(GLTimeQueryGLTest TimeQueryGLTest.cpp LIBRARIES MagnumOpenGLTester) @@ -125,16 +127,49 @@ if(BUILD_GL_TESTS) if(CORRADE_TARGET_EMSCRIPTEN OR CORRADE_TARGET_ANDROID) set(SHADERGLTEST_FILES_DIR "ShaderGLTestFiles") + set(RENDERERGLTEST_FILES_DIR "RendererGLTestFiles") else() set(SHADERGLTEST_FILES_DIR ${CMAKE_CURRENT_SOURCE_DIR}/ShaderGLTestFiles) + set(RENDERERGLTEST_FILES_DIR ${CMAKE_CURRENT_SOURCE_DIR}/RendererGLTestFiles) + endif() + + # CMake before 3.8 has broken $ expressions for iOS (see + # https://gitlab.kitware.com/cmake/cmake/merge_requests/404) and since + # Corrade doesn't support dynamic plugins on iOS, this sorta works around + # that. Should be revisited when updating Travis to newer Xcode (xcode7.3 + # has CMake 3.6). + if(NOT BUILD_PLUGINS_STATIC) + if(WITH_ANYIMAGEIMPORTER) + set(ANYIMAGEIMPORTER_PLUGIN_FILENAME $) + endif() + if(WITH_TGAIMPORTER) + set(TGAIMPORTER_PLUGIN_FILENAME $) + endif() endif() + # First replace ${} variables, then $<> generator expressions configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configure.h.cmake - ${CMAKE_CURRENT_BINARY_DIR}/configure.h) + ${CMAKE_CURRENT_BINARY_DIR}/configure.h.in) + file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$/configure.h + INPUT ${CMAKE_CURRENT_BINARY_DIR}/configure.h.in) + + corrade_add_test(GLRendererGLTest RendererGLTest.cpp + LIBRARIES MagnumOpenGLTester MagnumDebugTools + FILES RendererGLTestFiles/pointcoord.tga) + target_include_directories(GLRendererGLTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) + if(BUILD_PLUGINS_STATIC) + if(WITH_ANYIMAGEIMPORTER) + target_link_libraries(GLRendererGLTest PRIVATE AnyImageImporter) + endif() + if(WITH_TGAIMPORTER) + target_link_libraries(GLRendererGLTest PRIVATE TgaImporter) + endif() + endif() + corrade_add_test(GLShaderGLTest ShaderGLTest.cpp LIBRARIES MagnumOpenGLTester FILES ShaderGLTestFiles/shader.glsl) - target_include_directories(GLShaderGLTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) + target_include_directories(GLShaderGLTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) set_target_properties( GLAbstractTextureGLTest diff --git a/src/Magnum/GL/Test/RendererGLTest.cpp b/src/Magnum/GL/Test/RendererGLTest.cpp index 355a64a61..64bada9ab 100644 --- a/src/Magnum/GL/Test/RendererGLTest.cpp +++ b/src/Magnum/GL/Test/RendererGLTest.cpp @@ -23,10 +23,29 @@ DEALINGS IN THE SOFTWARE. */ +#include +#include +#include +#include + +#include "Magnum/Image.h" +#include "Magnum/DebugTools/CompareImage.h" +#include "Magnum/GL/AbstractShaderProgram.h" #include "Magnum/GL/Context.h" -#include "Magnum/GL/Renderer.h" +#include "Magnum/GL/Framebuffer.h" +#include "Magnum/GL/Mesh.h" #include "Magnum/GL/OpenGLTester.h" +#include "Magnum/GL/PixelFormat.h" +#include "Magnum/GL/Renderer.h" +#include "Magnum/GL/Renderbuffer.h" +#include "Magnum/GL/RenderbufferFormat.h" +#include "Magnum/GL/Shader.h" +#include "Magnum/GL/Version.h" #include "Magnum/Math/Range.h" +#include "Magnum/Math/Color.h" +#include "Magnum/Trade/AbstractImporter.h" + +#include "configure.h" namespace Magnum { namespace GL { namespace Test { namespace { @@ -34,10 +53,44 @@ struct RendererGLTest: OpenGLTester { explicit RendererGLTest(); void maxLineWidth(); + void pointCoord(); + + private: + PluginManager::Manager _manager{"nonexistent"}; + std::string _testDir; + + GL::Renderbuffer _color{NoCreate}; + GL::Framebuffer _framebuffer{NoCreate}; }; +using namespace Math::Literals; + RendererGLTest::RendererGLTest() { - addTests({&RendererGLTest::maxLineWidth}); + addTests({&RendererGLTest::maxLineWidth, + &RendererGLTest::pointCoord}); + + /* Load the plugins directly from the build tree. Otherwise they're either + static and already loaded or not present in the build tree */ + #ifdef ANYIMAGEIMPORTER_PLUGIN_FILENAME + CORRADE_INTERNAL_ASSERT(_manager.load(ANYIMAGEIMPORTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + #ifdef TGAIMPORTER_PLUGIN_FILENAME + CORRADE_INTERNAL_ASSERT(_manager.load(TGAIMPORTER_PLUGIN_FILENAME) & PluginManager::LoadState::Loaded); + #endif + + #ifdef CORRADE_TARGET_APPLE + if(Utility::Directory::isSandboxed() + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + /** @todo Fix this once I persuade CMake to run XCTest tests properly */ + && std::getenv("SIMULATOR_UDID") + #endif + ) { + _testDir = Utility::Directory::join(Utility::Directory::path(Utility::Directory::executableLocation()), "RendererGLTestFiles"); + } else + #endif + { + _testDir = RENDERERGLTEST_FILES_DIR; + } } void RendererGLTest::maxLineWidth() { @@ -60,6 +113,113 @@ void RendererGLTest::maxLineWidth() { MAGNUM_VERIFY_NO_GL_ERROR(); } +constexpr Vector2i RenderSize{16, 16}; + +void RendererGLTest::pointCoord() { + /* Pick a color that's directly representable on RGBA4 as well to reduce + artifacts */ + GL::Renderer::setClearColor(0x111111_rgbf); + GL::Renderer::enable(GL::Renderer::Feature::FaceCulling); + + _color = GL::Renderbuffer{}; + _color.setStorage( + #if !defined(MAGNUM_TARGET_GLES2) || !defined(MAGNUM_TARGET_WEBGL) + GL::RenderbufferFormat::RGBA8, + #else + GL::RenderbufferFormat::RGBA4, + #endif + RenderSize); + _framebuffer = GL::Framebuffer{{{}, RenderSize}}; + _framebuffer.attachRenderbuffer(GL::Framebuffer::ColorAttachment{0}, _color) + .clear(GL::FramebufferClear::Color) + .bind(); + + /* Verify that gl_PointCoord works. On desktop compatibility profile this + needs an explicit glEnable(GL_POINT_SPRITE), which is done in + RendererState */ + + struct SpriteShader: AbstractShaderProgram { + explicit SpriteShader() { + #ifndef MAGNUM_TARGET_GLES + Shader vert{ + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + , Shader::Type::Vertex}; + Shader frag{ + #ifndef CORRADE_TARGET_APPLE + Version::GL210 + #else + Version::GL310 + #endif + , Shader::Type::Fragment}; + #elif defined(MAGNUM_TARGET_GLES2) + Shader vert(Version::GLES200, Shader::Type::Vertex); + Shader frag(Version::GLES200, Shader::Type::Fragment); + #else + Shader vert(Version::GLES300, Shader::Type::Vertex); + Shader frag(Version::GLES300, Shader::Type::Fragment); + #endif + vert.addSource("#line " CORRADE_LINE_STRING "\n" R"GLSL( + void main() { + gl_PointSize = 12.0; + gl_Position = vec4(0.0, 0.0, 0.0, 1.0); + })GLSL"); + frag.addSource("#line " CORRADE_LINE_STRING "\n" R"GLSL( + #if !defined(GL_ES) && __VERSION__ == 120 + #define lowp + #endif + + #if (defined(GL_ES) && __VERSION__ < 300) || __VERSION__ == 120 + #define color gl_FragColor + #else + out lowp vec4 color; + #endif + void main() { + color = vec4(gl_PointCoord.x, gl_PointCoord.y, 0.0, 1.0); + })GLSL"); + + CORRADE_INTERNAL_ASSERT_OUTPUT(Shader::compile({vert, frag})); + + attachShaders({vert, frag}); + + CORRADE_INTERNAL_ASSERT_OUTPUT(link()); + } + } shader; + + #ifndef MAGNUM_TARGET_GLES + Renderer::enable(Renderer::Feature::ProgramPointSize); + #endif + + Mesh mesh{MeshPrimitive::Points}; + mesh.setCount(1) + .draw(shader); + + #ifndef MAGNUM_TARGET_GLES + Renderer::disable(Renderer::Feature::ProgramPointSize); + #endif + + MAGNUM_VERIFY_NO_GL_ERROR(); + + if(!(_manager.loadState("AnyImageImporter") & PluginManager::LoadState::Loaded) || + !(_manager.loadState("TgaImporter") & PluginManager::LoadState::Loaded)) + CORRADE_SKIP("AnyImageImporter / TgaImageImporter plugins not found."); + + #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) + const Float maxThreshold = 0.0f, meanThreshold = 0.0f; + #else + /* WebGL 1 doesn't have 8bit renderbuffer storage */ + const Float maxThreshold = 7.334f, meanThreshold = 2.063f; + #endif + CORRADE_COMPARE_WITH( + /* Dropping the alpha channel, as it's always 1.0 */ + Containers::arrayCast(_framebuffer.read(_framebuffer.viewport(), {Magnum::PixelFormat::RGBA8Unorm}).pixels()), + Utility::Directory::join(_testDir, "pointcoord.tga"), + (DebugTools::CompareImageToFile{_manager, maxThreshold, meanThreshold})); +} + }}}} CORRADE_TEST_MAIN(Magnum::GL::Test::RendererGLTest) diff --git a/src/Magnum/GL/Test/RendererGLTestFiles/pointcoord.tga b/src/Magnum/GL/Test/RendererGLTestFiles/pointcoord.tga new file mode 100644 index 0000000000000000000000000000000000000000..334e21c6c5a9ccb5574825b6c703d25d24a6c491 GIT binary patch literal 786 zcmciAF|nXo2!zqEHYItRZHNg>paK(^zyvBVfeB1p0TWHt{Q1;&)WHclX!wWU`QeBE z`1gMaPk72Rp7Vm2yy7))c*{H9`*R2!6Z{mGs7%%%-6TjN*nET&`B5F^w3Km{q-f3QbsuyR8mDXHPli^eSHe4 zq>)YrnPibo4!Pu!Umrp!VT2PwBvC{YLo9K`*Sp|~8}4}Ei5K4Z;ENyr^(L5NhB+2k yVudv}*kXr$y$Y(Rp^gTcXrYY`y6B-_FM=pyh$Dd{Qb;3%EON-l|M`u-+kXLXET#JZ literal 0 HcmV?d00001 diff --git a/src/Magnum/GL/Test/configure.h.cmake b/src/Magnum/GL/Test/configure.h.cmake index 892fea1d8..71eddad1d 100644 --- a/src/Magnum/GL/Test/configure.h.cmake +++ b/src/Magnum/GL/Test/configure.h.cmake @@ -23,4 +23,7 @@ DEALINGS IN THE SOFTWARE. */ +#cmakedefine ANYIMAGEIMPORTER_PLUGIN_FILENAME "${ANYIMAGEIMPORTER_PLUGIN_FILENAME}" +#cmakedefine TGAIMPORTER_PLUGIN_FILENAME "${TGAIMPORTER_PLUGIN_FILENAME}" #define SHADERGLTEST_FILES_DIR "${SHADERGLTEST_FILES_DIR}" +#define RENDERERGLTEST_FILES_DIR "${RENDERERGLTEST_FILES_DIR}"