From 1940488da4ee39e88b61d2e4219a843392f5542e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Sun, 15 Nov 2020 21:32:26 +0100
Subject: [PATCH] Vk: rework {Extension,Layer}Properties internals using
 ArrayTuple.

These were implemented long before ArrayTuple was a thing and while the
allocation saving optimization makes sense, the implementation is way
too error prone.

This also means Array (and thus <new> and other heavy shit) isn't
included in the headers, which leads to further slimming of compile
times across the library.
---
 doc/snippets/MagnumVk.cpp             |  2 +-
 src/Magnum/Vk/ExtensionProperties.cpp | 47 +++++++++++----------------
 src/Magnum/Vk/ExtensionProperties.h   |  9 +++--
 src/Magnum/Vk/LayerProperties.cpp     | 24 +++++---------
 src/Magnum/Vk/LayerProperties.h       |  7 ++--
 src/Magnum/Vk/Test/DeviceVkTest.cpp   |  1 +
 src/Magnum/Vk/Test/InstanceVkTest.cpp |  1 +
 7 files changed, 42 insertions(+), 49 deletions(-)

diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp
index a6291baab..ae14f364d 100644
--- a/doc/snippets/MagnumVk.cpp
+++ b/doc/snippets/MagnumVk.cpp
@@ -24,7 +24,7 @@
 */
 
 #include <string>
-#include <Corrade/Containers/ArrayView.h>
+#include <Corrade/Containers/Array.h>
 #include <Corrade/Utility/Algorithms.h>
 #include <Corrade/Utility/Directory.h>
 
diff --git a/src/Magnum/Vk/ExtensionProperties.cpp b/src/Magnum/Vk/ExtensionProperties.cpp
index 8760807d6..386d8f6c0 100644
--- a/src/Magnum/Vk/ExtensionProperties.cpp
+++ b/src/Magnum/Vk/ExtensionProperties.cpp
@@ -61,14 +61,11 @@ ExtensionProperties::ExtensionProperties(const Containers::ArrayView<const Conta
     /* Allocate extra for a list of string views that we'll use to sort &
        search the values and a layer index so we can map the extensions back
        to which layer they come from */
-    _extensions = Containers::Array<VkExtensionProperties>{
-        reinterpret_cast<VkExtensionProperties*>(new char[totalCount*(sizeof(VkExtensionProperties) + sizeof(Containers::StringView) + sizeof(UnsignedInt))]),
-        totalCount,
-        [](VkExtensionProperties* data, std::size_t) {
-            delete[] reinterpret_cast<char*>(data);
-        }};
-    Containers::ArrayView<Containers::StringView> extensionNames{reinterpret_cast<Containers::StringView*>(_extensions.end()), totalCount};
-    Containers::ArrayView<UnsignedInt> extensionLayers{reinterpret_cast<UnsignedInt*>(extensionNames.end()), totalCount};
+    _data = Containers::ArrayTuple{
+        {NoInit, totalCount, _extensions},
+        {totalCount, _names},
+        {NoInit, totalCount, _extensionLayers}
+    };
 
     /* Query the extensions, save layer ID for each */
     std::size_t offset = 0;
@@ -77,8 +74,8 @@ ExtensionProperties::ExtensionProperties(const Containers::ArrayView<const Conta
         MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(enumerator(state,
             i == 0 ? nullptr :
                 Containers::String::nullTerminatedView(layers[i - 1]).data(),
-            &count, reinterpret_cast<VkExtensionProperties*>(_extensions.data()) + offset));
-        for(std::size_t j = 0; j != count; ++j) extensionLayers[offset + j] = i;
+            &count, _extensions + offset));
+        for(std::size_t j = 0; j != count; ++j) _extensionLayers[offset + j] = i;
         offset += count;
     }
 
