Merge pull request #247 from NoxHarmonium/more-tiledit-cleanup

test(tiledit): add some integration tests with real images

.github/workflows/sirc-tiledit.yml

@@ -40,7 +40,7 @@ jobs:
           ln -s $(which clang-format-18) /usr/local/bin/clang-format
       - name: Install ubuntu dependencies
         run: |
-          sudo apt-get install catch2
+          sudo apt-get install libpng-dev
       - name: Set up Python
         uses: actions/setup-python@v5
         with:
@@ -66,13 +66,16 @@ jobs:
       - name: Test Project
         run: |
           cd build-debug
-          meson test
+          meson test --print-errorlogs
           ninja coverage-xml
       - name: Archive test coverage
         uses: actions/upload-artifact@v4
+        if: always()
         with:
           name: codecov-tiledit
-          path: ./sirc-tiledit/build-debug/meson-logs/coverage.xml
+          path: |
+            ./sirc-tiledit/build-debug/meson-logs/coverage.xml
+            ./sirc-tiledit/libs/shared/tests/resources/*actual*.png
           if-no-files-found: error
       - name: Lint Project
         run: |

sirc-tiledit/.clang-format

@@ -3,3 +3,7 @@
 BasedOnStyle: LLVM
 Language: Cpp
 Standard: c++20
+
+# Stop the formatter from breaking up clang-tidy nolint comments
+CommentPragmas: '^NOLINTNEXTLINE|^NOLINTBEGIN|^NOLINTEND'
+InsertBraces: true

sirc-tiledit/.clang-format-ignore

@@ -0,0 +1 @@
+libs/shared/tests/catch2/*

sirc-tiledit/.clang-tidy

@@ -1,5 +1,5 @@
 ---
-Checks: "clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,modernize-*,bugprone-*,llvm-*,performance-*,-modernize-use-trailing-return-type,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-type-vararg,-bugprone-macro-parentheses,-llvm-header-guard"
+Checks: "clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,modernize-*,bugprone-*,llvm-*,performance-*,-modernize-use-trailing-return-type,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-type-vararg,-bugprone-macro-parentheses,-llvm-header-guard,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers"
 FormatStyle: llvm
 CheckOptions:
   cppcoreguidelines-avoid-do-while.IgnoreMacros: "true"
@@ -8,3 +8,4 @@ CheckOptions:
 # cppcoreguidelines-pro-type-vararg can be useful sometimes, I'll try to use it sparingly
 # bugprone-macro-parentheses because it will cause too many changes right now with all the guards for double header inclusion
 # llvm-header-guard because I couldn't get auto fix to work and it is a pretty trivial rule
+# readability-magic-numbers because sometimes magic numbers are more readable, and this isn't really something I need a machine to detect

sirc-tiledit/.clang-tidy-ignore

@@ -0,0 +1 @@
+libs/shared/tests/catch2/*

sirc-tiledit/gcovr.cfg

@@ -0,0 +1 @@
+exclude = .*\/tests\/.*

sirc-tiledit/libs/gui/src/pixmapadapter.cpp

@@ -2,7 +2,9 @@
 #include "pixmapadapter.hpp"
 #include "sircimage.hpp"
 
-u_int16_t sircColorFromQRgb(const QColor qColor) {
+#include <miscadapter.hpp>
+
+SircColor sircColorFromQRgb(const QColor qColor) {
   const unsigned int r = qColor.red() / Q_TO_SIRC_COLOR_RATIO;
   const unsigned int g = qColor.green() / Q_TO_SIRC_COLOR_RATIO;
   const unsigned int b = qColor.blue() / Q_TO_SIRC_COLOR_RATIO;
@@ -11,7 +13,7 @@ u_int16_t sircColorFromQRgb(const QColor qColor) {
          b;
 }
 
-QColor qRgbFromSircColor(const u_int16_t sircColor) {
+QColor qRgbFromSircColor(const SircColor sircColor) {
   const unsigned int sircR =
       sircColor >> SIRC_COLOR_COMPONENT_BITS * 2 & SIRC_COLOR_RANGE;
   const unsigned int sircG =
@@ -29,7 +31,7 @@ QColor qRgbFromSircColor(const u_int16_t sircColor) {
 
 SircImage PixmapAdapter::pixmapToSircImage(const QPixmap &qPixmap) {
   const auto image = qPixmap.toImage();
-  PackedPixelData pixelData;
+  PackedSircPixelData pixelData;
   assert(image.width() >= WIDTH_PIXELS && image.height() >= HEIGHT_PIXELS);
 
   for (int x = 0; x < WIDTH_PIXELS; x++) {
@@ -40,14 +42,14 @@ SircImage PixmapAdapter::pixmapToSircImage(const QPixmap &qPixmap) {
       pixelData[x][y] = convertedPixel;
     }
   }
-  auto sircImage = SircImage::fromPixelData(pixelData);
+  auto sircImage = MiscAdapter::packedSircPixelDataToSircImage(pixelData);
 
   return sircImage;
 }
 
 QPixmap PixmapAdapter::sircImageToPixmap(const SircImage &sircImage) {
   auto image = QImage(WIDTH_PIXELS, HEIGHT_PIXELS, QImage::Format_RGB32);
-  auto [palette, pixelData] = sircImage.getImageData();
+  auto [palette, pixelData] = sircImage;
 
   for (int x = 0; x < WIDTH_PIXELS; x++) {
     for (int y = 0; y < HEIGHT_PIXELS; y++) {
@@ -65,7 +67,7 @@ QPixmap PixmapAdapter::sircImageToPixmap(const SircImage &sircImage) {
 std::vector<QColor>
 PixmapAdapter::getPaletteColors(const SircImage &sircImage) {
   auto convertedPalette = std::vector<QColor>();
-  const auto [palette, pixelData] = sircImage.getImageData();
+  const auto [palette, pixelData] = sircImage;
 
   std::vector<QColor> output;
   std::ranges::transform(

sirc-tiledit/libs/shared/include/constants.hpp

@@ -0,0 +1,11 @@
+//
+// Created by Sean Dawson on 27/6/2024.
+//
+
+#ifndef CONSTANTS_HPP
+#define CONSTANTS_HPP
+
+constexpr int WIDTH_PIXELS = 256;
+constexpr int HEIGHT_PIXELS = 256;
+
+#endif // CONSTANTS_HPP

sirc-tiledit/libs/shared/include/imageloader.hpp

@@ -0,0 +1,27 @@
+
+#ifndef IMAGELOADER_HPP
+#define IMAGELOADER_HPP
+
+#include "constants.hpp"
+
+#include <array>
+#include <cstdint>
+#include <limits>
+
+using RgbaComponent = uint8_t;
+using RgbaPixel = uint32_t;
+using RgbaPixelData =
+    std::array<std::array<RgbaPixel, HEIGHT_PIXELS>, WIDTH_PIXELS>;
+
+constexpr RgbaComponent RGBA_COMPONENT_MIN =
+    std::numeric_limits<RgbaComponent>::min();
+constexpr RgbaComponent RGBA_COMPONENT_MAX =
+    std::numeric_limits<RgbaComponent>::max();
+
+class ImageLoader {
+public:
+  static RgbaPixelData loadImageFromPng(const char *filename);
+  static void saveImageToPng(const char *filename, const RgbaPixelData &data);
+};
+
+#endif // IMAGELOADER_HPP

sirc-tiledit/libs/shared/include/miscadapter.hpp

@@ -0,0 +1,13 @@
+
+#ifndef MISCADAPTER_HPP
+#define MISCADAPTER_HPP
+
+#include "sircimage.hpp"
+
+class MiscAdapter {
+public:
+  static SircImage
+  packedSircPixelDataToSircImage(const PackedSircPixelData &pixelData);
+};
+
+#endif // MISCADAPTER_HPP

sirc-tiledit/libs/shared/include/rgbaadapter.hpp

@@ -0,0 +1,19 @@
+#ifndef RGBAADAPTER_H
+#define RGBAADAPTER_H
+
+#include <imageloader.hpp>
+#include <sircimage.hpp>
+
+// PNGs are loaded with standard 32 bit colour RGBA (8bpp)
+constexpr unsigned int RGBA_COLOR_RANGE = 0xFF;
+constexpr unsigned int RGBA_TO_SIRC_COLOR_RATIO =
+    RGBA_COLOR_RANGE / SIRC_COLOR_RANGE;
+constexpr unsigned int RGBA_BLACK = 0x000000FF;
+
+class RgbaAdapter {
+public:
+  static SircImage rgbaToSircImage(const RgbaPixelData &pixelData);
+  static RgbaPixelData sircImageToRgba(const SircImage &sircImage);
+};
+
+#endif // RGBAADAPTER_H

sirc-tiledit/libs/shared/include/sircimage.hpp

@@ -4,11 +4,10 @@
 #include <array>
 #include <cstddef>
 #include <cstdint>
-#include <map>
 #include <vector>
 
-constexpr int WIDTH_PIXELS = 256;
-constexpr int HEIGHT_PIXELS = 256;
+#include "constants.hpp"
+
 // The number of palette slots in the SIRC PPU
 constexpr int MAX_PALETTE_SIZE = 256;
 
@@ -18,42 +17,24 @@ constexpr unsigned int SIRC_COLOR_RANGE =
     (1 << (SIRC_COLOR_COMPONENT_BITS)) - 1;
 
 using SircColor = uint16_t;
-using ArgbColor = uint32_t;
-using PaletteReference = uint8_t;
-using PackedPixelData =
+using SircColorComponent = uint8_t;
+using PaletteReference = size_t;
+using PackedSircPixelData =
     std::array<std::array<SircColor, HEIGHT_PIXELS>, WIDTH_PIXELS>;
 using IndexedPixelData =
     std::array<std::array<PaletteReference, HEIGHT_PIXELS>, WIDTH_PIXELS>;
 
-struct SircImageData {
-  std::vector<SircColor> palette;
-  IndexedPixelData pixelData;
-  bool operator==(const SircImageData &) const = default;
-};
-
 /**
  * @brief Represents an image in the format supported by the SIRC PPU
  *
  * The SIRC PPU uses a 15 bit (5bpp) color format with a palette.
  * The palette can store 256 colors but usually tile data will
  * only support max 4bpp (16 colors).
  */
-class SircImage {
-
-public:
-  static SircImage fromPixelData(const PackedPixelData &pixelData);
-  static SircImage fromSircImageData(const SircImageData &imageData);
-
-  [[nodiscard]] SircImageData getImageData() const;
-  [[nodiscard]] PaletteReference paletteIndexForColor(SircColor color) const;
-
-private:
-  SircImageData imageData = {};
-  // This isn't doing anything at the moment because we are part way through a
-  // refactor
-  std::map<SircColor, size_t> paletteLookup;
-
-  SircImage();
+struct SircImage {
+  std::vector<SircColor> palette;
+  IndexedPixelData pixelData;
+  bool operator==(const SircImage &) const = default;
 };
 
 #endif // IMAGEPROCESSOR_H