GL: port Shader and ShaderProgram away from std::string.

Same as in the previous commit, most cases are inputs so a StringStl.h
compatibility include will do, the only breaking change is
GL::Shader::sources() which now returns a StringIterable instead of a
std::vector<std::string> (ew).

Awesome about this whole thing is that The Shader API now allows
creating a shader from sources coming either from string view literals
or Utility::Resource completely without having to allocate any strings
internally, because all those can be just non-owning references wrapped
with String::nullTerminatedGlobalView(). The only parts which aren't
references are the #line markers, but (especially on 64bit) those can
easily fit into the 22-byte (or 10-byte on 32bit) SSO storage.

Also, various Shader constructors and assignment operators had to be
deinlined in order to avoid having to include the String header, which
would be needed for Array destruction during a move.

Co-authored-by: Hugo Amiard <[email protected]>

Author: mosra

Diff for: doc/changelog.dox

@@ -1137,6 +1137,12 @@ See also:
 -   As part of the ongoing STL header dependency cleanup, the following
     breaking changes related to @ref std::string and a @ref std::vector are
     done:
+    -   @ref GL::AbstractShaderProgram::validate() now returns a
+        @relativeref{Corrade,Containers::String} instead of a @ref std::string;
+        @ref GL::Shader::sources() now returns a
+        @relativeref{Corrade,Containers::StringIterable} instead of a
+        @ref std::vector of a @ref std::string See also [mosra/magnum#499](https://github.com/mosra/magnum/pull/499)
+        and [mosra/magnum#608](https://github.com/mosra/magnum/pull/608).
     -   @ref GL::Context::vendorString(),
         @relativeref{GL::Context,rendererString()},
         @relativeref{GL::Context,versionString()},

Diff for: doc/snippets/MagnumGL.cpp

@@ -27,6 +27,8 @@
 #include <Corrade/Containers/ArrayViewStl.h>
 #include <Corrade/Containers/Iterable.h>
 #include <Corrade/Containers/Reference.h>
+#include <Corrade/Containers/StringIterable.h>
+#include <Corrade/Containers/StringView.h>
 #include <Corrade/Containers/Triple.h>
 #include <Corrade/TestSuite/Tester.h>
 

Diff for: doc/snippets/MagnumShaders-gl.cpp

@@ -28,7 +28,8 @@
 #include <Corrade/Containers/ArrayViewStl.h>
 #include <Corrade/Containers/Iterable.h>
 #include <Corrade/Containers/Pair.h>
-#include <Corrade/Utility/FormatStl.h>
+#include <Corrade/Containers/String.h>
+#include <Corrade/Utility/Format.h>
 
 #include "Magnum/ImageView.h"
 #include "Magnum/PixelFormat.h"
@@ -683,7 +684,7 @@ bindAttributeLocation(Shaders::GenericGL3D::Normal::Location, "normal");
 {
 GL::Shader vert{GL::Version::None, GL::Shader::Type::Vertex};
 /* [GenericGL-custom-preprocessor] */
-vert.addSource(Utility::formatString(
+vert.addSource(Utility::format(
     "#define POSITION_ATTRIBUTE_LOCATION {}\n"
     "#define NORMAL_ATTRIBUTE_LOCATION {}\n",
     Shaders::GenericGL3D::Position::Location,

Diff for: src/Magnum/DebugTools/TextureImage.cpp

@@ -37,7 +37,6 @@
 #if defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_GLES2)
 #include <Corrade/Containers/Iterable.h>
 #include <Corrade/Containers/Reference.h>
-#include <Corrade/Containers/StringStl.h> /** @todo remove once GL::Shader is <string>-free */
 #include <Corrade/Utility/Resource.h>
 
 #include "Magnum/GL/AbstractShaderProgram.h"

Diff for: src/Magnum/GL/AbstractShaderProgram.cpp

@@ -29,16 +29,12 @@
 
 #include <Corrade/Containers/Array.h>
 #include <Corrade/Containers/Iterable.h>
-#include <Corrade/Containers/StridedArrayView.h>
-#ifndef MAGNUM_TARGET_WEBGL
-#include <Corrade/Containers/String.h>
-#endif
-#include <Corrade/Containers/StringStl.h> /** @todo remove once <string>-free */
 #ifdef MAGNUM_BUILD_DEPRECATED
 #include <Corrade/Containers/Reference.h>
 #endif
-#include <Corrade/Utility/DebugStl.h>
-#include <Corrade/Utility/String.h>
+#include <Corrade/Containers/StridedArrayView.h>
+#include <Corrade/Containers/String.h>
+#include <Corrade/Containers/StringIterable.h>
 
 #include "Magnum/GL/Context.h"
 #include "Magnum/GL/Extensions.h"
@@ -343,26 +339,29 @@ AbstractShaderProgram& AbstractShaderProgram::setLabel(const Containers::StringV
 }
 #endif
 
-std::pair<bool, std::string> AbstractShaderProgram::validate() {
+std::pair<bool, Containers::String> AbstractShaderProgram::validate() {
     glValidateProgram(_id);
 
     /* Check validation status */
     GLint success, logLength;
     glGetProgramiv(_id, GL_VALIDATE_STATUS, &success);
     glGetProgramiv(_id, GL_INFO_LOG_LENGTH, &logLength);
 
-    /* Error or warning message. The string is returned null-terminated, scrap
-       the \0 at the end afterwards */
-    std::string message(logLength, '\n');
-    if(message.size() > 1)
-        glGetProgramInfoLog(_id, message.size(), nullptr, &message[0]);
-    message.resize(Math::max(logLength, 1)-1);
+    /* Error or warning message. The length is reported including the null
+       terminator and the string implicitly has a storage for that, thus
+       specify one byte less. */
+    Containers::String message{NoInit, std::size_t(Math::max(logLength, 1) - 1)};
+    if(logLength > 1)
+        glGetProgramInfoLog(_id, logLength, nullptr, message.data());
 
     /* On some drivers (such as SwiftShader) the message contains a newline at
        the end, on some (such as Mesa) it doesn't. Same as with link() or
        compile() message trimming it doesn't make sense to add driver-specific
        workarounds for this, so just trim it always. */
-    return {success, Utility::String::trim(std::move(message))};
+    /** @todo this allocates a new string, revisit once String is capable of
+        trimming in-place, e.g. `std::move(message).trimmed()` would just
+        shift the data around */
+    return {success, message.trimmed()};
 }
 
 AbstractShaderProgram& AbstractShaderProgram::draw(Mesh& mesh) {
@@ -547,48 +546,62 @@ void AbstractShaderProgram::attachShaders(const Containers::Iterable<Shader>& sh
     for(Shader& s: shaders) attachShader(s);
 }
 
-void AbstractShaderProgram::bindAttributeLocationInternal(const UnsignedInt location, const Containers::ArrayView<const char> name) {
-    glBindAttribLocation(_id, location, name);
+void AbstractShaderProgram::bindAttributeLocation(const UnsignedInt location, const Containers::StringView name) {
+    glBindAttribLocation(_id, location, Containers::String::nullTerminatedView(name).data());
 }
 
 #ifndef MAGNUM_TARGET_GLES
-void AbstractShaderProgram::bindFragmentDataLocationInternal(const UnsignedInt location, const Containers::ArrayView<const char> name) {
-    glBindFragDataLocation(_id, location, name);
+void AbstractShaderProgram::bindFragmentDataLocation(const UnsignedInt location, const Containers::StringView name) {
+    glBindFragDataLocation(_id, location, Containers::String::nullTerminatedView(name).data());
 }
-void AbstractShaderProgram::bindFragmentDataLocationIndexedInternal(const UnsignedInt location, UnsignedInt index, const Containers::ArrayView<const char> name) {
-    glBindFragDataLocationIndexed(_id, location, index, name);
+void AbstractShaderProgram::bindFragmentDataLocationIndexed(const UnsignedInt location, UnsignedInt index, const Containers::StringView name) {
+    glBindFragDataLocationIndexed(_id, location, index, Containers::String::nullTerminatedView(name).data());
 }
 #endif
 
 #ifndef MAGNUM_TARGET_GLES2
-void AbstractShaderProgram::setTransformFeedbackOutputs(const Containers::ArrayView<const std::string> outputs, const TransformFeedbackBufferMode bufferMode) {
+void AbstractShaderProgram::setTransformFeedbackOutputs(const Containers::StringIterable& outputs, const TransformFeedbackBufferMode bufferMode) {
     (this->*Context::current().state().shaderProgram.transformFeedbackVaryingsImplementation)(outputs, bufferMode);
 }
 
-void AbstractShaderProgram::setTransformFeedbackOutputs(const std::initializer_list<std::string> outputs, const TransformFeedbackBufferMode bufferMode) {
-    setTransformFeedbackOutputs(Containers::arrayView(outputs), bufferMode);
-}
-
-void AbstractShaderProgram::transformFeedbackVaryingsImplementationDefault(const Containers::ArrayView<const std::string> outputs, const TransformFeedbackBufferMode bufferMode) {
-    /** @todo VLAs */
-    Containers::Array<const char*> names{outputs.size()};
-
-    Int i = 0;
-    for(const std::string& output: outputs) names[i++] = output.data();
+void AbstractShaderProgram::transformFeedbackVaryingsImplementationDefault(const Containers::StringIterable& outputs, const TransformFeedbackBufferMode bufferMode) {
+    Containers::ArrayView<Containers::String> nameData;
+    Containers::ArrayView<const char*> names;
+    Containers::ArrayTuple data{
+        {NoInit, outputs.size(), nameData},
+        {NoInit, outputs.size(), names}
+    };
+    for(std::size_t i = 0; i != outputs.size(); ++i) {
+        new(&nameData[i]) Containers::String{Containers::String::nullTerminatedView(outputs[i])};
+        names[i] = nameData[i].data();
+    }
 
     glTransformFeedbackVaryings(_id, outputs.size(), names, GLenum(bufferMode));
 }
 
 #ifdef CORRADE_TARGET_WINDOWS
-void AbstractShaderProgram::transformFeedbackVaryingsImplementationDanglingWorkaround(const Containers::ArrayView<const std::string> outputs, const TransformFeedbackBufferMode bufferMode) {
+void AbstractShaderProgram::transformFeedbackVaryingsImplementationDanglingWorkaround(const Containers::StringIterable& outputs, const TransformFeedbackBufferMode bufferMode) {
     /* NVidia on Windows doesn't copy the names when calling
-       glTransformFeedbackVaryings() so it then fails at link time because the
-       char* are dangling. We have to do the copy on the engine side and keep
-       the values until link time (which can happen any time and multiple
-       times, so basically for the remaining lifetime of the shader program) */
-    _transformFeedbackVaryingNames.assign(outputs.begin(), outputs.end());
+       glTransformFeedbackVaryings() so it then might fail at link time because
+       the char* get dangling. We have to do the copy on the engine side and
+       keep the values until link time (which can happen any time and multiple
+       times, so basically for the remaining lifetime of the shader program).
+
+       Compared to the above case we thus do a copy also if the view is not
+       global in addition to not null-terminated, and we keep the string memory
+       in a member variable. */
+    Containers::ArrayView<Containers::String> nameData;
+    Containers::ArrayView<const char*> names;
+    _transformFeedbackVaryingNames = Containers::ArrayTuple{
+        {NoInit, outputs.size(), nameData},
+        {NoInit, outputs.size(), names}
+    };
+    for(std::size_t i = 0; i != outputs.size(); ++i) {
+        new(&nameData[i]) Containers::String{Containers::String::nullTerminatedGlobalView(outputs[i])};
+        names[i] = nameData[i].data();
+    }
 
-    transformFeedbackVaryingsImplementationDefault({_transformFeedbackVaryingNames.data(), _transformFeedbackVaryingNames.size()}, bufferMode);
+    glTransformFeedbackVaryings(_id, outputs.size(), names, GLenum(bufferMode));
 }
 #endif
 #endif
@@ -613,12 +626,12 @@ bool AbstractShaderProgram::checkLink(const Containers::Iterable<Shader>& shader
     glGetProgramiv(_id, GL_LINK_STATUS, &success);
     glGetProgramiv(_id, GL_INFO_LOG_LENGTH, &logLength);
 
-    /* Error or warning message. The string is returned null-terminated,
-       strip the \0 at the end afterwards. */
-    std::string message(logLength, '\n');
-    if(message.size() > 1)
-        glGetProgramInfoLog(_id, message.size(), nullptr, &message[0]);
-    message.resize(Math::max(logLength, 1)-1);
+    /* Error or warning message. The length is reported including the null
+       terminator and the string implicitly has a storage for that, thus
+       specify one byte less. */
+    Containers::String message{NoInit, std::size_t(Math::max(logLength, 1)) - 1};
+    if(logLength > 1)
+        glGetProgramInfoLog(_id, logLength, nullptr, message.data());
 
     /* Some drivers are chatty and can't keep shut when there's nothing to
        be said, handle that as well. */
@@ -662,16 +675,16 @@ bool AbstractShaderProgram::isLinkFinished() {
     return success == GL_TRUE;
 }
 
-void AbstractShaderProgram::cleanLogImplementationNoOp(std::string&) {}
+void AbstractShaderProgram::cleanLogImplementationNoOp(Containers::String&) {}
 
 #if defined(CORRADE_TARGET_WINDOWS) && !defined(MAGNUM_TARGET_GLES)
-void AbstractShaderProgram::cleanLogImplementationIntelWindows(std::string& message) {
+void AbstractShaderProgram::cleanLogImplementationIntelWindows(Containers::String& message) {
     if(message == "No errors.\n") message = {};
 }
 #endif
 
 #if defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_WEBGL)
-void AbstractShaderProgram::cleanLogImplementationAngle(std::string& message) {
+void AbstractShaderProgram::cleanLogImplementationAngle(Containers::String& message) {
     if(message == "\n") message = {};
 }
 #endif
@@ -680,18 +693,18 @@ void AbstractShaderProgram::completionStatusImplementationFallback(GLuint, GLenu
     *value = GL_TRUE;
 }
 
-Int AbstractShaderProgram::uniformLocationInternal(const Containers::ArrayView<const char> name) {
-    const GLint location = glGetUniformLocation(_id, name);
+Int AbstractShaderProgram::uniformLocation(const Containers::StringView name) {
+    const GLint location = glGetUniformLocation(_id, Containers::String::nullTerminatedView(name).data());
     if(location == -1)
-        Warning{} << "GL::AbstractShaderProgram: location of uniform \'" << Debug::nospace << std::string{name, name.size()} << Debug::nospace << "\' cannot be retrieved";
+        Warning{} << "GL::AbstractShaderProgram: location of uniform \'" << Debug::nospace << name << Debug::nospace << "\' cannot be retrieved";
     return location;
 }
 
 #ifndef MAGNUM_TARGET_GLES2
-UnsignedInt AbstractShaderProgram::uniformBlockIndexInternal(const Containers::ArrayView<const char> name) {
-    const GLuint index = glGetUniformBlockIndex(_id, name);
+UnsignedInt AbstractShaderProgram::uniformBlockIndex(const Containers::StringView name) {
+    const GLuint index = glGetUniformBlockIndex(_id, Containers::String::nullTerminatedView(name).data());
     if(index == GL_INVALID_INDEX)
-        Warning{} << "GL::AbstractShaderProgram: index of uniform block \'" << Debug::nospace << std::string{name, name.size()} << Debug::nospace << "\' cannot be retrieved";
+        Warning{} << "GL::AbstractShaderProgram: index of uniform block \'" << Debug::nospace << name << Debug::nospace << "\' cannot be retrieved";
     return index;
 }
 #endif