@@ -100,27 +97,24 @@ ExtensionProperties::ExtensionProperties(const Containers::ArrayView<const Conta
        costly enumeration. */
     if(offset < totalCount) {
         Warning{} << "Vk::ExtensionProperties: inconsistent extension count reported by the driver, expected" << totalCount << "but got only" << offset;
-        extensionNames = {extensionNames, offset};
-        extensionLayers = {extensionLayers, offset};
+        _names = {_names, offset};
+        _extensionLayers = {_extensionLayers, offset};
     } else CORRADE_INTERNAL_ASSERT(offset == totalCount);
 
     /* Populate the views, sort them and remove duplicates so we can search in
        O(log n) later */
-    for(std::size_t i = 0; i != extensionNames.size(); ++i)
-        extensionNames[i] = _extensions[i].extensionName;
-    std::sort(extensionNames.begin(), extensionNames.end());
-    _uniqueExtensionCount = std::unique(extensionNames.begin(), extensionNames.end()) - extensionNames.begin();
+    for(std::size_t i = 0; i != _names.size(); ++i)
+        _names[i] = _extensions[i].extensionName;
+    std::sort(_names.begin(), _names.end());
+    _names = _names.prefix(std::unique(_names.begin(), _names.end()));
 }
 
 Containers::ArrayView<const Containers::StringView> ExtensionProperties::names() const {
-    return {reinterpret_cast<const Containers::StringView*>(_extensions.end()), _uniqueExtensionCount};
+    return _names;
 }
 
 bool ExtensionProperties::isSupported(const Containers::StringView extension) const {
-    return std::binary_search(
-        reinterpret_cast<const Containers::StringView*>(_extensions.end()),
-        reinterpret_cast<const Containers::StringView*>(_extensions.end()) + _uniqueExtensionCount,
-        extension);
+    return std::binary_search(_names.begin(), _names.end(), extension);
 }
 
 bool ExtensionProperties::isSupported(const Extension& extension) const {
@@ -148,12 +142,9 @@ UnsignedInt ExtensionProperties::revision(const UnsignedInt id) const {
 }
 
 UnsignedInt ExtensionProperties::revision(const Containers::StringView extension) const {
-    /* Thanks, C++, for forcing me to do one more comparison than strictly
-       necessary */
-    auto found = std::lower_bound(
-        reinterpret_cast<const Containers::StringView*>(_extensions.end()),
-        reinterpret_cast<const Containers::StringView*>(_extensions.end()) + _uniqueExtensionCount,
-        extension);
+    /* Thanks, C++, for forcing me to have a larger bug surface instead of
+       providing a library helper to find the damn thing. */
+    auto found = std::lower_bound(_names.begin(), _names.end(), extension);
     if(*found != extension) return 0;
 
     /* The view target is contents of the VkExtensionProperties structure,
@@ -164,7 +155,7 @@ UnsignedInt ExtensionProperties::revision(const Containers::StringView extension
 UnsignedInt ExtensionProperties::layer(const UnsignedInt id) const {
     CORRADE_ASSERT(id < _extensions.size(),
         "Vk::xtensionProperties::layer(): index" << id << "out of range for" << _extensions.size() << "entries", {});
-    return reinterpret_cast<const UnsignedInt*>(reinterpret_cast<const Containers::StringView*>(_extensions.end()) + _extensions.size())[id];
+    return _extensionLayers[id];
 }
 
 InstanceExtensionProperties::InstanceExtensionProperties(InstanceExtensionProperties&&) noexcept = default;
diff --git a/src/Magnum/Vk/ExtensionProperties.h b/src/Magnum/Vk/ExtensionProperties.h
index 97a432838..9bf18b96a 100644
--- a/src/Magnum/Vk/ExtensionProperties.h
+++ b/src/Magnum/Vk/ExtensionProperties.h
@@ -30,7 +30,8 @@
  * @m_since_latest
  */
 
-#include <Corrade/Containers/Array.h>
+#include <Corrade/Containers/ArrayTuple.h>
+#include <Corrade/Containers/ArrayView.h>
 
 #include "Magnum/Tags.h"
 #include "Magnum/Magnum.h"
@@ -180,8 +181,10 @@ class MAGNUM_VK_EXPORT ExtensionProperties {
 
         explicit ExtensionProperties(const Containers::ArrayView<const Containers::StringView> layers, VkResult(*enumerator)(void*, const char*, UnsignedInt*, VkExtensionProperties*), void* state);
 
-        Containers::Array<VkExtensionProperties> _extensions;
-        std::size_t _uniqueExtensionCount;
+        Containers::ArrayTuple _data;
+        Containers::ArrayView<VkExtensionProperties> _extensions;
+        Containers::ArrayView<Containers::StringView> _names;
+        Containers::ArrayView<UnsignedInt> _extensionLayers;
 };
 
 /**
diff --git a/src/Magnum/Vk/LayerProperties.cpp b/src/Magnum/Vk/LayerProperties.cpp
index 92d45c48b..b72b63989 100644
--- a/src/Magnum/Vk/LayerProperties.cpp
+++ b/src/Magnum/Vk/LayerProperties.cpp
@@ -43,14 +43,11 @@ LayerProperties::~LayerProperties() = default;
 LayerProperties& LayerProperties::operator=(LayerProperties&&) noexcept = default;
 
 Containers::ArrayView<const Containers::StringView> LayerProperties::names() {
-    return {reinterpret_cast<const Containers::StringView*>(_layers.end()), _layers.size()};
+    return _names;
 }
 
 bool LayerProperties::isSupported(const Containers::StringView layer) {
-    return std::binary_search(
-        reinterpret_cast<const Containers::StringView*>(_layers.end()),
-        reinterpret_cast<const Containers::StringView*>(_layers.end()) + _layers.size(),
-        layer);
+    return std::binary_search(_names.begin(), _names.end(), layer);
 }
 
 UnsignedInt LayerProperties::count() {
@@ -95,22 +92,19 @@ LayerProperties enumerateLayerProperties() {
 
     /* Allocate extra for a list of string views that we'll use to sort &
        search the values; query the layers */
-    out._layers = Containers::Array<VkLayerProperties>{
-        reinterpret_cast<VkLayerProperties*>(new char[count*(sizeof(VkLayerProperties) + sizeof(Containers::StringView))]),
-        count,
-        [](VkLayerProperties* data, std::size_t) {
-            delete[] reinterpret_cast<char*>(data);
-        }};
+    out._data = Containers::ArrayTuple{
+        {NoInit, count, out._layers},
+        {count, out._names}
+    };
     MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(vkEnumerateInstanceLayerProperties(&count, out._layers.data()));
 
     /* Expect the layer count didn't change between calls */
     CORRADE_INTERNAL_ASSERT(count == out._layers.size());
 
     /* Populate the views and sort them so we can search in O(log n) later */
-    Containers::ArrayView<Containers::StringView> layerNames{reinterpret_cast<Containers::StringView*>(out._layers.end()), count};
-    for(std::size_t i = 0; i != layerNames.size(); ++i)
-        layerNames[i] = out._layers[i].layerName;
-    std::sort(layerNames.begin(), layerNames.end());
+    for(std::size_t i = 0; i != out._names.size(); ++i)
+        out._names[i] = out._layers[i].layerName;
+    std::sort(out._names.begin(), out._names.end());
 
     return out;
 }
diff --git a/src/Magnum/Vk/LayerProperties.h b/src/Magnum/Vk/LayerProperties.h
index 1d79bddb3..4348f517a 100644
--- a/src/Magnum/Vk/LayerProperties.h
+++ b/src/Magnum/Vk/LayerProperties.h
@@ -30,7 +30,8 @@
  * @m_since_latest
  */
 
-#include <Corrade/Containers/Array.h>
+#include <Corrade/Containers/ArrayTuple.h>
+#include <Corrade/Containers/ArrayView.h>
 
 #include "Magnum/Magnum.h"
 #include "Magnum/Tags.h"
@@ -146,7 +147,9 @@ class MAGNUM_VK_EXPORT LayerProperties {
 
         explicit LayerProperties();
 
-        Containers::Array<VkLayerProperties> _layers;
+        Containers::ArrayTuple _data;
+        Containers::ArrayView<VkLayerProperties> _layers;
+        Containers::ArrayView<Containers::StringView> _names;
 };
 
 /**
diff --git a/src/Magnum/Vk/Test/DeviceVkTest.cpp b/src/Magnum/Vk/Test/DeviceVkTest.cpp
index 35336c67a..488d2995c 100644
--- a/src/Magnum/Vk/Test/DeviceVkTest.cpp
+++ b/src/Magnum/Vk/Test/DeviceVkTest.cpp
@@ -24,6 +24,7 @@
 */
 
 #include <sstream>
+#include <Corrade/Containers/Array.h>
 #include <Corrade/Containers/Optional.h>
 #include <Corrade/Containers/StringStl.h>
 #include <Corrade/TestSuite/Compare/Numeric.h>
diff --git a/src/Magnum/Vk/Test/InstanceVkTest.cpp b/src/Magnum/Vk/Test/InstanceVkTest.cpp
index a37ebb1e4..817639a05 100644
--- a/src/Magnum/Vk/Test/InstanceVkTest.cpp
+++ b/src/Magnum/Vk/Test/InstanceVkTest.cpp
@@ -24,6 +24,7 @@
 */
 
 #include <sstream>
+#include <Corrade/Containers/Array.h>
 #include <Corrade/Containers/StringStl.h>
 #include <Corrade/TestSuite/Tester.h>
 #include <Corrade/TestSuite/Compare/Numeric.h>