Browse Source

TgaImporter: report error message on empty file being opened.

Also updated the test cases to be consistent with other image plugins.
pull/324/head
Vladimír Vondruš 7 years ago
parent
commit
25ef63ecd7
  1. 2
      doc/changelog.dox
  2. 36
      src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp
  3. 12
      src/MagnumPlugins/TgaImporter/TgaImporter.cpp

2
doc/changelog.dox

@ -96,6 +96,8 @@ See also:
@ref Trade::AnySceneImporter "AnySceneImporter" plugins now correctly
recognize also uppercase file extensions (see
[mosra/magnum#312](https://github.com/mosra/magnum/pull/312))
@ref Trade::TgaImporter "TgaImporter" now properly reports an error message
when opening an empty file
@subsection changelog-latest-bugfixes Bug fixes

36
src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp

@ -41,7 +41,9 @@ namespace Magnum { namespace Trade { namespace Test { namespace {
struct TgaImporterTest: TestSuite::Tester {
explicit TgaImporterTest();
void openEmpty();
void openShort();
void paletted();
void compressed();
@ -52,14 +54,17 @@ struct TgaImporterTest: TestSuite::Tester {
void grayscaleBits8();
void grayscaleBits16();
void useTwice();
void openTwice();
void importTwice();
/* Explicitly forbid system-wide plugin dependencies */
PluginManager::Manager<AbstractImporter> _manager{"nonexistent"};
};
TgaImporterTest::TgaImporterTest() {
addTests({&TgaImporterTest::openShort,
addTests({&TgaImporterTest::openEmpty,
&TgaImporterTest::openShort,
&TgaImporterTest::paletted,
&TgaImporterTest::compressed,
@ -70,7 +75,8 @@ TgaImporterTest::TgaImporterTest() {
&TgaImporterTest::grayscaleBits8,
&TgaImporterTest::grayscaleBits16,
&TgaImporterTest::useTwice});
&TgaImporterTest::openTwice,
&TgaImporterTest::importTwice});
/* Load the plugin directly from the build tree. Otherwise it's static and
already loaded. */
@ -79,6 +85,17 @@ TgaImporterTest::TgaImporterTest() {
#endif
}
void TgaImporterTest::openEmpty() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
std::ostringstream out;
Error redirectError{&out};
char a{};
/* Explicitly checking non-null but empty view */
CORRADE_VERIFY(!importer->openData({&a, 0}));
CORRADE_COMPARE(out.str(), "Trade::TgaImporter::openData(): the file is empty\n");
}
void TgaImporterTest::openShort() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
const char data[] = { 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
@ -201,11 +218,20 @@ void TgaImporterTest::grayscaleBits16() {
CORRADE_COMPARE(debug.str(), "Trade::TgaImporter::image2D(): unsupported grayscale bits-per-pixel: 16\n");
}
void TgaImporterTest::useTwice() {
void TgaImporterTest::openTwice() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TGAIMPORTER_TEST_DIR, "file.tga")));
CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TGAIMPORTER_TEST_DIR, "file.tga")));
/* Shouldn't crash, leak or anything */
}
void TgaImporterTest::importTwice() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
CORRADE_VERIFY(importer->openFile(Utility::Directory::join(TGAIMPORTER_TEST_DIR, "file.tga")));
/* Verify that the file is rewinded for second use */
/* Verify that everything is working the same way on second use */
{
Containers::Optional<Trade::ImageData2D> image = importer->image2D(0);
CORRADE_VERIFY(image);

12
src/MagnumPlugins/TgaImporter/TgaImporter.cpp

@ -53,6 +53,18 @@ bool TgaImporter::doIsOpened() const { return _in; }
void TgaImporter::doClose() { _in = nullptr; }
void TgaImporter::doOpenData(const Containers::ArrayView<const char> data) {
/* Because here we're copying the data and using the _in to check if file
is opened, having them nullptr would mean openData() would fail without
any error message. It's not possible to do this check on the importer
side, because empty file is valid in some formats (OBJ or glTF). We also
can't do the full import here because then doImage2D() would need to
copy the imported data instead anyway. This way it'll also work nicely
with a future openMemory(). */
if(data.empty()) {
Error{} << "Trade::TgaImporter::openData(): the file is empty";
return;
}
_in = Containers::Array<char>{data.size()};
std::copy(data.begin(), data.end(), _in.begin());
}

Loading…
Cancel
Save