[Banner] Keep horizontal user banner crops inside the artwork (#7742)

cockatrice/src/interface/widgets/cards/card_art_utils.cpp

@@ -2,6 +2,8 @@
 
 #include "libcockatrice/card/card_info.h"
 
+#include <QRect>
+#include <QSize>
 #include <QTransform>
 #include <libcockatrice/card/printing/exact_card.h>
 #include <qnamespace.h>
@@ -18,4 +20,23 @@ QPixmap rotateSidewaysLayoutArt(const QPixmap &art, const ExactCard &card)
     transform.rotate(90);
     return art.transformed(transform, Qt::SmoothTransformation);
 }
-} // namespace CardArtUtils
+
+QPixmap cropCardArt(const QPixmap &art, const ExactCard &card)
+{
+    if (art.isNull()) {
+        return {};
+    }
+
+    const QPixmap source = rotateSidewaysLayoutArt(art, card);
+    const bool landscape = card.getInfo().getUiAttributes().landscapeOrientation;
+    const QSize size = source.size();
+    const int marginX = size.width() * 0.07;
+    // Landscape name boxes extend below 5% of the upright image. Remove the
+    // whole box here, rather than relying on the banner's narrow viewport to
+    // hide it: the same saved offset is also used by the taller profile popup.
+    const int topMargin = size.height() * (landscape ? 0.15 : 0.11);
+    const int bottomMargin = size.height() * 0.45;
+    const QRect crop(marginX, topMargin, size.width() - 2 * marginX, size.height() - topMargin - bottomMargin);
+    return source.copy(crop.intersected(source.rect()));
+}
+} // namespace CardArtUtils

cockatrice/src/interface/widgets/cards/card_art_utils.h

@@ -20,6 +20,13 @@ namespace CardArtUtils
  * @return @p art rotated 90° clockwise for sideways-layout cards, else @p art.
  */
 QPixmap rotateSidewaysLayoutArt(const QPixmap &art, const ExactCard &card);
+
+/**
+ * Return the upright artwork used by banner previews, user rows and profile
+ * popups. The crop excludes conventional name and rules boxes before callers apply
+ * their saved pan and zoom, so a taller popup cannot expose the card frame.
+ */
+QPixmap cropCardArt(const QPixmap &art, const ExactCard &card);
 } // namespace CardArtUtils
 
-#endif // CARD_ART_UTILS_H
+#endif // CARD_ART_UTILS_H

cockatrice/src/interface/widgets/server/user/user_card_art_provider.cpp

@@ -12,7 +12,6 @@
 #include <QChar>
 #include <QMetaObject>
 #include <QPointer>
-#include <QRect>
 #include <QSize>
 #include <QStringList>
 #include <libcockatrice/card/database/card_database_manager.h>
@@ -66,27 +65,6 @@ void UserCardArtProvider::requestCardArt(const QString &userName, const QString
     processQueue();
 }
 
