Buffer: deprecate templated overloads of data(), subData() and map().

They were utterly confusing, as it was completely unclear what the units
of offset/size parameters are, whether byte sizes or element counts (and
moreover, some of these APIs had offset in bytes and size in count and
some not). All of those are deprecated now, with hinting the user to
convert to non-templated APIs in combination with
Containers::arrayCast(). Moreover, the non-templated range map()
function doesn't return just void* anymore, but a properly sized
ArrayView<char>. The old map() (which doesn't take range) still returns
just a pointer (but also a char* instead of void* for consistency), as
getting size there is non-trivial (and impossible on old ES/WebGL).
The switch to ArrayView might be a source breaking change, but I
silently hope that everyone was just using the templated functions
anyway (that are deprecated now). So, in short, this was before:

   T* a = buf.map<T>(0, size_in_what_i_have_no_idea);

And this is now, with proper size safety and clear API:

   ArrayView<T> a = Containers::arrayCast<T>(buf.map(0, size_in_bytes);

The deprecated APIs will be removed at some point in the future, as
usual.

doc/changelog.dox

@@ -301,8 +301,6 @@ namespace Magnum {
 
 ## Changes and improvements
 
-### Core library
-
 -   Functionality that is not available on WebGL (like debug output etc.) is
     not present in WebGL builds to reduce compiled code size
 -   The @ref Mesh class now stores a copy of @ref Buffer instance instead of
@@ -322,6 +320,8 @@ namespace Magnum {
 -   It's no longer possible to call @ref Image::data() "Image*::data()" on
     r-value instances as that would cause accessing freed data. Use
     @ref Image::release() instead.
+-   @ref Buffer::map() now returns @ref Corrade::Containers::ArrayView instead
+    of a plain pointer for better security
 -   Graceful handling of broken GL contexts
 -   Behavior of @ref Version::GLES200 and upwards on desktop OpenGL is changed
     to request an ES dialect of GLSL when used in @ref Shader (instead of a
@@ -478,6 +478,9 @@ namespace Magnum {
     and @ref ImageView::setData() taking `void*` are deprecated, use
     constructors and functions taking @ref Corrade::Containers::Array /
     @ref Corrade::Containers::ArrayView instead
+-   Templated @ref Buffer::data(), @ref Buffer::subData() and
+    @ref Buffer::map() are deprecated, use the non-templated versions in
+    combination with @ref Corrade::Containers::arrayCast() instead
 -   @ref Context::current() and @ref Audio::Context::current() returning a
     pointer is deprecated, it's returning a reference now and asserts that a
     context exists. Use @ref Context::hasCurrent() and @ref Audio::Context::hasCurrent()

src/Magnum/Buffer.cpp

@@ -341,6 +341,12 @@ Int Buffer::size() {
     return size;
 }
 
+#ifndef MAGNUM_TARGET_GLES
+Containers::Array<char> Buffer::data() {
+    return subData(0, size());
+}
+#endif
+
 Buffer& Buffer::setData(const Containers::ArrayView<const void> data, const BufferUsage usage) {
     (this->*Context::current().state().buffer->dataImplementation)(data.size(), data, usage);
     return *this;
@@ -362,8 +368,8 @@ Buffer& Buffer::invalidateSubData(const GLintptr offset, const GLsizeiptr length
 }
 
 #ifndef MAGNUM_TARGET_WEBGL
-void* Buffer::map(const MapAccess access) {
+char* Buffer::map(const MapAccess access) {
-    return (this->*Context::current().state().buffer->mapImplementation)(access);
+    return static_cast<char*>((this->*Context::current().state().buffer->mapImplementation)(access));
 }
 
 #if defined(DOXYGEN_GENERATING_OUTPUT) || defined(CORRADE_TARGET_NACL)
@@ -373,8 +379,8 @@ void* Buffer::mapSub(const GLintptr offset, const GLsizeiptr length, const MapAc
 }
 #endif
 
-void* Buffer::map(const GLintptr offset, const GLsizeiptr length, const MapFlags flags) {
+Containers::ArrayView<char> Buffer::map(const GLintptr offset, const GLsizeiptr length, const MapFlags flags) {
-    return (this->*Context::current().state().buffer->mapRangeImplementation)(offset, length, flags);
+    return {static_cast<char*>((this->*Context::current().state().buffer->mapRangeImplementation)(offset, length, flags)), std::size_t(length)};
 }
 
 Buffer& Buffer::flushMappedRange(const GLintptr offset, const GLsizeiptr length) {
@@ -394,10 +400,19 @@ void Buffer::unmapSub() {
 #endif
 
 #ifndef MAGNUM_TARGET_GLES
+Containers::Array<char> Buffer::subData(const GLintptr offset, const GLsizeiptr size) {
+    Containers::Array<char> data(size);
+    if(size) (this->*Context::current().state().buffer->getSubDataImplementation)(offset, size, data);
+    return data;
+}
+
+/** @todo remove when this is not used anymore */
+#ifdef MAGNUM_BUILD_DEPRECATED
 void Buffer::subDataInternal(GLintptr offset, GLsizeiptr size, GLvoid* data) {
     (this->*Context::current().state().buffer->getSubDataImplementation)(offset, size, data);
 }
 #endif
+#endif
 
 #ifndef MAGNUM_TARGET_GLES2
 void Buffer::bindImplementationFallback(const Target target, const GLuint firstIndex, Containers::ArrayView<Buffer* const> buffers) {

src/Magnum/Buffer.h

@@ -32,7 +32,7 @@
 #include <cstddef>
 #include <array>
 #include <vector>
-#include <Corrade/Containers/Array.h>
+#include <Corrade/Containers/ArrayView.h>
 #include <Corrade/Containers/EnumSet.h>
 #include <Corrade/Utility/Assert.h>
 
@@ -41,6 +41,7 @@
 #include "Magnum/Tags.h"
 
 #ifdef MAGNUM_BUILD_DEPRECATED
+#include <Corrade/Containers/Array.h>
 #include <Corrade/Utility/Macros.h>
 #endif
 
@@ -165,18 +166,21 @@ buffer.setData({nullptr, 200*sizeof(Vector3)}, BufferUsage::StaticDraw);
 @endcode
 Then you can map the buffer to client memory and operate with the memory
 directly. After you are done with the operation, call @ref unmap() to unmap the
-buffer again.
+buffer again. The @ref map() functions return a view on `char` array and you
+may want to cast it to some useful type first using @ref Containers::arrayCast():
 @code
-Vector3* data = buffer.map<Vector3>(0, 200*sizeof(Vector3), Buffer::MapFlag::Write|Buffer::MapFlag::InvalidateBuffer);
+Containers::ArrayView<Vector3> data = Containers::arrayCast<Vector3>(buffer.map(0, 200*sizeof(Vector3), Buffer::MapFlag::Write|Buffer::MapFlag::InvalidateBuffer));
-for(std::size_t i = 0; i != 200; ++i)
+CORRADE_INTERNAL_ASSERT(data);
-    data[i] = ...;
+for(Vector3& d: data)
+    d = ...;
 CORRADE_INTERNAL_ASSERT_OUTPUT(buffer.unmap());
 @endcode
 If you are updating only a few discrete portions of the buffer, you can use
 @ref MapFlag::FlushExplicit and @ref flushMappedRange() to reduce number of
 memory operations performed by OpenGL on unmapping. Example:
 @code
-Vector3* data = buffer.map<Vector3>(0, 200*sizeof(Vector3), Buffer::MapFlag::Write|Buffer::MapFlag::FlushExplicit);
+Containers::ArrayView<Vector3> data = Containers::arrayCast<Vector3>(buffer.map(0, 200*sizeof(Vector3), Buffer::MapFlag::Write|Buffer::MapFlag::FlushExplicit));
+CORRADE_INTERNAL_ASSERT(data);
 for(std::size_t i: {7, 27, 56, 128}) {
     data[i] = ...;
     buffer.flushMappedRange(i*sizeof(Vector3), sizeof(Vector3));
@@ -1072,13 +1076,21 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
          *      WebGL. Use @ref map() or @ref DebugTools::bufferData() in
          *      OpenGL ES instead.
          */
+        Containers::Array<char> data();
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /** @copybrief data()
+         * @deprecated Use non-templated @ref subData() and @ref Containers::arrayCast()
+         *      instead.
+         */
         /* MinGW complains loudly if the declaration doesn't also have inline */
-        template<class T = char> inline Containers::Array<T> data();
+        template<class T> CORRADE_DEPRECATED("use non-templated data() and Containers::arrayCast() instead") inline Containers::Array<T> data();
+        #endif
 
         /**
          * @brief Buffer subdata
          * @param offset    Byte offset in the buffer
-         * @param size      Data size (count of @p T values)
+         * @param size      Data size in bytes
          *
          * Returns data of given buffer portion. If neither
          * @extension{ARB,direct_state_access} (part of OpenGL 4.5) nor
@@ -1092,8 +1104,15 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
          *      WebGL. Use @ref map() or @ref DebugTools::bufferData() in
          *      OpenGL ES instead.
          */
+        Containers::Array<char> subData(GLintptr offset, GLsizeiptr size);
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /** @copybrief subData()
+         * @deprecated Use non-templated @ref subData() and @ref Containers::arrayCast() instead
+         */
         /* MinGW complains loudly if the declaration doesn't also have inline */
-        template<class T = char> inline Containers::Array<T> subData(GLintptr offset, GLsizeiptr size);
+        template<class T> CORRADE_DEPRECATED("use non-templated subData() and Containers::arrayCast() instead") inline Containers::Array<T> subData(GLintptr offset, GLsizeiptr size);
+        #endif
         #endif
 
         /**
@@ -1126,7 +1145,7 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
 
         /**
          * @brief Set buffer subdata
-         * @param offset    Offset in the buffer
+         * @param offset    Byte offset in the buffer
          * @param data      Data
          * @return Reference to self (for method chaining)
          *
@@ -1164,7 +1183,7 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
 
         /**
          * @brief Invalidate buffer subdata
-         * @param offset    Offset into the buffer
+         * @param offset    Byte offset into the buffer
          * @param length    Length of the invalidated range
          * @return Reference to self (for method chaining)
          *
@@ -1178,7 +1197,7 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
         /**
          * @brief Map buffer to client memory
          * @param access    Access
-         * @return Pointer to buffer data
+         * @return Pointer to mapped buffer data or `nullptr` on error
          *
          * If neither @extension{ARB,direct_state_access} (part of OpenGL 4.5)
          * nor @extension{EXT,direct_state_access} desktop extension is
@@ -1195,12 +1214,17 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
          * @deprecated_gl Prefer to use @ref map(GLintptr, GLsizeiptr, MapFlags)
          *      instead, as it has more complete set of features.
          */
-        void* map(MapAccess access);
+        char* map(MapAccess access);
 
-        /** @overload */
+        #ifdef MAGNUM_BUILD_DEPRECATED
-        template<class T> T* map(MapAccess access) {
+        /** @overload
-            return static_cast<T*>(map(access));
+         * @deprecated Use non-templated @ref map() and cast the result
+         *      manually instead.
+         */
+        template<class T> CORRADE_DEPRECATED("use non-templated map() and cast the result manually instead") T* map(MapAccess access) {
+            return reinterpret_cast<T*>(map(access));
         }
+        #endif
 
         #if defined(DOXYGEN_GENERATING_OUTPUT) || defined(CORRADE_TARGET_NACL)
         /**
@@ -1231,11 +1255,11 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
 
         /**
          * @brief Map buffer to client memory
-         * @param offset    Offset into the buffer
+         * @param offset    Byte offset into the buffer
-         * @param length    Length of the mapped memory
+         * @param length    Length of the mapped memory in bytes
          * @param flags     Flags. At least @ref MapFlag::Read or
          *      @ref MapFlag::Write must be specified.
-         * @return Pointer to buffer data
+         * @return Sized view to buffer data or `nullptr` on error
          *
          * If neither @extension{ARB,direct_state_access} (part of OpenGL 4.5)
          * nor @extension{EXT,direct_state_access} desktop extension is
@@ -1251,17 +1275,22 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
          *      OpenGL ES 2.0.
          * @requires_gles Buffer mapping is not available in WebGL.
          */
-        void* map(GLintptr offset, GLsizeiptr length, MapFlags flags);
+        Containers::ArrayView<char> map(GLintptr offset, GLsizeiptr length, MapFlags flags);
 
-        /** @overload */
+        #ifdef MAGNUM_BUILD_DEPRECATED
-        template<class T> T* map(GLintptr offset, GLsizeiptr length, MapFlags flags) {
+        /** @overload
-            return static_cast<T*>(map(offset, length, flags));
+         * @deprecated Use non-templated @ref map() and @ref Containers::arrayCast()
+         *      instead.
+         */
+        template<class T> CORRADE_DEPRECATED("use non-templated map() and Containers::arrayCast() instead") T* map(GLintptr offset, GLsizeiptr length, MapFlags flags) {
+            return Containers::arrayCast<T>(map(offset, length, flags));
         }
+        #endif
 
         /**
          * @brief Flush mapped range
-         * @param offset    Offset relative to start of mapped range
+         * @param offset    Byte offset relative to start of mapped range
-         * @param length    Length of the flushed memory
+         * @param length    Length of the flushed memory in bytes
          * @return Reference to self (for method chaining)
          *
          * Flushes specified subsection of mapped range. Use only if you called
@@ -1364,8 +1393,8 @@ class MAGNUM_EXPORT Buffer: public AbstractObject {
         Buffer& setLabelInternal(Containers::ArrayView<const char> label);
         #endif
 
-        #ifndef MAGNUM_TARGET_GLES
+        #if !defined(MAGNUM_TARGET_GLES) && defined(MAGNUM_BUILD_DEPRECATED)
-        void subDataInternal(GLintptr offset, GLsizeiptr size, GLvoid* data);
+        CORRADE_DEPRECATED("used only by deprecated subData<T>()") void subDataInternal(GLintptr offset, GLsizeiptr size, GLvoid* data);
         #endif
 
         void MAGNUM_LOCAL getParameterImplementationDefault(GLenum value, GLint* data);
@@ -1483,7 +1512,11 @@ inline GLuint Buffer::release() {
     return id;
 }
 
-#ifndef MAGNUM_TARGET_GLES
+#if !defined(MAGNUM_TARGET_GLES) && defined(MAGNUM_BUILD_DEPRECATED)
+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+#endif
 template<class T> Containers::Array<T> inline Buffer::data() {
     const Int bufferSize = size();
     CORRADE_ASSERT(bufferSize%sizeof(T) == 0, "Buffer::data(): the buffer size is" << bufferSize << "bytes, which can't be expressed as array of types with size" << sizeof(T), nullptr);
@@ -1495,6 +1528,9 @@ template<class T> Containers::Array<T> inline Buffer::subData(const GLintptr off
     if(size) subDataInternal(offset, size*sizeof(T), data);
     return data;
 }
+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
 #endif
 
 }