Skip to content

Commit

Permalink
[usdImaging][hdPrman] Fix texture-derived cards geometry for oiio 2.3
Browse files Browse the repository at this point in the history
Oiio 2.2 made a change to how it handles worldtoscreen metadata that makes exr images generated with < 2.2 incompatible with our texture-based cards geometry implementation in usdImaging. For details on the oiio change, see AcademySoftwareFoundation/OpenImageIO#2609.

Regenerating the texture asset with oiio >= 2.2 is the "correct" fix. To aid with the transition, however, this change checks for and uses "worldToNDC" as a fallback if it fails to find "worldtoscreen". We issue a warning recommending regenerating the texture asset if "worldToNDC" is present instead of "worldtoscreen".

(Internal change: 2291623)
  • Loading branch information
tgvarik authored and pixar-oss committed Aug 18, 2023
1 parent 0d8e1e1 commit 2fb1556
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 33 deletions.
69 changes: 47 additions & 22 deletions pxr/usdImaging/usdImaging/drawModeAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ TF_DEFINE_PRIVATE_TOKENS(
(subsetMaterialZNeg)

(worldtoscreen)
(worldToNDC)

(displayRoughness)
(diffuseColor)
Expand Down Expand Up @@ -1399,30 +1400,54 @@ UsdImagingDrawModeAdapter::_GetMatrixFromImageMetadata(
// in row major order.
// - GfMatrix4f or GfMatrix4d
VtValue worldtoscreen;
if (img->GetMetadata(_tokens->worldtoscreen, &worldtoscreen)) {
if (worldtoscreen.IsHolding<std::vector<float>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<float>>(), mat);
}
else if (worldtoscreen.IsHolding<std::vector<double>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<double>>(), mat);
}
else if (worldtoscreen.IsHolding<GfMatrix4f>()) {
*mat = GfMatrix4d(worldtoscreen.UncheckedGet<GfMatrix4f>());
return true;
}
else if (worldtoscreen.IsHolding<GfMatrix4d>()) {
*mat = worldtoscreen.UncheckedGet<GfMatrix4d>();
return true;
}
else {
TF_WARN(
"worldtoscreen metadata holding unexpected type '%s'",
worldtoscreen.GetTypeName().c_str());

// XXX: OpenImageIO >= 2.2 no longer flips 'worldtoscreen' with 'worldToNDC'
// on read and write, so assets where 'worldtoscreen' was written with > 2.2
// have 'worldToNDC' actually in the metadata, and OIIO < 2.2 would read
// and return 'worldToNDC' from the file in response to a request for
// 'worldtoscreen'. OIIO >= 2.2 no longer does either, so 'worldtoscreen'
// gets written as 'worldtoscreen' and returned when asked for
// 'worldtoscreen'. Issues only arise when trying to read 'worldtoscreen'
// from an asset written with OIIO < 2.2, when the authoring program told
// OIIO to write it as 'worldtoscreen'. Old OIIO flipped it to 'worldToNDC'.
// So new OIIO needs to read 'worldToNDC' to retrieve it.
//
// See https://github.com/OpenImageIO/oiio/pull/2609
//
// OIIO's change is correct -- the two metadata matrices have different
// semantic meanings, and should not be conflated. Unfortunately, users will
// have to continue to conflate them for a while as assets transition into
// vfx2022 (which uses OIIO 2.3). So we will need to check for both.

if (!img->GetMetadata(_tokens->worldtoscreen, &worldtoscreen)) {
if (img->GetMetadata(_tokens->worldToNDC, &worldtoscreen)) {
TF_WARN("The texture asset '%s' referenced at <%s> may have been "
"authored by an earlier version of the VFX toolset. To silence this "
"warning, please regenerate the asset with the current toolset.",
file.c_str(), attr.GetPath().GetText());
} else {
TF_WARN("The texture asset '%s' referenced at <%s> lacks a "
"worldtoscreen matrix in metadata. Cards draw mode may not appear "
"as expected.", file.c_str(), attr.GetPath().GetText());
return false;
}
}


if (worldtoscreen.IsHolding<std::vector<float>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<float>>(), mat);
} else if (worldtoscreen.IsHolding<std::vector<double>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<double>>(), mat);
} else if (worldtoscreen.IsHolding<GfMatrix4f>()) {
*mat = GfMatrix4d(worldtoscreen.UncheckedGet<GfMatrix4f>());
return true;
} else if (worldtoscreen.IsHolding<GfMatrix4d>()) {
*mat = worldtoscreen.UncheckedGet<GfMatrix4d>();
return true;
}
TF_WARN("worldtoscreen metadata holding unexpected type '%s'",
worldtoscreen.GetTypeName().c_str());
return false;
}