-QPixmap UserCardArtProvider::cropCardArt(const QPixmap &fullRes, const ExactCard &card)
-{
-    QPixmap source = fullRes;
-
-    // Sideways-layout cards (plane, siege/battle, split) store their landscape
-    // artwork rotated 90° inside a portrait frame. Rotate it upright first so
-    // the crop below lands on the horizontal art, mirroring the way
-    // CardInfoPictureWidget displays these cards.
-    const bool landscape = card.getInfo().getUiAttributes().landscapeOrientation;
-    source = CardArtUtils::rotateSidewaysLayoutArt(source, card);
-
-    const QSize sz = source.size();
-    const int marginX = sz.width() * 0.07;
-    const int topMargin = landscape ? sz.height() * 0.05 : sz.height() * 0.11;
-    const int bottomMargin = landscape ? sz.height() * 0.42 : sz.height() * 0.45;
-
-    const QRect artRect(marginX, topMargin, sz.width() - 2 * marginX, sz.height() - topMargin - bottomMargin);
-
-    return source.copy(artRect.intersected(source.rect()));
-}
-
 void UserCardArtProvider::insertIntoCache(const QString &key, const QPixmap &pixmap)
 {
     if (pixmap.isNull()) {
@@ -134,7 +112,7 @@ void UserCardArtProvider::processQueue()
 
         // Synchronous hit (already loaded/on disk)
         if (!fullRes.isNull()) {
-            insertIntoCache(key, cropCardArt(fullRes, card));
+            insertIntoCache(key, CardArtUtils::cropCardArt(fullRes, card));
             pending.remove(key);
 
             emit cardArtUpdated(userName);
@@ -158,7 +136,7 @@ void UserCardArtProvider::processQueue()
                             CardPictureLoader::getPixmap(fullRes, card, QSize(745, 1040));
 
                             if (!fullRes.isNull()) {
-                                self->insertIntoCache(key, self->cropCardArt(fullRes, card));
+                                self->insertIntoCache(key, CardArtUtils::cropCardArt(fullRes, card));
                             }
 
                             self->pending.remove(key);
@@ -172,4 +150,4 @@ void UserCardArtProvider::processQueue()
         // Stop here. We'll continue when the async load finishes.
         return;
     }
-}
+}

cockatrice/src/interface/widgets/server/user/user_card_art_provider.h

@@ -10,8 +10,6 @@
 #include <QString>
 #include <qtmetamacros.h>
 
-class ExactCard;
-
 class UserCardArtProvider : public QObject
 {
     Q_OBJECT
@@ -21,7 +19,6 @@ public:
 
     void requestCardArt(const QString &userName, const QString &cardName, const QString &providerId);
     const QMap<QString, QPixmap> &cache() const;
-    static QPixmap cropCardArt(const QPixmap &fullRes, const ExactCard &card);
 
 signals:
     void cardArtUpdated(const QString &userName);

cockatrice/src/interface/widgets/server/user/user_card_settings_dialog.cpp

@@ -2,6 +2,7 @@
 
 #include "../../../card_picture_loader/card_picture_loader.h"
 #include "../../cards/art_crop_attribution.h"
+#include "../../cards/card_art_utils.h"
 #include "../../utility/completer_utils.h"
 #include "card_database_display_model.h"
 #include "card_database_model.h"
@@ -10,7 +11,6 @@
 #include "libcockatrice/card/printing/exact_card.h"
 #include "libcockatrice/card/printing/printing_info.h"
 #include "libcockatrice/card/set/card_set.h"
-#include "user_card_art_provider.h"
 #include "user_list_painter.h"
 
 #include <QBrush>
@@ -580,7 +580,7 @@ void UserCardArtSettingsDialog::reloadPreview()
         return;
     }
 
-    currentPixmap = UserCardArtProvider::cropCardArt(fullRes, card);
+    currentPixmap = CardArtUtils::cropCardArt(fullRes, card);
     preview->setPixmap(currentPixmap);
     preview->setParams(currentParams);
 

tests/CMakeLists.txt

@@ -220,6 +220,18 @@ cockatrice_add_test(
   libcockatrice_deck_list
   libcockatrice_utility
 )
