Merge pull request #604 from LebedevRI/next

And some more `ArrayRef` touchups

Author: LebedevRI

fuzz/librawspeed/common/DngOpcodes.cpp

@@ -61,8 +61,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) {
       rawspeed::Array2DRef<uint16_t> img =
           mRaw->getU16DataAsUncroppedArray2DRef();
       const uint16_t fill = bs.getU16();
-      for (auto row = 0; row < img.height; ++row) {
-        for (auto col = 0; col < img.width; ++col) {
+      for (auto row = 0; row < img.height(); ++row) {
+        for (auto col = 0; col < img.width(); ++col) {
           img(row, col) = fill;
         }
       }
@@ -71,8 +71,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) {
     case rawspeed::RawImageType::F32: {
       rawspeed::Array2DRef<float> img = mRaw->getF32DataAsUncroppedArray2DRef();
       const float fill = bs.getFloat();
-      for (auto row = 0; row < img.height; ++row) {
-        for (auto col = 0; col < img.width; ++col) {
+      for (auto row = 0; row < img.height(); ++row) {
+        for (auto col = 0; col < img.width(); ++col) {
           img(row, col) = fill;
         }
       }

src/librawspeed/adt/Array1DRef.h

@@ -79,6 +79,7 @@ template <class T> class Array1DRef final {
         numElts(sizeof(T2) * RHS.numElts) {}
 
   [[nodiscard]] CroppedArray1DRef<T> getCrop(int offset, int numElts) const;
+  [[nodiscard]] CroppedArray1DRef<T> getBlock(int numElts, int index) const;
 
   [[nodiscard]] int RAWSPEED_READONLY size() const;
 
@@ -116,6 +117,17 @@ template <class T>
   return {*this, offset, size};
 }
 
+template <class T>
+[[nodiscard]] CroppedArray1DRef<T> Array1DRef<T>::getBlock(int size,
+                                                           int index) const {
+  establishClassInvariants();
+  invariant(index >= 0);
+  invariant(size >= 0);
+  invariant(index <= numElts);
+  invariant(size <= numElts);
+  return getCrop(size * index, size);
+}
+
 template <class T> inline T* Array1DRef<T>::addressOf(const int eltIdx) const {
   establishClassInvariants();
   invariant(eltIdx >= 0);

src/librawspeed/adt/Array2DRef.h

@@ -22,6 +22,7 @@
 #pragma once
 
 #include "adt/Array1DRef.h"
+#include "adt/CroppedArray1DRef.h" // IWYU pragma: keep
 #include "adt/Invariant.h"
 #include "adt/Optional.h"
 #include <cstddef>
@@ -31,27 +32,34 @@
 namespace rawspeed {
 
 template <class T> class Array2DRef final {
-  T* _data;
+  Array1DRef<T> data;
   int _pitch;
 
+  int _width;
+  int _height;
+
   friend Array2DRef<const T>; // We need to be able to convert to const version.
 
   // We need to be able to convert to std::byte.
   friend Array2DRef<std::byte>;
   friend Array2DRef<const std::byte>;
 
+  Array2DRef(Array1DRef<T> data, int width, int height, int pitch);
+
 public:
   void establishClassInvariants() const noexcept;
 
   using value_type = T;
   using cvless_value_type = std::remove_cv_t<value_type>;
 
-  int width;
-  int height;
+  [[nodiscard]] int RAWSPEED_READONLY width() const;
+  [[nodiscard]] int RAWSPEED_READONLY height() const;
 
   Array2DRef() = delete;
 
-  Array2DRef(T* data, int width, int height, int pitch = 0);
+  Array2DRef(T* data, int width, int height, int pitch);
+
+  Array2DRef(T* data, int width, int height);
 
   // Can not cast away constness.
   template <typename T2>
@@ -70,8 +78,8 @@ template <class T> class Array2DRef final {
     requires(!std::is_const_v<T2> && std::is_const_v<T> &&
              std::is_same_v<std::remove_const_t<T>, std::remove_const_t<T2>>)
   Array2DRef(Array2DRef<T2> RHS) // NOLINT google-explicit-constructor
-      : _data(RHS._data), _pitch(RHS._pitch), width(RHS.width),
-        height(RHS.height) {}
+      : data(RHS.data), _pitch(RHS._pitch), _width(RHS.width()),
+        _height(RHS.height()) {}
 
   // Const-preserving conversion from Array2DRef<T> to Array2DRef<std::byte>.
   template <typename T2>
@@ -80,8 +88,8 @@ template <class T> class Array2DRef final {
                               std::remove_const_t<T2>>) &&
              std::is_same_v<std::remove_const_t<T>, std::byte>)
   Array2DRef(Array2DRef<T2> RHS) // NOLINT google-explicit-constructor
-      : _data(reinterpret_cast<T*>(RHS._data)), _pitch(sizeof(T2) * RHS._pitch),
-        width(sizeof(T2) * RHS.width), height(RHS.height) {}
+      : data(RHS.data), _pitch(sizeof(T2) * RHS._pitch),
+        _width(sizeof(T2) * RHS.width()), _height(RHS.height()) {}
 
   template <typename AllocatorType =
                 typename std::vector<cvless_value_type>::allocator_type>
@@ -102,53 +110,81 @@ template <class T> class Array2DRef final {
 
 // CTAD deduction guide
 template <typename T>
-explicit Array2DRef(T* data, int width, int height, int pitch = 0)
+explicit Array2DRef(Array1DRef<T> data, int width, int height, int pitch)
     -> Array2DRef<T>;
 
+// CTAD deduction guide
+template <typename T>
+explicit Array2DRef(T* data, int width, int height, int pitch) -> Array2DRef<T>;
+
+// CTAD deduction guide
+template <typename T>
+explicit Array2DRef(T* data, int width, int height) -> Array2DRef<T>;
+
 template <class T>
 inline void Array2DRef<T>::establishClassInvariants() const noexcept {
-  invariant(_data);
-  invariant(width >= 0);
-  invariant(height >= 0);
+  data.establishClassInvariants();
+  invariant(_width >= 0);
+  invariant(_height >= 0);
+  invariant(_pitch != 0);
   invariant(_pitch >= 0);
-  invariant(_pitch >= width);
+  invariant(_pitch >= _width);
+  invariant((_width == 0) == (_height == 0));
+  invariant(data.size() == _pitch * _height);
 }
 
 template <class T>
-Array2DRef<T>::Array2DRef(T* data, const int width_, const int height_,
-                          const int pitch_ /* = 0 */)
-    : _data(data), _pitch(pitch_ == 0 ? width_ : pitch_), width(width_),
-      height(height_) {
+Array2DRef<T>::Array2DRef(Array1DRef<T> data_, const int width_,
+                          const int height_, const int pitch_)
+    : data(data_), _pitch(pitch_), _width(width_), _height(height_) {
+  establishClassInvariants();
+}
+
+template <class T>
+Array2DRef<T>::Array2DRef(T* data_, const int width_, const int height_,
+                          const int pitch_)
+    : Array2DRef({data_, pitch_ * height_}, width_, height_, pitch_) {
+  establishClassInvariants();
+}
+
+template <class T>
+Array2DRef<T>::Array2DRef(T* data_, const int width_, const int height_)
+    : Array2DRef(data_, width_, height_, /*pitch=*/width_) {
+  establishClassInvariants();
+}
+
+template <class T> inline int Array2DRef<T>::width() const {
+  establishClassInvariants();
+  return _width;
+}
+
+template <class T> inline int Array2DRef<T>::height() const {
   establishClassInvariants();
+  return _height;
 }
 
 template <class T>
 [[nodiscard]] inline Optional<Array1DRef<T>>
 Array2DRef<T>::getAsArray1DRef() const {
   establishClassInvariants();
-  if (height == 1 || _pitch == width)
-    return {{_data, width * height}};
+  if (height() == 1 || _pitch == width())
+    return data.getCrop(/*offset=*/0, width() * height()).getAsArray1DRef();
   return std::nullopt;
 }
 
 template <class T>
 inline Array1DRef<T> Array2DRef<T>::operator[](const int row) const {
   establishClassInvariants();
   invariant(row >= 0);
-  invariant(row < height);
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpragmas"
-#pragma GCC diagnostic ignored "-Wunknown-warning-option"
-#pragma GCC diagnostic ignored "-Wunsafe-buffer-usage"
-  return Array1DRef<T>(&_data[row * _pitch], width);
-#pragma GCC diagnostic pop
+  invariant(row < height());
+  return data.getCrop(row * _pitch, width()).getAsArray1DRef();
 }
 
 template <class T>
 inline T& Array2DRef<T>::operator()(const int row, const int col) const {
   establishClassInvariants();
   invariant(col >= 0);
-  invariant(col < width);
+  invariant(col < width());
   return (operator[](row))(col);
 }
 

src/librawspeed/adt/CroppedArray2DRef.h

@@ -60,8 +60,8 @@ template <class T> class CroppedArray2DRef final {
 
   // Conversion from Array2DRef<T> to CroppedArray2DRef<T>.
   CroppedArray2DRef(Array2DRef<T> RHS) // NOLINT google-explicit-constructor
-      : base(RHS), offsetCols(0), offsetRows(0), croppedWidth(base.width),
-        croppedHeight(base.height) {}
+      : base(RHS), offsetCols(0), offsetRows(0), croppedWidth(base.width()),
+        croppedHeight(base.height()) {}
 
   CroppedArray2DRef(Array2DRef<T> base_, int offsetCols_, int offsetRows_,
                     int croppedWidth_, int croppedHeight_);
@@ -94,12 +94,13 @@ inline void CroppedArray2DRef<T>::establishClassInvariants() const noexcept {
   invariant(offsetRows >= 0);
   invariant(croppedWidth >= 0);
   invariant(croppedHeight >= 0);
-  invariant(offsetCols <= base.width);
-  invariant(offsetRows <= base.height);
-  invariant(croppedWidth <= base.width);
-  invariant(croppedHeight <= base.height);
-  invariant(offsetCols + croppedWidth <= base.width);
-  invariant(offsetRows + croppedHeight <= base.height);
+  invariant(offsetCols <= base.width());
+  invariant(offsetRows <= base.height());
+  invariant(croppedWidth <= base.width());
+  invariant(croppedHeight <= base.height());
+  invariant(offsetCols + croppedWidth <= base.width());
+  invariant(offsetRows + croppedHeight <= base.height());
+  invariant((croppedWidth == 0) == (croppedHeight == 0));
 }
 
 template <class T>

src/librawspeed/common/BayerPhase.h

@@ -84,8 +84,8 @@ inline std::array<T, 4> applyPhaseShift(std::array<T, 4> srcData,
 
   std::array<T, 4> tgtData;
   const Array2DRef<T> tgt(tgtData.data(), 2, 2);
-  for (int row = 0; row < tgt.height; ++row) {
-    for (int col = 0; col < tgt.width; ++col) {
+  for (int row = 0; row < tgt.height(); ++row) {
+    for (int col = 0; col < tgt.width(); ++col) {
       tgt(row, col) = src((coordOffset.y + row) % 2, (coordOffset.x + col) % 2);
     }
   }
@@ -133,8 +133,8 @@ inline Optional<BayerPhase> getAsBayerPhase(const ColorFilterArray& CFA) {
 
   std::array<CFAColor, 4> patData;
   const Array2DRef<CFAColor> pat(patData.data(), 2, 2);
-  for (int row = 0; row < pat.height; ++row) {
-    for (int col = 0; col < pat.width; ++col) {
+  for (int row = 0; row < pat.height(); ++row) {
+    for (int col = 0; col < pat.width(); ++col) {
       pat(row, col) = CFA.getColorAt(col, row);
     }
   }

src/librawspeed/common/Common.h

@@ -61,19 +61,19 @@ inline void copyPixelsImpl(Array1DRef<std::byte> dest,
 
 inline void copyPixelsImpl(Array2DRef<std::byte> dest,
                            Array2DRef<const std::byte> src) {
-  invariant(src.width > 0);
-  invariant(src.height > 0);
-  invariant(dest.width > 0);
-  invariant(dest.height > 0);
-  invariant(src.height == dest.height);
-  invariant(src.width == dest.width);
+  invariant(src.width() > 0);
+  invariant(src.height() > 0);
+  invariant(dest.width() > 0);
+  invariant(dest.height() > 0);
+  invariant(src.height() == dest.height());
+  invariant(src.width() == dest.width());
   if (auto [destAsStrip, srcAsStrip] =
           std::make_tuple(dest.getAsArray1DRef(), src.getAsArray1DRef());
       destAsStrip && srcAsStrip) {
     copyPixelsImpl(*destAsStrip, *srcAsStrip);
     return;
   }
-  for (int row = 0; row != src.height; ++row)
+  for (int row = 0; row != src.height(); ++row)
     copyPixelsImpl(dest[row], src[row]);
 }
 

src/librawspeed/common/RawImage.cpp

@@ -299,7 +299,7 @@ void RawImageData::fixBadPixelsThread(int start_y, int end_y) {
 
   for (int y = start_y; y < end_y; y++) {
     for (int x = 0; x < gw; x++) {
-      const auto block = bad[y].getCrop((32 * x) / 8, 4);
+      const auto block = bad[y].getBlock(32 / 8, x);
 
       // Test if there is a bad pixel within these 32 pixels
       if (std::all_of(block.begin(), block.end(),

src/librawspeed/common/RawImage.h

@@ -161,7 +161,7 @@ class RawImageData : public ErrorLog {
   ColorFilterArray cfa;
   int blackLevel = -1;
   std::array<int, 4> blackLevelSeparateStorage;
-  Array2DRef<int> blackLevelSeparate = {blackLevelSeparateStorage.data(), 0, 0};
+  Optional<Array2DRef<int>> blackLevelSeparate;
   int whitePoint = 65536;
   std::vector<BlackArea> blackAreas;
 

src/librawspeed/common/RawImageDataFloat.cpp

@@ -88,7 +88,7 @@ void RawImageDataFloat::calculateBlackAreas() {
   }
 
   blackLevelSeparate = Array2DRef(blackLevelSeparateStorage.data(), 2, 2);
-  auto blackLevelSeparate1D = *blackLevelSeparate.getAsArray1DRef();
+  auto blackLevelSeparate1D = *blackLevelSeparate->getAsArray1DRef();
 
   if (!totalpixels) {
     for (int& i : blackLevelSeparate1D)
@@ -122,8 +122,7 @@ void RawImageDataFloat::scaleBlackWhite() {
 
   const int skipBorder = 150;
   int gw = (dim.x - skipBorder) * cpp;
-  if ((blackAreas.empty() && blackLevelSeparate.width == 0 &&
-       blackLevelSeparate.height == 0 && blackLevel < 0) ||
+  if ((blackAreas.empty() && !blackLevelSeparate && blackLevel < 0) ||
       whitePoint == 65536) { // Estimate
     float b = 100000000;
     float m = -10000000;
@@ -143,7 +142,7 @@ void RawImageDataFloat::scaleBlackWhite() {
   }
 
   /* If filter has not set separate blacklevel, compute or fetch it */
-  if (blackLevelSeparate.width == 0 && blackLevelSeparate.height == 0)
+  if (!blackLevelSeparate)
     calculateBlackAreas();
 
   startWorker(RawImageWorker::RawImageWorkerTask::SCALE_VALUES, true);
@@ -154,8 +153,8 @@ void RawImageDataFloat::scaleValues(int start_y, int end_y) {
   int gw = dim.x * cpp;
   std::array<float, 4> mul;
   std::array<float, 4> sub;
-  assert(blackLevelSeparate.width == 2 && blackLevelSeparate.height == 2);
-  auto blackLevelSeparate1D = *blackLevelSeparate.getAsArray1DRef();
+  assert(blackLevelSeparate->width() == 2 && blackLevelSeparate->height() == 2);
+  auto blackLevelSeparate1D = *blackLevelSeparate->getAsArray1DRef();
   for (int i = 0; i < 4; i++) {
     int v = i;
     if ((mOffset.x & 1) != 0)