This patch adds a new test to the CI matrix: verifying that we do not
break the ABI during releases.
Basically, in addition to building the current checkout, it also builds
a checkout of the designated "ABI standard" (generally, the first
non-beta release of a major version), then generates ABI dumps of both
versions and runs the ABI compliance checker using the tools from
https://github.com/lvc
Making two builds sounds expensive, but both builds are rigged to be
minimal, since only the main two libraries are needed for ABI checks (we
can turn off the binary tools, unit tests, python bindings, and support
for many formats), and of course the test suite need not run. So
actually, it's less expensive than most of the test matrix entries, and
it helps ensure that we don't accidentally break ABI when we don't mean
to.
I'm not sure how many false positives we'll have. We might need to
adjust the methodology slightly as we gain experience with this.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When writing out EXIF headers in PNG files, the `tiff_version` field
wasn't handling endianness properly.
This lead to some tools like `exiv2` refusing to process the file at
all[1]
[1] https://github.com/Exiv2/exiv2/issues/2745
---------
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
New theory of code examples in the docs: every one should live in the
testsuite somewhere, and merely be included by reference in the docs.
This ensures that every code example works, stays current with the
evolution of the APIs, and never breaks. It also beefs up our test cases
for more thorough code overage in the testsuite.
I've done this with just two examples here, but hopefully it serves as
an example for others to -- bit by bit -- convert all the other code
examples in the docs into tests in a similar manner.
Doing this for any one example is a perfect "good first issue," since
it's bite sized and can easily be done in a day, it doesn't require any
knowledge of deep OIIO internals, and in the process it also teaches you
something about how both the docs and the testsuite are set up.
A few notes:
* I'm structuring it (for now?) as two testsuite entry for each chapter
of the documentation (one for C++ and one for Python).
* Note how I can have many code snippets in a test source file, and use
the Sphinx `:start-after:` and `:end-before:` modifiers of
`literalinclude` to clip particular subsections of the file, using
special comment markers in the code.
While I was there, I also fixed some typos I noticed in the text of the
chapter, oops.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
To my eyes, these two constructors look like they should be treated as
doing exactly the same thing:
MyType()
: mymember(...)
{ }
and
MyType()
{
mymember = ...;
}
But nvcc in Cuda mode says no, if the constructor is constexpr, the
former is okey dokey, but latter gives a warning. Fine, Cuda, whatver
you say.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
While testing out the 2.5 beta for use in Blender, I hit an odd build
error[1] apparently related to include order. Apparently `windows.h`
should be included before `shellapi.h`[2]. Swapping the order does fix
things, however, it turns out neither is used in this particular file so
just remove them both. Unsure why we didn't see it until now.
[1]
```
20>[9/146] Building CXX object src\libutil\CMakeFiles\OpenImageIO_Util.dir\strutil.cpp.obj
20>FAILED: src/libutil/CMakeFiles/OpenImageIO_Util.dir/strutil.cpp.obj
20>cl : Command line warning D9002: ignoring unknown option '/arch:sse2'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): error C2146: syntax error: missing ';' before identifier 'DECLSPEC_IMPORT'
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(79): error C2065: 'HDROP': undeclared identifier
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(82): error C2086: 'int EXTERN_C': redefinition
20>C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um\shellapi.h(68): note: see declaration of 'EXTERN_C'
```
[2]
https://stackoverflow.com/questions/2953704/vs2010-lots-of-errors-when-including-standard-librarieshttps://social.msdn.microsoft.com/forums/vstudio/en-us/ed93ae48-969a-4280-8fd5-ef57a332022b/problems-with-shellexecute-and-shellapih?forum=vcgeneral
## Checklist:
<!-- Put an 'x' in the boxes as you complete the checklist items -->
- [X] I have read the [contribution
guidelines](https://github.com/OpenImageIO/oiio/blob/master/CONTRIBUTING.md).
- [ ] I have updated the documentation, if applicable.
- [ ] I have ensured that the change is tested somewhere in the
testsuite
(adding new test cases if necessary).
- [ ] If I added or modified a C++ API call, I have also amended the
corresponding Python bindings (and if altering ImageBufAlgo functions,
also
exposed the new functionality as oiiotool options).
- [X] My code follows the prevailing code style of this project. If I
haven't
already run clang-format before submitting, I definitely will look at
the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
Bump fmt minimum to 7.0 for OIIO 2.5. fmt 7.0 is 3 years old, so let's
stop doing extra work to support 6.x. It's not that we depend on newer
features per se, but I'd like to no longer need separate reference
output for some tests whose formatting behavior changed slightly between
6.x and 7.0.
Rig CI to properly test "latest" and "bleeding edge" fmt. It turns out
we had not yet been testing 10.1 or keeping up with their master.
In the process, I discovered that the combo of fmt >= 10.1 and gcc >= 11
results in a mangled AVX math heisenbug, symptomatic in our simd_test. I
struggled with this on and off for many days, having great difficulty
reproducing (though it does fail every time and deterministically, I
just can't seem to narrow it to a smaller example). It only happens with
gcc, only gcc >= 11, only when we use FMT_EXCEPTIONS=0 to disable true
exceptions in fmt. The change happens specifically at fmt commit 9a034b0
(midway between 10.0 and 10.1), which changes the definition of
FMT_THROW when exceptions are disabled. Why this should affect SIMD math
is a total mystery, and currently I am suspecting either a gcc compiler
error or that it's exposing undefined behavior. I found that redefining
FMT_THROW on our side to something innocuous is a good workaround. If I
no longer disable fmt exceptions, that also makes it work, but I don't
know what that might break for us or downstream.
My report of this to the fmt project is
[here](https://github.com/fmtlib/fmt/issues/3620), maybe they'll uncover
the true cause.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Advanced users know to check error returns and retrieve errors.
New users often just try OIIO calls with no error checks when they don't
get behavior they want, ask questions to the dev team. We hypothesize
that if they only saw the text of the error messages that they never
retrieved, they could probably self-diagnose many of their problems.
This patch makes an ImageBuf, when it is destryed, print any pending
error messages that were never retrieved. It happens by default, but can
be disabled by setting the new global attribute
"imagebuf:print_uncaught_errors" to 0 (it defaults to 1).
This doesn't cover everything, but for ImageBuf and ImageBufAlgo, maybe
it will help some users? It could annoy advanced users who intend to not
retrieve error messages (why?) but still don't want this printed. But
that's who the new attribute is for.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Typo introduced in #3894 broke our ability to build against libheif <
1.13. A conditional compilation that should have kicked in for >=1.16
was mistakenly written as 1.6.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Add documentation for oiiotool --normalize
* Add tests for oiiotool --normalize
* Revamp existing normalize tests in testsuite/python-imagebofalgo: Get
rid of old test images and refs, because their values were already
normalized, so was probably not testing the math. Make new tests that
are much smaller, 3x3 images that contain specific (non-normalized)
values to verify they all get normalized corectly. They'll run a lot
faster, too.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Not unlike the ImageOutput::open_check we added a few months back, this
creates an ImageInput utility function that centralizes a number of
validation checks. Currently, this is primarily a check on allowed
resolution and total size, aimed at making a guess at files that are
corrupted or malicious.
Also adds a new `supports("noimage")` query.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This is Update 4, the original PR is ongoing and we will continue to
update as additional people relicense.
Per PR #3905 (https://github.com/OpenImageIO/oiio/pull/3905), update the
RELICENSING document with the names of people and companies who have
signed on to the relicensing thus far, and update the SPDX license
notices on source files whose extant authorship (per 'git blame')
comprises only authors who have relicensed.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Proposal from Thiago to add Jesse Yurkovich, representing Blender, to
the TSC.
Vote passed by majority of TSC members voting yes on the PR itself.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Avoid possible double define of NOMINMAX in platform.h
* Move other Windows symbol definitions into the cmake file instead of
repeating them in certain .cpp files.
Fixes#3964
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Normalize 3D vectors (i.e., divide by their length) textures.
inCenter and outCenter define 0.0 coordinate in input and output
3D vectors encodings (e.g. 0.0f for [-1,1] or 0.5f for [0,1] encoding).
scale defines the scale factor to apply to the normalized vectors
(i.e. 1.0f for a full range normalization, 0.5f for 0.0-1.0 range
encoding).
Default values are for [-1,1] encoding and full range normalization.
Functions required 3 or 4 channels. If destination have no alpha
channel,
but source have, alpha channel will be dropped.
## Tests
```
int main(int argc, const char* argv[])
{
ImageBuf input(argv[1]);
ImageBuf output;
bool ok = ImageBufAlgo::normalize(output, input, 0.0f, 0.0f, 1.0f);
if (ok)
{
ok = output.write(argv[2]);
}
else {
std::cerr << "Could not normalize image: " << argv[1] << std::endl;
return -1;
}
return 0;
}
```
---------
Signed-off-by: ssh4net <libalias@gmail.com>
Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>
Signed-off-by: Vlad Erium <libalias@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Anton Dukhovnikov <131838425+antond-weta@users.noreply.github.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com>
Fixes#3950
Fix the div-by-0 when handling unassociated alpha during a DXT4 DDS
load. The code mirrors what other formats do by first checking to see if
the alpha value is not 0 before continuing.
Added the 218-byte file from the bug to the local test suite.
---------
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
Enable set of Boost_NO_BOOST_CMAKE through make using
MY_CMAKE_FLAGS=-DBoost_NO_BOOST_CMAKE:BOOL=OFF.
Signed-off-by: Mikael Sundell <mikael.sundell@gmail.com>
The incoming length of the RLE data could be 0. In this case we would
attempt to take the address of the 1st element of an empty vector.
Added a new 1004-byte file to the local test suite.
Fixes#3951
---------
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
We had disabled the use of the "openexr core" library by default, even
for OpenEXR versions that nominally support it, when it was discovered
that it stil didn't support the DWA compression types.
That has finally been added to the OpenEXR trunk as well as the OpenEXR
3.1.10 release. When we are building against a sufficiently new OpenEXR
to fully support all the compression types for exr core, then re-enable
it by default.
Bump the "latest" CI tests to OpenEXR 3.1.11.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Fixes#3952
Several interrelated fixes to guard against corrupted files:
* Extend the `limits:imagesize_MB` sanity check to Targa files to
address #3952 specifically, and check in that failing example file into
the testsuite. (N.B.: Only a couple readers use this, TIFF and now
Targa. it should be extended to the others. Mackerel!)
* Improve the error message used for the above, both for Targa and TIFF.
* For low-memory situations, perhaps the default of 32GB is too large.
Make the default be the minimum of 32GB and 1/2 the size of physical
memory on the machine. (An app that legit expects humongous files is
expected to raise the limit appropriately.)
* The generic "Read error: hit end of file" we sometimes issued from
`ImageInput::ioread()` was hard to discern which reader it came from, in
cases where the file extension was absent or misleading and a succession
of files were tried. By changing this error message to say which format
reader triggered the error, it might make these errors easier to track
down in the future. This also changed the reference output of a few test
involving truncated or corrupted files.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
On overloaded VMs, the timer_test is ureliable and fails spuriously too
often.
Two strategies in this patch, hope some combination does the trick:
1. Increase the absolute error allowed for the timing tests when doing
CI runs.
2. Add just a bit more relative error (via a new unit test macro,
OIIO_CHECK_EQUAL_THRESH_REL).
Hopefully this will cut down on CI failures for unit_timer test case.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Disable GCC warnings around mismatched new/delete for shared_texturesys
creation
Signed-off-by: Shootfast <markboo99@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com>
* Add string literal operators for ustring and ustringhash so you can
write: ``` "foo"_us "bar"_ush ``` Note that on Cuda (but not on the
host), this ustringhash construction is constexpr.
* Add a new `ustringhash(const char*, size_t)` constructor, which is
also constexpr on the device side.
* Mark the existing `ustringhash(string_view)` constructor as constexpr
on the device side.
We can mark those things as constexpr on the device side because on the
Cuda side, the implementation only takes the hash (which is constexpr),
and does not actually add a ustring to the table (which cannot be
constexpr). To make such declarations easy, add a new macro to
platform.h, `OIIO_DEVICE_CONSTEXPR` which is constexpr for the device,
merely inline for the host.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The most important fix is to remove links to the old CLAs and update
references to the mail list to the new one.
Also spruced up some language while I was there, hopefully to coax out
improved PRs and Issues.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When building the code with Xcode ("cmake -G Xcode"), there are 1000+
warnings about implicit 64 to 32 bit conversion.
This change adds a compile option to suppress such warnings when
generating an Xcode project.
Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz>
This is Update 3, the original PR is ongoing and we will continue to
update as additional people relicense.
Per PR #3905 (https://github.com/OpenImageIO/oiio/pull/3905), update the
RELICENSING document with the names of people and companies who have
signed on to the relicensing thus far, and update the SPDX license
notices on source files whose extant authorship (per 'git blame')
comprises only authors who have relicensed.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Text rendering doesn't work with utf-8...
I can't render chinese, arabic... or special chars as éèûîà...
It seems that the string function to convert utf-8 to unicode is quiet
simple.
Fixes#3933
[Note]: OpenImageIO is used in the [gpx2video
project](https://github.com/progweb/gpx2video).
---------
Signed-off-by: Nicolas VIVIEN <nicolas@vivien.fr>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com>
I think it will be handy to be able to have ParamValue contain
ustringhashes and have a TypeDesc that can describe it explicitly.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Rationale:
* LibRaw 0.18 was released in last 2016, so we are stil supporting all
libraw from up to 6 years back.
* LibRaw 0.18 is necessary for ACES, plus it improves a lot of metadata
handling.
* LibRaw 0.20 is the minimum that can be used when compiling with C++17,
and we will almost certainly move to C++17 as a minimum in OIIO 2.6, so
it seems like LibRaw 0.18 is reasonable minimum for the upcmoming 2.5.
Remove some cruft that's no longer necessary if we drop support for
0.15-0.17.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This bump brings access to the following interesting features (not a
comprehensive list):
- 3.13 Nov 21, 2018
- -B, -S let you specify source and build directories without the "cd" dance
- target_link_options()
- ctest --progress / CTEST_PROGRESS_OUTPUT
- 3.14 Mar 14, 2019
- cmake --build now supports -v/--verbose
- file(CREATE_LINK )
- FindGIF now has imported targets
- 3.15 Jul 17, 2019
- env CMAKE_GENERATOR can be used instead of -G
- when using --build, the --target can now list multiple targets (and also can be `-t`)
- --install can be used separately without running through the whole build
- message() now supports NOTICE, VERBOSE, DEBUG and TRACE.
- CMAKE_FIND_PACKAGE_PREFER_CONFIG variable
- Enable C++20 for Apple Clang
Make sure all CI cases have a new enough cmake
Use GIF targets, now enabled with CMake 3.14
Use message(VERBOSE ...) feature to simplify lots of `if` blocks.
Clean up code for older cmake versions
Note that this will NOT be backported to the current release branch,
but it will be part of the upcoming 2.5 family of reseases.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When an incoming 1-dimensional, buffer-like, object was used in Python,
the corresponding check inside OIIO would overflow if its length
exceeded a signed 32bit value.
An error similar to `Pixel data array error: Python array shape is
[2304000000] but expecting h=24000, w=24000, ch=4` would be raised.
I modified a few similar calculations and tried to follow the
predominant pattern already used.
Tested using the following snippet which would previously fail:
```
import OpenImageIO as oiio
import numpy as np
filename = "t:/foo.png"
xres = 24000
yres = 24000
channels = 4 # RGB
pixels = np.zeros(xres * yres * channels, dtype=np.uint8)
spec = oiio.ImageSpec(xres, yres, channels, "uint8")
image = oiio.ImageOutput.create(filename)
image.open(filename, spec)
if not image.write_image(pixels):
print("error = ", image.geterror())
image.close()
```
---------
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
Per PR #3905 (https://github.com/OpenImageIO/oiio/pull/3905), update the
RELICENSING document with the names of people and companies who have
signed on to the relicensing thus far, and update the SPDX license
notices on source files whose extant authorship (per 'git blame')
comprises only authors who have relicensed.
This is part 2, the original PR is ongoing and we will continue to
update as additional people relicense.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Intel's servers have again updated the default icx that you get when you
install from their yum repository, and the new one seems incompatible
with our libstdc++ toolchain. So lock down on 2023.1.0 for now so that
our CI can pass.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Optimises the code in the tile hash function. During a render this
function is called billions of times so this small improvement adds up,
in some test scenes it reduces total render time by an average of about
2%.
See https://godbolt.org/z/jMzz4PTce for the exact details. A summary
would be:
- Reduced instruction count from 75 to 61, this is due to the redundant
add and shift operations that can be implicitely handled by taking
advantage of the member layout in the class.
- Halves the number of memory fetch instructions because all accesses
are now 64-bit.
- Removes branching instructions and extra memory indirection inside
`ustring::hash()`.
There is a slight change in behaviour, which is that the hash is now
computed from the `m_file` pointer rather than from
`m_file->filename().hash()`, so runtime reproducibility is lost. However
the performance gain seems worth it.
Signed-off-by: Curtis Black <curtis.w.black@gmail.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com>
This updates ICOInput to use the IOProxy model so it can read from files
or the IOMemReader
Signed-off-by: Jason Baumeister <jason@jasonbaumeister.com>
During the CMYK to RGB conversion in PSDInput, the alpha channel was
never copied to the RGBA output. Converting a PSD to JPEG worked because
it ignored all the 0 alpha values, but converting to WebP would result
in a fully transparent image.
I added a cmyk-with-alpha.psd test to the psd-colormodes test to check
this
Signed-off-by: Jason Baumeister <jason@jasonbaumeister.com>
Clean up the `#if` tests that disable Intel SIMD when not on Intel
architectures, which incorrectly disabled ARM NEON when on ARM.
Fixes#3909
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Update the ImageInput documentation to not use the deprecated
`read_image` APIs.
Based on the discussion in #3902
---------
Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com>
Per PR #3905 (https://github.com/OpenImageIO/oiio/pull/3905), update the
RELICENSING document with the names of people and companies who have
signed on to the relicensing thus far, and update the SPDX license
notices on source files whose extant authorship (per 'git blame')
comprises only authors who have relicensed.
This is part 1, the original PR is ongoing and we will continue to
update as additional people relicense.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Our own FindWebP.cmake was setting up targets that didn't
match the capitalization of the cmake configs that come with WebP itself.
Signed-off-by: Benjamin Buch <benjamin.buch@technoteam.de>
This batch is for files that LG (relicensed!) has contributed, in
addition to >2 other authors. Also add appropriate license notices to
some that had no posted license before due to oversight.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Add copyright/license notices to some mixed-authorship files that
lacked them before (all related to tests)
* Get rid of the pointless year in many copyright notices (change to the
current LF recommended wording)
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Since LG has re-licensed all his old code under Apache-2.0, we can
change the license notice from BSD to Apache-2.0 for all files in which
LG is the only extant author. Also change all files that LG co-authored
with other people to specify "BSD-3-Clause and Apache-2.0".
Some files (mostly test scripts) had no license notice, so in those
cases, we added one.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
By majority vote of the TSC, we are changing OpenImageIO's main
incoming/outgoing open source license from BSD-3-Clause to Apache-2.0.
New code submitted from July 1, 2023 will be contributed under the
Apache 2.0 license.
Old code submitted prior to July 1 2023 is under the original BSD
3-clause license, unless relicensed per the process described in the new
RELICENSING.md file. We hope to get the vast majority of the code base
relicensed quickly and we will document which set of files contain a
mixture of licenses.
Instructions to everybody reading this:
* If you are contributing code under the authorization of an employer,
please let them know we are changing to Apache 2.0 and make sure that
you are authorized to make all future contributions to under that
license.
* If you made prior contributions to OpenImageIO over the years and are
willing to relicense your code under the Apache 2.0 license, please
submit a PR adding your name to the list in RELICENSING.md. If you were
working for a company that might own your IP output at the time you made
those contributions, please also ask somebody there (who has the
authority to do so) to submit a PR adding the company name as well,
indicating that they are re-submitting all of the old contributions
under the new license.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
By unanimous vote of the OpenImageIO TSC, as of July 1, 2023, we
adopt the technical charter, Code of Conduct, new CLAs, and GOVERNANCE
document.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The underlying farmhash algorithm (which is the basis for
Strutil::strhash, and thus ustring's hash representation) turned out to
not be properly constexpr under certain circumstances and needed to be
expressed a different way to be fully constexpr on all platforms and for
all strings.
New implementation suggested by Alex Wells.
Also, make sure string_view ctr from char* can be constexpr, even for
C++14. This is related for the sake of the strhash calls that, when
taking string literals, actually choose the variety that accepts a
string_view.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
repology has a handy tool that will show all the package distros
and which version of openimageio they supply.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Remove leftover logic from when we used TravisCI.
* Pick a default for LLVM_DISTRO_NAME so it works when the script is run
standalone when not on GitHub Actions CI. (But it's still really only
meant to run on x86 Ubuntu, and not designed for anything useful outside
of our CI.)
* Eliminate all the complexity with old LLVM versions -- in practice,
our CI only uses this to download fairly modern clang versions.
Fixes#3891
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The strukturag repo for libheif recently upgraded the version it was
serving from 1.14 to 1.16, and that caused our tests to fail because (a)
some of the codec support has been moved to plugins that are loaded at
runtime and those are now distributed in packages we need to install
separately; (b) you must call heif_init() or it won't know about the
optional plugins.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
If colorconvert, ociolook, ociodisplay, ociofiletransform fail
because OCIO support is not enabled, make sure the error message
makes that clear.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Make sure we can input exif tags as strings containing numbers when
number are required.
E.G. if my input is "Orientation"=>"8", i would like the jpeg output to
contains the value 8 for the exif tag orientation.
PR #2147, addressing issue #1844, is related to an apparent OpenEXR bug
wherein single channel tiled images where the tile dimensions were not a
power of 2 and >= 16 would not work properly with dwa compression. We
work around the problem by automatically substituting zip compression in
those cases. But it was implemented with a subtle bug, forgetting that
tile_size == 0 for untiled images, and thus we were silently switching
from dwa to zip compression for all *untiled* single channel images of
any size (which presumably were never problematic). This patch fixes the
test so we are not changing the compression for the untiled files.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
J2K output was defaulting to UINT8 when asked to output an unsupported
type. It should have been UINT16 to try to minimize the amount of
precision loss.
Fixes#3876
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Everybody please go to https://lists.aswf.io/g/oiio-dev and sign up for
the new mail list as soon as possible. The old list is on its last legs
and having trouble, let's get this done.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This change adds a `shutdown()` method to the API that can be used to
perform shutdown of the thread-pool(s) (or other clean-up activities).
Applications that utilize OpenImageIO are advised to call it before exiting their
main function to prevent possible deadlocks during application shutdown
(see #3851)
This change also updates the documentation and all oiio-tools to call
the new `shutdown` method.
This is the final change for fixing #3851
Part of solving #3851.
When decreasing the number of threads in `thread_pool::Impl::resize` it
was possible for the threads marked for termination to still be fully
alive at process exit time if the call to `resize` and the exit of the
main function were close enough to each other.
This change makes `resize` wait for the threads to actually join
guaranteeing the threads have been shut down in a orderly manner when
the `resize` function exits.
This cleanly terminates all worker threads of the default thread pool
by resizing the pool to 0 threads... but only if the pool has been
created in the first place. Note that the alternate idiom
default_thread_pool()->resize(0);
seems like it should accomplish the same thing, but in fact will start
by creating a pool and its threads, if it has not done so before, only
to immediately terminate them.
---------
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When reading a PSD file, if it contains a thumbnail, we read it from the
in-memory blob with a combination of an IOMemReader and an ImageBuf.
But... we always named it "thumbnail.jpg" without considering that
multiple simultaneous PSD files reading identically named images (but
which are in fact different) could cause a weird kind of clashing in any
underlying use of ImageCache!
The simple fix is to use ordinary ImageInput approach to reading the
thumbnail from the IOMemReader, but just store it in the ImageBuf.
Fixes#3824
Signed-off-by: Larry Gritz <lg@larrygritz.com>
I got the test backwards, and it was incorrectly hiding the definition
for the default case of OIIO_DISABLE_DEPRECATED == 0.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
A variety of minor documentation fixups:
- Use Python 3 style 'print' statements consistently everywhere (some
snippets had a mix of both)
- Correct the usage of `ImageSpec::channels` that should be
`ImageSpec::nchannels`
- Template syntax errors in two C++ snippets
- Document the new default for `max_memory_MB`
Background info: oiiotool multithreads *within* any individual image
operation (such as `--over`), but a series of operations comprising the
oiiotool command are done serially -- each command finishes before the
next starts. And similarly, for multi-frame operations using frame
ranges, frame number wildcards, or view wildcards, the set of commands
for each frame runs to completion before the next begins. For complex
oiiotool commands, especially when running over frame ranges, this is
not a particularly efficient way of parallelizing, since any serialized
operation (which often includes the expensive writing of the result
image from each frame to disk) can substantially serialize the entire
run.
This PR adds a new oiiotool --parallel-frames option that assumes all
frames in the range can be computed independently and in any order, and
can therefore be run in parallel. As long as there are a similar number
(or more) of frames versus cores, and you have enough RAM to handle that
many image pipelines at once, this is a more efficient way of
parallelizing the operations than multithreading individual image
processing operations.
Benchmarking: On my 32-core workstation, I tried the following oiiotool
command, contrived but representative of a reasonable oiiotool command
complexity that might be a production task:
oiiotool --runstats -v --frames 1-100 tmp/noise.exr -cut
2880x1620+960+540 -resize 1920x1080 -colorconvert lin_srgb srgb -d uint8
-o "tmp/test.#.tif"
The noise.exr file was a previously created 4k x 3channel 'half' exr
image. The command loads the file, cuts out a 3k section of it, resizes
that to HD res, does a color space transformation, then outputs as an 8
bit TIFF file, for each of 100 sequential frames.
threads real time user sys peak memory
1 4:44 4:23 0:21 122 MB
2 2:58 4:24 0:29 122 MB
4 1:40 4:36 0:29 122 MB
8 1:10 5:00 0:30 127 MB
16 0:56 5:28 0:37 131 MB
all(32) 0:55 8:12 0:54 127 MB
parallel 0:25 8:13 1:04 3.4 GB
Here we can see the diminishing returns of using multiple threads within
each frame, where for low thread counts we are improving steadily, but
we pretty much peak at 16 threads, seeing virtually no further gain at
32, and maxing out at about 5x the single threaded performance. This is
due to serialization of the I/O and increasingly bumping into various
locking at higher thread levels.
However, using `--parallel-frames` gives us another doubling of total
throughput (giving us a total of 11.3x speedup versus single threaded),
though at the expense of 28x the memory use! That memory explosion is
acceptable in this case, even 3.4 GB is really nothing on a modern
machine, but we can imagine that other possible oiiotool command lines
might be more memory intensive and having dozens of frame pipelines
computing simultaneously would have problematic memory use. So use this
feature carefully and with purpose, always checking that your own
workflows experience sufficient speedup and don't use more memory than
you have available.
There's some code I put in not long ago to automatically call exr's
thread pool setNumThreads(0) upon exit, as an attempt to work around
some exr thread pool deadlock conditions. But... we've seen some
weirdness from this workaround as well, so try to limit its application
by not bothering when using OpenEXR >= 3.1.8 (at which point we believe
the exr side bug was fixed) and also to do so only on Windows.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Various improvements to OpenCV support:
* IBA::from_OpenCV and to_OpenCV now set error messages for every type
of failure they encounter, instead of returning false with no
explanation. For from_OpenCV, the error message can be retrieved from
the ImageBuf returned; for to_OpenCV, the error message can be retrieved
from the global OIIO::geterror(). Several assertions were removed,
instead relying on error return codes and messages.
* Add global OIIO attribute "opencv_version" which allows you to find
out what version of OpenCV OIIO was built against, which was previously
impossible to know without seeing the build-time status messages. This
can be retrieved via
OIIO::get_int_attribute("opencv_version")
and also you can find out from the command line with
oiiotool --echo "{getattribute(opencv_version)}"
The return value is an integer encoded as `10000*major + 100*minor +
patch`, and it will return 0 if no OpenCV support was enabled when
OpenImageIO was built.
* For OpenCV >= 4.0, to_OpenCV can now correctly construct a 'half'
matrix. For OpenCV 3.x, we turn it into a float matrix as before, since
CV_16F was only supported starting in 4.0.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Just a routine update of these files as I proofread them in light of the
move to ASWF. This patch is addressing the "old" project, there will be
further ASWF-specific edits for changes that will take effect after the
project is fully moved.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
In `jpegoutput.cpp` when computing the density fields the relationship
between `XRes`, `YRes` and `aspect` is `aspect = XRes / YRes`.
However this relationship was not respected in one specific case, when
`aspect >= 1.0` and `XRes` and `YRes` are not set.
This PR solves this problem by enforcing the correct relationship.
Related to: https://github.com/OpenImageIO/oiio/issues/2822
The Intel yum repo renamed this package a couple weeks ago, and it broke
our CI. Update to the new name.
Adjust fmt use to solve icc warning.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
When reading a raw image, setting the `user_flip` to 1 meant ignoring
the `flip` value (to avoid buffer rotation from LibRaw).
However the value 1 for the `user_flip` is **not neutral**: values lie
between 0 and 7 (see
https://www.libraw.org/docs/API-datastruct-eng.html#libraw_image_sizes_t),
and 1 corresponds to a horizontal mirror (in my understanding of the
source code).
The proposed fix is to use 0 as `user_flip` value for ignoring the
`flip` value.
This modification invalidates the changes made in this PR:
https://github.com/OpenImageIO/oiio/pull/3847.
Therefore, flip to orientation conversion has been adapted to match
exactly LibRaw's documentation, which handles only rotations (+90, +180,
-90 degrees).
The previous PR was only tested on images of landscape and human faces,
hence the mirroring issue remained unnoticed.
When reading images containing text however the problem is clearly
visible.
This PR fixes this mirroring issue on the following 4 orientations: 0,
+90, +180 and -90 degrees rotation.
Other orientations could not be tested as they don't seem to ever appear
on raw images datasets.
Make all oiiotool actions take ot as a parameter.
This is a necessary first step to being able to parallelize over frame
ranges.
Also finally convert the last of the `int argc, const char**argv` idioms
to `cspan<const char*>`.
When reading a raw image while having a `user_flip` set to 1, we set the
orientation Exif tag to LibRaw's flip value (so that LibRaw doesn't
rotate and/or mirror the buffer but the image still gets displayed
correctly).
However LibRaw's convention for flip values differ from Exif orientation
values: https://www.libraw.org/comment/5176#comment-5176.
In this PR we fix this issue by converting LibRaw flip values to Exif
orientation values. This conversion is based on:
- https://www.libraw.org/comment/5178#comment-5178 (conversion proposed
by LibRaw's maintainer)
-
https://github.com/LibRaw/LibRaw/blob/6fffd414bfda63dfef2276ae07f7ca36660b8888/src/write/file_write.cpp#L22
(LibRaw's function for applying the flip).
Tested on:
- CR2 images with flip values 1, 5 and 6
- ARW images with flip values 1 and 5
(unfortunately I could not find any image covering other flip values).
Since the conversion is `[1, 2, 3, 4, 5, 6, 7, 8] -> [1, 2, 4, 3, 5, 8,
6, 7]`, the only test that came out with different results before and
after the change is the one with flip value 6:
in that case, the image orientation got corrected by the fix.
Following up #3826, this implements efficient `valid_file` support for
DDS, DPX, PSD, HDR, and WEBP.
The quick magic/signature checks were abstracted into small helper
functions where necessary to eliminate duplicate code.
For WEBP, some redundant checks were also removed and there's also a fix
for a `uint64_t` vs `size_t` check which would have caused issues on
32-bit builds.
The PSD changes exposed an issue with array attributes. During a normal
ImageInput create+open sequence, an ImageInput may be opened then closed
and then opened again.
Array attributes are not cleared as part of close and so their contents
will be doubled up next time they are read into the existing `m_spec`.
This PR does not fix that general problem but now that PSD has a proper
valid_file which doesn't do an open, the array attributes are now
correct:
```
This PR
2023-05-06T02:32:05.8774584Z - photoshop:ColorMode: 3
2023-05-06T02:32:05.8774888Z - photoshop:ICCProfile: "sRGB IEC61966-2.1"
2023-05-06T02:32:05.8775194Z - photoshop:LayerName: 1, 2, 3
2023-05-06T02:32:05.8775436Z - photoshop:LayerText: 1, 2, 3
vs. the current
2023-05-06T02:32:05.8775725Z + photoshop:ColorMode: 3, 3
2023-05-06T02:32:05.8776023Z + photoshop:ICCProfile: "sRGB IEC61966-2.1"
2023-05-06T02:32:05.8776494Z + photoshop:LayerName: 1, 2, 3, 1, 2, 3
2023-05-06T02:32:05.8776746Z + photoshop:LayerText: 1, 2, 3, 1, 2, 3
```
Fixes#3837
oiiotool already attempted to trim extra channels when outputting to a
format that can't support it. It correctly handled the two common cases:
* More than 4 channels for formats that don't have
`supports("nchannels")` will drop after the first 4 channels.
* More than 3 channels for formats that don't have `supports("alpha")`
will drop after the first 3 channels.
But this omits an interesting (but rare) corner case: a 4 channel file
where the last channel is NOT alpha, and the output file supports an
alpha channel but not arbitrary channels. A concrete example might be an
input file that is exr with channels R, G, B, Z, writing to a PNG file
(which can support RGB or RGBA). We wouldn't want to write what appears
to be an RGBA file if that last channel isn't really alpha.
Also, noticed that oiiotool --chnames didn't properly clear the
alpha_channel and z_channel flags, which is important if the new channel
lists drop one of those channels.
fmt 10.0 was recently released, and it breaks a lot for us because they
have fully removed legacy support for automatically being able to format
anything that has an iostream `<<` operator. This facility was deprecatd
in 8.1 but could be re-enabled with FMT_DEPRECATED_OSTREAM, but even
that was removed in 10.0.
So this means that we had to write several explicit format handlers for
some types for use with format() and print().
Also, they are even more strict with sprintf/printf which don't honor
the custom formatters as far as I can tell, so in several cases I also
switched those to the std::format/print style.
Fixes#3835
As Fabrice Macagno points out on the mail list, we weren't setting the
correct variable name CMAKE_CXX_VISIBILITY_PRESET.
Fixing this -- and finally hiding all the symbols correctly -- uncovered
an instance where testtex was relying on some classes that should have
been hidden in libOpenImageIO. For symbol visibility reasons, move the
functionality of testtex's test_hash() function to be in TextureSystem
itself. This is a much cleaner separation.
We have some OIIO 1.x era type aliases float4, int4, etc., for the
simd types that have long since been renamed vfloat4, vint4, etc.
Hide those aliases when buiding OIIO itself, to be sure we aren't
inadvertently using them. And indeed, in one spot, we were! (Fix it.)
Introuce a new preprocessor symbol, `OIIO_DISABLE_DEPRECATED`, which
if set to an encoded version number (such as 20100 for version 2.1) by
a downstream package, indicates that we are permitted to hide
declarations that were considered deprecated from that version
on. This is an aid to downstream projects who wish to enforce that
they are not inadvertently using any deprecated OIIO calls which may
disappear in a future release.
So back to the deprecated simd names: Also hide them if
`OIIO_DISABLE_DEPRECATED` is defined to exclude things that were
deprecated as of OIIO 1.9+, which apparently is when we changed this
definition.
I'm honestly not sure why some GHA CI jobs suddenly started warning
about this in the past day or so. Something must have updated on their
end. But switching to the safer Strutil equivalents fixes it.
`ssize_t` is a posix type, msvc seemingly doesn't know about leading to
a build error with MSVC 2019
usage elsewhere in this files used the `py::ssize_t` type, which seems
to fix this
`ssize_t` is a posix type, msvc seemingly doesn't know about leading to
a build error with MSVC 2019
usage elsewhere in this files used the `py::ssize_t` type, which seems
to fix this
`make_texture()` already took a configuration option "maketx:forcefloat"
(defaults to 1), which if set to 0 disables the conversion to a float
buffer for intermediate computation.
For `oiiotool -otex`, there was no way to specify this. This patch
allows it to be disabled with the new optional modifier
`-otex:forcefloat=0`.
Also, in make_texture, this option only controlled the initial
conversion of the whole buffer to float, but the downsized levels all
were float either way. This patch now uses the forcefloat config option
to also control the buffers used for downsizing.
Disabling forcefloat like this has a speed penalty (the downsizing math
on float buffers is faster than going in and out of float for every
indvidual value) and loses some precision in the process of repeatedly
downsizing to generate the MIP levels. But it can sure save a lot of
memory if you have a ginormous uint8 image and want to avod the 4x
balooning of memory. I don't recommend disabling forcefloat except for
images that can't fit into memory for the texture creation without it.
But when this is the right hammer to use, it can be the difference
between being able to create the texture, or not.
Some other improvements:
* `oiiotool -otex` wasn't outputting ANY status output during texture
output as maketx does, and for that matter, -v and -debug were not being
influencing the behavior of make_texture at all despite it having such
support. So fix to pass down the right flags. Now there are proper
verbose status and debug messages when requested.
* Eliminate useless call to is_monochrome by calling it only when the
previously-computed pixel stats indicate that the average value of all
the channels is the same (if they don't have the same average, there's
no way the pixel-by-pixel values will be the same in all channels for
all pixels). This saves an extra traversal of all the pixel memory
during texture creation.
* Use synchronized/flushed status messages so users can see them as they
are issued for very long maketx tasks. This also included some
conversion from old iostream stuff to fmt-based print.
* For verbose/debug, more fine-grained timing for make_texture, in
particular, print the time spent downsizing and writing each MIP level.
ABI-BREAKING CHANGE!
Every format that has an existing `valid_file(std::string)`
implementation, and which supports an
ioproxy, now also has a `valid_file(IOProxy)` implementation for
completeness.
The existing std::string overloads have been implemented in terms of the
IOProxy overload where
possible to cut down on the boilerplate. This is done at the base of the
hierarchy in ImageInput.
The remaining formats that support ioproxy, but do not currently have a
`valid_file` method, will be
submitted in a follow-up PR as the additional logic required for those
will require closer review.
The PSD reader was written 12 years ago by a GSOC student and has
changed little since. The following is not a knock on that student, who
clearly did a good job if his code has survived mostly unomdified for
over a decade. In chasing down problems with handling extremely large
psd files, I've noticed some oddities and inefficiencies that need
fixing. It's not an emergency, so I'll handle it in pieces.
The item addressed by this patch is memory usage and thread concurrency.
It turns out that the reader allocates memory for the scanline reads as
it goes... and basically keeps it in the ImageInput until it's
destroyed, even though it will never actually be used again. I guess
it's just trying to save the next allocation if it reads the same
scanline again or on the next subimage? But this means that if you read
a really big psd image, it's basically doubling the memory use -- it
reads one copy into the user's buffer, but also holds on to this extra
copy for no reason. Also, a bunch of the decoding helper functions
implicitly use this memory, which means we lock a mutex for the duration
of read_native_scanline (which in theory should not be necessary since
all I/O is happening through thread-safe IOProxy calls).
So this patch does the following:
1. Remove both this retained scanline memory as well as some other
scratch memory from the ImageInput, and make it allocated and freed
local to read_native_scanline.
2. Change all the helper methods that used it implicitly so that they
are passed buffers to use. In many cases these have been changed to
`const` methods and in a few cases to `static`, making it much easier to
assure ourselves that they don't modify (or sometimes use at all)
anything in the PSDInput to do their task.
3. The result of these changes means that read_native_scanline doen't
need to modify any of the data methods of the class (though it does use,
without modifcation, some things set up in the course of open). Because
it's now non-mutable, we can also remove the mutex, now allowing
multiple threads to read scanlines from the same file simultaneously
without blocking each other.
4. Some other miscellaneous cleanup, including that some scratch memory
was allocated as std::string (!), whereas it should really be a
`vector<unsigned char>` or `unique_ptr<unsigned char>`, and also
changing some computations from double to float.
There is still some more work to do after this is merged. In particular,
I think now that scanline read is stateless / non-mutable with respect
to the PSDInput object, this is a candidate for adding a multi-scanline
read_native_scanlines method (replacing the default implementation of
looping and calling read_native_scanline serially for each scanline),
which should also be able to read multiple scanlines with some overhead
and allocations done once per block, and with blocks of scanlines
running in parallel using the thread pool. This should be a huge
efficiency gain when reading psd files. But it will wait until the next
patch.
The latest version of libfmt began using the `__float128` type but this
is [not supported in the NVPTX backend for
LLVM](https://discourse.llvm.org/t/compiling-cuda-code-fails/61240/13).
A way to detect this particular platform seems to be to check both
`__CUDA_ARCH__` and `__clang__`.
I am not disabling it entirely on any CUDA target because NVCC 10+
understands `_float128` and generates instructions for it when tested on
godbolt.
Following up from https://github.com/OpenImageIO/oiio/issues/3809.
Co-authored-by: Edoardo Dominici <edoardod@sy1wse0003.al.com.au>
Windows isn't smart about symbolic links. There were a few places in the
testsuite where we used symlinks to essentially de-duplicate test
reference images that were expected to be identical. But then the
resulting archive you get when you ask GitHub for a zip file of the
whole repo (which some people download to get OIIO source rather than
git clone) can't be unzipped correctly on Windows (at least not by
cmake's unzip command).
OK, so just change those links to full copies.
Fixes#3811
The `openexr-images` repo was never updated after the default branch was
renamed to `main`. This will fix the following silent errors in CI:
```
Cloning into '/__w/oiio/oiio/build/testsuite/openexr-images'...
warning: Could not find remote branch master to clone.
fatal: Remote branch master not found in upstream origin
```
Additionally, the cmake attempted to symbolically link local repos like
`oiio-images` if they were found. Unfortunately this type of link will
fail unless running as admin on Windows (which is rare) and the
COPY_ON_ERROR directive did _not_ work here (cmake 3.24; weird;
sigh...). The output will now look something like the following:
```
-- Linking oiio-images from C:/Users/jesse/source/oiio-images
-- Link result failed to create symbolic link 'C:/Users/jesse/source/oiio/build/testsuite/oiio-images': A required privilege is not held by the client.
-- Copying oiio-images from C:/Users/jesse/source/oiio-images
```
We know that there are some corner cases on Windows when it can be
unsafe to exit because the Ilm thread pool (which is used implicitly by
OpenEXR) hangs. The exact conditions that cause this are not well
understood.
Some time back, in PR 3582, we added an explicit shutdown in oiiotool
right before we exit, to alleviate the problem. But this doesn't help
for other apps like idiff, nor for apps that use libOpenImageIO and
aren't aware of the problem, so wouldn't know to do this.
This patch changes to a different strategy: In set_exr_threads(), which
will always be called any time we use openexr files, use `std::atexit()`
to ensure that when the program terminates -- any program using
OpenImageIO in cases when openexr was used -- we set the IlmThread
global thread pool size to 0 to shut it down cleanly. Hopefully, that
will cover even more cases where this OpenEXR bug might be tickled.
When decoding RLE data, some arguments to decompress_packbits() were
inexplicably declared uint16_t (they were passed uint32_t arguments, and
I honestly don't know why we didn't get a warning all along). This
prevented us from correctly reading PSD images whose width was more than
64k, which I suppose are rare enough that it's not hard to understand
why this went unreported for so long.
But anyway, just changing those parameters to uint32_t does the trick.
I ran into the problem described in #2929 and did a little digging to
see if I could find the source of the bad color conversions. I tracked
it down to `fast_exp2()`
https://github.com/OpenImageIO/oiio/blob/a77e36250fadfb4c391688eb1df4b3dd88d5e367/src/include/OpenImageIO/fmath.h#L2050-L2080
and in particular the return statement on L2071. In the process of
trying to figure out what part of that return statement was going awry,
I found that separating the bitcasts to separate variables as in this
PR, the problem went away. All testing was done with the `color_test`
added in #3023.
It also succeeded when adding various `std::cout` lines to check what
the values of each computation were, so it does sound like a compiler
optimization that does or does not get triggered is the problem.
Unfortunately I don't know why it's not failing on the CI builds, I
double checked to make sure I'm using the same settings and flags. I
also tested on two different machines and get the same failures, and
success with this patch, on both.
One strange wrinkle is that even without this patch, color conversions
worked fine when using AVX2 for the SIMD mode.
Summary :
Previous results (without this patch)
- USE_SIMD=0 - PASS
- USE_SIMD=SSE4.2 - FAIL
- USE_SIMD=AVX - FAIL
- USE_SIMD=AVX2 - PASS
With this patch
- USE_SIMD=0 - PASS
- USE_SIMD=SSE4.2 - PASS
- USE_SIMD=AVX - PASS
- USE_SIMD=AVX2 - PASS
Visual Studio 2019 versions 16.11.32228.343, 16.11.33328.57 and
16.11.33423.256(same as current Github build runners) in RelWithDebInfo
mode.
Chris Gloor points out that after my latest change, the "font_dir_list"
got loaded with every directory underneath that it discovered through
that one level of recursion. Not harmful, but very cluttered. My
original intention was for font_dir_list to only have the search path
directories.
So this prompted me to do a bigger refactor to fix that up and clean up
some other things. Now all the searching is done once when it builds up
the initial inventory the first time a font is needed, no more potential
directory searching or touching non-essential files for each font-using
operation.
Not long ago, we removed recursion from the font searching, for
performance reasons. But now I see at there are some systems where none
of the font directories likely in the searchpath have any of the font
files directly there, but instead they are in subdirectories one level
further down.
To compromise, then, I'm adding ONE level of recurson to our font
search. If you need to search deeper than that, you should directly add
the paths to the OPENIMAGEIO_FONTS searchpath.
For cases of calls like `ImageOutput.write_image(array)`, this improves
the error messages when the array dimensions don't match the size of the
image we're writing, in two ways:
1. For the case of 2D images where the dims don't match, print what the
array shape actually is and what was expected. That's more helpful than
just saying "can't figure out array shape" and printing the number of
dimensions (but not the shape).
2. A mistake caused a number of error messages to be substituted by a
cryptic "can't handle numpy array with noncontiguous channels", but that
was only supposed to be for one particular case; fix that.
We are not ready to throw the switch quite yet, because the exr core
library still cannot read dwaa/dwab compressed images. It's only safe
to use if you are sure you will not encounter those files.
I feel really stupid for not catching this earlier. Inadequate testing!
Add a new "openexr-compression" test that writes one of every compression
methods available to openexr and ensures it can be read back. We have to
disable it for openexr:core=1 because it will fail on the dwa cases.
Currently, if you run oiiotool with --bumpslopes and --cdf at the same time (or -obump:cdf=1), the CDF is only produced for the first four (of six) channels. The techniques described in Burley (2019) actually do apply reasonably to the squared derivative channels, so it's useful to allow all six channels to have CDFs here.
maketx and oiiotool-maketx tests updated with new relevant tests.
When we build against OpenEXR >= 3.1, default to using the new
"exrcore" APIs unless disabled at build time with
`-DOIIO_USE_EXR_C_API=OFF` or at runtime through the global openexr:core
attribute.
TIFF output using JPEG compression seems unreliable, and after several
hours trying to chase it down, I'm not making much progress. As far
as I can tell, this has always (or at least for a long time) been
broken, and nobody noticed, which is strong circumstantial evidence
that it's not something people need or is trying to use, and therefore
it doesn't seem like a good use of more time to fix a feature that
no one uses.
So I'm hiding all the TIFF output with jpeg compression support behind
`#if` guards and disabling them for now. This will cause requests to
use jpeg compression for TIFF output to just revert to the default zip
compression.
Reading TIFF files that are jpeg-compressed seems fine, this doesn't
disable our ability to read the files, just to produce them.
It is also worth noting: It's one thing to write an actual .jpg file
as *final* output for web display, viewing, or other "terminal" uses.
But JPEG-compressed image data should never ever be used as input to
further image computation operations, since each read/write/read cycle
with jpeg compression can lead to further data loss. So I think for
this reason also, this patch should help avoid the temptation or
inadvertent ability to use this lossy compression method when writing
TIFFs.
There were still some bolded (indicating newly changed) dependency
minimums that actually were bumped in 2.4 or even 2.3 and should have
already been unbolded.
* Don't use deprecated ubuntu-18.04 runner. Switch to running on
ubuntu-20.04, but use docker containers that supply ubundu-18.04.
* Bump 'latest' version of openexr, webp, pugixml.
There turns out to be a way to get wedged into a mutual deadlock on
the file handle cache, with how get_texture_info on a udim pattern
checks to see if the attribute requested is present and equally valued
in all constituent files of the udim set. This hits the handle set
pretty hard, and if multiple threads are hammering the same sets and
rapidly cycling textures out of the cache, there can be trouble, it
seems.
Remedies in this patch:
* When it's trying to close files because the number of handles is too
high, we already had it so that if we only exceeded the max_files
limit by a little bit, we would simply try to get the lock, and
abandon the attempt if somebody else is holding it, but if we are
quite a bit over, then stay and wait for the lock to be released.
In this patch, we make the that default behavior -- if we are over
the limit and need to free some handles, simply try to get the lock,
and if somebody else is holding it, leave it all up to them and
return immediately. The pros of this are (a) snappier response by
never waiting at this point, and (b) removing the possibility of a
deadlock, because if you can't acquire the lock, you just move
on. But the con is that lots of threads adding new files may run
faster than the one thread who has the file and is trying to release
files, so it's possible that the number of handles will exceed the
limit by a wider margin now, but hopefully only momentarily. (But
note that this can be overridden with a new ImageCache attribute
"max_open_files_strict", which if set to nonzero revert to the old
behavior. At least for now, in case I am neglecting to understand
some bad consequence of this change.)
* The recursive_mutex controlling access to the ImageInput in each
tile cache record is changed to a recursive_timed_mutex, and when
closing a handle by calling release(), instead of an unconditional
lock, try acquiring a lock for 100ms. If it fails to do so, it may
be in a possible deadlock, so just give up and move on. The only
consequence is that maybe that handle isn't freed after all. It will
then move on to the next one, not the end of the world, but at least
it can't get stuck there.
* In find_file, eliminate the call to check_max_files -- it's not
needed here, since it's also called by open(), which is the only
time that new file handles might be opened. We just don't also need
it again simply because we're adding a file to the cache.
Setting the cmake option OIIO_DISABLE_BOOST_STACKTRACE to ON disables the use of
Boost stacktrace at compile time. This avoids dependencies on helper libraries
like dbgeng.dll.
* test: Add a corrupted test file
* fix(IC): upon tile read error, mark the file as broken
The idea is to keep fruitlessly reading from a corrupted file.
* fix: race condition in TIFF reader
TIFFInput::read_native_tiles had a very subtle race condition. There
is a section where it has a task_set of decompression tasks that are
sent to the thread queue. When the method ends, the task_set goes out
of scope, and its destructor will wait for all the remaining tasks to
finish. All fine, right? Except that other variables in the function
include unique_ptr's with allocated memory that the tasks need. So
what can happen is that if the compiler generates the code such that
the destructor of the unique_ptr happens before the destructor of the
task_set, the memory that the unique_ptr held may be deallocated
before some of the tasks still running in threads have finished.
In practice, we seem only to hit this race condition for particular
cases involving corrupted files, because the early `return` seems
quite likely to run those destructors before all the threads are done,
but it was possible to encounter it for valid files as well, if the
timing is just right. Shocking that we never had it reported.
The solution is just to have an explitit tasks.wait(), which will
block until all the tasks have finished before hitting the actual end
of the method and letting the other destructors to run. And for the
"early return" case, change it to a break so that we go through the
wait call at the end.
I believe that this fix addresses TALOS-2023-1709 / CVE-2023-24472.
There are two ways to fail the FITS magic number check: fail to read
the bytes, or the bytes don't match the magic number. It seemed
suspicious that we call close() for the latter but not the
former. Combine them into a single error response (they had the same
message, anyway).
Also, make sure FitsInput::init() fully clears some other data
structures. Oversight?
I don't know if any of these things are real bugs, but they look
sloppy and potentially problematic, so fixing them up.
* Make lookups of const unordered_maps thread-safe
* added back deleted end brace
* clang-format
---------
Co-authored-by: Larry Gritz <lg@larrygritz.com>
For non-0-origin images, we were checking the resolution range
incorrectly (`>=` vs `>`).
Also were printing the error message with the wrong notation, should
have shown a half-open range as `[beg,end)` and not `(beg,end]`.
* Be more careful constructing strings from what we read in the header;
corrupted files may make them not be zero-terminated.
Fixes TALOS-2023-1707 / CVE-2023-24473
* Watch out for pixels with palette indices that exceed the size we
allocated for the palette.
Fixes TALOS-2023-1708 / CVE-2023-22845
* Check alpha_type for valid range.
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 parameter 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 attribute "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 transform 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 entries, 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.
When we recently added IOProxy support to the Maya iff file reader in
PR 3647, something broke for certain (but not all) iff files, and I
have productions stumbling over this.
It's proving tricky to track down and I have shows needing a fix
immediatly, so this change sets up a CMake variable
ENABLE_RLA_IOPROXY, default to ON, but when set to OFF, it switches
back to the old "no proxy" versions of the iff reader.
Disabling this will get me out of my current emergency -- at the cost
of not having IOProxy for iff, which I don't need at this moment
anyway -- and give me some breathing room to fix the problem for real,
at which point I will re-remove the old code.
There's a recurring idiom spread all over the python bindings for
ImageBufAlgo, which I encapsulate in a new local helper template
vecresize(). This ensures that the logic is identical in all the
places it's used, and also improves our coverage analysis (because it
was tricky to make tests that exercise the uninitialized-handling
cases separately for each of the many different places the idiom was
used).
Tweak the heuristics for trying to determine which arbitrary-named
color spaces are equivalent to certain known common names.
For OCIO 2.2+, some of this is easier and more reliable by using the
new built-in color spaces -- for example, we can transform a set of
sentinel colors from an arbitrary cs into the known built-in acescg,
or lin_srgb, etc., and if the values don't change, we know that cs
*is* the canonical one, even if named something weird.
But one problem is that for older OCIO, the built-in config is not
present, so if you have a color space named, I dunno, "linear", it's
hard to know what that is. Is it what we now call lin_srgb? Is it
ACEScg? Something else? So the hack is: first, we try to identify a
space that we think is sRGB (almost all configs have something called
"srgb", because everybody needs it, and we take it at face value that
if they call a cs "srgb", it's the real "srgb", fools who use the name
for something else get what they deserve). We transform the sentinel
colors from the srgb into the cs we are interrogating, and if they
match what we expect to see from an srgb->lin_srgb transformation,
then guess what -- the unknown color space must be lin_srgb!
We may, in the future, expand this to deduce other spaces. But this is
a start.
There are other changes I made here to make this whole process much
less expensive than it had been getting, there was some redundant
work, and some work that only was useful for 2.2 or only useful for <=
2.1, so I only do those in the right circumstances now.
Also along for the ride: improve test coverage for maketx features of
colorconvert, unpremult, attrib, attrib.
Examining some timings and function call counts, I saw that a typical
oiiotool command was spending a surprising amount of time in
ImageBufAlgo::zero(), not just more runtime overall, but also calling
it many more times than I thought it should. For example,
oiiotool -create 4096x4096 3 -colorconvert srgb acescg
called ImageBufAlgo::zero()... 4 times, totaling as much time as the
colorconvert operation.
The idiom used in `IBA::op(dst, src, ...)` which initializes the `dst`
IB if it's not already initialized will call the IB constructor that
zeros out its contents (so you don't have every new IB full of garbage
pixel values). But for IBA functions, this is almost never necessary,
since the IBA function itself will tend to overwrite all pixel values.
Not always, of course -- since IBA functions let you restrict the
spatial area or channel range -- but in the cases where the
destination starts out uninitialized and is allocated to exactly mtch
the ROI, I *think* you can count on the calling IBA function filling
in all those values.
So I changed IBAPrep() to call the IB constructor that says not to
zero out the pixels.
Just in case we ever need the ability, I made a new flag value that
lets you call IBAPrep and tell it, yes, do zero out any newly
allocated buffers. But we pass the testsuite without needing that flag
anywhere, so I think we're ok.
Also, the --create command itself would make new buffers (with the
implied zeroing), and then explicitly zero it again!
So after these changes, the oiiotool example command line above only
calls IBA::zero() ONCE (correctly, in the initial new image --create
makes), eliminating three redundant calls.
I also hypothesized that each call to zero was taking too much time,
because underneath it was calling IBA::fill which laboriously walked
through the IB with iterators and copying a fixed pixel value that
happened to contain 0 values. So I rewrote a special case where if the
buffer had contiguous strides, it would just use memset instead.
Surprisingly, it was not appreciably faster, and I'm guessing that the
memory bandwidth necessary to touch every pixel in a 4k buffer is the
limiting factor, and the other pixel-by-pixel iterator diddling was
inconsequential compared to waiting for the memory bus. I'm still
going to leave this "optimization" in, even though it was barely
measurable for the case I was trying.
Timer:
* Fix goof where m_printdtr was a bool but was tested for equality to
an enum. It happened to work like that, but only coincidentally.
* Add another enum value that allows a debugging message to be printed
upon construction of the timer, in addition to its destruction.
* Switch from printf style to std::print style formatting of the
debugging messages.
LoggedTimer:
* Modify to allow modifying the call count. This turns out to be
helpful for times when one logged function calls another logged
function (like if a special case is broken out) and they both log to
the same timer -- it makes it look like just one invocation instead
of two.
Currently, --resize takes a single argument giving an "image geometry"
(like "640x480") that designates the size of the result image, and the
rescaling specifically maps the "full size" (a.k.a. display window) of
the input image to the output.
This experimental change extends --resize to accept additional new
optional modifiers:
- ":from=<geom>" Overrides the default of the full window of the
source.
- ":to=<geom>" Overrides the default of the full window of the
destination.
- ":offset=+x+y" Adds an additional offset transformation to the
destination.
- ":edgeclamp=<0|1>" Turns on and off some special edge sample
clamping to try to reduce certain ringing artifacts. (Experimental.)
Unlike the output resolution, which like all image sizes must be an
integer resolution, the overrides on the source and destination
windows and the offset is allowed to be floating point, that is,
allowing for fractional pixel sizes or offsets.
The from and to windows do not restrict the boundaries of pixels
computed, but rather, simply define the transformational mapping
between the two images: the transformation will be chosen such that
the float-valued coordinates of the source image correspond exactly to
the float-valued coordintes within the destination image. In both
cases, coordinates refer to the true continuous coordinates in the
image plane, with (0.0, 0.0) being the "upper left" corner, and pixel
centers being at half-unit positions such as (ipixel+0.5, jpixel+0.5).
For example,
oiiotool in.exr -resize:from=100x50.3+50+20 1000x503 -o out.exr
creates a 1000x503 pixel image where the full output image is a
resized version of the 100x50.3 pixel window starting at coordinates
(50,20) in the input image.
This is mostly implemented (when it doesn't end up equivalent to the
old style resize) in terms of the existing IBA::warp() function. I needed
to add a new variety of warp() to avoid some artifacts, but I do this
tentatively and so currently do not expose it as part of the public API.
I want to gain some experience with the oiiotool usage by the users who
requested this, then will return later to exactly how it should be
exposed directly as IBA functions.
Some background: TIFF allows for pixel values to be encoded in some
alternative color spaces, like YCbCr, CIELAB, etc., and this is
indicated by the TIFF PhotometricInterpretation tag. Since in practice
we almost never encounter these encodings in the wild, and they
require a lot of custom code to handle (for example, YCbCr also comes
along with the ability to subsample the chroma, and options for
different placements of the subsampled positions, yuck), for most of
these cases we just lean on libtiff's ability to convert the whole
thing to RGBA for us, and so in that case, we return to the user the
pixels as RGBA, and set the "tiff:ColorSpace" metadata just as an
advisory to indicate how it had been encoded in the file. But we don't
support writing any of those. (One exception is that we do allow
reading and writing of CMYK files.)
OK, but now what about a case where the app already has the data in
one of these spaces (such as YCrCb; and let's ignore the whole
subsampling miasma and assume we don't want to support that), and all
we want to do is write that data to a TIFF file -- without any kind of
data conversion -- but to set the TIFF tags properly to indicate that
the data in the file is what we know it to be?
A related special case of this is: what if we have a TIFF file that
has YCbCr data but neglected to mark it as such (I'm staring daggers
at a certain widely used compositing package right now), and to fix
this, we want to use OIIO to just copy the file and set the right tags?
It turns out that in the status quo, it is surprisingly difficult to
do this. There is just nothing in our existing TIFF output code that
will write any of those photometric tags for those spaces, and no way
to communicate that this is what we desire.
Whew. Now here's what this patch does:
* If the "oiio:ColorSpace" attribute (our usual way to indicate
presumed color space of the pixel data) says that the pixels are
already in one of those color spaces (specifically, "YCbCr",
"CIELAB", "ICCLAB", "ITULAB", "LOGL", "LOGLUV"), then we set the
PhotometricInterpretation tag as such in the TIFF file that we
write.
* We still DO NOT interpret that as a request to convert pixel data
from any other space into this one. In other words, the TIFF output
does not convert color spaces (except for RGB <-> CMYK); this merely
lets us output a file that correctly says the data are what the app
thinks they are.
* A bunch of minor refactoring was necessary because of some unobvious
coupling and order dependencies. For example, the sgilog compression
techniques MUST be used with the LOGLUV color space, and vice versa.
You have to know how both should be set before checking that they
are compatible, and if they are not, adjust something that it's not
too late to change.
So the punchline at the end is that if you had a TIFF file that has
YCbCr encoded pixels, but for some reason it's marked as ordinary RGB,
you can fix it as follows:
oiiotool ycbcr_disguised_as_rgb.tif --iscolorspace YCbCb -o real_ycbcr.tif
Refactoring in #1641 incorrectly caused this to be set on the last layer
rather than the composite image as before. Now set it on the composite
image and every layer, as all of them use unassociated alpha.
* Tests for maketx -u, --attrib, --sattrib, --cdf
* maketx was documeted to take --cdf as a simple flag, but was
incorrectly implemented as requiring a (pointless) nonzero argument.
Fix it and hope nobody was depending on the undocumented behavior.
* maketx -v arg never had any effect. Add it back.
* Better Typedesc testing of 'tostring' with fmt style formatting
commands, and fix a bug that uncovered.
* Refine 'make CODECOV=1' behavior
* Beef up python-imagecache test to test getpixels, more attribute,
invalidate.
* Better testing of python IB construction from numpy array.
* testtex: Use correct guard to avoid compiling unused code for
coverage analysis testtex: get rid of unused 3d warp code.
* Add texture-threadtimes test
* Eliminate redundant code in exroutput.c by implementing
write_scanline using write_scanlines.
* Add Exif write support to PNG output.
Stores any Exif data from the given ImageSpec into the PNG eXIf chunk.
* add test for writing Exif data in png
* add test reference output for libpng versions not supporting the eXIf chunk.
* Update testsuite/png/ref/out-libpng15.txt
Co-authored-by: Joris Nijs <joris.nijs@esko.com>
Sets color space metadata. While conceptually, the main job is to call
`attribute("oiio:ColorSpace",name)`, this is a central spot to add
logic that adjusts or removes other contradictory metadata.
Note that these intrinsics don't seem to be available for gcc.
They are available on clang, but are defined in terms of the memcpy
trick. But in Intel, they seem to actually help.
The new 2023 one on the Intel site seems to require a glibc too new to
be compatible with the one on the ASWF CentOS-7 based containers, so
has been breaking our CI for a couple weeks.
It should not have been set in platform.h (inside the cpuid function!)
in the first place. And apparently, though this was ok for Windows ARM,
on Mac ARM it's an error because that symbol is already defined.
So only redefine it if it's currently undefined. And move the whole
check and definition into simd.h where all the other ARM_NEON stuff
lives.
We never had a test for `maketx --lightprobe`, so add it to
testsuite/maketx.
Immediately discovered that it only worked correctly for float images,
fails for half or other types. Fix.
We didn't have thorough tests of pbm/pgm/ppm files, add some.
That immediately uncovered a problem with pbp files.
By spec, pbm (1 bit) files use 1 to mean black, 0 for white.
Confusing, because pgm (grey) and ppm (rgb) use 0 for black.
* When checking code coverage, don't abort for failed tests
* Rejigger imagebufalgo_test a bit for better IBA test coverage
* Test zover
* Have testsuite/oiiotool-fixnan test with deep images
* Add to the extisting --fixnan tests a test for 'error' mode.
* Make testsuite/oiiotool-color test all colormap names
* Test 2D filters in filter_test
* Exclude python scripts in src/doc from coverage analysis
* Remove redundant attribute-seting code from the python internals
* Remove unused python_array_code function
* Exclude some imported code from coverage testing
* Re-enable and improve coverage testing from 'make CODECOV=1'
* Exclude unused/unexposed parts of xxhash.cpp from compilation
Turns out the clang/gcc intrinsics themselves compile to the same
memcpy idiom that I was using for the general case. So there is no
need for the clutter of the specialization.
libtiff 4.5 was just released, so confirm that we build against it,
update the "latest versions" CI test to use 4.5.
More careful sorting out of libtiff version numbers for conditional
compilation.
Get rid of some ancient tiff version guards that are related to
libtiff versions that are older than we currently support.
When building against libiff 4.5, use the new open calls that allow
per-file error handlers, which is better than the old global error
handlers for thread safety of multiple threads reading from the same
ImageInput if there are errors.
Convert error messages to std::format form.
We had no testsuite exercise of --pdiff, add it.
Along the way, I saw tha we were getting the "you didn't have any
output" warning when using pdiff, so fix that.
And some other minor changes: formatted output update, some other
tightening up of things.
A little bit of jiggling the contents of XYZtoLAB_color to work around
what I think is a compiler error with icx -- it was generating NaNs
for reasons I was unable to understand or fix, but not if I rearranged
the code slightly. All the other compilers and versions had correct
behavior.
Fixes#3721
Please read the comments in #3721. This is the "local" fix for the
build break due to the type mismatch. It is a band-aid. And it may be
the best solution for the 2.4 branch if we don't want to potentially
break ABIs by changing the definition of any public types.
Still pending is to examine the issue of whether it was a mistake to
define vbool4 storage for neon as uint32x4_t or if we should change it
to int32x4_t to better match the non-simd reference implementation.
After debating that (and identifying somebody with access to an
ARM-based machine to test the solution for us), we may return to
tackle this more fundamental change.
A few minor fixe to the term code, add a "term:filename" optional hint
to redirect output rather than go to stdout, and also change to the
new std::format notation for output.
* OpenColorIO improvements, especially for 2.2
* When OIIO builds against OCIO >= 2.2, in the event that there is no
config supplied (e.g., specified by env `$OCIO`), use he new
build-in "cg" config to supply a basic set of color spaces and
transformations. Yay for OCIO 2.2 and its built-in configs!
* New OIIO-understood aliases (regardless of whether they were in the
config or not): "scene_linear", "srgb", "lin_srgb", and "ACEScg".
It will do its best to know which, if any, supported color spaces
are equivalent to these, even if arbitrarily named. For OCIO >= 2.2,
some trickery is employed to use the new features that can convert
to built-in config color spaces, transforming several colors in this
manner and seeing if it gets what would be expected for various
canonical color spaces.
* `oiiotool --help` now a much abbreviated set of color information
(just the OCIO version, and the config, and a hint about how to get
more info).
* A new command `oiiotool --colorconfiginfo` prints the FULL
inventory of color management information, which now has been
expanded to reveal the aliases of each color space, print the roles,
and generally spruce up the output to be much more readable (though
longer) for large configs.
* New methods for our ColorConfig class:
- `resolve(name)` turns the name, which could be a color space, an
alias, a role, or an OIIO-understood universal name (like "sRGB")
into a canonical color space name.
- `equivalent(name1,name2)` returns whether or not the two names
refer, ultimately, to equivalent color spaces (to the best of its
ability to figure out).
* Some minor changes (including additional `const` qualifiers)
on the ColorConfig class.
* Make testsuite tests use the built-in OCIO config if OCIO 2.2+ is
found.
* Build sonar against ocio2.2 so we are primarily testing the new
code paths.
* For OCIO >= 2.2, because we can count on the built-in configs,
conditionally compile a bunch of hard-coded color transforms. These
will eventually disappear completely, someday, when OCIO 2.2 is the
minimum supported version.
I think the behavior should all be largely unchanged when running
OIIO in a facility that has a proper OCIO config pointed to by the
`$OCIO` env variable and using color space names contained therein. So
studio users will (I hope) see no changes. For users that have no
OCIO config, or that are writing software that they want to work
reasonably well for common cases no matter what config they encounter
at runtime, this should give more helpful and consistent behavior when
OCIO 2.2+ is used (which I now strongly recommend for anybody not
constrained to 2.1 for compatibility reasons.
But because of the risk and the possible behavioral changes, this is
all currently envisioned as OIIO 2.5 features, and I am not planning
to backport it to the 2.4 release branch. Additionally, a lot has
changed and I expect some additional tweaking and identification of
edge cases I may have missed.
* docs, etc
IOMemReader::pread catch out-of-range read positions.
Fixes#3711
Along the way, I noticed that the system pread & pwrite return a
ssize_t rather than a size_t as our IOProxy methods do, so I need to
catch the negative value returns that they use to indicate errors.
Remove the xxhash stuff from hashes.cpp -- it long ago moved to
xxhash.cpp. Also, comment out the bjhash implementations that OIIO
itself doesn't use or expose publicly, since there's no point
compiling it (or having it count against us in coverage analysis).
Seems like a reasonable assumption for BMP files. It's not strictly
correct -- there are header fields on V4 and V5 BMP files that
communicate color primaries and gamma, but honestly, I do not have any
idea if anybody uses them or whether they're more misleading than
helpful. For now, assuming that a BMP file is meant for output on
an sRGB display seems like it's likely to be true.
* More extensive testing of texture3d: handle vs filename, subimages,
>4 channels, closest sample interpolation.
* Fixes for tex3d > 4 chan that turned out to be broken.
* Some rearrangement for the >4 channel case
* Refactor and simplify accum3d_sample_bilinear so multiple data types
all use the same templated code path.
* This all reduces the amount of untested code in OIIO by another 0.4%.
* Build: fmt external version flexibility
The current default behavior is to
1. Check if there's already a copy of fmt found.
2. If not (or if BUILD_FMT_FORCE is ON), download it locally.
3. Copy the relevant fmt .h files as part of the OIIO install in
OpenImageIO_ROOT/include/OpenImageIO/detail/fmt
This patch adds a new INTERNALIZE_FMT option, which defaults to ON,
whch is the old behavior. But if set to OFF at build time, the fmt
headers will NOT be copied into OIIO's install area (instead, it will
have dummy files that in turn include the real ones).
Set INTERNALIZE_FMT to OFF if you are really sure you want users of
OIIO to use the system install of fmt and not have OIIO install a new
copy.
When an externalized fmt is used, also make sure our exported cmake
config files cause the downstream user to find fmt and pass on the
dependency properly (necessary because strutil.h actually requires
properly including fmt headers).
Eliminate extraneous include_directories of fmt
Bump the auto-download version of fmtlib to 9.1
Many ImageOutput::open implementations start out with various validity
checks -- such as whether the ImageSpec requests a resolution, number
of channels, or other features simply not allowed by that file format.
This is not only repetitive, but which checks are done by each file
type is somewhat haphazard, error-prone, hard to get full test
coverage (must we separately test each possible failure case for every
file format?), and the error messages are not always consistent across
formats. Frankly, most of the format writers assume that the caller
has a full understanding of supports() and will never send requests
for features that are unsupported.
This PR proposes adding a utility method ImageOutput::check_open(),
which performs all validity checks on the ImageSpec that can be
deduced from supports(). The various ImageOutput::open()
implementations may call this function and avoid having to replicate
(or forget) all the checks done in other output formats. This is a
protected method that is used by ImageOutput implementations
internally, but has no use for client-side code.
I've implemented check_open, and also used it here for all the output
formats. If this seems ok to everyone, I also plan to do an analogous
helper for ImageInput common validity checks (which will be an even
bigger reduction of redundant code and greatly increase the safety
against corrupted or nonsensical input files).
Along the way I also instituted a new supports() query: "noimage".
That is to indicate a file for which it is permitted to have a pixel
data window consisting of 0 pixels (i.e. an "image" that is only a
container for metadata). Currently, FITS is the only format we support
that allows this, though it's something we occasionally talk about for
OpenEXR. Allowing an explicit way for formats to say they allow no
pixels is what allows check_open to issue error messages for all the
other formats if it is passed zero-sized width, height, depth, or
nchannels.
Would you believe we never had a testsuite test for environment
mapping? Well, it wasn't totally untested, in the sense that OSL
testsuite had one, and obviously it's used in production all the
time. But OIIO's testsuite didn't have any tests of
TextureSystem::environmemt(). We now remedy that.
* Add testsuite/texture-env, contains several environment tests,
including handles vs filenames, subimage selection, different interp
and mip modes, ask for >4 channels.
* New CMake build option OIIO_TEX_IMPLEMENT_VARYINGREF can be used for
testing to disable implementation of the old 1.x style of batch
texture calls. We don't use them, they are deprecated. This option
give us a way to exclude them from code analysis, because we really
don't care.
* Fix bug with >4 channels for environment batch shading!
* Avoid unnecessary spec lookup in TileID constructor.
* Avoid building testtex test_hash during code coverage -- it is an
extremely specialized test, not run in the testsuite, it shouldn't
really count againt good code coverage.
When creating a texture, the old logic was to set the file metadata
"DateTime" to the current time, unless update mode was used, in which
case it was set to the file's write timestamp of the input file.
But that made it impossible to explicitly set that metadata, and also
a big PITA for testing because it meant the metadata of any written
texture would be the time of its writing, and so appear different
every time.
This change alters the logic: update mode still sets DateTime to the
last update time of the input file, as before; other cases will only
set DateTime if it isn't already set, but not alter it if the input
carries that metadata already.
* There was lots of duplicated code between iinfo and `oiiotool -info`.
Refactor much of that into a new src/libOpenImageIO/printinfo.cpp.
These are internal functions, no change to public API.
* Test iinfo --hash, --stats, and with nonzero origin and full size
* Test iinfo and oiiotool --dumpdata of deep image
* Test iinfo for subimages, mips, -f, -s
* Some conversion to new print instead of printf
There is probably more such refactoring to do later, I did the easy
parts for now and that's a start.
When reading PSD file, if reading the thumbnail doesn't pass basic
sanity checks, we consider that a hard error reading the file. One
check that we do is to make sure that our computation of the byte
width of the uncompressed thumbnail (`bpp/8 * width`) matches what the
file says the `widthbytes` is.
BUT... it seems that it might round up to an even (or maybe 4-multiple?)
of columns, presumably for scanline alignment reasons. And so we were
considering whole PDF files unreadable if they fell into this case.
This PR primarily loosens the test to allow some padding up to a
multiple of 4.
Fixes#3658
* Maya IFF documentation says IFF file has max resolution of 8192, and
our implementation only allows writing RGB and RGBA, so check these
limits when an IFF file is opened for output.
* Make sure scratch space allocates enough for tile padding.
* Check for non-zero image origin coordinates (which are not allowed
in IFF files).
* Change some 16-bit loop variables to uint32_t to avoid possible
overflow.
Fixes TALOS-2022-1654, TALOS-2022-1655, TALOS-2022-1656
In open(), check for non-zero image offsets, which is not supported by
BMP, and issue an error if found. If the spec passed in has nonzero
spec.y, other code in the write functions would fail because they
assumed spec.y == 0 and could therefore access memory incorrectly.
More generally, also put checks in the write functions that issue
errors and take early outs if they find that they are called on a
BMPOutput that is not currently open, for example if open() fails but
the user doesn't check the return codes and proceeds to call the write
functions anyway.
This addresses TALOS-2022-1653 / CVE-2022-43594-CVE-2022-43595
There was a subtle error where we had a reference to an ImageSpec for
one of the subimages, confusingly called m_spec, which we altered, but
then didn't make the same change to the this->m_spec, and in a later
spot we referenced the `m_spec` meaning the one in `this`. This could
lead to a crash.
Fix, but not only that, also rename to avoid the confusion. `spec0` is the
alias we us for the spec for subimage 0, and `spec_s` is the alias we
use when referencing subimage S, and we really never use m_spec
anymore to avoid any confusion. This should make it easier to reason
about this code in the future and avoid these kinds of mistakes.
A second error resulted from the situation where a failure in open()
would return before one or both of m_buf or m_tilebuf were allocated.
If the return status of open() were ignored and the caller
subsequently attempted calls to write_scanline or write_tile, invalid
memory could be accessed. Fix this with a variety of guards not only
based on the allocation status of these buffers, but also issuing
errors (and exiting early) from write_scanline or write_tile if they
are called on an ImageInput that is not in fact open.
Addresses TALOS-2022-1651 / CVE-2022-43592 and
TALOS-2022-1652 / CVE-2022-43593
Fixes#3660
In investigating this issue, I also noticed that now that IOProxy
support can be relied on, the old code that in the psd reader that
directly fumbles with libjpeg is not needed and can be replaced by
just a couple of lines to make an ImageBuf read from an IOProxy.
And it fixes the bug, too. So I'm not even going to try to figure
out what was going wrong with the old code.
When writing a zfile, anything that causes open() to fail will leave
m_tilebuffer at 0 size. Subsequent calls to write_tiles, and also
to close (which cleans up unwritten parts of the buffer at the end)
did not check for this condition and would access out of bounds memory.
While I was in there, I converted the zfile.cpp error messages to the
new formatting convention.
Fixes TALOS-2022-1657
I uncovered a very subtle problem that can happen with the interaction
between ImageBuf, using an IOProxy, and the underlying ImageCache.
Remember that when using an IOProxy, it is critical that the lifetime
of the proxy outlasts any ImageInput, ImageBuf, or ImageCache that was
told about it.
Consider the following code:
IOProxy proxy(...); // temporary proxy in this local scope
ImageBuf A ("foo.exr", 0, 0, proxy);
We've set up an ImageBuf that will read from an IOProxy. Behind the
scenes, it made an ImageCache entry that remembers the IOProxy, as it
has to, since it needs to use that proxy for subsequent reads.
Now we read the image, and we use the `force=true` parameter to
force an immediate read of the file:
A.read (0, 0, true);
It reads from the proxy. But the IB and the underlying entry in the IC
still know about the proxy. But in the user's mind, they have fully
read the image, will never read from it again, so of course (they
think) it is safe to close or destroy the proxy, knowing that they
made it last past its use:
proxy.close();
But -- oops! -- the cache entry still has a pointer to the proxy, and
even though it won't need to read from it again, it still will access
it, close it, etc., but the user's proxy may by that point be out of
scope.
This is *technically* not a "bug" in the sense that it is the user who
violated the contract by closing or destroying the proxy before its
last use by anything they passed it to. But in context, it's very easy
to be confused by this -- the user thinks that the only observer of
the proxy is the ImageBuf, and they're done using the ImageBuf at that
point, but because they don't explicitly mention an ImageCache, they
don't realize that the ImageBuf passed the proxy reference along to
the underlying IC that could be backing any ImageBuf.
It's the user's fault in some sense, but let's just say that we have
given them an "attractive nuisance."
That's a long bit of background explanation, but I wanted to force
myself to explain it clearly and make sure it was spelled out in the
commit comments. Now here is the fix:
* When IB::read does an immediate forced read involving an IOProxy,
after the read is complete, tell the cache to invalidate the entry
for the file so that it no longer retains a pointer to the proxy.
* When an IB itself is destroyed or cleared, if it used an IOProxy,
also invalidate the IC entry for that file.
* Unrelated cleanup: avoid a redundant invalidate call in init_spec().
* Make it so that when we build against OCIO 2.2, it is possible to
specify the built-in configs. The logic had previously precluded
this, because at one point it checks to see if the passed config or
contents of env $OCIO was an existing file. Now you can set OCIO to
"ocio://default", for example, and it will be used by OIIO.
* Expose getAliases (I think that has been there for a while but we
didn't expose in in our ColorConfig wrapper).
* Expose isColorSpaceLinear (new to OCIO 2.2)
* New OIIO global getattribute query: "opencolorio_version" returns the
human-readable version string (such as "2.2.0").
* Some miscellaneous other cleanup in color_ocio.cpp
* Change in color methodology for test suite: Previously, for every
call to oiiotool in the testsuite, we passed a `--colorconfig`
argument to oiiotool pointing it to the sample colorconfig that we
ship in the testsuite. Here, I'm changing it so that the cmake setup
for tests sets the OCIO env variable for all tests, so no explicit
--colorconfig is necessary, and thus you won't see a path to the
config in any of the test output files if they need to echo the
command line arguments (and you'll see several test outputs altered
in this way). This is a prelude to later changes that will use the
built-in configs themselves for tests, when available.
More interesting changes coming in subsequent PRs.
Lately, it has been experiencing intermittent availability problems
that causes our CI to fail while setting up dependencies.
What happens if we don't use the problematic ubuntu-toolchain-r at all?
Nothing, it seems. So eliminate it.
I think that once upon a time, it was necessary to add this repo to
get certain new packages? But maybe they are mainstream enough now
that it's not necessary to add this repo? In any case, it doesn't seem
to hurt anything, it's one less moving part that can't fail if it's
not there.
We use this all the time in our internals, and I'm tired of having to
write Strutil::print. Too verbose. By putting it in the main OIIO
namespace, OIIO internals can reference as simply print(), and
external users now have a choice of saying Strutil::print() or
OIIO::print().
Kai Pastor (dg0yt) made an important observation that the way I
recently made OpenImageIO_VERSION be a cache variable might lead to
unexpeted behavior by inadvertently retaining a version number upon
recompiles if the cache itself was not cleared, and this can happen
unexpectedly even when the user isn't trying to override the version.
This patch changs the behavior as follows:
You must use the special `-DOpenImageIO_VERSION_OVERRIDE=...` to
override the version.
* if you do so, you are playing with fire and get what you deserve
if you misuse it or change branches and don't remember to clear
the cache.
If you do a normal build (without _trying_ to override the version),
* you will get the real version number because it IS NOT cached
* if you switch branches, you'll always get the right version number
* ...but switching branches without clearing the cache can still, as
always, have other subtle behavior differences versus doing a
truly fresh build from scratch.
So in short, it still allows a way to override the version if you
really want to, but if you are not trying to do that, you can no
longer get yourself inadvertently into unexpected trouble.
* testtex: better control over temp file creation
* Additional texture tests to improve coverage of udim generation,
and deadling with cache limits.
* Better ImageCache unit tests for perthread create/destroy and
individual tile retrieval. Fix a bug in get_tile for nonexistant
subimage or miplevel.
* Dead code removal: ImageCacheFile::udim_to_concrete() and
find_file_no_add(), which were no longer called anywhere.
* IC stats: fix messed up 'slowest' list ordering when there are broken files
* IC stats: when stats level is high, enable printing of some stats that
we previously skipped over when they were "uninteresting" (e.g., too
little time spent on something, no problematic files, etc).
* IC testing: test a variety of get_pixels error conditions
* IC testing: more tests of IC::imagespec() and get_imagespec(), incluing
error conditions.
* More get_texture_info query testing
* Minor bug fix for int-to-float data conversion in get_image_info
* Test IC query for resolution[3]
* Test IC::reset_stats()
* Fix ImageCache::reset_stats in cases where threads ended
* Add casual tests of oiiotool with no args (print brief usage message)
and when an unknown command.
* Also changed the meaning of -q (quiet mode) to be more quiet -- in
particular, not to print the full help message for errors when quiet
mode is used, only print the error.
* oiiotool test sequence errors.
* Test (and fix) --selectmip and --subimage with out of range indices
Add IOProxy support.
Fixes#3623
Along the way, noticed an important bug at line 209 of this change --
the header "imagename" field should be fully zeroed out even if there
is no ImageDescription metadata!
Changes:
* Scorecards and analysis -- allow newly launched job to cancel
running old job from the same PR.
* Scorecard only runs on main repo, no need to run on forks.
* Nightly CI only runs on main repo, no need to run on forks.
* Test oiiotool control flow syntax mistakes
* Test oiiotool --set with array of strings
* oiiotool: better adjust_geometry unit testing
Helper utility additions:
* oiiotool --no-error-exit
`--no-error-exit` causes an error to not exit immediately, but
rather to try its best to continue execuing command line
operations. Use with caution! This is mainly for unit tests and
debugging.
`--unittest` also turns this on automatically, so that tests that
are probing failures don't cause an exit.
* unittest.h: If OIIO_UNIT_TEST_QUIET_SUCCESS is defined, don't print
anything extra for unit test success
It surprised me a little to realize that --printinfo prints the
current spec of the top image (I mean, duh), but that may not match
what you'd get with `oiiotool -info -v`, which prints the data about
each file as it's read from disk, inasmuch as the "current" spec
reflects the in-memory representation. So, for example, it might say
the image is float, because oiiotool tends to store everything
in-memory as float, rather than uint8 of the disk file.
So these changes help fix this oddity and other ambiguities I realized
follow:
* Give --printinfo an optional modifier `:native=1` that will print
the native (from the original file) info, if that's what you need,
rather than the up-to-the-moment in-memory data.
* Also add --printinfo optional modifiers `:verbose=` and `:stats=`.
* Expression substitution gets some new names it knows: "nativeformat"
is the data type in the file, versus "format" which I now see would
return the in-memory version; METANATIVE and METANDATIVEBRIEF, which
are the native versions of META and METABRIEF.
Make extra sure to clear the oiio:ConstantColor, oiio:AverageColor,
and oio:SHA-1 metadata when writing textures, if those things are not
going to be set.
* Add OIIO::getattribute() queries for "font_list", "font_file_list",
and "font_dir_list", which are, respectively, the lists of fonts
that were found, the list of font files (full paths), and the list
of directories it searched. All return a single string that is a
semicolon-separated list of the items.
* As a consequence of the new attributes, if you are curious to see
the full inventory of fonts it can find, you can just query the
attribute:
oiiotool --echo "{getattribute(font_list)}"
* Some refactoring of the font inventory process (in
imagebufalgo_draw.cpp) was necessary to achieve this.
* We now search for ".ttc" (TrueType collection) files, which we didn't
notice before.
* We now search explicitly in share/fonts/OpenImageIO, which we
neglected to do before, but found those fonts because we searched
/share/fonts recursively. In exchange, we now do not search the
directories recursively, because that seems unwise for performance
reasons.
* We pass the full testsuite, but it's possible that this
change may fail to find fonts that were previously found because of
the recusion. If that is the case, you should fix it by explicitly
adding the deep directories with fonts to the search path specified
in the environment variable `$OPENIMAGEIO_FONTS`.
* Unit test adjust_geometry
* Tests of --set for various implicit and explicit types not
previously covered in the testsuite
* Comment out Oiiotool:get_position, which seems to be unused
* Exercise --iconfig for explicit types
* Add test for --origin and --originoffset
* Add test for --ociodisplay
* Add test for --unmip and --selectmip
It's similar to trim_whitespace, but returns the trimmed result, rather
than operating in-place on a `string_view&`.
Also add unit testing of trim_whitespace and trimmed_whitespace
* Add an optional size parameter to read_text_file to limit the
maximum size that will be allocated and read (default to a limit of
16MB).
* New read_text_from_command, similar use to read_text_file, but reads
from the console output of a shell command.
Along the way, improve the runtest.py script by giving the
info_command() fuction the same silent, concat, and failureok
parameters that the oiiotool() function has. (This lets individual
tests assemble iinfo command lines with a little more flexibility.)
* In the case of DDS cubemap files where a cube face was not present,
we were filling m_buf with 0 bytes before allocating it.
* Add check for invalid bits per pixel.
* Add regression test for another bug that was perviously fixed in
2.4.3.
In one case, we actually had a check for this, but an assignment to an
int made the nonsensical offset appear negative, and we only tested
whether the necessary offset it was bigger than the buffer size.
Keeping it as (unsigned) size_t makes the test work as intended.
In another case, there were several places where we never checked that
we were staying within the exif block, and here we address this by
changing the utility decode_ifd so instead of passing it a pointer to
the ifd, it passes the offset (the pointer turned out to always be
inside the buffer) so it can check the extent for subsequent accesses.
Also some fixes related to squashing undefined behavior sanitizer
cases.
Also, change `operator<<` to output the string, not the hash itself.
These changes help push us more toward the original intent of
ustringhash being a drop-in replacement for ustring, any time it's
more desirable to be using around the hashes rather than the pointers.
* oiiotool tests for expressions var() and getattribute()
* Make sure we test oiiotool IMG[] expressions and several specific
special attributes to retrieve.
* Add coverage of oiiotool --keyword and --clear-keywords
When reading the image (which can have signals to jump around in the
array), we checked for out-of-bounds x coordinate, but not y, and some
corrupted files could overrun buffers as a result.
The existing syntax `TOP.name`, if the name wasn't found, produced
an error.
This PR adds another syntax: `TOP[name]` which is identical if the
named metadata is found. But if not found, this syntax just evaluates
to the empty string, with no error.
This test was taking almost 30 seconds to run, without a lot of images
being read or written. Turns out it was just a large number of separte
oiiotool invocations, and the overhead of all those shells and
programs launched adds up. Most of them were just firing up oiiotool
for the sake of doing an `--echo` to print things. A little bit of
restructuring to make fewer oiiotool invocations (each one doing a
bunch of echo commands, each testing a different thing) cuts the time
for the whole test by 50%.
I also made the runtest.py script allow certain test debugging by
letting a test set the `wrapper_cmd`, and if so, will prepend every
oiio utility launch with it. For example, a test could say
`wrapper_cmd='time'` to time every command.
Add a hidden --unittest command to oiiotool, only functional for Debug
builds, which enables additional testing of internal utility funcions
and other things that are difficult to fully exercise in the testsuite
from normal oiiotool commands (could be done, but would take tons of
additional tests to get into particular deep edge cases).
Prototype by adding unit tests for scan_box. More will be added later
now that the mechanism exists.
Simplify string_to_dataformat
Make sure to use --list-formats and --runstats during testing.
ustring::hash() and the representation of ustringhash were size_t,
which might be 32 bits on some platforms. But we really would prefer
ustrings hash to 64 bits, to greatly reduce the chance of hash
collisions.
* Add strhash64() which is like strhash() but guaranteed to be a 64
bit value (returning uint64_t instead of size_t -- though note that
these are the same on 64 bit CPUs).
* ustring::hash() and ustringhash::rep_t are now uint64_t.
* Note that std::hash(ustring) and std::hash(ustringhash) are still
size_t, per the C++ definition of std::hash. So ustring::hash() may
have more bits than std::hash(ustring) on 32 bit platforms.
This will NOT be backported to the release branch, since it changes
ABI on 32 bit platforms.
* We mis-named color_test, and that caused it to never get run!
* imagespeed_test is meant for benchmarking only, run by hand.
We don't make a testsuite test, so don't even build the source
(which would never be counted for code coverage because it doesn't
run) during CI.
* Add a test for cineon -- we never directly tested it before
* Don't compile code used only for writing. We don't supporting
writing cineon, so there is no point compiling the cineon module
used only for writing.
* And of course, once cineon was in the testsuite, we immediately
found a problem via the sanitizers! So fix buffer overflows when
reading FilmEdgeCode in Cineon. (And clean up the equally sloppy,
though not technically buggy, code in DPX as well.)
On any one platform, our testsuite was only testing the old C++ and
the new C core exr implementations.
Rig it to run the openexr-oriented tests twice -- once in each state
(when using OpenEXR new enough to support the core library).
Doing this cleanly required adding SUFFIX and ENVIRONMENT optional
parameters to the oiio_add_tests utility macro. Once that was added,
the texture batch testing could be implemented that way as well,
removing a bunch of hackier special purpose code.
* Protect against 0-sized allocation
* Exception safety in IffOutput destructor
* Using sonar.coverage.exclusions as a better way to exclude files
from coverage analysis
* Null pointer safety
I realized that the recently added ustringhash class is confusingly
similar to the (obscure, barely used, mostly forgotten) ustringHash
functor. But also ustringHash doesn't need to exist, because it's
equivalent to `std::hash<ustring>`, if we define it. So now we do define
it, and also `std::hash<ustringhash>`, and deprecate ustringHash, to
be removed entirely for 3.0.
Also, add a ustring ctr from ustringhash (explicit only for now),
and `operator<` for ustringhash.
And add from_hash(size_t) method to ustringhash, analogous to
ustring::from_hash().
Modernize a few places we still used `(void)` to just `()`.
Give ustring and ustringhash a `using rep_t =`
rep_t is the type of the underlying representation (const char* for
ustring, size_t for ustringhash).
Modernize some typedef into using.
Make `==` and `!=(ustringhash, const char*)` be constexpr for C++ >= 17
Including windows.h leads to a lot of possibly common names being
"taken", mostly by preprocessor defines. Some of these are possible
to turn off (like "max" and "min" by defining "NOMINMAX" before
including <windows.h>). But many others are not possible to turn off,
e.g. "near" and "far" just get defined to nothing, preventing from using
variables named like that.
Additionally, includes have a cost, and while <windows.h>, being a C
header does not have expensive language constructs that would slow down
compilation a lot, it's still something close to 100k lines of
preprocessed code to wade through.
Now, only the OIIO implementation .cpp files that actually need
<windows.h> include it (all of them under "libutil").
There were two exceptions:
1) thread.h pause() implementation uses YieldProcessor() macro when on
a Windows system. Replaced it with manually "re-implemented"
things that YieldProcessor expands to. Right now made the
implementation for all 4 platforms that Windows normally supports
(x64, x86, arm, arm64); the latter two OIIO does not support though
when on Windows so it's untested.
2) timer.h Timer::now() inline function used QueryPerformanceCounter
and LARGE_INTEGER. These are quite hard to somehow "forward-declare"
or similar, without resulting in duplicate type definitions. So on
Windows I moved the now() function to be non-inline.
In particular, defining NOGDI before including <windows.h> achieves
several things amongh others:
- No longer causes ERROR symbol to be defined to 0. ERROR being defined
prevents using e.g. google/glog library, since it wants ERROR to mean
something else.
- No longer defines some possibly widely used names e.g. GetObject.
16 bit per pixel (e.g. 555, 565, 444) formats probably have never
worked. One part of the code was assuming 5 bits in the mask, another
was shifting by 4 bits, and the 16-bit pixel fetch code was reading just
one byte.
The code assumes "char" is "signed char" (as on x86), but on ARM "char" is
"unsigned char".
Co-authored-by: Joachim Reichel <joachim.reichel@posteo.de>
In some cases (at least on Windows, with static linking) the oiiotool
main returns zero, but the full process returns 3 error code due to
abort() being called. And that is called because a global shared
image cache is already destroyed, but the image references from another
global (Oiiotool itself) try to use it, get a call into a "pure virtual
function" stub that is put by MSVC C runtime into a destroyed object
vtable, and that ends up in an abort().
Work around that by making sure various member variables holding
ImageRecRef are cleared before main() finishes and global destructors
are called.
Most of the cost in HDR file reading was converting from RGBE to floats,
inside ldexpf(), which at least on Windows/VS2022 spends most of the
time inside _ctrlfp(). But we only have 256 possible exponents in total,
so just use a precomputed table.
On my PC (Windows 10, VS2022, Ryzen 5950X) this change gets file read
time for an 8x resolution .HDR image from 1.10s down to 0.27s
HDR code overhaul to support IOProxy (#3218) changed all reads into
a `pread()` with explicit file position tracking. Turns out, that is
quite a larger overhead compared to simple sequential reads; even more
so on Windows.
On my PC (Windows 10, VS2022, Ryzen 5950X) this change gets file read
time for an 8x resolution .HDR image (with RLE compression) from
8.28s back to 1.10s just like it was in OIIO 2.3.
On Windows the extra cost when using pread() is a bit more extra mutex
locks, but the real cost is in enventual ReadFile; looks like any seek
operations make it take some sort of "way slower" path with various
callbacks and whatnot inside the kernel.
---
Note repoduced from discussion #3588 from LG:
Switching from IOProxy::pread to using IOProxy::seek + IOProxy::read makes the IOProxy stateful and not thread-safe without an external lock (which this HdrInput has, so it's not going to break anything), precluding any future removal of the locks from read_native_scanline to make the whole ImageInput able to read concurrently. That's probably not important for this file format, but for others (especially crucial those usable as texture), it would be desirable to rely on those stateless pread calls and remove the lock from the ImageInput.
I think this patch is fine. We don't expect a lot of concurrent reads from hdr like we do for tiff and openexr that we use extensively for highly threaded on-demand reading of texture. It fixes the recent performance regression, and it doesn't really cost us concurrency because we already had the lock in HdrInput. But in general, we want to prefer eschewing read/seek and using pread instead, so we can expect maximum concurrency from multiple threads using an ImageInput (as we do in ImageCache/TextureSystem).
So I believe that a revised and more complete diagnosis is that the problem isn't the pread vs read per se, but that for RLE-compressed images we were calling pread separately for every 4 bytes! The calls to read are faster simply because they are buffered (in FILE) and require fewer OS system calls. But the real sin is that we call it way too many times.
I think that the rle case could go back to pread if we want, but do just one pread per scanline (rather than one per pixel value run), asking for the maximum amount that will be needed to read the scanline for the worst case rle representation (it's ok if it's the end of the file, pread will just read up to the end and return the true number of bytes read). In other words, we'd be doing the buffering ourselves.
But anyway, the real misdesign of this HDR reader is ancient, and consists of doing many many tiny reads. Even the buffered fread would be sped up a lot by not doing it separately for every 2-4 bytes of the file. I don't know if it's worth fixing for this format. Depends on whether anybody really needs to maximize its performance or expects multiple threads reading the same hdr file to be fully concurrent.
Something changed in libtiff master and it doesn't build shared libs
by default, breaking our "bleeding edge" CI test. So when we build it,
be sure to ask for it. It can be overridden in build_libtiff by
invoking it with env variable `LIBTIFF_BUILD_SHARED_LIBS=OFF`.
Comparing the time it takes to load DXT/BCn compressed DDS images via
OIIO, I noticed that it's several times slower than in another codebase
(Blender). So I started profiling, and fixing some low hanging fruit.
- After decoding a BCn block (always 4x4 pixels), it was written into
destination image using quite many branches in the innermost loop.
Now it still takes care of not writing outside of a destination image
that might not be multiple-of-4 in size, just with way fewer branches.
- BCn decompression is multi-threaded via parallel_for_chunked, using
at least 8 block rows per job (i.e. 32 pixel rows). This is
similar to internal multi-threading done by EXR or TIF readers.
- Avoid one case of std::vector resize (which clears memory) followed
by immediate overwriting of that buffer with bytes read from the file.
Switched to using unique_ptr for that array, similar to how other
input plugins do it.
Performance data: I'm timing how long it takes to do "iinfo --hash" on
24 DDS files, each 4096x4096 in size. Quite a lot of time there is taken
by just the SHA hash calculation which is not related to loading, so
also have timing info for just DDSInput::read_native_scanline where
all the actual DDS loading/decoding happens.
Timings on PC (Windows 10, VS2022 Release build), AMD Ryzen 5950X.
- BC1: total 3.68s -> 2.76s, read_native_scanline 1.54s -> 0.59s
- BC3: total 3.74s -> 2.84s, read_native_scanline 1.83s -> 0.66s
- BC4: total 1.26s -> 0.82s, read_native_scanline 0.74s -> 0.24s
- BC5: total 2.29s -> 1.57s, read_native_scanline 1.31s -> 0.46s
- BC6: total 7.50s -> 4.30s, read_native_scanline 4.67s -> 1.60s
- BC7: total 6.18s -> 3.09s, read_native_scanline 4.34s -> 0.84s
Possible future optimization: implement DDSInput::read_native_scanlines,
that could avoid one extra temporary memory buffer copy. This extra
buffer clear & eventual copy from it into final user destination pixels
is non-trivial cost from profiling. But it's also quite involved
to handle all the possible edge cases (e.g. user requests scanlines
not on 4-pixel row boundaries); the code would have to do similar
juggling as EXR reader does to handle "not on chunk boundaries" reads.
It could potentially also interplay with DDS cubemap/volume texture
reading ("tiled") code paths, for which there is no test coverage at all
right now. Maybe for some other day.
Suggested by SideFX: explicitly shut down the OpenEXR thread pool
before exiting oiiotool, because sometimes on Windows it seems to hang
otherwise. A better solution is probably possible on the OpenEXR side,
but I don't know what it would be. (And in any case, we'd still need
to do this be be well behaved when using OpenEXR versions earlier than
where a fix might land.)
Solution tracked down by Edward Lam.
* DDS: fixes for A8, L8, A8L8 formats
While developing #3573 at one time I had them working properly, but then
while fixing the address sanitizer failure for 3-channel formats I
botched them again. Note to self: never again do a "yeah I'll add tests
later", since if I had all of them running all the time this would
not have happened. 🤦
* DDS: extend test coverage for more formats & channel size cases
With more test images recently added (https://github.com/OpenImageIO/oiio/pull/3459),
start using them for DDS tests. This covers now:
- Alpha, Luminance and Alpha+Luminance formats,
- RGB formats with rarer channel sizes (rgb10 a2, r3g3b2),
- Several "broken" DDS file cases
- Removed usage of sample-DXT1 since it is well covered by others.
Add a new build-time cmake cache variable: FORTIFY_SOURCE
Default to empty, but can be set to 1, 2, or 3, corresponding to
setting the _FORTIFY_SOURCE macro available in recent versions of
clang and gcc. (I'm not sure exactly which minimum compiler version is
needed for each fortification level, except that for level 3, gcc must
be 12+.)
Fortification involves replacing several "unsafe" memory-related
functions such as memcpy, memset, strcpy, etc, with special versions
that do bounds checking, aided by some compiler smarts for
understanding the likely bounds of different buffers. If I understand
correctly, at level 2 it can figure out bounds of constant-sized
arrays, and at level 3 it can figure out certain dynamic cases as
well.
There are two use cases for this:
1. For our own CI, this is yet another bit of static and dynamic
anslysis to enable (currently, just in the gcc12 test) to possibly
catch bugs.
2. Users who are building OIIO to deploy it in security-sensitive
environment may wish to build with some fortification level enabled to
help prevent certain memory errors or security issues, understanding
that it may slightly impact performance.
- If a DDS pixel format header contains "this is a normal map" flag
set, then handle that properly (#3525):
- For a DXT5, this means it's a "DXT5nm" format. Normal X & Y
components are in green and alpha channel respectively. Compute Z
component from those, and the result is a 3-channel tangent space
normal map.
- For ATI2/BC5 format that also has a "normal map" flag set, compute
the Z component automatically too, and result is a 3-channel image.
However a bunch of ATI2/BC5 normal maps out there in the wild do
not have that bit set, so also have a global "dds:bc5normal"
attribute to optionally turn this on.
- Fix support of R10G10B10A2 format - was producing garbage data due to
code assumption that truncation down to 8 bits never needs to happen.
- Fix bit expansion ("too dark") of very low-bit formats, e.g. R3G3B2.
Code was simply shifting the bits, instead of properly repeating
the high bits into new low bit positions (i.e. a 3-bit "111" needs
to become an 8-bit "11111111" and not a "11100000").
- Support "RXGB" (DXT5, with red & alpha swapped) as well as "BC4U"
and "BC5U" formats (the latter two are just BC4 and BC5, produced by
some older tools).
- Support alpha-only (e.g. A8) formats.
All this mostly prompted by doing DDS improvements in Blender, and
wanting to get Cycles renderer (which uses OIIO) to the same feature
parity. See https://developer.blender.org/T101405 and
https://developer.blender.org/D16087.
* Refactor the clang-format job to be part of the aswf batch of jobs
rather than separate (reduces redundant logic in the workflow file).
* Give proper names to distinguish the two Mac jobs' artifacts for
broken builds and their caches.
* Revise scorecard workflow triggers: no need to run weekly, and run
on push only to the main repo, no reason to run on forks.
Cmake-generated visual studio project files were using just one CPU core for all the compilation, which is not terribly fast. Add the `/MP` option so that it compiles separate .cpp files in parallel. On my (16 core, 32 thread) machine this makes a Release build of OpenImageIO.lib go from 136 sec down to 30 sec.
Also remove three raw test reference output files that we apparently
no longer use (corresponding to platform + libraw combinations that
aren't in our current set of CI configurations).
This was an experiment that was added to master at one point but
removed before we branched for a release. But we forgot to remove a
few headers. Removing them now is not a compatibility break, since we
never shipped a supported release that contained the library that
implemented the functions declared in these headers.
In addition to address and leak sanitizers, also turn on the undefined
behavior sanitizer in our CI run. We can do it in the same job as
asan/lsan.
Some things that come along for the ride, including fixes for things
that ubsan uncovered:
* src/build-scripts/ubsan-suppressions.txt can hold suppression list
(though we don't have any now, it just sets up the space for it for
future needs).
* Split the "targa" test so that the parts that directly need the OIIO
python module (which are all related to thumbnails) are now in
"targa-thumbnails" test. That has to be disabled for the sanitizers,
since we can't arrange for python to b linked to the sanitizer
libraries.
* Found vint4::store has a potentially unaligned integer store.
* Found Strutil::concat is not necessarily safe if either of the
strings are empty.
This was part of a set of obsolete calls where we recently removed the
implementations, but for link compatibility, made matching shim calls
and wrap the modern versions. But I missed one case.
Adapted from the similar document I wrote for OSL.
This serves both as a checklist for me to make sure I don't miss any
steps during a release, but also so that other people could take over
release procedures if necessary or desired.
I also rearranged the order of some lines in CMakeLists.txt for
clarity (but did not change the logic).
ImageSpec and ParamValueList have had (since 2.1) a getattributetype()
method, which simply returns the TypeDesc of the named attribute, or
TypeUnknown if no such attribute exits. This goes along well with the
related getattribute() and attribute() family of methods.
The getattribute() method was not present in ImageCache and
TextureSystem, despite their having very similar
getattribute()/attribute() interfaces.
A recent issue made me realize that in the Python bindings, the IC and
TS bindings for `getattribute(name)` did not work properly, always
returning None, and *required* you to use the full
`getattribute(name,type)` variety where you have to supply -- and
already know -- the type. This isn't especially Pythonic, where you
should just be able to ask by name and get a polymorphic result. And
when I went to look at how to repair this, it was going to be very
awkward without getattributetype(), but trivial with it.
So this patch sets up getattributetype() for both IC and TS, and
exposes them to both C++ and Python APIs, and also fully fixes the
getattribute() so that it works properly with the name only.
Unfortunately, because getattributename is a new virtual method, it
changes the vtable, breaking the ABI, and thus cannot be backported to
any supported release that makes ABI compatibility promises.
Currently when cross-compiling for Android build fails due to usage of
TRY_RUN(). In the particular case it tries to figure out whether atomics
can be used without the libatomic library.
Fortunately we don't need this check because on Android NDK atomics are
built-in and libatomic.so is not present. Only static dummy libatomic.a
exists for compatibility with -latomic flag.
This patch allows ParamValue, ParamValueList, and ImageSpec to support
attribute/getattribute of metadata consisting of uint8[] arrays, which
is how we store certain binary blobs or raw byte arrays. We translate
this to Python as a numpy array of type uint8 (dtype='B').
This fixes a limitation where it was not possible to directly retrieve
or set the "ICCProfile" metadata (which is stored as a uint8 array)
from the Python APIs, as you could easily do from the C++ APIs.
In the process, I found similar bits of code in several different
places, in various states of incompleteness. I refactored to make a
single make_pyobject() utility and then used that call in place of
several other redundant pieces of functionality.
TBB::tbb is added as a library dependency via TARGET_NAME_IF_EXISTS.
Therefore if an executable links OpenImageIO static library without the
executable itself depending on TBB, then TBB will not be linked in and
result in link time errors.
Just a couple missing pieces were preventing getattribute() from
retrieving attributes that were stored as int64 or uint64. That
doesn't come up very often, but one place is retrieving certain
statistics that use 64 bit counters. (This comes up in ImageCache,
among other places.)
While we're at it, there was no existing test dedicated to the python
binding of ImageCache. So to add these getattribute calls, I made one.
There is an important caveat about ImageCache.getattribute(): In this
process, I discovered that the flavor of IC.getattribute(name) where
you only pass the name is BROKEN, and will always return None. You
MUST use the form where you pass the type, like
x = ic.getattribute("foo", "int")
This is unfortunate and not very Pythonic, but it is the workaround
for now. I will addess getting the name-only form working in a
separate PR.
Closes#3537
For a while, we have found embedded ICC profiles in TIFF, JPEG,
JPEG-2000, PNG, and PSD (PhotoShop) files and store them in the
metadata as a byte array with the name "ICCProfile".
With this patch, we further add the facility of parsing the binary ICC
profile data to extract a number of other fields that we add to the
ImageSpec metadata so that "oiiotool -info -v" or "iinfo 'v"
will print this info.
Important caveat: You can read this for informational purposes, but
changing it (e.g. with spec.attribute()) does not change the contents
of the ICCProfile byte array, which is still the canonical ground
truth of the profile. You have to replace the "ICCProfile" byte array
to actually change the profile.
* Add `Strutil::utf16_to_utf8(const std::u16string& utf16str)`.
This is subtly different from our existing utf16_to_utf8() that
takes a std::wstring, because wchar_t is not the same on all
platforms. If it really and truly is utf16-in-char16_t, you need
this and can't use the utf-in-wchar_t which may be 32 bit chars.
* Add `Strutil::utf8_to_utf16wstring()` (identical to the old
utr8_to_utf16) to make it more self-documenting as returning a
utf16-encoded wstring and NOT a u16string. Note that wchar_t is not
necessarily char16_t on all platforms (it is on Windows, but it's 32
bits on Linux).
* Deprecate the ambiguously named `utf8_to_utf16()` for OIIO >= 2.5 and
remove it for OIIO >= 3.0.
This fixes builds in projects that compile OpenImageIO as static
libraries and don't themselves depend on the image libraries used by
OpenImageIO such as PNG, JPEG or TIFF.
When OpenImageIO is built as a static library its targets have e.g.
PNG::PNG among INTERFACE_LINK_LIBRARIES. If the including project does
not itself depend on PNG, the following error would be observed:
Target "<...>" links to target "PNG::PNG" but the target was not found.
Perhaps a find_package() call is missing for an IMPORTED target, or an
ALIAS target is missing?
Rather than specify the version in the usual project() statement,
project (OpenImageIO VERSION 2.5.0.0 ...)
this patch decouples it
set (OpenImageIO_VERSION "2.5.0.0" CACHE STRING "Version")
project (OpenImageIO VERSION ${OpenImageIO_VERSION} ...)
thus allowing the possibility of overriding the version number at
build confuguration time (`cmake -DOpenImageIO_VERSION=3.1.4.1`).
This should be used with extreme caution. Probably never used by most
people. I don't need to describe all the ways that things can go
sideways if you make your build of x.y.z.w actually be arbitrarily
different from the x.y.z.w that everyone else uses and that is
documented.
That said, there are two scenarios I have in mind for which this
ability is very useful:
1. Sometimes, we add or deprecate API features "in advance" by having
an `#if` enable or disable code based on the version, essentially
putting in place changes that can't be enabled yet because they
would break compatibility rules, but we want to integrate the code
now and have it become active upon reaching a particular future
version in which that incompatibility will be allowed. The version
number override makes it easy to test this by quickly doing a build
"as if" we had already bumped the version number to see what will
happen when we actually do.
2. Every once in a while we need to do an internal build that, to
conform to version compatibility rules, needs to appear to still be
part of a past version family. This lets individual sites play some
games with version number overrides rather than have to create new
branches and tags for certain one-off internal releases.
--iccread reads a named file and directly adds its contents as the
"ICCProfile" metadata of the top image (as a byte array).
--iccwrite takes the "ICCProfile" metadata of the top image and
writes it to a named file.
Oops, at some point, something about the way we set up the CI test
that runs the sanitizers, and ever since then (embarrassingly, perhaps
as much as a couple years ago), the "sanitizer" test has not in fact
been running the sanitizers. Here we fix that up, get sanitizers
running again.
* Minor png refactoring to fix some memory leaks for broken png files.
* Remove unused string variable from read_info() and simplify the
surrounding "if"
* Split png test into png and png-damaged, because I just can't seem
to squash some very minor memory leaks for broken png files, I think
it's not worth worrying about and I'll just exclude the broken case
from testing for the address sanitizer.
Reading uncompressed scanlines would copy 4 bytes from each pixel into
an int and then decode the bits, incrementing where it was reading
from by Bpp bytes each time. But when Bpp == 3, the last pixel of
every row would therefore read an extra byte past the end of the heap
storage. Allocation padding would in practice keep it from crashing,
and that meaningless extra byte was not used in the decode because it
knew it only needed the first 3 of the integer. But found by address
sanitizer!
OpenImageIO has been accepted to be an ASWF-administered project, by vote of
the ASWF Technical Advisory Council (TAC) on May 3, 2023.
At this time, the ASWF subdirectory contains *proposed* (and sometimes TBD/WIP
placeholders) project organization and policy documents that will be formally
adopted as the OpenImageIO project is formally turned over to ASWF and moved
to the AcademySoftwareFoundation GitHub account. Please file issues or PRs
or review comments against these documents to make any suggested changes while
we are revising the drafts.
Summary of the current state of documents:
| File | Status | Notes |
| ---- | ------ | ----- |
| CLA-corporate.md | Final | Standard ASWF corporate CLA, approved by the TSC. Will take effect when the repo moves. |
| CLA-individual.md | Final | Standard ASWF individual CLA, approved by the TSC. Will take effect when the repo moves. |
| CODE_OF_CONDUCT.md | Final | Links to the standard LF CoC, approved by the TSC and now in effect. |
| CONTRIBUTING.md | Preliminary | Modified version of our existing CONTRIBUTING guidelines, lightly amended to reflect the changes that will come along with moving the repo, the new CLA/DCO rules, etc.
| GOVERNANCE.md | Final | Approved by the TSC and now in effect. |
| project_intake.md | Ongoing | This is the latest ASWF project intake checklist. We will check off items as we get them done. |
| Technical-Charter.md | Final | Approved by the TSC. |
This is a checklist for TSC's to review as part of the intake process. The TSC should review this entire list during the kickoff meeting. For anything outstanding, create an [issue](../issues) to track and link to it in the list
- Existing Project Governance
- [X] A [LICENSE.md](../LICENSE.md) file in every code repository, with the license chosen an OSI-approved license.
- [X] Any third-party components/dependencies included are listed along with thier licenses ( [THIRD-PARTY.md](../THIRD-PARTY.md) )
- [X] A [README.md](../README.md) file welcoming new community members to the project and explaining why the project is useful and how to get started.
- [X] A [CONTRIBUTING.md](../CONTRIBUTING.md) file explaining to other developers and your community of users how to contribute to the project. The file should explain what types of contributions are needed and how the process works, along with how to disclose security issues responsibly ( may also point to a [SECURITY.md](../SECURITY.md) file ).
- [ ] A [CODEOWNERS](../CODEOWNERS) or [COMMITTERS](../COMMITERS.csv) file to define individuals or teams that are responsible for code in a repository; document current project owners and current and emeritus committers.
- [X] A [CODE_OF_CONDUCT.md](../CODE_OF_CONDUCT.md) file that sets the ground rules for participants’ behavior associated and helps to facilitate a friendly, welcoming environment. By default, projects should leverage the Linux Foundation Code of Conduct unless an alternate Code of Conduct was previously approved.
- [X] A [RELEASING.md](../RELEASING.md) file that provides documentation on the release methodology, cadence, criteria, etc.
- [X] A [GOVERNANCE.md](../GOVERNANCE.md) file that documents the project’s technical governance.
- [ ] A [SUPPORT.md](../SUPPORT.md) file to let users and developers know about ways to get help with your project.
- Infrastructure/Assets
- [X] Technical Charter Approved
- [X] License scan completed and no issues found
- [ ] Code repository imported to ASWF GitHub organization or ownership of current GitHub organization given to `thelinuxfoundation` user
- [X] Developer Certificate of Origin past commit signoff done and DCO Probot enabled.
- [X] Issue/feature tracker established (JIRA, GitHub issues)
- [X] Mailing lists ( one of )
- [X] Setup new lists ( -dev@ and -tsc@ ) on [ASWF groups.io](https://lists.aswf.io) ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to setup )
- [X] (In progress) Move to [ASWF groups.io](https://lists.aswf.io) ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to transfer )
- [X] Slack ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to setup project channel on [ASWF Slack](https://slack.aswf.io) )
- [ ] Website
- [X] CI/build environment
- [X] Trademarks/mark ownership rights ( complete 'LF Projects - Form of Trademark and Account Assignment' - create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) - only needed if project using existing name )
- [ ] Domain name ( create [issue with the LF IT staff](https://jira.linuxfoundation.org/plugins/servlet/theme/portal/2/group/19) to setup/transfer )
- [ ] Zoom account access ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to get access to credentials )
- [X] (N/A) Social media accounts or other project accounts ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to transfer )
- [X] Logo(s) ( create [issue on artwork repo](https://github.com/AcademySoftwareFoundation/artwork/issues/new) to add in SVG and PNG format and color/black/white )
- New Project Governance
- [X] TSC members identified
- [X] First TSC meeting held
- [X] TSC meeting cadence set and added to project calendar
- [X] CLA Approved ( if used ) ( [CCLA](CLA-corporate.md) and [ICLA](CLA-individual.md) )
- Project charter ( [Technical-Charter.md](Technical-Charter.md) )
- [X] [Core Infrastructure Initiative Best Practices Badge](https://bestpractices.coreinfrastructure.org/) achieved as the 'Passing' level.
- Outreach
- [ ] New project announcement done ( create [issue on foundation repo](https://github.com/AcademySoftwareFoundation/foundation/issues/new) to trigger )
- [X] Project added to ASWF website and ASWF landscape
- Incubation Stage requirements
- [X] Have achieved and maintained a [OpenSSF Best Practices Badge at the passing level](https://bestpractices.coreinfrastructure.org/en/criteria).
- [ ] Have had a successful license scan with any critical issues remedied.
- [ ] Have a defined project mission and scope
- [ ] An overview of the project’s architecture and features defined.
- [ ] A project roadmap defined, which should address the following questions.
- What use cases are possible now?
- What does the next year look like in terms of additional features and use cases covered?
- [ ] Community and contributor growth assessment
- The current number of contributors and committers, and the number of different organizations contributing to the project.
- Demonstrate a sustained flow of commits / merged contributions
- A credible plan for developing a thriving user community, in particular expanding the number of committers and contributors?
- Outline of the plan for the project to complete the requirements for Adopted Stage
- [ ] Obtain an affirmative vote of the TAC.
- Adopted Stage graduation requirements
- [ ] Demonstrate a substantial ongoing flow of commits and merged contributions, authored by a healthy number of diverse contributors.
- [ ] Demonstrable roadmap progress.
- [ ] A healthy number of public adopters that are identified within the project ( using an ADOPTERS file or showcased on the project’s website ).
- [ ] Have achieved and maintained an [OpenSSF Best Practices Badge at the gold level](https://bestpractices.coreinfrastructure.org/en/criteria/2).
- [ ] Have a technical lead appointed for voting representation of the project to the TAC.
- [ ] Be deemed by the TAC to add value to the mission of ASWF.
- [ ] 2/3 supermajority vote of the TAC
- [ ] Affirmative majority vote of the Governing Board
@@ -246,9 +252,8 @@ The command 'make help' will list all possible options.
You can also ignore the top level Makefile wrapper, and instead use
CMake directly:
mkdir build
cd build
cmake ..
cmake -B build -S .
cmake --build build --target install
If the compile stops because of warnings, try again with
@@ -257,8 +262,9 @@ If the compile stops because of warnings, try again with
or, if you are using CMake directly,
cd build
cmake -DSTOP_ON_WARNING=0 ..
rm -rf build
cmake -B build -S . -DSTOP_ON_WARNING=0
cmake --build build --target install
@@ -268,8 +274,87 @@ Building on Windows
**Method 1 - from source**
I really need someone to write correct, modern docs about how to build
from source on Windows.
You will need to have Git, CMake and Visual Studio installed.
The minimal set of dependencies for OIIO is: Boost, zlib, libTIFF, OpenEXR, and libjpeg or libjpeg-turbo. If you have them built somewhere then you skip
the section below, and will only have to point OIIO build process so their locations.
* Boost: get the boost source archive, extract into `{BOOST_ROOT}`.
```
cd {BOOST_ROOT}
bootstrap
b2
```
* zlib: this will build it, and then delete the non-static library, so they don't get picked up:
* OpenEXR: you'll have to point it to your `{ZLIB_ROOT}` location from the above. If copy-pasting the multi-line command (with lines ending in `^`) into
cmd.exe prompt, make sure to copy all the lines at once.
| master | :white_check_mark: :construction: ALL fixes immediately, but this is a branch under development with a frequently unstable ABI and occasionally unstable API. |
| 2.3.x | :white_check_mark: All fixes that can be backported without breaking ABI compatibility. New tagged releases monthly. |
| 2.2.x | :x: Only receives occasional critical fixes, upon request. |
| <= 2.1.x | :x: No longer receiving patches of any kind. |
| 2.4.x | :white_check_mark: All fixes that can be backported without breaking ABI compatibility. New tagged releases monthly. |
| 2.3.x | :x: Only receives occasional critical fixes, upon request. |
| <= 2.2.x | :x: No longer receiving patches of any kind. |
## Reporting a Vulnerability
If you think you've found a potential vulnerability in OpenImageIO, please
report it by emailing security@openimageio.org. Only the project administrator
has access to these messages. Include detailed steps to reproduce the issue,
report it by emailing security@openimageio.org. Only the project administrators
have access to these messages. Include detailed steps to reproduce the issue,
and any other information that could aid an investigation. Our policy is to
message(STATUS"${ColorYellow}WARNING When building for C++17, LibRaw should be 0.20 or higher (found ${LibRaw_VERSION}). You may get errors, depending on the compiler.${ColorReset}")
@@ -246,21 +253,22 @@ checked_find_package (R3DSDK) # RED camera
set(NUKE_VERSION"7.0"CACHESTRING"Nuke version to target")
checked_find_package(Nuke)
checked_find_package(OpenGL)#usedforiv
#Qt--usedforiv
set(qt5_modulesCoreGuiWidgets)
if(OPENGL_FOUND)
list(APPENDqt5_modulesOpenGL)
endif()
option(USE_QT"Use Qt if found"ON)
checked_find_package(Qt5COMPONENTS${qt5_modules})
if(USE_QTANDNOTQt5_FOUNDANDAPPLE)
message(STATUS" If you think you installed qt5 with Homebrew and it still doesn't work,")
set(ILMBASE_LIBRARIES${OPENEXR_IMATH_LIBRARY}${OPENEXR_IEX_LIBRARY}${OPENEXR_HALF_LIBRARY}${OPENEXR_ILMTHREAD_LIBRARY}${ILMBASE_PTHREADS}CACHESTRING"The libraries needed to use IlmBase")
set(OPENEXR_LIBRARIES${OPENEXR_ILMIMF_LIBRARY}${ILMBASE_LIBRARIES}${ZLIB_LIBRARIES}CACHESTRING"The libraries needed to use OpenEXR")
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Larry Gritz
