From 89af6ff82e6421f13843dae4686fd51cde4374b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 16 Oct 2023 16:08:29 +0200 Subject: [PATCH] TextureTools: fix DistanceField to not be slightly shifted. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original implementation wrongly assumed that the input and output pixel centers align, which would only be a case if the ratio of the input and output sizes would be odd. Which it in practice isn't, usually it's a 1024x1024 texture scaled down to 128x128 or something like that. The flipped test cases added in the previous commit now pass. According to the benchmark, the new code is very slightly slower (~815 µs vs ~805 before). The new code isn't really more complex than the old one, it just does slightly different work -- there are new corner case in the initial logic for marking the pixel inside or outside, on the other hand some corner cases that had to be handled in the previous case are no longer a thing. --- doc/changelog.dox | 3 + src/Magnum/TextureTools/DistanceField.cpp | 11 +- .../TextureTools/DistanceFieldShader.frag | 215 ++++++++++-------- .../TextureTools/Test/DistanceFieldGLTest.cpp | 42 +++- .../Test/DistanceFieldGLTestFiles/output.tga | Bin 4114 -> 2617 bytes 5 files changed, 179 insertions(+), 92 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 9e87734af..4b6b40451 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1077,6 +1077,9 @@ See also: propagate errors when delegating to @ref Text::AbstractFontConverter::exportFontToSingleData() / @ref Text::AbstractFontConverter::exportGlyphCacheToSingleData() +- @ref TextureTools::DistanceField no longer generates an output that's + slightly shifted, which also makes the output invariant to flips and + transpositions - @ref Trade::MeshData::attributeData(UnsignedInt) const was not correctly propagating attribute array size, causing array attributes to appear as non-array diff --git a/src/Magnum/TextureTools/DistanceField.cpp b/src/Magnum/TextureTools/DistanceField.cpp index 2ba73ac89..30eb88e7d 100644 --- a/src/Magnum/TextureTools/DistanceField.cpp +++ b/src/Magnum/TextureTools/DistanceField.cpp @@ -208,11 +208,20 @@ void DistanceField::operator()(GL::Texture2D& input, GL::Framebuffer& output, co CORRADE_ASSERT(output.checkStatus(GL::FramebufferTarget::Draw) == GL::Framebuffer::Status::Complete, "TextureTools::DistanceField: output texture format not framebuffer-drawable:" << output.checkStatus(GL::FramebufferTarget::Draw), ); + /* The shader assumes that the ratio between the output and input is a + multiple of 2, causing output pixel *centers* to be aligned with input + pixel *edges* */ + const Vector2i scaling = imageSize/rectangle.size(); + CORRADE_ASSERT(imageSize % rectangle.size() == Vector2i{0} && + scaling % 2 == Vector2i{0}, + "TextureTools::DistanceField: expected input and output size ratio to be a multiple of 2, got" << Debug::packed << imageSize << "and" << Debug::packed << rectangle.size(), ); + output .clear(GL::FramebufferClear::Color) .bind(); - _state->shader.setScaling(Vector2(imageSize)/Vector2(rectangle.size())) + _state->shader + .setScaling(Vector2{scaling}) .bindTexture(input); #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/TextureTools/DistanceFieldShader.frag b/src/Magnum/TextureTools/DistanceFieldShader.frag index b854c8853..db06c2969 100644 --- a/src/Magnum/TextureTools/DistanceFieldShader.frag +++ b/src/Magnum/TextureTools/DistanceFieldShader.frag @@ -47,11 +47,7 @@ layout(binding = 7) #endif uniform lowp sampler2D textureData; -#ifdef TEXELFETCH_USABLE -#ifndef GL_ES -layout(pixel_center_integer) in mediump vec4 gl_FragCoord; -#endif -#else +#ifndef TEXELFETCH_USABLE #ifdef EXPLICIT_UNIFORM_LOCATION layout(location = 1) #endif @@ -68,11 +64,11 @@ bool hasValue(const mediump ivec2 position, const mediump ivec2 offset) { } #else bool hasValue(const mediump vec2 position, const mediump ivec2 offset) { - return texture(textureData, position + (vec2(offset) + vec2(0.5))*imageSizeInverted).r > 0.5; + return texture(textureData, position + vec2(offset)*imageSizeInverted).r > 0.5; } #endif -mediump int findMinDistanceSquared(const mediump +mediump float findMinDistanceSquared(const mediump #ifdef TEXELFETCH_USABLE ivec2 #else @@ -80,57 +76,40 @@ mediump int findMinDistanceSquared(const mediump #endif position, const bool isInside) { - /* Initialize minimal distance to a value just outside the radius */ - mediump int minDistanceSquared = (RADIUS+1)*(RADIUS+1); + /* Initialize minimal distance to a value just outside the radius. As the + output coordinate is shifted by half a pixel from the input (see the + diagram in main() below), the X/Y distances are always a whole pixel + minus 0.5. Thus the max distance found in the below loop can be at most + `RADIUS - 0.5`, and the next nearest distance is thus `RADIUS + 0.5`. */ + mediump float minDistanceSquared = (float(RADIUS)+0.5)*(float(RADIUS)+0.5); /* Go in cocentric squares around the point */ - for(int i = 1; i <= RADIUS; ++i) { - /* First check the nearest points, since that's only four combinations. - If any of the four values is opposite of what is on `position`, we - found the nearest value. If the distance is not less than what's - found already, we don't even check the texture. - - i = 1 i = 2 i = 3 - - 0 - 0 - 0 - 1o3 1 o 3 1 o 3 - 2 - 2 - 2 - - Since everything else in the cocentric square and all others will be - further away, we can stop if we found something. */ - const mediump int centerDistanceSquared = i*i; - if(centerDistanceSquared >= minDistanceSquared) - return minDistanceSquared; - if(hasValue(position, ivec2(0, i)) != isInside || - hasValue(position, ivec2(-i, 0)) != isInside || - hasValue(position, ivec2(0, -i)) != isInside || - hasValue(position, ivec2(i, 0)) != isInside) { - return centerDistanceSquared; - } - - /* Now check for points further away, except for the corner points. - Every iteration checks all eight rotations/reflections at the same - distance. Again, if the distance is not less than what's found - already, we don't even check the texture -- but can't return, since - next iteration can still have closer values. - - i = 1 i = 2 i = 3 - (none) - 91 08 - 1 0 a f - 2 7 2 7 - o o o - 3 6 3 6 - 4 5 b e - c4 5d + for(int i = 2; i <= RADIUS; ++i) { + /* Actual distance from the center for this iteration is half a pixel + less (see the diagram in main() below) */ + const mediump float iF = float(i) - 0.5; + + /* Check for points further away from the initial 2x2 square, except + for corners. Every iteration checks all eight rotations/reflections + at the same distance. If the distance in given iteration is not less + than what's found already, we don't even check the texture -- but + can't return, since next iteration can still have closer values. + + i = 2 i = 3 + + 9108 + 10 a f + 2 7 2 7 + 3p 6 3 p 6 + 45 b e + c45d Once we find something, it's the closest value possible in this cycle, so we stop the cycle. But next iterations can still have - values that are closer, so can't return. */ + values that are closer, so can't return. + + Note that the integer `position` isn't at the center, which means + the offsets aren't symmetric. */ for(int j = 1; j < RADIUS; ++j) { /* Don't go further than current radius - 1 (i.e., excluding the corner). The loop needs to be compile-time bound otherwise some @@ -138,43 +117,47 @@ mediump int findMinDistanceSquared(const mediump this directly in the loop condition. */ if(j >= i) break; - const mediump int sideDistanceSquared = i*i + j*j; + /* Again, actual distance from the center is half a pixel less (see + the diagram in main() below) */ + const mediump float jF = float(j) - 0.5; + const mediump float sideDistanceSquared = iF*iF + jF*jF; if(sideDistanceSquared >= minDistanceSquared) break; - if(hasValue(position, ivec2( j, i)) != isInside || - hasValue(position, ivec2(-j, i)) != isInside || - hasValue(position, ivec2(-i, j)) != isInside || - hasValue(position, ivec2(-i, -j)) != isInside || - hasValue(position, ivec2(-j, -i)) != isInside || - hasValue(position, ivec2( j, -i)) != isInside || - hasValue(position, ivec2( i, -j)) != isInside || - hasValue(position, ivec2( i, j)) != isInside) { + if(hasValue(position, ivec2( j, i)) != isInside || /* 0, 8 */ + hasValue(position, ivec2(1-j, i)) != isInside || /* 1, 9 */ + hasValue(position, ivec2(1-i, j)) != isInside || /* 2, a */ + hasValue(position, ivec2(1-i, 1-j)) != isInside || /* 3, b */ + hasValue(position, ivec2(1-j, 1-i)) != isInside || /* 4, c */ + hasValue(position, ivec2( j, 1-i)) != isInside || /* 5, d */ + hasValue(position, ivec2( i, 1-j)) != isInside || /* 6, e */ + hasValue(position, ivec2( i, j)) != isInside) { /* 7, f */ minDistanceSquared = sideDistanceSquared; break; } } - /* Finally, check for the corners, which is again just four cases: + /* Then check for the corners, which is just four cases. Again the + integer `position` isn't at the center, which means the offsets + aren't symmetric. + + i = 2 i = 3 - i = 1 i = 2 i = 3 + 1 0 + 1 0 - 1 0 - 1 0 - 1 0 - o o o - 2 3 - 2 3 - 2 3 + p p + 2 3 + 2 3 If we find something, it's most probably not the nearest distance, since the following iterations can be much closer. */ - const mediump int cornerDistanceSquared = 2*i*i; + const mediump float cornerDistanceSquared = 2.0*iF*iF; if(cornerDistanceSquared >= minDistanceSquared) continue; - if(hasValue(position, ivec2( i, i)) != isInside || - hasValue(position, ivec2(-i, i)) != isInside || - hasValue(position, ivec2(-i, -i)) != isInside || - hasValue(position, ivec2( i, -i)) != isInside) { + if(hasValue(position, ivec2( i, i)) != isInside || + hasValue(position, ivec2(1-i, i)) != isInside || + hasValue(position, ivec2(1-i, 1-i)) != isInside || + hasValue(position, ivec2( i, 1-i)) != isInside) { minDistanceSquared = cornerDistanceSquared; } } @@ -183,24 +166,76 @@ mediump int findMinDistanceSquared(const mediump } void main() { + /* The -0.5 is to make the position aligned with the center of the input + pixel, and in the integer case to make the conversion not round up */ #ifdef TEXELFETCH_USABLE - #ifndef GL_ES - const mediump ivec2 position = ivec2(gl_FragCoord.xy*scaling); + const mediump ivec2 position = ivec2(gl_FragCoord.xy*scaling - vec2(0.5)); #else - const mediump ivec2 position = ivec2((gl_FragCoord.xy - vec2(0.5))*scaling); - #endif - #else - const mediump vec2 position = (gl_FragCoord.xy - vec2(0.5))*scaling*imageSizeInverted; + const mediump vec2 position = (gl_FragCoord.xy*scaling - vec2(0.5))*imageSizeInverted; #endif - /* If pixel at the position is inside (its value > 0.5), we are looking for - nearest pixel outside and the value will be positive (or > 0.5 after - normalization). If it is outside (its value < 0), we are looking for - nearest pixel inside and the value will be negative (or < 0.5). */ - const bool isInside = hasValue(position, ivec2(0, 0)); - const mediump float minDistance = sqrt(float(findMinDistanceSquared(position, isInside))); + /* +=======+===== + H | | | H | | Assuming the ratio of input and output sizes is a + H-+-+-+-H-+-+ multiple of 2 (in this diagram it's scaled 4x), the + H |k|l| H | center of the output pixel `o` is always between four + H-+-o-+-H-+ input pixels `i`, `j`, `k`, `l`.Then, depending on + H |i|j| H which of the four input pixels have a value > 0.5, the + H-+-+-+-H- following six cases can happen. Other combinations are + H | | | just variants of these. + +===== + + - In case A and F, the pixel is either inside or outside and we have to + look further around to know the distance to the edge. + - In case B and C, the pixel is exactly on the edge, i.e. distance is + 0, and we don't need to look further. + - In case D and E, the pixel is at a distance of 0.5 or (0.5, 0.5) from + the edge, and we don't need to look further. + + A B C D | E F + k---l k---l k l k l -k l k l + | | | / / | + | o | | o o | o o o + | | |/ / | + i---j i j i j i j i j i j + + The main complication is distinguishing cases C and D, in all other + cases it's simply a matter of counting the number of pixels that have a + value of > 0.5. + + Note that the integer `position` isn't at `o` (which is between pixels) + but aliases `i`. Which means the offsets aren't symmetric. */ + const bool i = hasValue(position, ivec2(0, 0)); + const bool j = hasValue(position, ivec2(1, 0)); + const bool k = hasValue(position, ivec2(0, 1)); + const bool l = hasValue(position, ivec2(1, 1)); + + mediump float minDistance; + bool isInside; + const int sum = int(i) + int(j) + int(k) + int(l); + /* Case B */ + if(sum == 3) { + isInside = false; + minDistance = 0.0; + /* Case C and D */ + } else if(sum == 2) { + isInside = false; + if((i && l) || (j && k)) + minDistance = 0.0; + else + minDistance = 0.5; + /* Case E */ + } else if(sum == 1) { + isInside = false; + /* sqrt(0.5*0.5 + 0.5*0.5) */ + minDistance = 0.7071067811865475; + /* Case A and F */ + } else { + isInside = sum == 4; + minDistance = sqrt(float(findMinDistanceSquared(position, isInside))); + } - /* Final signed distance, normalized from [-radius-1, radius+1] to [0, 1] */ + /* Final signed distance, normalized from [-radius + 0.5, radius + 0.5] to + [0, 1] */ const highp float halfSign = isInside ? 0.5 : -0.5; - value = halfSign*minDistance/float(RADIUS + 1) + 0.5; + value = halfSign*minDistance/(float(RADIUS) + 0.5) + 0.5; } diff --git a/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp b/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp index b70498512..ef7764629 100644 --- a/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp +++ b/src/Magnum/TextureTools/Test/DistanceFieldGLTest.cpp @@ -70,6 +70,7 @@ struct DistanceFieldGLTest: GL::OpenGLTester { void run(); void formatNotDrawable(); + void sizeRatioNotMultipleOfTwo(); #ifndef MAGNUM_TARGET_WEBGL void benchmark(); @@ -100,7 +101,8 @@ DistanceFieldGLTest::DistanceFieldGLTest() { addInstancedTests({&DistanceFieldGLTest::run}, Containers::arraySize(RunData)); - addTests({&DistanceFieldGLTest::formatNotDrawable}); + addTests({&DistanceFieldGLTest::formatNotDrawable, + &DistanceFieldGLTest::sizeRatioNotMultipleOfTwo}); #ifndef MAGNUM_TARGET_WEBGL addBenchmarks({&DistanceFieldGLTest::benchmark}, 5, BenchmarkType::GpuTime); @@ -313,6 +315,44 @@ void DistanceFieldGLTest::formatNotDrawable() { #endif } +void DistanceFieldGLTest::sizeRatioNotMultipleOfTwo() { + 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), {23*14, 23*14}); + + GL::Texture2D output; + output.setStorage(1, GL::textureFormat(PixelFormat::RGBA8Unorm), {23, 23}); + + GL::Texture2D outputInvalid; + outputInvalid.setStorage(1, GL::textureFormat(PixelFormat::RGBA8Unorm), {23*2, 23*2}); + + DistanceField distanceField{4}; + + /* This should be fine */ + distanceField(input, output, {{}, Vector2i{23}} + #ifdef MAGNUM_TARGET_GLES + , Vector2i{23*14} + #endif + ); + + std::ostringstream out; + Error redirectError{&out}; + distanceField(input, output, {{}, Vector2i{23*2}} + #ifdef MAGNUM_TARGET_GLES + , Vector2i{23*14} + #endif + ); + CORRADE_COMPARE(out.str(), "TextureTools::DistanceField: expected input and output size ratio to be a multiple of 2, got {322, 322} and {46, 46}\n"); +} + #ifndef MAGNUM_TARGET_WEBGL void DistanceFieldGLTest::benchmark() { #ifdef MAGNUM_TARGET_GLES diff --git a/src/Magnum/TextureTools/Test/DistanceFieldGLTestFiles/output.tga b/src/Magnum/TextureTools/Test/DistanceFieldGLTestFiles/output.tga index f6982a590b4b3d72e3020d92e783652587ffbb30..41e020ccd244df63d561413d479db5fb4c5eac4b 100644 GIT binary patch literal 2617 zcmaKu-%lD@6vwBn9V|9(HpbnxwcTvYCc1s{p{Z@RYHLeZ4gH~%pr%%SlnMwFCQQgY zFk!UbK*+G5{3yS|k1&u?QDm_$F16k5)@)Ogeb{~4>>smtsJa7K;O)%)+h9`n zYhFa2Bd5`}i#@%4-1XCay*(G((C09Qws!Vh9*~S=N6`_Nn}c0Rxi>iWs<(`4sj)Z_WY&mBa=F7mK0falOxwJoj-dN zADz^bSw=+aCr1zC&HQ>k&akm&M)>5+(UTa+HLa#&p#bea9SlVi?COuXGqI}0;{O-t z7RqQL6AieX4jRx-5#tR+(`&^7y?L_uvb0zh=c>_m0b2g>x{!%@927~=_!+`V+8ypd zJe#JIZETjyTd}EK!~M*Ft!ki!danmP>yal`C}^ZZ;UL zG-$Wr7J{@<{1(gA^mjtKkn#ayc0xKR8K0T6Qh*{YMuIe|o@(_bGfvQ?n5m|_h4gyd zLt!)HclxDMIs&+T9v2{Sy+$EdOin+WH?`Pm;2r^7T7h{R9!vX&9?oDC6G_EGOD>yf z`krJM25dCdV)s@bEu??uLQa_X$Tf-lsRj5_`Sonr#oOrnJ2Ev6=Dp$<)v)lbE3j66 zw_;dlDEo3N6Lb<<#qia;axLKq&`B{{O$$HQ5nZuwbakhY_5)n=NOG_pw^?JAdRQ@h zLplY$AIp_DlU~ZK9J|#oeQY2ZxLX6b#KCtoJidSH-n0oyn}eQoQ&6#in-89tC{KL7 z)}+Fd%kX2_am|FZl6HhE@cQegq&IGlt8lQC+-wN{#0H(vWfKo4W(mh~G?hJ&pBD`H z*|zZN-Inb4M6r3QlzbuxBbH8%87K&V$O zTveDKc413chOQ*chFKkEv69eX_vyj5`_m@M9m~C{rK=R%-+NWy63~{gEC?Jk>Wrpm z&|yPkD%|c(Zoa97KNHxyytk8Ip}B9fiLl?pIN=Uf0#nQGT&WlqD)zRPt~z=*pN@pW z@eG%aM}j^t+<<_Fy*2{n=MB5p5awHg)8&b-7S>lXsYG-M;HvT49~FzNl-@Uli>x0I zB!9@O(pG`5aMP0qHwO;|+iwW-?F6^*hWP#cyPehOL4_X*))?MMcqt4A{`AK7?%S8p zVU>$S@<;t#=J!zM@IJkpzq{l4R2n z1~#>aL$ImoNmrOF{ZR*hDfrM^Y`{TiHFFjlV7#>df*TOJDQQ3Krx@?B@`pP5a{*mZ z8#mIkr!c~1r@;lvIyW;8TOi+1eW}8mKkM)>1UOWQaoC|bjkBhC{DN7hl#h)ork~-k zp>o^r{;q@13E=WZIuh`H*g5Q8QHw`b& zbNr1#+FgOALeSuJ>e*LI3E57u<{GR literal 4114 zcmeH~|4-9+7{})+g5LsR;+GgiAjlbv5dy|2!p0#YFkv9`790+2WP@fb`;s+Fe`}UC zc1u~R+eld_+tN7(Wljc61&<(S;^lJjxBGMM(?N%xVePM%-1~>E{j9I2PoK~8rIX1H z%LFN83R#J8^8dH2sJKL`ijVvduaeT!XC>>5{M6C!Ec|q`qO$gKy;RjzUyvXF*1*Yg zH4Tj|H>9fh`jtx+$My}BRMcF%sp#yIs?N5ShN|+Syn)inhMT{rhK8kTuvgh!fBuBP z7oDzcRH(G$kW}e4J+0TO$`0)rC_a0+rE_R}+9p--=zy}ZMlRGs$=UiFUBi&gEm4?x zOx@8`D;KgL&j%!lI+$k##lj()rkQY5SlIuL6yjHTaj}r!M-qgG^u3r3vrC!W>fT4X zFu${t<)T3n$4~@8QOr&Fg0B)umVKS%Qg2t^ZG05tH{Qg8UJS9oQwA6|+nv~Rie>_V z**PW>i!X2~5&lyqPLr5@3L1T^)s2o%z*f{fL*Z`1OH%$I%|xPw{ohPQD3@dEiDp3c zKsydxFibmK9@L(DnCBjndchQo-+F@fqUJHpePy2(ns%V5!vZ@n`-BlTTWq`$JW!Bd zVFPZ5aYUu)dN^isx;-8@h=m!}kLdJIpBX?v+=Y4IK4yi6yIOm-28WBHX+MdhR!GyQ zQfnvd2!ar^1@F%>Bx=^HJCp;XFzTV>iC73+KR(piuF}CMK@c z0k&t4x^D^b6y-mEcoSzp(qgE?YKB4C*rq}D-BCTBLI^ssF3NAMrMM*Dz(D1oPQ+%0 zA+1W0OJ_2)_+FG>OGX$b5|4%H07VkG3){`U!7)Camp8?D&_7<1Bt^%$1RG(14FWNa z^xfgZ=b|F%EiwLGa?XpQ{H5zFsig(b4eS%Y-WS4``6R}J{zD*Uj0|%d+aGhu0=mK7 zeE2?#@fp7Vz#!AO?8o0XxG_b_0OU_*gi1&tR~}>h3~eO$tOc9wdTnr z=*9Eg#vh`5DjKA`{PYCLZ!j1j-3aLX5NyZ1bUd^DRg}+i?EG9HXTr%B5NtHUCM$w@ zgVEH+m%l~%Pw!TilQ|P4;l?m@cZ}mR!6=vA`6_<@t*x!iEN>#0{65l4@RK(XV!15% z38--YNavMW0tFOi=4g73nP-#v^@4mviVmJ%lr$YQ|1hcg*ZeF#Q`g)%sE5r`HL2@Y zG**`j@1IBH)z{m4HKSuvrF+*~c);r&^5>cW*4WzT?Gs`|Uzn(EH&|AROxztqs& zs_c*|#jU2FE6W7;fg8sws_L&bHc3_E)w;^^;=KFd;8^+js+wA^od0D>_jo bm#p&A