Expand Down
47 changes: 36 additions & 11 deletions pxr/usdImaging/usdImaging/drawModeStandin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,7 @@ TF_DEFINE_PRIVATE_TOKENS(
_imageMetadataTokens,

(worldtoscreen)
(worldToNDC)
);

// Helper to produce, e.g., FooXPosBar.
Expand Down Expand Up @@ -1146,29 +1147,53 @@ GetWorldToScreenFromImageMetadata(
// in row major order.
// - GfMatrix4f or GfMatrix4d
VtValue worldtoscreen;

// XXX: OpenImageIO >= 2.2 no longer flips 'worldtoscreen' with 'worldToNDC'
// on read and write, so assets where 'worldtoscreen' was written with > 2.2
// have 'worldToNDC' actually in the metadata, and OIIO < 2.2 would read
// and return 'worldToNDC' from the file in response to a request for
// 'worldtoscreen'. OIIO >= 2.2 no longer does either, so 'worldtoscreen'
// gets written as 'worldtoscreen' and returned when asked for
// 'worldtoscreen'. Issues only arise when trying to read 'worldtoscreen'
// from an asset written with OIIO < 2.2, when the authoring program told
// OIIO to write it as 'worldtoscreen'. Old OIIO flipped it to 'worldToNDC'.
// So new OIIO needs to read 'worldToNDC' to retrieve it.
//
// See https://github.com/OpenImageIO/oiio/pull/2609
//
// OIIO's change is correct -- the two metadata matrices have different
// semantic meanings, and should not be conflated. Unfortunately, users will
// have to continue to conflate them for a while as assets transition into
// vfx2022 (which uses OIIO 2.3). So we will need to check for both.

if (!img->GetMetadata(_imageMetadataTokens->worldtoscreen, &worldtoscreen)) {
return false;
if (img->GetMetadata(_imageMetadataTokens->worldToNDC, &worldtoscreen)) {
TF_WARN("The texture asset '%s' may have been authored by an "
"earlier version of the VFX toolset. To silence this warning, "
"please regenerate the asset with the current toolset.",
file.c_str());
} else {
TF_WARN("The texture asset '%s' lacks a worldtoscreen matrix in "
"metadata. Cards draw mode may not appear as expected.",
file.c_str());
return false;
}
}

if (worldtoscreen.IsHolding<std::vector<float>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<float>>(), mat);
}
if (worldtoscreen.IsHolding<std::vector<double>>()) {
} else if (worldtoscreen.IsHolding<std::vector<double>>()) {
return _ConvertToMatrix(
worldtoscreen.UncheckedGet<std::vector<double>>(), mat);
}
if (worldtoscreen.IsHolding<GfMatrix4f>()) {
} else if (worldtoscreen.IsHolding<GfMatrix4f>()) {
*mat = GfMatrix4d(worldtoscreen.UncheckedGet<GfMatrix4f>());
return true;
}
if (worldtoscreen.IsHolding<GfMatrix4d>()) {
} else if (worldtoscreen.IsHolding<GfMatrix4d>()) {
*mat = worldtoscreen.UncheckedGet<GfMatrix4d>();
return true;
}

TF_WARN(
"worldtoscreen metadata holding unexpected type '%s'",
TF_WARN("worldtoscreen metadata holding unexpected type '%s'",
worldtoscreen.GetTypeName().c_str());
return false;
}
Expand Down

0 comments on commit 2fb1556

Please sign in to comment.