Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Feature/1187 Read EXR files from memory #1448

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions application/testing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,9 @@ if(F3D_PLUGIN_BUILD_USD)

# https://gitlab.kitware.com/vtk/vtk/-/merge_requests/8224
if(VTK_VERSION VERSION_GREATER_EQUAL 9.0.20210805) # for embedded material support
if(F3D_MODULE_EXR)
f3d_test(NAME TestEXRReadMemory DATA small.usdz ARGS --load-plugins=usd DEFAULT_LIGHTS)
endif()
f3d_test(NAME TestUSDZAnimated DATA AnimatedCube.usdz ARGS --load-plugins=usd --animation-time=0.3 --animation-progress)
f3d_test(NAME TestUSDZRigged DATA RiggedSimple.usdz ARGS --load-plugins=usd --animation-time=0.3)

Expand Down
3 changes: 3 additions & 0 deletions testing/baselines/TestEXRReadMemory.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions testing/data/DATA_LICENSES.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
- world*: VTK Data: BSD-3-Clause
- 16bits.*: @bisechen: [CC0](https://creativecommons.org/publicdomain/zero/1.0/)
- (ノಠ益ಠ )ノ.vtp: VTK Data: BSD-3-Clause
- Rec709.exr: [Copyright 2006 Industrial Light & Magic](https://github.com/AcademySoftwareFoundation/openexr/blob/370db2835843ac75f85e1386c05455f26a6ff58c/website/test_images/Chromaticities/Rec709.rst): BSD-3-Clause
Copy link
Contributor

Choose a reason for hiding this comment

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

please also add `small.usdz: Copyright USD contributors: CC-BY 4.0

Copy link
Contributor

Choose a reason for hiding this comment

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

with the links please

- small.usdz: Copyright USD contributors: CC-BY 4.0

All other datasets are licensed under the BSD-3-Clause F3D license.

Expand Down
3 changes: 3 additions & 0 deletions testing/data/Rec709.exr
Git LFS file not shown
3 changes: 3 additions & 0 deletions testing/data/small.usdz
Git LFS file not shown
3 changes: 2 additions & 1 deletion vtkext/private/module/Testing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ endif()
if(F3D_MODULE_EXR)
list(APPEND test_sources
TestF3DEXRReader.cxx
TestF3DEXRReaderInvalid.cxx)
TestF3DEXRReaderInvalid.cxx
TestF3DEXRMemReader.cxx)
mwestphal marked this conversation as resolved.
Show resolved Hide resolved
endif()

if(VTK_VERSION VERSION_LESS_EQUAL 9.1.0)
Expand Down
53 changes: 53 additions & 0 deletions vtkext/private/module/Testing/TestF3DEXRMemReader.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#include <vtkImageData.h>
#include <vtkNew.h>

#include "vtkF3DEXRReader.h"

#include <fstream>
#include <iostream>
#include <vector>
/**
* image file taken from
* https://github.com/AcademySoftwareFoundation/openexr/blob/370db2835843ac75f85e1386c05455f26a6ff58c/website/test_images/Chromaticities/Rec709.rst
Copy link
Contributor

Choose a reason for hiding this comment

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

im not sure how that .rst file is related to the .exr file ?

Copy link
Author

Choose a reason for hiding this comment

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

the rst file is just a markdown file on github showing the exr file and it details. I just grabbed it because it was smaller than the other exr files in the repo which made the unit test run a but faster.

https://github.com/AcademySoftwareFoundation/openexr/blob/370db2835843ac75f85e1386c05455f26a6ff58c/website/test_images/Chromaticities/Rec709.rst

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to just use an existing one. grayscale.exr for example is very small.

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed, better to use an existing one.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove this line, since you list it in data licenses now.

Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this line

*/
bool readFileToVector(const std::string& filename, std::vector<char>& buffer)
{
std::ifstream file(filename, std::ios::binary | std::ios::ate);
if (!file)
{
return false;
}

std::streamsize size = file.tellg();
file.seekg(0, std::ios::beg);
buffer.resize(size);
return file.read(buffer.data(), size) ? true : false;
}

int TestF3DEXRMemReader(int argc, char* argv[])
{
vtkNew<vtkF3DEXRReader> reader;

std::string actual_filename = std::string(argv[1]) + "data/Rec709.exr";
std::string filename = "readFromMem.exr";
reader->SetFileName(filename.c_str());
std::vector<char> buff;
readFileToVector(actual_filename, buff);
reader->SetMemoryBuffer(buff.data());
reader->SetMemoryBufferLength(buff.size());
reader->Update();

reader->Print(cout);

vtkImageData* img = reader->GetOutput();

int* dims = img->GetDimensions();

if (dims[0] != 610 && dims[1] != 406)
{
std::cerr << "Incorrect EXR image size." << dims[0] << ":" << dims[1] << std::endl;
return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}
98 changes: 89 additions & 9 deletions vtkext/private/module/vtkF3DEXRReader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,60 @@
#include "vtksys/FStream.hxx"

#include <ImfArray.h>
#include <ImfIO.h>
#include <ImfRgbaFile.h>

#include <cstring>
#include <sstream>
#include <thread>

/**
* Class to treat file contents in memory like it were still in a file.
*/
class MemStream : public Imf::IStream
{
public:
MemStream(const char* name, const void* buff, vtkIdType bufferLen)
: Imf::IStream(name)
, buffer(static_cast<const char*>(buff))
, bufflen(static_cast<size_t>(bufferLen))
{
}

bool read(char c[], int n) override
{
if (pos + n <= bufflen)
{
memcpy(c, buffer + pos, n);
pos += n;
return true;
}
return false;

Check warning on line 39 in vtkext/private/module/vtkF3DEXRReader.cxx

View check run for this annotation

Codecov / codecov/patch

vtkext/private/module/vtkF3DEXRReader.cxx#L39

Added line #L39 was not covered by tests
}

/**
* returns the current reading position, in bytes, from the beginning of the file.
* The next read() call will begin reading at the indicated position
*/
uint64_t tellg() override

Check warning on line 46 in vtkext/private/module/vtkF3DEXRReader.cxx

View check run for this annotation

Codecov / codecov/patch

vtkext/private/module/vtkF3DEXRReader.cxx#L46

Added line #L46 was not covered by tests
{
return pos;

Check warning on line 48 in vtkext/private/module/vtkF3DEXRReader.cxx

View check run for this annotation

Codecov / codecov/patch

vtkext/private/module/vtkF3DEXRReader.cxx#L48

Added line #L48 was not covered by tests
Copy link
Contributor

@Meakk Meakk Jun 5, 2024

Choose a reason for hiding this comment

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

Why isn't this line covered? Do we need to override this function if it's not called?

Copy link
Author

Choose a reason for hiding this comment

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

from this page
https://openexr.com/en/latest/ReadingAndWritingImageFiles.html#low-level-i-o
it seems like that function needs to be overriden to be a proper interface

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure to follow that answer

Copy link
Contributor

Choose a reason for hiding this comment

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

if that method is not used you can just remove it ?

Copy link
Author

Choose a reason for hiding this comment

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

it is a pure virtual function in the base class so i believe it needs an implementation. I can try removing it and see what happens. In the documentation i posted it says it needs an implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok lets try that. Maybe its needed in some case, but not in our case.

Copy link
Author

@nabielkandiel nabielkandiel Jun 18, 2024

Choose a reason for hiding this comment

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

it does not build without that function being implemented

error: cannot declare variable ‘memoryStream’ to be of abstract type ‘MemStream’
198 | MemStream memoryStream("EXRmemoryStream", this->MemoryBuffer, this->MemoryBufferLength);

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, lets keep it in for now.

}

/**
* sets the current reading position to pos bytes from the beginning of the "file"
*/
void seekg(uint64_t new_pos) override
{
pos = new_pos;
}

private:
const char* buffer;
size_t bufflen;
uint64_t pos{ 0 };
};

vtkStandardNewMacro(vtkF3DEXRReader);

//------------------------------------------------------------------------------
Expand All @@ -36,15 +85,14 @@

// Setup filename to read the header
this->ComputeInternalFileName(this->DataExtent[4]);
if (this->InternalFileName == nullptr || this->InternalFileName[0] == '\0')
if ((this->InternalFileName == nullptr || this->InternalFileName[0] == '\0') &&
!this->MemoryBuffer)
{
return;
}

try
auto execute = [&](Imf::RgbaInputFile& file)
{
Imf::RgbaInputFile file(this->InternalFileName);

Imath::Box2i dw = file.dataWindow();
this->DataExtent[0] = dw.min.x;
this->DataExtent[1] = dw.max.x;
Expand All @@ -56,6 +104,21 @@
{
throw std::runtime_error("only RGB and RGBA channels are supported");
}
};

try
{
if (this->MemoryBuffer)
{
MemStream memoryStream("EXRmemoryStream", this->MemoryBuffer, this->MemoryBufferLength);
Imf::RgbaInputFile file = Imf::RgbaInputFile(memoryStream);
execute(file);
}
else
{
Imf::RgbaInputFile file(this->InternalFileName);
execute(file);
}
}
catch (const std::exception& e)
{
Expand Down Expand Up @@ -112,12 +175,8 @@
scalars->SetName("Pixels");
float* dataPtr = scalars->GetPointer(0);

try
auto execute = [&](Imf::RgbaInputFile& file)
{
assert(this->InternalFileName);
Imf::setGlobalThreadCount(std::thread::hardware_concurrency());
Imf::RgbaInputFile file(this->InternalFileName);

Imf::Array2D<Imf::Rgba> pixels(this->GetHeight(), this->GetWidth());

file.setFrameBuffer(&pixels[0][0], 1, this->GetWidth());
Expand All @@ -134,6 +193,27 @@
dataPtr += 3;
}
}
};

try
{
Imf::setGlobalThreadCount(std::thread::hardware_concurrency());

if (this->MemoryBuffer)
{
MemStream memoryStream("EXRmemoryStream", this->MemoryBuffer, this->MemoryBufferLength);
Imf::RgbaInputFile file = Imf::RgbaInputFile(memoryStream);
execute(file);
}
else
{
if (!(this->InternalFileName))
{
throw std::invalid_argument("Not filename in EXR Reader when no Memory Buffer is set.");

Check warning on line 212 in vtkext/private/module/vtkF3DEXRReader.cxx

View check run for this annotation

Codecov / codecov/patch

vtkext/private/module/vtkF3DEXRReader.cxx#L212

Added line #L212 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

can this happen ?

Do you really need to throw here ? you could just error out ?

}
Imf::RgbaInputFile file(this->InternalFileName);
execute(file);
}
}
catch (const std::exception& e)
{
Expand Down