fix(ImageSpec): ImageSpec::get_string_attribute didn't correctly translate to string (#5161)

This method is advertised as being able to convert any type of the
attribute into a string, but actually, it would only *search* for
attributes that were entered as strings, so there was no conversion
happening.

Beef up testing of paramlist and imagespec to cover the holes that let
this mistake happen.

Fixed a couple typos in imageio.h comments as well.

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/include/OpenImageIO/imageio.h

@@ -721,7 +721,7 @@ class OIIO_API ImageSpec {
     /// Retrieve the named metadata attribute and return its value as an
     /// `int`. Any integer type will convert to `int` by truncation or
     /// expansion, string data will parsed into an `int` if its contents
-    /// consist of of the text representation of one integer. Floating point
+    /// consist of the text representation of one integer. Floating point
     /// data will not succeed in converting to an `int`. If no such metadata
     /// exists, or are of a type that cannot be converted, the `defaultval`
     /// will be returned.
@@ -730,7 +730,7 @@ class OIIO_API ImageSpec {
     /// Retrieve the named metadata attribute and return its value as a
     /// `float`. Any integer or floating point type will convert to `float`
     /// in the obvious way (like a C cast), and so will string metadata if
-    /// its contents consist of of the text representation of one floating
+    /// its contents consist of the text representation of one floating
     /// point value. If no such metadata exists, or are of a type that cannot
     /// be converted, the `defaultval` will be returned.
     float get_float_attribute (string_view name, float defaultval=0) const;

src/libOpenImageIO/formatspec.cpp

@@ -646,7 +646,7 @@ string_view
 ImageSpec::get_string_attribute(string_view name, string_view defaultval) const
 {
     ParamValue tmpparam;
-    const ParamValue* p = find_attribute(name, tmpparam, TypeDesc::STRING);
+    const ParamValue* p = find_attribute(name, tmpparam);
     return p ? p->get_ustring() : defaultval;
 }
 

src/libOpenImageIO/imagespec_test.cpp

@@ -259,6 +259,8 @@ test_get_attribute()
     spec.attribute("pi", float(M_PI));
     spec.attribute("bar", "barbarbar?");
     spec["baz"] = (unsigned int)14;
+    spec.attribute("foostring", "42");
+    spec.attribute("pistring", Strutil::to_string(float(M_PI)));
 
     OIIO_CHECK_EQUAL(spec.get_int_attribute("width"), 640);
     OIIO_CHECK_EQUAL(spec.get_int_attribute("height"), 480);
@@ -282,8 +284,13 @@ test_get_attribute()
     OIIO_CHECK_EQUAL(spec.get_int_attribute("foo"), 42);
     OIIO_CHECK_EQUAL(spec.get_int_attribute("pi", 4), 4);  // should fail int
     OIIO_CHECK_EQUAL(spec.get_float_attribute("pi"), float(M_PI));
+    OIIO_CHECK_EQUAL(spec.get_float_attribute("foo"), 42.0f);
+    OIIO_CHECK_EQUAL(spec.get_float_attribute("pistring"),
+                     spec.get_float_attribute("pi"));
     OIIO_CHECK_EQUAL(spec.get_int_attribute("bar"), 0);
-    OIIO_CHECK_EQUAL(spec.get_int_attribute("bar"), 0);
+    OIIO_CHECK_EQUAL(spec.get_int_attribute("foostring"), 42);
+    OIIO_CHECK_EQUAL(spec.get_string_attribute("foostring"), "42");
+    OIIO_CHECK_EQUAL(spec.get_string_attribute("foo"), "42");
     OIIO_CHECK_EQUAL(spec.get_string_attribute("bar"), "barbarbar?");
     OIIO_CHECK_ASSERT(spec.find_attribute("foo") != NULL);
     OIIO_CHECK_ASSERT(spec.find_attribute("Foo") != NULL);

src/libutil/paramlist_test.cpp

@@ -284,8 +284,11 @@ test_from_string()
 void
 populate_pvl(ParamValueList& pl)
 {
-    pl["foo"]  = 42;
-    pl["pi"]   = float(M_PI);
+    pl["foo"]       = 42;
+    pl["foostring"] = "42";
+    pl["pi"]        = float(M_PI);
+    pl["pistring"]  = Strutil::to_string(float(M_PI));
+
     pl["bar"]  = "barbarbar?";
     pl["bar2"] = std::string("barbarbar?");
     pl["bar3"] = ustring("barbarbar?");
@@ -307,12 +310,17 @@ test_paramlist()
     print("ParamValueList pl footprint is: {}\n", pvt::footprint(pl));
 
     OIIO_CHECK_EQUAL(pl.get_int("foo"), 42);
+    OIIO_CHECK_EQUAL(pl.get_int("foostring"), 42);
+    OIIO_CHECK_EQUAL(pl.get_int("foostring", -12, false, false), -12);
     OIIO_CHECK_EQUAL(pl.get_int("pi", 4), 4);  // should fail int
     OIIO_CHECK_EQUAL(pl.get_float("pi"), float(M_PI));
+    OIIO_CHECK_EQUAL(pl.get_float("pistring"), float(M_PI));
+    OIIO_CHECK_EQUAL(pl.get_float("pistring", 3.0f, false, false), 3.0f);
     OIIO_CHECK_EQUAL(pl.get_int("bar"), 0);
     OIIO_CHECK_EQUAL(pl.get_int("bar"), 0);
     OIIO_CHECK_EQUAL(pl.get_string("bar"), "barbarbar?");
     OIIO_CHECK_EQUAL(pl.get_string("foo"), "42");
+    OIIO_CHECK_EQUAL(pl.get_string("foo", "fail", false, false), "fail");
     OIIO_CHECK_ASSERT(pl.find("foo") != pl.cend());
     OIIO_CHECK_ASSERT(pl.find("Foo") == pl.cend());
     OIIO_CHECK_ASSERT(pl.find("Foo", TypeDesc::UNKNOWN, false) != pl.cend());