+cockatrice_add_test(
+  NAME
+  card_art_crop_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/cards/card_art_utils.cpp
+  ${VERSION_STRING_CPP}
+  card_art_crop_test.cpp
+  LIBS
+  libcockatrice_card
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/cockatrice/src
+)
 cockatrice_add_test(
   NAME
   server_rate_limiter_test

tests/card_art_crop_test.cpp

@@ -0,0 +1,92 @@
+#include "interface/widgets/cards/card_art_utils.h"
+
+#include <QApplication>
+#include <QImage>
+#include <QPainter>
+#include <QTransform>
+#include <gtest/gtest.h>
+#include <libcockatrice/card/card_info.h>
+#include <libcockatrice/card/printing/exact_card.h>
+
+namespace
+{
+ExactCard makeCard(bool landscape)
+{
+    CardInfo::UiAttributes attributes;
+    attributes.landscapeOrientation = landscape;
+    return ExactCard(CardInfo::newInstance("Artwork", "", false, {}, {}, {}, {}, attributes));
+}
+
+QPixmap landscapeCardWithFrame()
+{
+    // Normalized landscape frame: the title reaches 14% down the upright
+    // image, and the Battle type line starts at 56%. Artwork has two colors
+    // so the test also detects a wrong or missing rotation.
+    QImage upright(1400, 1000, QImage::Format_RGB32);
+    upright.fill(Qt::red);
+    QPainter painter(&upright);
+    painter.fillRect(QRect(70, 140, 1260, 420), Qt::green);
+    painter.fillRect(QRect(700, 140, 630, 420), Qt::blue);
+    painter.end();
+    QTransform storedOrientation;
+    storedOrientation.rotate(-90);
+    return QPixmap::fromImage(upright.transformed(storedOrientation));
+}
+
+TEST(CardArtCrop, LandscapeCropExcludesNameAndRulesBoxesAndRotatesUpright)
+{
+    const QImage crop = CardArtUtils::cropCardArt(landscapeCardWithFrame(), makeCard(true)).toImage();
+    ASSERT_GT(crop.width(), crop.height());
+    ASSERT_FALSE(crop.isNull());
+    for (int y = 0; y < crop.height(); ++y) {
+        for (int x = 0; x < crop.width(); ++x) {
+            EXPECT_EQ(x < crop.width() / 2 ? QColor(Qt::green) : QColor(Qt::blue), crop.pixelColor(x, y));
+        }
+    }
+}
+
+TEST(CardArtCrop, PortraitCropRetainsItsExistingArtworkBounds)
+{
+    QImage source(745, 1040, QImage::Format_RGB32);
+    for (int y = 0; y < source.height(); ++y) {
+        for (int x = 0; x < source.width(); ++x) {
+            source.setPixelColor(x, y, QColor(x % 256, y % 256, (x + y) % 256));
+        }
+    }
+    const QPixmap input = QPixmap::fromImage(source);
+    const QImage crop = CardArtUtils::cropCardArt(input, makeCard(false)).toImage();
+    EXPECT_EQ(source.copy(QRect(52, 114, 641, 458)), crop);
+}
+
+TEST(CardArtCrop, DevicePixelRatioDoesNotChangeTheArtworkPixels)
+{
+    QPixmap source = landscapeCardWithFrame();
+    const QPixmap normal = CardArtUtils::cropCardArt(source, makeCard(true));
+    source.setDevicePixelRatio(2.0);
+    const QPixmap highDpi = CardArtUtils::cropCardArt(source, makeCard(true));
+    EXPECT_EQ(normal.size(), highDpi.size());
+    EXPECT_EQ(2.0, highDpi.devicePixelRatio());
+    QImage normalPixels = normal.toImage();
+    QImage highDpiPixels = highDpi.toImage();
+    normalPixels.setDevicePixelRatio(1.0);
+    highDpiPixels.setDevicePixelRatio(1.0);
+    EXPECT_EQ(normalPixels, highDpiPixels);
+}
+
+TEST(CardArtCrop, EmptyAndSmallImagesStayWithinTheirBounds)
+{
+    EXPECT_TRUE(CardArtUtils::cropCardArt({}, makeCard(true)).isNull());
+    QPixmap small(1, 1);
+    small.fill(Qt::green);
+    EXPECT_EQ(small.toImage(), CardArtUtils::cropCardArt(small, makeCard(true)).toImage());
+    EXPECT_EQ(small.toImage(), CardArtUtils::cropCardArt(small, makeCard(false)).toImage());
+}
+} // namespace
+
+int main(int argc, char **argv)
+{
+    qputenv("QT_QPA_PLATFORM", "offscreen");
+    QApplication app(argc, argv);
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}