Skip to content

Commit 30142ed

Browse files
authored
fix(texture): fix texture overblur with st-blur parameters (#5071)
Fixes #5069 This PR fixes the texture blur overstimate reported in #5069 We provide the following changes: * Use the mathematically correct Pythagorean form to properly adjust the ellispse footprint with st-blur parameters. * Add a new legacy_texture_blur attribute in TextureSystem to opt into the fix without breaking existing renders. * Add a new --fix-texture-blur option in testtex to enable the fix. * Add a new texture-blurfix test. Note: This PR has been partially edited using the Claude coding assistant. --------- Signed-off-by: Pascal Lecocq <pascal.lecocq@gmail.com>
1 parent 583ace5 commit 30142ed

15 files changed

Lines changed: 81 additions & 9 deletions

src/cmake/testing.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ macro (oiio_add_all_tests)
186186
texture-uint8
187187
texture-width0blur
188188
texture-wrapfill
189+
texture-blurfix
189190
texture-fat texture-skinny
190191
texture-stats
191192
texture-threadtimes

src/libtexture/texture_pvt.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,13 @@ class TextureSystemImpl {
467467
int m_max_tile_channels; ///< narrow tile ID channel range when
468468
///< the file has more channels
469469
int m_stochastic;
470+
#if OIIO_VERSION_GREATER_EQUAL(3, 2, 0)
471+
bool m_legacy_texture_blur = false; ///< Use legacy texture blur behavior?
472+
// Opt OUT of the fix for 3.2 and beyond
473+
#else
474+
bool m_legacy_texture_blur = true; ///< Use legacy texture blur behavior?
475+
// Opt IN to get the fix for 3.1
476+
#endif
470477
static EightBitConverter<float> uchar2float;
471478

472479
enum StochasticStrategyBits {

src/libtexture/texturesys.cpp

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -746,10 +746,11 @@ void
746746
TextureSystemImpl::init()
747747
{
748748
m_Mw2c.makeIdentity();
749-
m_gray_to_rgb = false;
750-
m_flip_t = false;
751-
m_max_tile_channels = 6;
752-
m_stochastic = StochasticStrategy_None;
749+
m_gray_to_rgb = false;
750+
m_flip_t = false;
751+
m_max_tile_channels = 6;
752+
m_stochastic = StochasticStrategy_None;
753+
m_legacy_texture_blur = false;
753754
hq_filter.reset(Filter1D::create("b-spline", 4));
754755
m_statslevel = 0;
755756

@@ -907,6 +908,10 @@ TextureSystemImpl::attribute(string_view name, TypeDesc type, const void* val)
907908
unit_test_texture_blur = *(const float*)val;
908909
return true;
909910
}
911+
if (name == "legacy_texture_blur" && type == TypeInt) {
912+
m_legacy_texture_blur = (*(const int*)val != 0);
913+
return true;
914+
}
910915

911916
// Maybe it's meant for the cache?
912917
return m_imagecache->attribute(name, type, val);
@@ -926,6 +931,7 @@ TextureSystemImpl::getattributetype(string_view name) const
926931
{ "flip_t", TypeInt },
927932
{ "max_tile_channels", TypeInt },
928933
{ "stochastic", TypeInt },
934+
{ "legacy_texture_blur", TypeInt },
929935
};
930936
// clang-format on
931937

@@ -975,6 +981,10 @@ TextureSystemImpl::getattribute(string_view name, TypeDesc type,
975981
*(int*)val = m_stochastic;
976982
return true;
977983
}
984+
if (name == "legacy_texture_blur" && type == TypeInt) {
985+
*(int*)val = m_legacy_texture_blur;
986+
return true;
987+
}
978988

979989
// If not one of these, maybe it's an attribute meant for the image cache?
980990
return m_imagecache->getattribute(name, type, val);
@@ -1890,7 +1900,7 @@ adjust_width(float& dsdx, float& dtdx, float& dsdy, float& dtdy, float swidth,
18901900
// back to this and solve it better.
18911901
inline void
18921902
adjust_blur(float& majorlength, float& minorlength, float& theta, float sblur,
1893-
float tblur)
1903+
float tblur, bool legacy_textblur = false)
18941904
{
18951905
if (sblur + tblur != 0.0f /* avoid the work when blur is zero */) {
18961906
// Carefully add blur to the right derivative components in the
@@ -1901,8 +1911,22 @@ adjust_blur(float& majorlength, float& minorlength, float& theta, float sblur,
19011911
fast_sincos(theta, &sintheta, &costheta);
19021912
sintheta = fabsf(sintheta);
19031913
costheta = fabsf(costheta);
1904-
majorlength += sblur * costheta + tblur * sintheta;
1905-
minorlength += sblur * sintheta + tblur * costheta;
1914+
1915+
if (legacy_textblur) {
1916+
// The legacy blur code just adds the blur to the major and minor
1917+
// axes, which is a crude approximation that is wrong at some angles
1918+
// but is very simple and fast. I'm leaving it here as an option
1919+
// for now, in case the new code causes any unforeseen problems.
1920+
majorlength += sblur * costheta + tblur * sintheta;
1921+
minorlength += sblur * sintheta + tblur * costheta;
1922+
} else {
1923+
const float sintheta2 = sintheta * sintheta;
1924+
const float costheta2 = costheta * costheta;
1925+
const float sblur2 = sblur * sblur;
1926+
const float tblur2 = tblur * tblur;
1927+
majorlength += sqrtf(sblur2 * costheta2 + tblur2 * sintheta2);
1928+
minorlength += sqrtf(sblur2 * sintheta2 + tblur2 * costheta2);
1929+
}
19061930
#if 1
19071931
if (minorlength > majorlength) {
19081932
// Wildly uneven sblur and tblur values might swap which axis is
@@ -2273,7 +2297,8 @@ TextureSystemImpl::texture_lookup(TextureFile& texturefile,
22732297
// or bicubic texture probes, and therefore runtime!
22742298
ellipse_axes(dsdx, dtdx, dsdy, dtdy, majorlength, minorlength, theta);
22752299

2276-
adjust_blur(majorlength, minorlength, theta, options.sblur, options.tblur);
2300+
adjust_blur(majorlength, minorlength, theta, options.sblur, options.tblur,
2301+
m_legacy_texture_blur);
22772302

22782303
float aspect, trueaspect;
22792304
aspect = anisotropic_aspect(majorlength, minorlength, options, trueaspect);
@@ -3388,7 +3413,8 @@ TextureSystemImpl::visualize_ellipse(const std::string& name, float dsdx,
33883413
ellipse_axes(dsdx, dtdx, dsdy, dtdy, majorlength, minorlength, theta, ABCF);
33893414
std::cout << " ellipse major " << majorlength << ", minor " << minorlength
33903415
<< ", theta " << theta << "\n";
3391-
adjust_blur(majorlength, minorlength, theta, sblur, tblur);
3416+
adjust_blur(majorlength, minorlength, theta, sblur, tblur,
3417+
m_legacy_texture_blur);
33923418
std::cout << " post " << sblur << ' ' << tblur << " blur: major "
33933419
<< majorlength << ", minor " << minorlength << "\n\n";
33943420

src/testtex/testtex.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ static bool test_derivs = false;
8686
static bool test_statquery = false;
8787
static bool invalidate_before_iter = true;
8888
static bool close_before_iter = false;
89+
static bool legacy_texture_blur = false;
8990
static bool runstats = false;
9091
static bool udim_tests = false;
9192
static bool do_gettextureinfo = true;
@@ -200,6 +201,8 @@ getargs(int argc, const char* argv[])
200201
.help("Do not warp the image->texture mapping");
201202
ap.arg("--tube", &tube)
202203
.help("Make a tube projection");
204+
ap.arg("--legacy-texture-blur", &legacy_texture_blur)
205+
.help("Use mathematically correct texture blur instead of legacy overblur");
203206
ap.arg("--ctr", &test_construction)
204207
.help("Test TextureOpt construction time");
205208
ap.arg("--gettexels", &test_gettexels)
@@ -1831,6 +1834,10 @@ main(int argc, const char* argv[])
18311834
texsys->attribute("gray_to_rgb", gray_to_rgb);
18321835
texsys->attribute("flip_t", flip_t);
18331836
texsys->attribute("stochastic", stochastic);
1837+
if (legacy_texture_blur)
1838+
texsys->attribute("legacy_texture_blur", 1);
1839+
else
1840+
texsys->attribute("legacy_texture_blur", 0);
18341841
texcolortransform_id
18351842
= std::max(0, texsys->get_colortransform_id(ustring(texcolorspace),
18361843
ustring("scene_linear")));
113 KB
Binary file not shown.
177 KB
Binary file not shown.
147 KB
Binary file not shown.
105 KB
Binary file not shown.
51.7 KB
Binary file not shown.
113 KB
Binary file not shown.

0 commit comments

Comments
 (0)