Skip to content

Commit

Permalink
refactor: de-virtualize IC and TS
Browse files Browse the repository at this point in the history
Background: The ImageCache and TextureSystem had historically been
implemented as pure virtual abstract base classe defining an
interface, then a hidden concrete subclass. The flexibility this was
intended to provide was never really used; instead the imagined main
client for the flexibility, OSL, has a different mechanism for
customizing the texture system (via their RendererServices
class). Meanwhile, this has made the interfaces to IC and TS
impossible to change without breaking the ABI.

So this PR does the following:

* ImageCache and TextureSytem are full de-virtualized, and are now
  concrete classes, no hidden subclassing, no virtual methods. As
  such, removing or changing existing methods will be an ABI break,
  but adding new methods is not.

* Each use a PIMPL idiom, where all the data amd internal methods
  are hidden from the public enterface and do not affect the ABI.

* Roughly speaking, we added the PIMPL pointers and made the
  ImageCacheImpl/TextureSystemImpl -- what used to be the hidden
  concrete subclasses -- into the hidden PIMPL classes.

* Moved a lot of classes that used to be in a "pvt" namespace into the
  main namespace. If they are opaque types not exposed in public
  headers or as symbols exported from the library, it doesn't matter,
  so this just removes some arbitrary clutter.

* Had to rearrange some of the recently added heapsize/footprint code,
  moved some inline functions in the headers into methods of the
  classes, defined in the cpp files.

This itself is a huge ABI change, so will only be merged into master,
to become part of the 3.0 release. This is not an API change, though!
This is all about the internals, and should not require any client
software to change a single line of code.

I may do further simplification or refactoring of this code in the
future, but that will all be smaller and have no further API/ABI
changes that break compatibility.

Although the structure of the class hierarchy has changed around, none
of the logic about how IC and TS actually *work* has changed, so there
should be no change in observable behavior.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
  • Loading branch information
lgritz committed Aug 25, 2024
1 parent 6b2d140 commit 52f13d7
Show file tree
Hide file tree
Showing 11 changed files with 1,534 additions and 732 deletions.
7 changes: 2 additions & 5 deletions src/include/OpenImageIO/imagebuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ OIIO_NAMESPACE_BEGIN
class ImageBuf;
class ImageBufImpl; // Opaque type for the unique_ptr.
class ImageCache;

namespace pvt {
class ImageCacheTile;
}; // namespace pvt



Expand Down Expand Up @@ -1369,7 +1366,7 @@ class OIIO_API ImageBuf {
int m_rng_xbegin, m_rng_xend, m_rng_ybegin, m_rng_yend, m_rng_zbegin,
m_rng_zend;
int m_x, m_y, m_z;
pvt::ImageCacheTile* m_tile = nullptr;
ImageCacheTile* m_tile = nullptr;
int m_tilexbegin, m_tileybegin, m_tilezbegin;
int m_tilexend;
int m_nchannels;
Expand Down Expand Up @@ -1638,7 +1635,7 @@ class OIIO_API ImageBuf {
// tile for the given pixel, and return the ptr to the actual pixel
// within the tile. If any read errors occur, set haderror=true (but
// if there are no errors, do not modify haderror).
const void* retile(int x, int y, int z, pvt::ImageCacheTile*& tile,
const void* retile(int x, int y, int z, ImageCacheTile*& tile,
int& tilexbegin, int& tileybegin, int& tilezbegin,
int& tilexend, bool& haderr, bool exists,
WrapMode wrap) const;
Expand Down
264 changes: 145 additions & 119 deletions src/include/OpenImageIO/imagecache.h

Large diffs are not rendered by default.

366 changes: 176 additions & 190 deletions src/include/OpenImageIO/texture.h

Large diffs are not rendered by default.

52 changes: 50 additions & 2 deletions src/libtexture/environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,54 @@ OIIO_NAMESPACE_BEGIN
using namespace pvt;
using namespace simd;


bool
TextureSystem::environment(ustring filename, TextureOpt& options, V3fParam R,
V3fParam dRdx, V3fParam dRdy, int nchannels,
float* result, float* dresultds, float* dresultdt)
{
return m_impl->environment(filename, options, R, dRdx, dRdy, nchannels,
result, dresultds, dresultdt);
}


bool
TextureSystem::environment(TextureHandle* texture_handle,
Perthread* thread_info, TextureOpt& options,
V3fParam R, V3fParam dRdx, V3fParam dRdy,
int nchannels, float* result, float* dresultds,
float* dresultdt)
{
return m_impl->environment(texture_handle, thread_info, options, R, dRdx,
dRdy, nchannels, result, dresultds, dresultdt);
}


bool
TextureSystem::environment(ustring filename, TextureOptBatch& options,
Tex::RunMask mask, const float* R, const float* dRdx,
const float* dRdy, int nchannels, float* result,
float* dresultds, float* dresultdt)
{
return m_impl->environment(filename, options, mask, R, dRdx, dRdy,
nchannels, result, dresultds, dresultdt);
}


bool
TextureSystem::environment(TextureHandle* texture_handle,
Perthread* thread_info, TextureOptBatch& options,
Tex::RunMask mask, const float* R, const float* dRdx,
const float* dRdy, int nchannels, float* result,
float* dresultds, float* dresultdt)
{
return m_impl->environment(texture_handle, thread_info, options, mask, R,
dRdx, dRdy, nchannels, result, dresultds,
dresultdt);
}



namespace pvt {


Expand All @@ -225,6 +273,8 @@ vector_to_latlong(const Imath::V3f& R, bool y_is_up, float& s, float& t)
t = 0.0f;
}

} // namespace pvt



bool
Expand Down Expand Up @@ -594,6 +644,4 @@ TextureSystemImpl::environment(ustring filename, TextureOptBatch& options,
}


} // end namespace pvt

OIIO_NAMESPACE_END
Loading

0 comments on commit 52f13d7

Please sign in to comment.