Skip to content

Commit 1e301a5

Browse files
committed
Add color management to texture lookups
This isn't 100% final -- I'm sure there are some corner cases I missed, maybe things that will eventually need some performance work, but enough works and passes tests that I'm confident in the API and so I want to get this merged and we can iterate on internals later. The use case is as follows: User or shader knows that a particular texture is in something other than the default working color space, this is not necessarily apparent from the file itself and so cannot be fully automatic, but it is presumed that it can be communicated in the texture call. We aim to be efficient, fully converting each tile upon input. Note that we are sweeping under the rug the fact that a non-default color space lookup is not mathematically correct with respect to the MIP-map downsizing math if that was not considered at texture creation time. But by converting texels as they are read, rather than the shader itself trying to do a color transformation on the final texture result, at least we are doing the right math for the filtering within and among MIP levels. The new interface consists of communicating the presumed working color space, finding the colortransform ID of a given from/to (with the "to" defaulting to the working space, if not specified), and then on each texture call, supplying that colortransform ID as one of the settings in TextureOpt (or TextureOptBatch). The outline of changes are as follows: * BREAKING CHANGE: TextureOpt and TextureOptBatch now contain an additional int `colortransformid`. The default, 0, is the old (and, frankly, preferred) behavior. Nonzero values indicate that we want the texels to go through a color transformation as the tiles are read into the cache. * BREAKING CHANGE: New public methods have been added to TextureSystem `get_colortransform_id(fromspace, tospace)` that return a transform ID given the from and to color spaces (ustring or ustringhash). This is the value that goes in the TextureOpt to request a color transformation on the pixels as they are read. * BREAKING CHANGE: IC::get_image_handle / TS::get_texture_handle now take an additional optinal paramter that is a TextureOpt*. This is currently unused, but is reserved for possible future use and/or for alternate TextureSystem implementations that choose a different implementation strategy wherein an entirely different TextureHandle may correspond to each color transformation. That's not how I've done it here, but it's a valid approach (and maybe one we'll switch to in the future). We presume that it's ill-defined to ask for a handle with one explicit color transformation, then use that handle with a TextureOpt that implies a different color transformation. Using those consistently ensure that either TS strategy will work equally well. * New TextureSystem attributes "colorspace" gives the working color space name (defaulting to "scene_linear"), and "colorconfig" can optionally point to a color config file (though this is not yet functional -- it just uses the default color config). * Internally, the tile cache hash has been adjusted so that the same tile in the disk file will appear as a separate tile in the cache for each color space pair that is applied to it. Thus, there is no conflict if some different texture calls ask for different color space interpretations of the same texture file. (That said, it's not recommended to be throwing color spaces around willy nilly -- it still takes multiple cache entrie, plus extra color conversion math as the tiles are read in.) * A new preprocessor symbol OIIO_TEXTURESYSTEM_SUPPORTS_COLORSPACE in texture.h allows client software can easily tell if this is a version new enough to support color management and has the additional fields in the texture option structures. Because there are breaking changes here to ABI, this is destined only for master (future 2.5) and will not be backported to OIIO 2.4.
1 parent e2e5eba commit 1e301a5

File tree

16 files changed

+273
-73
lines changed

16 files changed

+273
-73
lines changed

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
cmake_minimum_required (VERSION 3.12)
66

7-
set (OpenImageIO_VERSION "2.5.0.2")
7+
set (OpenImageIO_VERSION "2.5.1.0")
88
set (OpenImageIO_VERSION_OVERRIDE "" CACHE STRING
99
"Version override (use with caution)!")
1010
mark_as_advanced (OpenImageIO_VERSION_OVERRIDE)

src/cmake/testing.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ macro (oiio_add_all_tests)
186186
texture-stats
187187
texture-threadtimes
188188
texture-env
189+
texture-colorspace
189190
)
190191
oiio_add_tests (${all_texture_tests})
191192
# Duplicate texture tests with batch mode

src/include/OpenImageIO/imagecache.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828

2929
OIIO_NAMESPACE_BEGIN
3030

31+
// Forward declaration
32+
class TextureOpt;
33+
3134
namespace pvt {
3235
// Forward declaration
3336
class ImageCacheImpl;
@@ -216,6 +219,11 @@ class OIIO_API ImageCache {
216219
/// enabled, this reduces the number of file opens, at the
217220
/// expense of not being able to open files if their format do
218221
/// not actually match their filename extension). Default: 0
222+
/// - `string colorspace` :
223+
/// The working colorspace of the texture system. Default: none.
224+
/// - `string colorconfig` :
225+
/// Name of the OCIO config to use. Default: "" (meaning to use
226+
/// the default color config).
219227
///
220228
/// - `string options`
221229
/// This catch-all is simply a comma-separated list of
@@ -467,6 +475,25 @@ class OIIO_API ImageCache {
467475
/// internals.
468476
typedef pvt::ImageCacheFile ImageHandle;
469477

478+
#if OIIO_VERSION_GREATER_EQUAL(2, 5, 1)
479+
/// Retrieve an opaque handle for fast texture lookups. The filename is
480+
/// presumed to be UTF-8 encoded. The `options`, if not null, may be used
481+
/// to create a separate handle for certain texture option choices
482+
/// (currently: the colorspace). The opaque pointer `thread_info` is
483+
/// thread-specific information returned by `get_perthread_info()`. Return
484+
/// nullptr if something has gone horribly wrong.
485+
virtual ImageHandle* get_image_handle(ustring filename,
486+
Perthread* thread_info = nullptr,
487+
const TextureOpt* options = nullptr) = 0;
488+
489+
/// Get an ImageHandle using a UTF-16 encoded wstring filename.
490+
ImageHandle* get_image_handle(const std::wstring& filename,
491+
Perthread* thread_info = nullptr,
492+
const TextureOpt* options = nullptr) {
493+
return get_image_handle(ustring(Strutil::utf16_to_utf8(filename)),
494+
thread_info, options);
495+
}
496+
#else
470497
/// Retrieve an opaque handle for fast image lookups. The filename is
471498
/// presumed to be UTF-8 encoded. The opaque `pointer thread_info` is
472499
/// thread-specific information returned by `get_perthread_info()`.
@@ -480,6 +507,7 @@ class OIIO_API ImageCache {
480507
return get_image_handle (ustring(Strutil::utf16_to_utf8(filename)),
481508
thread_info);
482509
}
510+
#endif
483511

484512
/// Return true if the image handle (previously returned by
485513
/// `get_image_handle()`) is a valid image that can be subsequently read.

src/include/OpenImageIO/texture.h

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
// Define symbols that let client applications determine if newly added
2121
// features are supported.
2222
#define OIIO_TEXTURESYSTEM_SUPPORTS_CLOSE 1
23+
#define OIIO_TEXTURESYSTEM_SUPPORTS_COLORSPACE 1
2324

2425
// Is the getattributetype() method present? (Added in 2.5)
2526
#define OIIO_TEXTURESYSTEM_SUPPORTS_GETATTRIBUTETYPE 1
@@ -232,6 +233,7 @@ class OIIO_API TextureOpt {
232233
fill(0.0f), missingcolor(nullptr),
233234
time(0.0f), rnd(-1.0f), samples(1),
234235
rwrap(WrapDefault), rblur(0.0f), rwidth(1.0f),
236+
colortransformid(0),
235237
envlayout(0)
236238
{ }
237239

@@ -264,6 +266,8 @@ class OIIO_API TextureOpt {
264266
float rblur; ///< Blur amount in the r direction
265267
float rwidth; ///< Multiplier for derivatives in r direction
266268

269+
int colortransformid; ///< Color space id of the texture
270+
267271
/// Utility: Return the Wrap enum corresponding to a wrap name:
268272
/// "default", "black", "clamp", "periodic", "mirror".
269273
static Wrap decode_wrapmode(const char* name)
@@ -313,9 +317,7 @@ class OIIO_API TextureOptBatch {
313317
alignas(Tex::BatchAlign) float twidth[Tex::BatchWidth];
314318
alignas(Tex::BatchAlign) float rwidth[Tex::BatchWidth];
315319
// Note: rblur,rwidth only used for volumetric lookups
316-
#if OIIO_VERSION_GREATER_EQUAL(2,4,0)
317320
alignas(Tex::BatchAlign) float rnd[Tex::BatchWidth];
318-
#endif
319321

320322
// Options that must be the same for all points we're texturing at once
321323
int firstchannel = 0; ///< First channel of the lookup
@@ -330,6 +332,7 @@ class OIIO_API TextureOptBatch {
330332
int conservative_filter = 1; ///< True: over-blur rather than alias
331333
float fill = 0.0f; ///< Fill value for missing channels
332334
const float *missingcolor = nullptr; ///< Color for missing texture
335+
int colortransformid = 0; ///< Color space id of the texture
333336

334337
private:
335338
// Options set INTERNALLY by libtexture after the options are passed
@@ -551,6 +554,10 @@ class OIIO_API TextureSystem {
551554
/// - `int max_errors_per_file` :
552555
/// Limits how many errors to issue for each file. (default:
553556
/// 100)
557+
/// - `string colorspace` :
558+
/// The working colorspace of the texture system.
559+
/// - `string colorconfig` :
560+
/// Name of the OCIO config to use (default: "").
554561
///
555562
/// Texture-specific settings:
556563
/// - `matrix44 worldtocommon` / `matrix44 commontoworld` :
@@ -788,16 +795,20 @@ class OIIO_API TextureSystem {
788795
class TextureHandle;
789796

790797
/// Retrieve an opaque handle for fast texture lookups. The filename is
791-
/// presumed to be UTF-8 encoded. The opaque pointer `thread_info` is
792-
/// thread-specific information returned by `get_perthread_info()`.
793-
/// Return nullptr if something has gone horribly wrong.
794-
virtual TextureHandle* get_texture_handle (ustring filename,
795-
Perthread *thread_info=nullptr) = 0;
798+
/// presumed to be UTF-8 encoded. The `options`, if not null, may be used
799+
/// to create a separate handle for certain texture option choices
800+
/// (currently: the colorspace). The opaque pointer `thread_info` is
801+
/// thread-specific information returned by `get_perthread_info()`. Return
802+
/// nullptr if something has gone horribly wrong.
803+
virtual TextureHandle* get_texture_handle(ustring filename,
804+
Perthread* thread_info = nullptr,
805+
const TextureOpt* options = nullptr) = 0;
796806
/// Get a TextureHandle using a UTF-16 encoded wstring filename.
797-
TextureHandle* get_texture_handle (const std::wstring& filename,
798-
Perthread *thread_info=nullptr) {
799-
return get_texture_handle (ustring(Strutil::utf16_to_utf8(filename)),
800-
thread_info);
807+
TextureHandle* get_texture_handle(const std::wstring& filename,
808+
Perthread* thread_info = nullptr,
809+
const TextureOpt* options = nullptr) {
810+
return get_texture_handle(ustring(Strutil::utf16_to_utf8(filename)),
811+
thread_info, options);
801812
}
802813

803814
/// Return true if the texture handle (previously returned by
@@ -810,6 +821,14 @@ class OIIO_API TextureSystem {
810821
/// This method was added in OpenImageIO 2.3.
811822
virtual ustring filename_from_handle(TextureHandle* handle) = 0;
812823

824+
/// Retrieve an id for a color transformation by name. This ID can be used
825+
/// as the value for TextureOpt::colortransformid. The returned value will
826+
/// be -1 if either color space is unknown, and 0 for a null
827+
/// transformation.
828+
virtual int get_colortransform_id(ustring fromspace,
829+
ustring tospace) const = 0;
830+
virtual int get_colortransform_id(ustringhash fromspace,
831+
ustringhash tospace) const = 0;
813832
/// @}
814833

815834
/// @{

src/libtexture/imagecache.cpp

Lines changed: 81 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,13 @@
1111
#include <vector>
1212

1313
#include <OpenImageIO/Imath.h>
14+
15+
#include <OpenImageIO/color.h>
1416
#include <OpenImageIO/dassert.h>
1517
#include <OpenImageIO/filesystem.h>
1618
#include <OpenImageIO/fmath.h>
1719
#include <OpenImageIO/imagebuf.h>
20+
#include <OpenImageIO/imagebufalgo.h>
1821
#include <OpenImageIO/imagecache.h>
1922
#include <OpenImageIO/imageio.h>
2023
#include <OpenImageIO/optparser.h>
@@ -791,36 +794,41 @@ ImageCacheFile::init_from_spec()
791794

792795

793796
bool
794-
ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info, int subimage,
795-
int miplevel, int x, int y, int z, int chbegin,
796-
int chend, TypeDesc format, void* data)
797+
ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info,
798+
const TileID& id, void* data)
797799
{
798-
OIIO_DASSERT(chend > chbegin);
800+
OIIO_DASSERT(id.chend() > id.chbegin());
799801

800802
// Mark if we ever use a mip level that's not the first
803+
int miplevel = id.miplevel();
801804
if (miplevel > 0)
802805
m_mipused = true;
803-
804806
// count how many times this mipmap level was read
805807
m_mipreadcount[miplevel]++;
806808

809+
int subimage = id.subimage();
807810
SubimageInfo& subinfo(subimageinfo(subimage));
808811

809812
// Special case for un-MIP-mapped
810813
if (subinfo.unmipped && miplevel != 0)
811-
return read_unmipped(thread_info, subimage, miplevel, x, y, z, chbegin,
812-
chend, format, data);
814+
return read_unmipped(thread_info, id, data);
813815

814816
std::shared_ptr<ImageInput> inp = open(thread_info);
815817
if (!inp)
816818
return false;
817819

818820
// Special case for untiled images -- need to do tile emulation
819821
if (subinfo.untiled)
820-
return read_untiled(thread_info, inp.get(), subimage, miplevel, x, y, z,
821-
chbegin, chend, format, data);
822+
return read_untiled(thread_info, inp.get(), id, data);
822823

823824
// Ordinary tiled
825+
int x = id.x();
826+
int y = id.y();
827+
int z = id.z();
828+
int chbegin = id.chbegin();
829+
int chend = id.chend();
830+
TypeDesc format = id.file().datatype(subimage);
831+
824832
bool ok = true;
825833
const ImageSpec& spec(this->spec(subimage, miplevel));
826834
for (int tries = 0; tries <= imagecache().failure_retries(); ++tries) {
@@ -852,6 +860,19 @@ ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info, int subimage,
852860
thread_info->m_stats.bytes_read += b;
853861
m_bytesread += b;
854862
++m_tilesread;
863+
if (id.colortransformid() > 0) {
864+
// print("CONVERT id {} {},{} to cs {}\n", filename(), id.x(), id.y(),
865+
// id.colortransformid());
866+
ImageBuf wrapper(ImageSpec(spec.tile_width, spec.tile_height,
867+
spec.nchannels, format),
868+
data);
869+
ImageBufAlgo::colorconvert(
870+
wrapper, wrapper,
871+
ColorConfig::default_colorconfig().getColorSpaceNameByIndex(
872+
(id.colortransformid() >> 16) - 1),
873+
m_imagecache.colorspace(), true, string_view(), string_view(),
874+
nullptr, ROI(), 1);
875+
}
855876
}
856877
return ok;
857878
}
@@ -860,9 +881,7 @@ ImageCacheFile::read_tile(ImageCachePerThreadInfo* thread_info, int subimage,
860881

861882
bool
862883
ImageCacheFile::read_unmipped(ImageCachePerThreadInfo* thread_info,
863-
int subimage, int miplevel, int x, int y, int z,
864-
int chbegin, int chend, TypeDesc format,
865-
void* data)
884+
const TileID& id, void* data)
866885
{
867886
// We need a tile from an unmipmapped file, and it doesn't really
868887
// exist. So generate it out of thin air by interpolating pixels
@@ -875,6 +894,15 @@ ImageCacheFile::read_unmipped(ImageCachePerThreadInfo* thread_info,
875894
// N.B. No need to lock the mutex, since this is only called
876895
// from read_tile, which already holds the lock.
877896

897+
int subimage = id.subimage();
898+
int miplevel = id.miplevel();
899+
int x = id.x();
900+
int y = id.y();
901+
// int z = id.z();
902+
int chbegin = id.chbegin();
903+
int chend = id.chend();
904+
TypeDesc format = id.file().datatype(id.subimage());
905+
878906
// Figure out the size and strides for a single tile, make an ImageBuf
879907
// to hold it temporarily.
880908
const ImageSpec& spec(this->spec(subimage, miplevel));
@@ -888,7 +916,7 @@ ImageCacheFile::read_unmipped(ImageCachePerThreadInfo* thread_info,
888916
// Figure out the range of texels we need for this tile
889917
x -= spec.x;
890918
y -= spec.y;
891-
z -= spec.z;
919+
// z -= spec.z;
892920
int x0 = x - (x % spec.tile_width);
893921
int x1 = std::min(x0 + spec.tile_width - 1, spec.full_width - 1);
894922
int y0 = y - (y % spec.tile_height);
@@ -956,10 +984,18 @@ ImageCacheFile::read_unmipped(ImageCachePerThreadInfo* thread_info,
956984
// of reading a "tile" from a file that's scanline-oriented.
957985
bool
958986
ImageCacheFile::read_untiled(ImageCachePerThreadInfo* thread_info,
959-
ImageInput* inp, int subimage, int miplevel, int x,
960-
int y, int z, int chbegin, int chend,
961-
TypeDesc format, void* data)
962-
{
987+
ImageInput* inp, const TileID& id, void* data)
988+
{
989+
int subimage = id.subimage();
990+
int miplevel = id.miplevel();
991+
int x = id.x();
992+
int y = id.y();
993+
int z = id.z();
994+
int chbegin = id.chbegin();
995+
int chend = id.chend();
996+
TypeDesc format = id.file().datatype(id.subimage());
997+
int colortransformid = id.colortransformid();
998+
963999
// Strides for a single tile
9641000
const ImageSpec& spec(this->spec(subimage, miplevel));
9651001
int tw = spec.tile_width;
@@ -1021,7 +1057,7 @@ ImageCacheFile::read_untiled(ImageCachePerThreadInfo* thread_info,
10211057
// tile-row, so let's put it in the cache anyway so
10221058
// it'll be there when asked for.
10231059
TileID id(*this, subimage, miplevel, i + spec.x, y0, z, chbegin,
1024-
chend);
1060+
chend, colortransformid);
10251061
if (!imagecache().tile_in_cache(id, thread_info)) {
10261062
ImageCacheTileRef tile;
10271063
tile = new ImageCacheTile(id, &buf[i * pixelsize], format,
@@ -1520,10 +1556,7 @@ ImageCacheTile::read(ImageCachePerThreadInfo* thread_info)
15201556
// Clear the end pad values so there aren't NaNs sucked up by simd loads
15211557
memset(m_pixels.get() + size - OIIO_SIMD_MAX_SIZE_BYTES, 0,
15221558
OIIO_SIMD_MAX_SIZE_BYTES);
1523-
m_valid = file.read_tile(thread_info, m_id.subimage(), m_id.miplevel(),
1524-
m_id.x(), m_id.y(), m_id.z(), m_id.chbegin(),
1525-
m_id.chend(), file.datatype(m_id.subimage()),
1526-
&m_pixels[0]);
1559+
m_valid = file.read_tile(thread_info, m_id, &m_pixels[0]);
15271560
file.imagecache().incr_mem(size);
15281561
if (m_valid) {
15291562
ImageCacheFile::LevelInfo& lev(
@@ -1636,6 +1669,7 @@ ImageCacheImpl::init()
16361669
m_failure_retries = 0;
16371670
m_latlong_y_up_default = true;
16381671
m_Mw2c.makeIdentity();
1672+
m_colorspace = ustring("scene_linear");
16391673
m_mem_used = 0;
16401674
m_statslevel = 0;
16411675
m_max_errors_per_file = 100;
@@ -2196,8 +2230,23 @@ ImageCacheImpl::attribute(string_view name, TypeDesc type, const void* val)
21962230
do_invalidate = true;
21972231
}
21982232
} else if (name == "substitute_image" && type == TypeDesc::STRING) {
2199-
m_substitute_image = ustring(*(const char**)val);
2200-
do_invalidate = true;
2233+
ustring uval(*(const char**)val);
2234+
if (uval != m_substitute_image) {
2235+
m_substitute_image = uval;
2236+
do_invalidate = true;
2237+
}
2238+
} else if (name == "colorconfig" && type == TypeDesc::STRING) {
2239+
ustring uval(*(const char**)val);
2240+
if (uval != m_colorconfigname) {
2241+
m_colorconfigname = uval;
2242+
do_invalidate = true;
2243+
}
2244+
} else if (name == "colorspace" && type == TypeDesc::STRING) {
2245+
ustring uval(*(const char**)val);
2246+
if (uval != m_colorspace) {
2247+
m_colorspace = uval;
2248+
do_invalidate = true;
2249+
}
22012250
} else if (name == "max_mip_res" && type == TypeInt) {
22022251
m_max_mip_res = *(const int*)val;
22032252
do_invalidate = true;
@@ -2339,6 +2388,14 @@ ImageCacheImpl::getattribute(string_view name, TypeDesc type, void* val) const
23392388
*(const char**)val = m_substitute_image.c_str();
23402389
return true;
23412390
}
2391+
if (name == "colorconfig" && type == TypeDesc::STRING) {
2392+
*(const char**)val = m_colorconfigname.c_str();
2393+
return true;
2394+
}
2395+
if (name == "colorspace" && type == TypeDesc::STRING) {
2396+
*(const char**)val = m_colorspace.c_str();
2397+
return true;
2398+
}
23422399
if (name == "all_filenames" && type.basetype == TypeDesc::STRING
23432400
&& type.is_sized_array()) {
23442401
ustring* names = (ustring*)val;

0 commit comments

Comments
 (0)