From f2352235777f673d259a0512e68befc1ce1d6e4b Mon Sep 17 00:00:00 2001 From: Basile Fraboni Date: Mon, 7 Oct 2024 11:24:54 -0700 Subject: [PATCH 1/5] first draft for removing nativespec from ImageBuf Signed-off-by: Basile Fraboni --- src/include/OpenImageIO/imagebuf.h | 18 +++-- src/iv/ivimage.cpp | 2 +- src/libOpenImageIO/imagebuf.cpp | 117 ++++++++++++++++++++------- src/libOpenImageIO/imagebuf_test.cpp | 1 - src/libOpenImageIO/maketexture.cpp | 4 +- src/libtexture/imagecache.cpp | 4 + src/oiiotool/expressions.cpp | 2 +- src/oiiotool/imagerec.cpp | 17 ++-- src/oiiotool/oiiotool.cpp | 19 ++--- 9 files changed, 130 insertions(+), 54 deletions(-) diff --git a/src/include/OpenImageIO/imagebuf.h b/src/include/OpenImageIO/imagebuf.h index e209dba08e..2db3d59826 100644 --- a/src/include/OpenImageIO/imagebuf.h +++ b/src/include/OpenImageIO/imagebuf.h @@ -1064,11 +1064,10 @@ class OIIO_API ImageBuf { /// knowledge of its pixel memory layout. USE WITH EXTREME CAUTION. ImageSpec& specmod(); - /// Return a read-only (const) reference to the "native" image spec - /// (that describes the file, which may be slightly different than - /// the spec of the ImageBuf, particularly if the IB is backed by an - /// ImageCache that is imposing some particular data format or tile - /// size). + /// Return the "native" channel format that describes the file, + /// which may be slightly different than the spec of the ImageBuf, + /// particularly if the IB is backed by an ImageCache that is imposing + /// some particular data format or tile size. /// /// This may differ from `spec()` --- for example, if a data format /// conversion was requested, if the buffer is backed by an ImageCache @@ -1076,7 +1075,14 @@ class OIIO_API ImageBuf { /// that of the file, or if the file had differing per-channel data /// formats (ImageBuf must contain a single data format for all /// channels). - const ImageSpec& nativespec() const; + TypeDesc file_format() const; + /// Same as the above for channel specific formats. + std::vector file_channelformats() const; + + /// DEPRECATED old API. This will now return spec() by default. + /// We recommend switching to the new API. + /// TODO: uncomment for backwards compatibility once everything else is in place. + // const ImageSpec& nativespec() const; /// Does this ImageBuf have an associated thumbnail? bool has_thumbnail() const; diff --git a/src/iv/ivimage.cpp b/src/iv/ivimage.cpp index 6bf1f79c3e..79f126ec41 100644 --- a/src/iv/ivimage.cpp +++ b/src/iv/ivimage.cpp @@ -119,7 +119,7 @@ std::string IvImage::longinfo() const { if (m_longinfo.empty()) { - const ImageSpec& m_spec(nativespec()); + const ImageSpec& m_spec(spec()); m_longinfo += ""; // m_longinfo += html_table_row (Strutil::fmt::format("{}", m_name.c_str()).c_str(), // std::string()); diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 257a11113e..386f2a7c79 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -269,11 +269,39 @@ class ImageBufImpl { validate_spec(); return m_spec; } - const ImageSpec& nativespec() const + + TypeDesc file_format() const { - validate_spec(); - return m_nativespec; + //! TODO: remove m_nativespec and store only the "native format" + return m_nativespec.format; + } + + TypeDesc& file_format() + { + //! TODO: remove m_nativespec and store only the "native format" + return m_nativespec.format; } + + std::vector file_channelformats() const + { + //! TODO: remove m_nativespec and store only the "native channels format" + return m_nativespec.channelformats; + } + + std::vector& file_channelformats() + { + //! TODO: remove m_nativespec and store only the "native channels format" + return m_nativespec.channelformats; + } + + /// DEPRECTATED + /// TODO: uncomment for backwards compatibility once everything else is in place. + // const ImageSpec& nativespec() const + // { + // validate_spec(); + // return m_nativespec; + // } + ImageSpec& specmod() { validate_spec(); @@ -338,7 +366,11 @@ class ImageBufImpl { int m_nmiplevels; ///< # of MIP levels in the current subimage mutable int m_threads; ///< thread policy for this image ImageSpec m_spec; ///< Describes the image (size, etc) + //! TODO: remove m_nativespec and store only "native channels formats" ImageSpec m_nativespec; ///< Describes the true native image + // TypeDesc format; ///< Data format of the channels as in the associated file. + // std:vector channelformats; ///< Optional per-channel data formats as in the associated file. + // ///< This will be empty if all native channels have the same format. std::unique_ptr m_pixels; ///< Pixel data, if local and we own it char* m_localpixels; ///< Pointer to local pixels span m_bufspan; ///< Bounded buffer for local pixels @@ -384,7 +416,7 @@ class ImageBufImpl { return m_write_format[channel]; if (m_write_format.size() == 1) return m_write_format[0]; - return m_nativespec.format; + return file_format(); } void lock() const { m_mutex.lock(); } @@ -784,7 +816,7 @@ void ImageBufImpl::clear() { if (m_imagecache && !m_name.empty() - && (storage() == ImageBuf::IMAGECACHE || m_rioproxy)) { + && (cachedpixels() || m_rioproxy)) { // If we were backed by an ImageCache, invalidate any IC entries we // might have made. Also do so if we were using an IOProxy, because // the proxy may not survive long after the ImageBuf is destroyed. @@ -914,7 +946,10 @@ ImageBufImpl::reset(string_view filename, const ImageSpec& spec, m_current_miplevel = 0; if (buforigin || bufspan.size()) { m_spec = spec; + //! TODO: ultimattely remove m_nativespec and only update + //! m_file_format and m_file_channelformats m_nativespec = nativespec ? *nativespec : spec; + m_channel_stride = stride_t(spec.format.size()); m_xstride = xstride; m_ystride = ystride; @@ -1077,7 +1112,9 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel, TypeString, &fmt); m_fileformat = ustring(fmt); + // Read native image spec from the ImageCache m_imagecache->get_imagespec(m_name, m_nativespec, subimage); + // Copy and override the spec with cache internal data size m_spec = m_nativespec; m_imagecache->get_cache_dimensions(m_name, m_spec, subimage, miplevel); @@ -1221,9 +1258,12 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_current_subimage = subimage; m_current_miplevel = miplevel; - if (chend < 0 || chend > nativespec().nchannels) - chend = nativespec().nchannels; - bool use_channel_subset = (chbegin != 0 || chend != nativespec().nchannels); + + //! TODO: should we pass the number of channels of nativespec() + // or consider chbeing/chend are correctly set ? + // if (chend < 0 || chend > nativespec().nchannels) + // chend = nativespec().nchannels; + // bool use_channel_subset = (chbegin != 0 || chend != nativespec().nchannels); if (m_spec.deep) { Timer timer; @@ -1278,7 +1318,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, } else { // No cache should take the "forced read now" route. force = true; - m_cachedpixeltype = m_nativespec.format; + m_cachedpixeltype = file_format(); } if (use_channel_subset) { @@ -1286,25 +1326,28 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, force = true; m_spec.nchannels = chend - chbegin; m_spec.channelnames.resize(m_spec.nchannels); + /// TODO: what to do here ? if we remove m_nativespec, we won't have the channelnames anymore.. for (int c = 0; c < m_spec.nchannels; ++c) m_spec.channelnames[c] = m_nativespec.channelnames[c + chbegin]; - if (m_nativespec.channelformats.size()) { + const std::vector& cformats = file_channelformats(); + if (cformats.size()) { m_spec.channelformats.resize(m_spec.nchannels); for (int c = 0; c < m_spec.nchannels; ++c) m_spec.channelformats[c] - = m_nativespec.channelformats[c + chbegin]; + = cformats[c + chbegin]; } } if (convert != TypeDesc::UNKNOWN) m_spec.format = convert; else - m_spec.format = m_nativespec.format; + m_spec.format = file_format(); realloc(); // N.B. realloc sets m_bufspan // If forcing a full read, make sure the spec reflects the nativespec's // tile sizes, rather than that imposed by the ImageCache. + /// TODO: what to do here again ? getting rid of nativespec will lose that info.. m_spec.tile_width = m_nativespec.tile_width; m_spec.tile_height = m_nativespec.tile_height; m_spec.tile_depth = m_nativespec.tile_depth; @@ -1312,7 +1355,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, if (force || !m_imagecache || m_rioproxy || (convert != TypeDesc::UNKNOWN && convert != m_cachedpixeltype && convert.size() >= m_cachedpixeltype.size() - && convert.size() >= m_nativespec.format.size())) { + && convert.size() >= file_format().size())) { // A specific conversion type was requested which is not the cached // type and whose bit depth is as much or more than the cached type. // Bypass the cache and read directly so that there is no possible @@ -1660,8 +1703,8 @@ ImageBuf::write(string_view _filename, TypeDesc dtype, string_view _fileformat, } else { // No override on the ImageBuf, nor on this call to write(), so // we just use what is known from the imagespec. - newspec.set_format(nativespec().format); - newspec.channelformats = nativespec().channelformats; + newspec.set_format(file_format()); + newspec.channelformats = file_channelformats(); } if (m_impl->m_wioproxy) { @@ -1712,13 +1755,16 @@ ImageBufImpl::copy_metadata(const ImageBufImpl& src) m_spec.full_width = srcspec.full_width; m_spec.full_height = srcspec.full_height; m_spec.full_depth = srcspec.full_depth; - if (src.storage() == ImageBuf::IMAGECACHE) { - // If we're copying metadata from a cached image, be sure to - // get the file's tile size, not the cache's tile size. - m_spec.tile_width = src.nativespec().tile_width; - m_spec.tile_height = src.nativespec().tile_height; - m_spec.tile_depth = src.nativespec().tile_depth; - } else { + //! TODO: now that nativespec() is deprecated what should we do here ? + // if (src.cachedpixels()) { + // // If we're copying metadata from a cached image, be sure to + // // get the file's tile size, not the cache's tile size. + // m_spec.tile_width = src.nativespec().tile_width; + // m_spec.tile_height = src.nativespec().tile_height; + // m_spec.tile_depth = src.nativespec().tile_depth; + // } + // else + { m_spec.tile_width = srcspec.tile_width; m_spec.tile_height = srcspec.tile_height; m_spec.tile_depth = srcspec.tile_depth; @@ -1752,14 +1798,31 @@ ImageBuf::specmod() -const ImageSpec& -ImageBuf::nativespec() const +TypeDesc +ImageBuf::file_format() const { - return m_impl->nativespec(); + return m_impl->file_format(); } +std::vector +ImageBuf::file_channelformats() const +{ + return m_impl->file_channelformats(); +} + + + +// DEPRECATED +// const ImageSpec& +// ImageBuf::nativespec() const +// { +// return m_impl->nativespec(); +// } + + + bool ImageBufImpl::has_thumbnail(DoLock do_lock) const { @@ -2167,12 +2230,12 @@ ImageBuf::copy(const ImageBuf& src, TypeDesc format) return true; } if (src.deep()) { - m_impl->reset(src.name(), src.spec(), &src.nativespec()); + m_impl->reset(src.name(), src.spec()); m_impl->m_deepdata = src.m_impl->m_deepdata; return true; } if (format.basetype == TypeDesc::UNKNOWN || src.deep()) - m_impl->reset(src.name(), src.spec(), &src.nativespec()); + m_impl->reset(src.name(), src.spec()); else { ImageSpec newspec(src.spec()); newspec.set_format(format); diff --git a/src/libOpenImageIO/imagebuf_test.cpp b/src/libOpenImageIO/imagebuf_test.cpp index 2785ccf96c..556487b390 100644 --- a/src/libOpenImageIO/imagebuf_test.cpp +++ b/src/libOpenImageIO/imagebuf_test.cpp @@ -423,7 +423,6 @@ test_read_channel_subset() true /*force*/, TypeDesc::FLOAT); std::cout << " After reading channels [2,5), we have:\n"; print(B); - OIIO_CHECK_EQUAL(B.nativespec().nchannels, 6); OIIO_CHECK_EQUAL(B.spec().nchannels, 3); OIIO_CHECK_EQUAL(B.spec().format, TypeDesc::FLOAT); OIIO_CHECK_EQUAL(B.spec().channelnames[0], "B"); diff --git a/src/libOpenImageIO/maketexture.cpp b/src/libOpenImageIO/maketexture.cpp index 55156a935b..87210d3065 100644 --- a/src/libOpenImageIO/maketexture.cpp +++ b/src/libOpenImageIO/maketexture.cpp @@ -1124,8 +1124,8 @@ make_texture_impl(ImageBufAlgo::MakeTextureMode mode, const ImageBuf* input, } // The cache might mess with the apparent data format, so make sure - // it's the nativespec that we consult for data format of the file. - TypeDesc out_dataformat = src->nativespec().format; + // it's the original file format that we consult. + TypeDesc out_dataformat = src->file_format(); if (configspec.format != TypeDesc::UNKNOWN) out_dataformat = configspec.format; diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index b71aba7e06..3279c64ca1 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -3143,6 +3143,10 @@ ImageCacheImpl::get_cache_dimensions(ImageCacheFile* file, file->subimages()); return false; } + + //! force mip level to zero for now + //! TODO: store nativespec in SubImageInfo, and extra overrides in LevelInfo + constexpr int miplevel = 0; if (miplevel < 0 || miplevel >= file->miplevels(subimage)) { if (file->errors_should_issue()) error("Unknown mip level {} (out of {})", miplevel, diff --git a/src/oiiotool/expressions.cpp b/src/oiiotool/expressions.cpp index cd4f5128ed..6950773cba 100644 --- a/src/oiiotool/expressions.cpp +++ b/src/oiiotool/expressions.cpp @@ -310,7 +310,7 @@ Oiiotool::express_parse_atom(const string_view expr, string_view& s, // OiioTool::print_stats(out, *this, (*img)()); std::string err; - if (!pvt::print_stats(out, "", (*img)(), (*img)().nativespec(), + if (!pvt::print_stats(out, "", (*img)(), (*img)().spec(), ROI(), err)) errorfmt("stats", "unable to compute: {}", err); diff --git a/src/oiiotool/imagerec.cpp b/src/oiiotool/imagerec.cpp index f732ae292f..b9439cf8ff 100644 --- a/src/oiiotool/imagerec.cpp +++ b/src/oiiotool/imagerec.cpp @@ -299,7 +299,8 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) int new_z_channel = -1; int chbegin = 0, chend = -1; if (channel_set.size()) { - decode_channel_set(ib->nativespec(), channel_set, + //! TODO: now that nativespec() is deprecated what should we do here ? + decode_channel_set(ib->spec(), channel_set, newchannelnames, channel_set_channels, channel_set_values, eh); for (size_t c = 0, e = channel_set_channels.size(); c < e; @@ -332,11 +333,11 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) TypeDesc convert = TypeDesc::FLOAT; if (m_input_dataformat != TypeDesc::UNKNOWN) { convert = m_input_dataformat; - if (m_input_dataformat != ib->nativespec().format) + if (m_input_dataformat != ib->file_format()) m_subimages[s].m_was_direct_read = false; forceread = true; } else if (readpolicy & ReadNative) - convert = ib->nativespec().format; + convert = ib->file_format(); if (!forceread && convert != TypeDesc::UINT8 && convert != TypeDesc::UINT16 && convert != TypeDesc::HALF && convert != TypeDesc::FLOAT) { @@ -377,11 +378,13 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) m_subimages[s].m_specs[m] = ib->spec(); // For ImageRec purposes, we need to restore a few of the // native settings. - const ImageSpec& nativespec(ib->nativespec()); + + //! TODO: now that nativespec() is deprecated what should we do here ? + const ImageSpec& spec(ib->spec()); // m_subimages[s].m_specs[m].format = nativespec.format; - m_subimages[s].m_specs[m].tile_width = nativespec.tile_width; - m_subimages[s].m_specs[m].tile_height = nativespec.tile_height; - m_subimages[s].m_specs[m].tile_depth = nativespec.tile_depth; + m_subimages[s].m_specs[m].tile_width = spec.tile_width; + m_subimages[s].m_specs[m].tile_height = spec.tile_height; + m_subimages[s].m_specs[m].tile_depth = spec.tile_depth; } } diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index bdeb415aeb..7cfdb91c2e 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -406,7 +406,7 @@ Oiiotool::read(ImageRecRef img, ReadPolicy readpolicy, string_view channel_set) // If this is the first tiled image we have come across, use it to // set our tile size (unless the user explicitly set a tile size, or // explicitly instructed scanline output). - const ImageSpec& nspec((*img)().nativespec()); + const ImageSpec& nspec((*img)().spec()); if (nspec.tile_width && !output_tilewidth && !output_scanline) { output_tilewidth = nspec.tile_width; output_tileheight = nspec.tile_height; @@ -452,28 +452,29 @@ void Oiiotool::remember_input_channelformats(ImageRecRef img) { for (int s = 0, subimages = img->subimages(); s < subimages; ++s) { - const ImageSpec& nspec((*img)(s, 0).nativespec()); + const ImageBuf& buf((*img)(s, 0)); + const ImageSpec& spec(buf.spec()); // Overall default format is the merged type of all subimages // of the first input image. input_dataformat = TypeDesc::basetype_merge(input_dataformat, - nspec.format); - std::string subimagename = nspec.get_string_attribute( + buf.file_format()); + std::string subimagename = spec.get_string_attribute( "oiio:subimagename"); if (subimagename.size()) { // Record a best guess for this subimage, if not already set. auto key = Strutil::fmt::format("{}.*", subimagename); if (input_channelformats[key] == "") - input_channelformats[key] = nspec.format.c_str(); + input_channelformats[key] = buf.file_format().c_str(); } if (!input_bitspersample) - input_bitspersample = nspec.get_int_attribute("oiio:BitsPerSample"); - for (int c = 0; c < nspec.nchannels; ++c) { + input_bitspersample = spec.get_int_attribute("oiio:BitsPerSample"); + for (int c = 0; c < spec.nchannels; ++c) { // For each channel, if we don't already have a type recorded // for its name, record it. Both the bare channel name, and also // "subimagename.channelname", so that we can remember the same // name differently for different subimages. - std::string chname = nspec.channel_name(c); - std::string chtypename = nspec.channelformat(c).c_str(); + std::string chname = spec.channel_name(c); + std::string chtypename = buf.file_channelformats()[c].c_str(); if (subimagename.size()) { std::string subchname = Strutil::fmt::format("{}.{}", subimagename, chname); From 44c55afc8f04f73b3a9c259db5f9bd4e87cfd943 Mon Sep 17 00:00:00 2001 From: Basile Fraboni Date: Mon, 7 Oct 2024 11:44:52 -0700 Subject: [PATCH 2/5] update ImageRec::read according to recent api changes --- src/oiiotool/imagerec.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/oiiotool/imagerec.cpp b/src/oiiotool/imagerec.cpp index b9439cf8ff..dc5b9d6b83 100644 --- a/src/oiiotool/imagerec.cpp +++ b/src/oiiotool/imagerec.cpp @@ -282,8 +282,8 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) // relying on the cache to read their frames on demand rather // than reading the whole movie up front, even though each frame // individually would be well below the threshold. - ImageSpec spec; - m_imagecache->get_imagespec(uname, spec, s); + const ImageSpec& nativespec = *m_imagecache->imagespec(uname, s); + ImageSpec spec = nativespec; m_imagecache->get_cache_dimensions(uname, spec, s, m); imagesize_t imgbytes = spec.image_bytes(); bool forceread = (s == 0 && m == 0 @@ -299,8 +299,7 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) int new_z_channel = -1; int chbegin = 0, chend = -1; if (channel_set.size()) { - //! TODO: now that nativespec() is deprecated what should we do here ? - decode_channel_set(ib->spec(), channel_set, + decode_channel_set(nativespec, channel_set, newchannelnames, channel_set_channels, channel_set_values, eh); for (size_t c = 0, e = channel_set_channels.size(); c < e; @@ -333,11 +332,11 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) TypeDesc convert = TypeDesc::FLOAT; if (m_input_dataformat != TypeDesc::UNKNOWN) { convert = m_input_dataformat; - if (m_input_dataformat != ib->file_format()) + if (m_input_dataformat != nativespec.format) m_subimages[s].m_was_direct_read = false; forceread = true; } else if (readpolicy & ReadNative) - convert = ib->file_format(); + convert = nativespec.format; if (!forceread && convert != TypeDesc::UINT8 && convert != TypeDesc::UINT16 && convert != TypeDesc::HALF && convert != TypeDesc::FLOAT) { @@ -379,12 +378,10 @@ ImageRec::read(ReadPolicy readpolicy, string_view channel_set) // For ImageRec purposes, we need to restore a few of the // native settings. - //! TODO: now that nativespec() is deprecated what should we do here ? - const ImageSpec& spec(ib->spec()); // m_subimages[s].m_specs[m].format = nativespec.format; - m_subimages[s].m_specs[m].tile_width = spec.tile_width; - m_subimages[s].m_specs[m].tile_height = spec.tile_height; - m_subimages[s].m_specs[m].tile_depth = spec.tile_depth; + m_subimages[s].m_specs[m].tile_width = nativespec.tile_width; + m_subimages[s].m_specs[m].tile_height = nativespec.tile_height; + m_subimages[s].m_specs[m].tile_depth = nativespec.tile_depth; } } From 09a2731a523078eb645fc0c2d808d1b6936e86d5 Mon Sep 17 00:00:00 2001 From: Basile Fraboni Date: Mon, 7 Oct 2024 11:57:11 -0700 Subject: [PATCH 3/5] update ImageRec::read according to recent api changes Signed-off-by: Basile Fraboni --- src/libOpenImageIO/imagebuf.cpp | 27 ++++++++++++--------------- src/libtexture/imagecache.cpp | 4 ---- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 386f2a7c79..84cdf3b874 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -366,11 +366,8 @@ class ImageBufImpl { int m_nmiplevels; ///< # of MIP levels in the current subimage mutable int m_threads; ///< thread policy for this image ImageSpec m_spec; ///< Describes the image (size, etc) - //! TODO: remove m_nativespec and store only "native channels formats" + //! TODO: remove m_nativespec ImageSpec m_nativespec; ///< Describes the true native image - // TypeDesc format; ///< Data format of the channels as in the associated file. - // std:vector channelformats; ///< Optional per-channel data formats as in the associated file. - // ///< This will be empty if all native channels have the same format. std::unique_ptr m_pixels; ///< Pixel data, if local and we own it char* m_localpixels; ///< Pointer to local pixels span m_bufspan; ///< Bounded buffer for local pixels @@ -946,8 +943,7 @@ ImageBufImpl::reset(string_view filename, const ImageSpec& spec, m_current_miplevel = 0; if (buforigin || bufspan.size()) { m_spec = spec; - //! TODO: ultimattely remove m_nativespec and only update - //! m_file_format and m_file_channelformats + //! TODO: remove m_nativespec m_nativespec = nativespec ? *nativespec : spec; m_channel_stride = stride_t(spec.format.size()); @@ -1755,15 +1751,16 @@ ImageBufImpl::copy_metadata(const ImageBufImpl& src) m_spec.full_width = srcspec.full_width; m_spec.full_height = srcspec.full_height; m_spec.full_depth = srcspec.full_depth; - //! TODO: now that nativespec() is deprecated what should we do here ? - // if (src.cachedpixels()) { - // // If we're copying metadata from a cached image, be sure to - // // get the file's tile size, not the cache's tile size. - // m_spec.tile_width = src.nativespec().tile_width; - // m_spec.tile_height = src.nativespec().tile_height; - // m_spec.tile_depth = src.nativespec().tile_depth; - // } - // else + if (src.cachedpixels()) { + // If we're copying metadata from a cached image, be sure to + // get the file's tile size, not the cache's tile size. + //! TOCHECK: is it actually correct to do as follows ? + const ImageSpec& nativespec = *m_imagecache->imagespec(m_name); + m_spec.tile_width = nativespec.tile_width; + m_spec.tile_height = nativespec.tile_height; + m_spec.tile_depth = nativespec.tile_depth; + } + else { m_spec.tile_width = srcspec.tile_width; m_spec.tile_height = srcspec.tile_height; diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index 3279c64ca1..b71aba7e06 100644 --- a/src/libtexture/imagecache.cpp +++ b/src/libtexture/imagecache.cpp @@ -3143,10 +3143,6 @@ ImageCacheImpl::get_cache_dimensions(ImageCacheFile* file, file->subimages()); return false; } - - //! force mip level to zero for now - //! TODO: store nativespec in SubImageInfo, and extra overrides in LevelInfo - constexpr int miplevel = 0; if (miplevel < 0 || miplevel >= file->miplevels(subimage)) { if (file->errors_should_issue()) error("Unknown mip level {} (out of {})", miplevel, From e3e3c232100a847c3a40617d35f3958d093f8bb7 Mon Sep 17 00:00:00 2001 From: Basile Fraboni Date: Mon, 7 Oct 2024 12:05:56 -0700 Subject: [PATCH 4/5] update ImageBuf::read to query the native spec from the cache Signed-off-by: Basile Fraboni --- src/libOpenImageIO/imagebuf.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 84cdf3b874..5b2086b1e1 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -1255,11 +1255,10 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_current_subimage = subimage; m_current_miplevel = miplevel; - //! TODO: should we pass the number of channels of nativespec() - // or consider chbeing/chend are correctly set ? - // if (chend < 0 || chend > nativespec().nchannels) - // chend = nativespec().nchannels; - // bool use_channel_subset = (chbegin != 0 || chend != nativespec().nchannels); + const ImageSpec& nativespec = *m_imagecache->imagespec(m_name, m_current_subimage); + if (chend < 0 || chend > nativespec.nchannels) + chend = nativespec.nchannels; + bool use_channel_subset = (chbegin != 0 || chend != nativespec.nchannels); if (m_spec.deep) { Timer timer; @@ -1322,9 +1321,8 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, force = true; m_spec.nchannels = chend - chbegin; m_spec.channelnames.resize(m_spec.nchannels); - /// TODO: what to do here ? if we remove m_nativespec, we won't have the channelnames anymore.. for (int c = 0; c < m_spec.nchannels; ++c) - m_spec.channelnames[c] = m_nativespec.channelnames[c + chbegin]; + m_spec.channelnames[c] = nativespec.channelnames[c + chbegin]; const std::vector& cformats = file_channelformats(); if (cformats.size()) { m_spec.channelformats.resize(m_spec.nchannels); From cb9871126984e51f36f067d25f1d72cf23713156 Mon Sep 17 00:00:00 2001 From: Basile Fraboni Date: Mon, 7 Oct 2024 15:34:23 -0700 Subject: [PATCH 5/5] fix compilation Signed-off-by: Basile Fraboni --- src/libOpenImageIO/imagebuf.cpp | 1 + src/oiiotool/expressions.cpp | 2 +- src/oiiotool/imagerec.cpp | 13 +++++++++++++ src/oiiotool/oiiotool.cpp | 2 +- src/oiiotool/oiiotool.h | 6 +----- src/oiiotool/printinfo.cpp | 4 ++-- src/python/py_imagebuf.cpp | 4 +++- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/libOpenImageIO/imagebuf.cpp b/src/libOpenImageIO/imagebuf.cpp index 5b2086b1e1..672c193c1c 100644 --- a/src/libOpenImageIO/imagebuf.cpp +++ b/src/libOpenImageIO/imagebuf.cpp @@ -1255,6 +1255,7 @@ ImageBufImpl::read(int subimage, int miplevel, int chbegin, int chend, m_current_subimage = subimage; m_current_miplevel = miplevel; + //! NOTE: m_imagecache can be null, make sure there is a fallback solution const ImageSpec& nativespec = *m_imagecache->imagespec(m_name, m_current_subimage); if (chend < 0 || chend > nativespec.nchannels) chend = nativespec.nchannels; diff --git a/src/oiiotool/expressions.cpp b/src/oiiotool/expressions.cpp index 6950773cba..a2c076e54a 100644 --- a/src/oiiotool/expressions.cpp +++ b/src/oiiotool/expressions.cpp @@ -239,7 +239,7 @@ Oiiotool::express_parse_atom(const string_view expr, string_view& s, read(img); ParamValue tmpparam; if (metadata == "nativeformat") { - result = img->nativespec(0, 0)->format.c_str(); + result = img->nativespec(0)->format.c_str(); } else if (auto p = img->spec(0, 0)->find_attribute(metadata, tmpparam)) { std::string val = ImageSpec::metadata_val(*p); diff --git a/src/oiiotool/imagerec.cpp b/src/oiiotool/imagerec.cpp index dc5b9d6b83..a72d2d59d4 100644 --- a/src/oiiotool/imagerec.cpp +++ b/src/oiiotool/imagerec.cpp @@ -247,6 +247,19 @@ ImageRec::read_nativespec() +const ImageSpec* +ImageRec::nativespec(int subimg) const +{ + if (subimg < subimages()) + { + ustring uname(name()); + return m_imagecache->imagespec(uname, subimg); + } + return nullptr; +} + + + bool ImageRec::read(ReadPolicy readpolicy, string_view channel_set) { diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index 7cfdb91c2e..507d01bd13 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -5648,7 +5648,7 @@ output_file(Oiiotool& ot, cspan argv) for (int s = 0, send = ir->subimages(); s < send; ++s) { for (int m = 0, mend = ir->miplevels(s); m < mend && ok; ++m) { ImageSpec spec = *ir->spec(s, m); - adjust_output_options(filename, spec, ir->nativespec(s, m), ot, + adjust_output_options(filename, spec, ir->nativespec(s), ot, supports_tiles, fileoptions, (*ir)[s].was_direct_read()); if (s > 0 || m > 0) { // already opened first subimage/level diff --git a/src/oiiotool/oiiotool.h b/src/oiiotool/oiiotool.h index 6a56d5df92..d2345b42af 100644 --- a/src/oiiotool/oiiotool.h +++ b/src/oiiotool/oiiotool.h @@ -544,11 +544,7 @@ class ImageRec { return subimg < subimages() ? m_subimages[subimg].spec(mip) : NULL; } - const ImageSpec* nativespec(int subimg = 0, int mip = 0) const - { - return subimg < subimages() ? &((*this)(subimg, mip).nativespec()) - : nullptr; - } + const ImageSpec* nativespec(int subimg = 0) const; bool was_output() const { return m_was_output; } void was_output(bool val) { m_was_output = val; } diff --git a/src/oiiotool/printinfo.cpp b/src/oiiotool/printinfo.cpp index 3d5ddef48d..7070aefd5e 100644 --- a/src/oiiotool/printinfo.cpp +++ b/src/oiiotool/printinfo.cpp @@ -257,7 +257,7 @@ OiioTool::print_stats(std::ostream& out, Oiiotool& ot, return; } std::string err; - if (!pvt::print_stats(out, indent, input, input.nativespec(), roi, err)) + if (!pvt::print_stats(out, indent, input, input.spec(), roi, err)) ot.errorfmt("stats", "unable to compute: {}", err); } @@ -472,7 +472,7 @@ print_info_subimage(std::ostream& out, Oiiotool& ot, int current_subimage, std::string err; if (!pvt::print_stats(out, nmip > 1 ? " " : " ", (*img)(current_subimage, m), - (*img)(current_subimage, m).nativespec(), + *img->nativespec(), opt.roi, err)) ot.errorfmt("stats", "unable to compute: {}", err); } diff --git a/src/python/py_imagebuf.cpp b/src/python/py_imagebuf.cpp index 409e0ba53d..f3efc99612 100644 --- a/src/python/py_imagebuf.cpp +++ b/src/python/py_imagebuf.cpp @@ -361,7 +361,9 @@ declare_imagebuf(py::module& m) "height"_a = 0, "depth"_a = 0) .def("spec", &ImageBuf::spec, py::return_value_policy::reference_internal) - .def("nativespec", &ImageBuf::nativespec, + .def("file_format", &ImageBuf::file_format, + py::return_value_policy::reference_internal) + .def("file_channelformats", &ImageBuf::file_channelformats, py::return_value_policy::reference_internal) .def("specmod", &ImageBuf::specmod, py::return_value_policy::reference_internal)