From 6fe0dd1078f654ad4e031694244bf6745abb5c9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Tue, 9 Nov 2021 19:56:03 +0100
Subject: [PATCH] Trade: report at least some *D object count if everything
 fails.

As the comment says -- before the user code expected that if the scene
hierarchy is broken, particular objects will fail to import. Now the
whole scene fails to import, so we don't even get to know the actual
(expanded) deprecated 2D/3D object count. To reduce the suffering,
return at least the dimension-less object count there. It won't include
the duplicates from the single-function-object conversion but better
than reporting 0.
---
 src/Magnum/Trade/AbstractImporter.cpp         | 11 +++++
 .../Trade/Test/AbstractImporterTest.cpp       | 43 +++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp
index 2146519d2..cda75437e 100644
--- a/src/Magnum/Trade/AbstractImporter.cpp
+++ b/src/Magnum/Trade/AbstractImporter.cpp
@@ -547,6 +547,17 @@ void AbstractImporter::populateCachedScenes() {
             newObjectOffset = Math::max(newObjectOffset, _cachedScenes->scenes[i]->objectCount());
         }
     }
+
+    /* If there are scenes but no objects (because for example all scenes
+       failed to import), use the dimension-less object count at least, and
+       assume the scene was 3D. Otherwise this may cause unexpected assertions
+       in code that expected proper object count to be reported even if a scene
+       contains errors.
+
+       Not ideal, especially regarding the 3D assumption, but better than
+       nothing. */
+    if(!_cachedScenes->scenes.empty() && !_cachedScenes->object2DCount && !_cachedScenes->object3DCount)
+        _cachedScenes->object3DCount = objectCount();
 }
 
 UnsignedInt AbstractImporter::object2DCount() const {
diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp
index d7d44c859..627ba15a9 100644
--- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp
+++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp
@@ -126,6 +126,8 @@ struct AbstractImporterTest: TestSuite::Tester {
     void sceneDeprecatedFallbackTransformless3D();
     void sceneDeprecatedFallbackMultiFunctionObjects2D();
     void sceneDeprecatedFallbackMultiFunctionObjects3D();
+    void sceneDeprecatedFallbackObjectCountNoScenes();
+    void sceneDeprecatedFallbackObjectCountAllSceneImportFailed();
     #endif
     void sceneNameNotImplemented();
     void objectNameNotImplemented();
@@ -410,6 +412,8 @@ AbstractImporterTest::AbstractImporterTest() {
               &AbstractImporterTest::sceneDeprecatedFallbackTransformless3D,
               &AbstractImporterTest::sceneDeprecatedFallbackMultiFunctionObjects2D,
               &AbstractImporterTest::sceneDeprecatedFallbackMultiFunctionObjects3D,
+              &AbstractImporterTest::sceneDeprecatedFallbackObjectCountNoScenes,
+              &AbstractImporterTest::sceneDeprecatedFallbackObjectCountAllSceneImportFailed,
               #endif
               &AbstractImporterTest::sceneForNameOutOfRange,
               &AbstractImporterTest::objectForNameOutOfRange,
@@ -3219,6 +3223,45 @@ void AbstractImporterTest::sceneDeprecatedFallbackMultiFunctionObjects3D() {
     }
     CORRADE_IGNORE_DEPRECATED_POP
 }
+
+void AbstractImporterTest::sceneDeprecatedFallbackObjectCountNoScenes() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doSceneCount() const override { return 0; }
+        UnsignedLong doObjectCount() const override { return 27; }
+    } importer;
+
+    /* There's no scenes to get data or hierarchy from, so there are no
+       2D/3D objects reported even though objectCount() says 27 */
+    CORRADE_IGNORE_DEPRECATED_PUSH
+    CORRADE_COMPARE(importer.object2DCount(), 0);
+    CORRADE_COMPARE(importer.object3DCount(), 0);
+    CORRADE_IGNORE_DEPRECATED_POP
+}
+
+void AbstractImporterTest::sceneDeprecatedFallbackObjectCountAllSceneImportFailed() {
+    struct: AbstractImporter {
+        ImporterFeatures doFeatures() const override { return {}; }
+        bool doIsOpened() const override { return true; }
+        void doClose() override {}
+
+        UnsignedInt doSceneCount() const override { return 1; }
+        UnsignedLong doObjectCount() const override { return 27; }
+        Containers::Optional<SceneData> doScene(UnsignedInt) override {
+            return {};
+        }
+    } importer;
+
+    /* There's a scene but it failed to import, assume it was 3D and proxy the
+       objectCount() */
+    CORRADE_IGNORE_DEPRECATED_PUSH
+    CORRADE_COMPARE(importer.object2DCount(), 0);
+    CORRADE_COMPARE(importer.object3DCount(), 27);
+    CORRADE_IGNORE_DEPRECATED_POP
+}
 #endif
 
 void AbstractImporterTest::sceneNameNotImplemented() {