From 6357ed52c066847eb3b995c10a898f6d8ba25c22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 16 Oct 2023 15:57:17 +0200 Subject: [PATCH] TextureTools: assert that DistanceField output is framebuffer-drawable. If it's not, it's a programmer error (i.e., don't use Luminance or packed formats, won't work), and since there's no way for the API to report a failure in a programmatic way, this was causing hard-to-track errors. --- doc/changelog.dox | 3 ++ src/Magnum/TextureTools/DistanceField.cpp | 8 +--- src/Magnum/TextureTools/DistanceField.h | 18 ++++---- src/Magnum/TextureTools/Test/CMakeLists.txt | 7 +++- .../TextureTools/Test/DistanceFieldGLTest.cpp | 41 +++++++++++++++++++ 5 files changed, 61 insertions(+), 16 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 41a99da30..9e87734af 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1654,6 +1654,9 @@ See also: - The @ref Text::AbstractGlyphCache::image() query now returns a @ref MutableImageView3D instead of an @ref Image2D in order to support incremental population and texture arrays +- @ref TextureTools::DistanceField now asserts that the output texture has a + framebuffer-drawable format. Before it only printed an error message and + silently did nothing, causing a hard-to-track error. - The @ref Trade::AnimationTrackTarget enum was extended from 8 to 16 bits to provide more room for custom targets, consistently with @ref Trade::MeshAttribute. diff --git a/src/Magnum/TextureTools/DistanceField.cpp b/src/Magnum/TextureTools/DistanceField.cpp index b003cbff7..2ba73ac89 100644 --- a/src/Magnum/TextureTools/DistanceField.cpp +++ b/src/Magnum/TextureTools/DistanceField.cpp @@ -205,12 +205,8 @@ void DistanceField::operator()(GL::Texture2D& input, GL::Framebuffer& output, co Vector2i imageSize = input.imageSize(0); #endif - const GL::Framebuffer::Status status = output.checkStatus(GL::FramebufferTarget::Draw); - if(status != GL::Framebuffer::Status::Complete) { - Error() << "TextureTools::DistanceField: cannot render to given output texture, unexpected framebuffer status" - << status; - return; - } + CORRADE_ASSERT(output.checkStatus(GL::FramebufferTarget::Draw) == GL::Framebuffer::Status::Complete, + "TextureTools::DistanceField: output texture format not framebuffer-drawable:" << output.checkStatus(GL::FramebufferTarget::Draw), ); output .clear(GL::FramebufferClear::Color) diff --git a/src/Magnum/TextureTools/DistanceField.h b/src/Magnum/TextureTools/DistanceField.h index 99d077658..df825dd8d 100644 --- a/src/Magnum/TextureTools/DistanceField.h +++ b/src/Magnum/TextureTools/DistanceField.h @@ -81,15 +81,6 @@ http://www.valvesoftware.com/publications/2007/SIGGRAPH2007_AlphaTestedMagnifica @attention This is a GPU-only implementation, so it expects an active GL context. -@note If internal format of @p output texture is not renderable, this function - prints a message to error output and does nothing. On desktop OpenGL and - OpenGL ES 3.0 it's common to render to @ref GL::TextureFormat::R8. On - OpenGL ES 2.0 you can use @ref GL::TextureFormat::Red if - @gl_extension{EXT,texture_rg} is available; if not, the smallest but still - inefficient supported format is in most cases @ref GL::TextureFormat::RGB, - rendering to @ref GL::TextureFormat::Luminance is not supported in most - cases. - @note This function is available only if Magnum is compiled with @ref MAGNUM_TARGET_GL enabled (done by default). See @ref building-features for more information. @@ -136,6 +127,15 @@ class MAGNUM_TEXTURETOOLS_EXPORT DistanceField { * on desktop GL the information is gathered automatically using * @ref GL::Texture2D::imageSize(). * @m_since_latest + * + * The @p output texture is expected to have a framebuffer-drawable + * @ref GL::TextureFormat. On desktop OpenGL and + * OpenGL ES 3.0 it's common to render to @ref GL::TextureFormat::R8. + * On OpenGL ES 2.0 you can use @ref GL::TextureFormat::Red if + * @gl_extension{EXT,texture_rg} is available; if not, the smallest but + * still inefficient supported format is in most cases + * @ref GL::TextureFormat::RGB. The @ref GL::TextureFormat::Luminance + * format usually isn't renderable. */ void operator()(GL::Texture2D& input, GL::Framebuffer& output, const Range2Di& rectangle, const Vector2i& imageSize #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/TextureTools/Test/CMakeLists.txt b/src/Magnum/TextureTools/Test/CMakeLists.txt index eb6cafd4e..dc669a981 100644 --- a/src/Magnum/TextureTools/Test/CMakeLists.txt +++ b/src/Magnum/TextureTools/Test/CMakeLists.txt @@ -99,7 +99,12 @@ if(MAGNUM_BUILD_GL_TESTS) list(APPEND TextureToolsDistanceFieldGLTest_SRCS DistanceFieldGLTestFiles) endif() corrade_add_test(TextureToolsDistanceFieldGLTest ${TextureToolsDistanceFieldGLTest_SRCS} - LIBRARIES MagnumTextureTools MagnumGL MagnumTrade MagnumDebugTools MagnumOpenGLTester + LIBRARIES + MagnumDebugTools + MagnumGL + MagnumOpenGLTester + MagnumTextureToolsTestLib + MagnumTrade FILES DistanceFieldGLTestFiles/input.tga DistanceFieldGLTestFiles/output.tga) diff --git a/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp b/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp index 4ce57741b..04ce04f20 100644 --- a/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp +++ b/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp @@ -67,6 +67,9 @@ struct DistanceFieldGLTest: GL::OpenGLTester { /* This tests the GL::Texture overload, which itself calls into the GL::Framebuffer overload so both are covered */ void run(); + + void formatNotDrawable(); + #ifndef MAGNUM_TARGET_WEBGL void benchmark(); #endif @@ -93,6 +96,8 @@ DistanceFieldGLTest::DistanceFieldGLTest() { addInstancedTests({&DistanceFieldGLTest::run}, Containers::arraySize(RunData)); + addTests({&DistanceFieldGLTest::formatNotDrawable}); + #ifndef MAGNUM_TARGET_WEBGL addBenchmarks({&DistanceFieldGLTest::benchmark}, 5, BenchmarkType::GpuTime); #endif @@ -255,6 +260,42 @@ void DistanceFieldGLTest::run() { (DebugTools::CompareImageToFile{_manager, 1.0f, 0.178f})); } +void DistanceFieldGLTest::formatNotDrawable() { + CORRADE_SKIP_IF_NO_ASSERT(); + + #ifndef MAGNUM_TARGET_GLES + if(!GL::Context::current().isExtensionSupported()) + CORRADE_SKIP(GL::Extensions::EXT::texture_shared_exponent::string() << "not supported, can't test"); + #endif + + GL::Texture2D input; + input.setMinificationFilter(GL::SamplerFilter::Nearest, GL::SamplerMipmap::Base) + .setMagnificationFilter(GL::SamplerFilter::Nearest) + .setStorage(1, GL::textureFormat(PixelFormat::R8Unorm), {64, 64}); + + GL::Texture2D output; + #ifdef MAGNUM_TARGET_GLES2 + output.setImage(0, GL::TextureFormat::Luminance, ImageView2D{GL::PixelFormat::Luminance, GL::PixelType::UnsignedByte, Vector2i{4}}); + #else + output.setImage(0, GL::TextureFormat::RGB9E5, ImageView2D{GL::PixelFormat::RGB, GL::PixelType::UnsignedInt5999Rev, Vector2i{4}}); + #endif + + DistanceField distanceField{4}; + + std::ostringstream out; + Error redirectError{&out}; + distanceField(input, output, {{}, Vector2i{4}} + #ifdef MAGNUM_TARGET_GLES + , Vector2i{64} + #endif + ); + #ifndef MAGNUM_TARGET_GLES + CORRADE_COMPARE(out.str(), "TextureTools::DistanceField: output texture format not framebuffer-drawable: GL::Framebuffer::Status::Unsupported\n"); + #else + CORRADE_COMPARE(out.str(), "TextureTools::DistanceField: output texture format not framebuffer-drawable: GL::Framebuffer::Status::IncompleteAttachment\n"); + #endif +} + #ifndef MAGNUM_TARGET_WEBGL void DistanceFieldGLTest::benchmark() { #ifdef MAGNUM_TARGET_GLES