ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Platform: replace UGLY error-prone C string handling with StringViews. 

We no longer have to use sizeof("...") to avoid useless strlen calls or
check for nulls because the StringView APIs are ACTUALLY SANE and not a
full of nasty surprises and performance / security pitfalls like with
both the C and C++ standard library functions. Good riddance.
euler-xxx
Vladimír Vondruš 5 years ago
parent
d9f16f30d7
commit
d1774ef4d2
5 changed files with 35 additions and 69 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 23
      src/Magnum/Platform/GlfwApplication.cpp
  2. 20
      src/Magnum/Platform/Sdl2Application.cpp
  3. 24
      src/Magnum/Platform/WindowlessEglApplication.cpp
  4. 14
      src/Magnum/Platform/WindowlessGlxApplication.cpp
  5. 17
      src/Magnum/Platform/WindowlessWglApplication.cpp

23
src/Magnum/Platform/GlfwApplication.cpp
Unescape View File

@ -28,7 +28,6 @@
#include "GlfwApplication.h"
#include <cstring>
#include <tuple>
#include <Corrade/Containers/Array.h>
#include <Corrade/Containers/StridedArrayView.h>
@ -498,9 +497,7 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf
succeeds, make the context current so we can query GL_VENDOR below.
If we are on Wayland, this is causing a segfault; a blinking window is
acceptable in this case. */
constexpr const char waylandString[] = "wayland";
const char* const xdgSessionType = std::getenv("XDG_SESSION_TYPE");
if(!xdgSessionType || std::strncmp(xdgSessionType, waylandString, sizeof(waylandString)) != 0)
if(std::getenv("XDG_SESSION_TYPE") != "wayland"_s)
glfwWindowHint(GLFW_VISIBLE, false);
else if(_verboseLog)
Warning{} << "Platform::GlfwApplication: Wayland detected, GL context has to be created with the window visible and may cause flicker on startup";
@ -515,25 +512,17 @@ bool GlfwApplication::tryCreate(const Configuration& configuration, const GLConf
version, they force the version to the one specified, which is
completely useless behavior. */
#ifndef CORRADE_TARGET_APPLE
constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation";
#ifdef CORRADE_TARGET_WINDOWS
constexpr static const char intelVendorString[] = "Intel";
#endif
constexpr static const char amdVendorString[] = "ATI Technologies Inc.";
const char* vendorString;
Containers::StringView vendorString;
#endif
if(glConfiguration.version() == GL::Version::None && (!_window
#ifndef CORRADE_TARGET_APPLE
/* If context creation fails *really bad*, glGetString() may actually
return nullptr. Check for that to avoid crashes deep inside
strncmp(). Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE
WORKAROUNDS */
/* Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE WORKAROUNDS */
|| (vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR)),
vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 ||
(vendorString == "NVIDIA Corporation"_s ||
#ifdef CORRADE_TARGET_WINDOWS
std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 ||
vendorString == "Intel"_s ||
#endif
std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0)
vendorString == "ATI Technologies Inc."_s)
&& !_context->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s))
#endif
)) {

20
src/Magnum/Platform/Sdl2Application.cpp
Unescape View File

@ -26,7 +26,7 @@
#include "Sdl2Application.h"
#include <cstring>
#include <cstring> /** @todo remove, needed for std::strlen() in TextInputEvent */
#ifdef CORRADE_TARGET_CLANG_CL
/* SDL does #pragma pack(push,8) and #pragma pack(pop,8) in different headers
(begin_code.h and end_code.h) and clang-cl doesn't like that, even though it
@ -545,25 +545,17 @@ bool Sdl2Application::tryCreate(const Configuration& configuration, const GLConf
version, they force the version to the one specified, which is
completely useless behavior. */
#ifndef CORRADE_TARGET_APPLE
constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation";
#ifdef CORRADE_TARGET_WINDOWS
constexpr static const char intelVendorString[] = "Intel";
#endif
constexpr static const char amdVendorString[] = "ATI Technologies Inc.";
const char* vendorString;
Containers::StringView vendorString;
#endif
if(glConfiguration.version() == GL::Version::None && (!_glContext
#ifndef CORRADE_TARGET_APPLE
/* If context creation fails *really bad*, glGetString() may actually
return nullptr. Check for that to avoid crashes deep inside
strncmp(). Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE
WORKAROUNDS */
/* Sorry about the UGLY code, HOPEFULLY THERE WON'T BE MORE WORKAROUNDS */
|| (vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR)),
vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 ||
(vendorString == "NVIDIA Corporation"_s ||
#ifdef CORRADE_TARGET_WINDOWS
std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 ||
vendorString == "Intel"_s ||
#endif
std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0)
vendorString == "ATI Technologies Inc."_s)
&& !_context->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s))
#endif
)) {

24
src/Magnum/Platform/WindowlessEglApplication.cpp
Unescape View File

@ -27,7 +27,7 @@
#include "WindowlessEglApplication.h"
#include <cstring>
#include <cstring> /** @todo used by extensionSupported(), cleann up */
#include <string>
#include <Corrade/Utility/Arguments.h>
#include <Corrade/Utility/Assert.h>
@ -346,13 +346,13 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
#endif
#if defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_WEBGL)
const char* version = eglQueryString(_display, EGL_VERSION);
const Containers::StringView version = eglQueryString(_display, EGL_VERSION);
/* SwiftShader 3.3.0.1 blows up on encountering EGL_CONTEXT_FLAGS_KHR with
a zero value, so erase these. It also doesn't handle them as correct
flags, but instead checks for the whole value, so a combination won't
work either: https://github.com/google/swiftshader/blob/5fb5e817a20d3e60f29f7338493f922b5ac9d7c4/src/OpenGL/libEGL/libEGL.cpp#L794-L8104 */
if(!(UnsignedLong(flags) & 0xffffffffu) && version && std::strstr(version, "SwiftShader") != nullptr && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("swiftshader-no-empty-egl-context-flags"_s))) {
if(!(UnsignedLong(flags) & 0xffffffffu) && version.contains("SwiftShader"_s) && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("swiftshader-no-empty-egl-context-flags"_s))) {
auto& contextFlags = attributes[Containers::arraySize(attributes) - 3];
CORRADE_INTERNAL_ASSERT(contextFlags == EGL_CONTEXT_FLAGS_KHR);
contextFlags = EGL_NONE;
@ -401,15 +401,13 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
/* The workaround check is the last so it doesn't appear in workaround
list on unrelated drivers */
constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation";
constexpr static const char amdVendorString[] = "ATI Technologies Inc.";
const char* const vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
/* If context creation fails *really bad*, glGetString() may actually
return nullptr. Check for that to avoid crashes deep inside
strncmp() */
if(vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 ||
std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0) &&
(!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
const Containers::StringView vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
/* Unlike GLFW/SDL2/WGL there's no Intel here because EGL doesn't work
with native drivers on Windows, only though ANGLE etc, and those
don't suffer from this issue. */
if((vendorString == "NVIDIA Corporation"_s ||
vendorString == "ATI Technologies Inc."_s)
&& (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
{
/* Destroy the core context and create a compatibility one */
eglDestroyContext(_display, _context);
@ -442,7 +440,7 @@ WindowlessEglContext::WindowlessEglContext(const Configuration& configuration, G
#if defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_WEBGL)
/* SwiftShader 3.3.0.1 needs some pbuffer, otherwise it crashes somewhere
deep inside when making the context current */
if(version && std::strstr(version, "SwiftShader") != nullptr && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("swiftshader-egl-context-needs-pbuffer"_s))) {
if(version.contains("SwiftShader"_s) && (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("swiftshader-egl-context-needs-pbuffer"_s))) {
EGLint surfaceAttributes[] = {
EGL_WIDTH, 32,
EGL_HEIGHT, 32,

14
src/Magnum/Platform/WindowlessGlxApplication.cpp
Unescape View File

@ -26,7 +26,6 @@
#include "WindowlessGlxApplication.h"
#include <cstring>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Utility/Assert.h>
#include <Corrade/Utility/Debug.h>
@ -186,15 +185,10 @@ WindowlessGlxContext::WindowlessGlxContext(const WindowlessGlxContext::Configura
/* The workaround check is the last so it doesn't appear in workaround
list on unrelated drivers */
constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation";
constexpr static const char amdVendorString[] = "ATI Technologies Inc.";
const char* const vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
/* If context creation fails *really bad*, glGetString() may actually
return nullptr. Check for that to avoid crashes deep inside
strncmp() */
if(vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 ||
std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0) &&
(!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
const Containers::StringView vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
if((vendorString == "NVIDIA Corporation"_s ||
vendorString == "ATI Technologies Inc."_s)
&& (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
{
/* Destroy the core context and create a compatibility one */
glXDestroyContext(_display, _context);

17
src/Magnum/Platform/WindowlessWglApplication.cpp
Unescape View File

@ -25,7 +25,6 @@
#include "WindowlessWglApplication.h"
#include <cstring>
#include <Corrade/Containers/StringView.h>
#include <Corrade/Utility/Assert.h>
#include <Corrade/Utility/Debug.h>
@ -183,17 +182,11 @@ WindowlessWglContext::WindowlessWglContext(const Configuration& configuration, G
/* The workaround check is the last so it doesn't appear in workaround
list on unrelated drivers */
constexpr static const char nvidiaVendorString[] = "NVIDIA Corporation";
constexpr static const char intelVendorString[] = "Intel";
constexpr static const char amdVendorString[] = "ATI Technologies Inc.";
const char* const vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
/* If context creation fails *really bad*, glGetString() may actually
return nullptr. Check for that to avoid crashes deep inside
strncmp() */
if(vendorString && (std::strncmp(vendorString, nvidiaVendorString, sizeof(nvidiaVendorString)) == 0 ||
std::strncmp(vendorString, intelVendorString, sizeof(intelVendorString)) == 0 ||
std::strncmp(vendorString, amdVendorString, sizeof(amdVendorString)) == 0) &&
(!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
const Containers::StringView vendorString = reinterpret_cast<const char*>(glGetString(GL_VENDOR));
if((vendorString == "NVIDIA Corporation"_s ||
vendorString == "Intel"_s ||
vendorString == "ATI Technologies Inc."_s)
&& (!magnumContext || !magnumContext->isDriverWorkaroundDisabled("no-forward-compatible-core-context"_s)))
{
/* Destroy the core context and create a compatibility one */
wglDeleteContext(_context);

Write Preview
Loading…
Cancel
Save