Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add get_texture_info_type to query TypeDesc of arbitrary texture metadata #3207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+100 −0
Open

Add get_texture_info_type to query TypeDesc of arbitrary texture metadata #3207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/include/OpenImageIO/imagecache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,13 @@ class OIIO_API ImageCache {
int subimage, int miplevel,
ustring dataname, TypeDesc datatype, void *data) = 0;


//Output the TypeDesc of a given attribute (if found). If not found
//we return unknown.
virtual bool get_image_info_type (ImageHandle *file, Perthread *thread_info,
int subimage, int miplevel,
ustring dataname, TypeDesc &datatype) = 0;

/// Copy the ImageSpec associated with the named image (the first
/// subimage & miplevel by default, or as set by `subimage` and
/// `miplevel`).
Expand Down
8 changes: 8 additions & 0 deletions src/include/OpenImageIO/texture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,14 @@ class OIIO_API TextureSystem {
Perthread *thread_info, int subimage,
ustring dataname, TypeDesc datatype, void *data) = 0;

// Find the TypeDesc of an "Anything else" metadata
virtual bool get_texture_info_type (ustring filename, int subimage,
ustring dataname, TypeDesc &datatype) = 0;

virtual bool get_texture_info_type (TextureHandle *texture_handle,
Perthread *thread_info, int subimage,
ustring dataname, TypeDesc &datatype) = 0;

/// Copy the ImageSpec associated with the named texture (the first
/// subimage by default, or as set by `subimage`).
///
Expand Down
42 changes: 42 additions & 0 deletions src/libtexture/imagecache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2822,6 +2822,48 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file,
#undef ATTR_DECODE
}

bool
ImageCacheImpl::get_image_info_type(ustring filename, int subimage,
int miplevel, ustring dataname,
TypeDesc& datatype)
{
ImageCachePerThreadInfo* thread_info = get_perthread_info();
ImageCacheFile* file = find_file(filename, thread_info, nullptr);
if (!file && dataname != s_exists) {
error("Invalid image file \"{}\"", filename);
return false;
}
return get_image_info_type(file, thread_info, subimage, miplevel, dataname,
datatype);
}

bool
ImageCacheImpl::get_image_info_type(ImageCacheFile* file,
ImageCachePerThreadInfo* thread_info,
int subimage, int miplevel,
ustring dataname, TypeDesc& datatype)
{
// Output the TypeDesc of a given attribute (if found). If not found
// we set it to UNKNOWN.

ImageCacheStatistics& stats(thread_info->m_stats);
Comment on lines +2848 to +2849
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See lines 2581ff, where get_image_info checks and gives appropriate errors and early exit for a null file, and lines 2672ff, where it handles non-existant subimage or miplevel. I think we need to do this here as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... that comment doesn't seem to have gotten associated with the right spot in your code. :-)
I was trying to point this out in get_image_info_type, line 2853 (of your changes), right before you dereference file and ask for the spec for this specific subimage and miplevel.

++stats.imageinfo_queries;
file = verify_file(file, thread_info, true);

const ImageSpec& spec(file->spec(subimage, miplevel));
ParamValue tmpparam;
const ParamValue* p = spec.find_attribute(dataname, tmpparam);

if (p) {
datatype = p->type();
return true;
}

// Does it make sense to set the type here, or should we just return
// false?
datatype.basetype = TypeDesc::UNKNOWN;
return false;
Comment on lines +2854 to +2865
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these lines can simply be

return spec.getattributetype(dataname);

and the return type for get_image_info_type can just be TypeDesc (and the datatype parameter to this method can go away). The returned TypeDesc will already be UNKNOWN if it's not found.

This matches the way we've already done getattributetype() in ImageSpec, and also in ParamList.

Also, the ImageSpec::getattributetype works even if it's one of the "built-in" fields, and isn't restricted to the "extra" attributes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take back that last sentence partially -- ImageSpec::getattributetype will only work for things that are in the ImageSpec (extra_args but also some other hard fields). But what this implementation of get_image_info_type won't be able to do is retrieve any of the other kinds of queries that get_image_info knows about that don't correspond to the ImageSpec, unless you are inclined to replicate much of that logic here.

}


bool
Expand Down
8 changes: 8 additions & 0 deletions src/libtexture/imagecache_pvt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,14 @@ class ImageCacheImpl final : public ImageCache {
int subimage, int miplevel, ustring dataname,
TypeDesc datatype, void* data);

virtual bool get_image_info_type(ustring filename, int subimage,
int miplevel, ustring dataname,
TypeDesc& datatype);
virtual bool get_image_info_type(ImageCacheFile* file,
Comment on lines +845 to +848
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just have these return the TypeDesc directly, since that's the way the equivalent works in other places we have a "get attribute type" kind of method. You don't need the bool return since there is already a special TypeDesc value (TypeUnknown) to indicate failure to find a matching item.

ImageCachePerThreadInfo* thread_info,
int subimage, int miplevel,
ustring dataname, TypeDesc& datatype);

/// Get the ImageSpec associated with the named image. If the file
/// is found and is an image format that can be read, store a copy
/// of its specification in spec and return true. Return false if
Expand Down
6 changes: 6 additions & 0 deletions src/libtexture/texture_pvt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,12 @@ class TextureSystemImpl final : public TextureSystem {
ustring dataname, TypeDesc datatype,
void* data);

virtual bool get_texture_info_type(ustring filename, int subimage,
ustring dataname, TypeDesc& datatype);
virtual bool get_texture_info_type(TextureHandle* texture_handle,
Perthread* thread_info, int subimage,
ustring dataname, TypeDesc& datatype);

virtual bool get_imagespec(ustring filename, int subimage, ImageSpec& spec);
virtual bool get_imagespec(TextureHandle* texture_handle,
Perthread* thread_info, int subimage,
Expand Down
29 changes: 29 additions & 0 deletions src/libtexture/texturesys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,35 @@ TextureSystemImpl::get_texture_info(TextureHandle* texture_handle,
}


bool
TextureSystemImpl::get_texture_info_type(ustring filename, int subimage,
ustring dataname, TypeDesc& datatype)
{
bool ok = m_imagecache->get_image_info_type(filename, subimage, 0, dataname,
datatype);
if (!ok) {
std::string err = m_imagecache->geterror();
if (!err.empty())
error("{}", err);
}
return ok;
}
bool
TextureSystemImpl::get_texture_info_type(TextureHandle* texture_handle,
Perthread* thread_info, int subimage,
ustring dataname, TypeDesc& datatype)
{
// This lets us ask for the datatype of a specific attribute
bool ok = m_imagecache->get_image_info_type(
(ImageCache::ImageHandle*)texture_handle,
(ImageCache::Perthread*)thread_info, subimage, 0, dataname, datatype);
if (!ok) {
std::string err = m_imagecache->geterror();
if (!err.empty())
error("{}", err);
}
return ok;
}

bool
TextureSystemImpl::get_imagespec(ustring filename, int subimage,
Expand Down