Skip to content
Draft
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
19 changes: 19 additions & 0 deletions c/image_c_api.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,25 @@ f3d_image_t* f3d_image_new_path(const char* path)
return reinterpret_cast<f3d_image_t*>(img);
}

//----------------------------------------------------------------------------
f3d_image_t* f3d_image_new_stream(unsigned char* byte, unsigned int size)
{

f3d::image* img = nullptr;

try
{
img = new f3d::image(reinterpret_cast<std::byte*>(byte), static_cast<size_t>(size));
}
catch (const f3d::image::read_exception& e)
{
std::cerr << "Error loading image: " << e.what() << "\n";
return nullptr;
}

return reinterpret_cast<f3d_image_t*>(img);
}

//----------------------------------------------------------------------------
void f3d_image_delete(f3d_image_t* img)
{
Expand Down
9 changes: 9 additions & 0 deletions c/image_c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ extern "C"
*/
F3D_EXPORT f3d_image_t* f3d_image_new_path(const char* path);

/**
* @brief Create a new image object from a stream
*
* The returned image must be deleted with f3d_image_delete().
*
* @return Pointer to the newly created image object, NULL on failure
*/
F3D_EXPORT f3d_image_t* f3d_image_new_stream(unsigned char* buffer, unsigned int size);

/**
* @brief Delete an image object
* @param img Pointer to the image object to be deleted
Expand Down
19 changes: 18 additions & 1 deletion c/testing/test_image.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,28 @@ int test_image()
return 1;
}

f3d_image_t* temp_image_stream = f3d_image_new_stream(tempBuffer, 0);
if (temp_image_stream != NULL){
return 1;
}

unsigned int buffer_size;
unsigned char* buffer = f3d_image_save_buffer(img, PNG, &buffer_size);
if (buffer)
{
f3d_image_free_buffer(buffer);
f3d_image_t* image_stream = f3d_image_new_stream(buffer, buffer_size);

if (image_stream)
{
double stream_error = f3d_image_compare(img, image_stream);
f3d_image_free_buffer(buffer);
f3d_image_delete(image_stream);

if (stream_error != 0)
{
return 1;
}
}
}

const char* tmp_path = "/tmp/f3d_test_image.png";
Expand Down
10 changes: 10 additions & 0 deletions java/F3DImageBindings.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ extern "C"
return reinterpret_cast<jlong>(img);
}

JNIEXPORT jlong JAVA_BIND(Image, nativeCreateFromStream)(
JNIEnv* env, jclass, jbyteArray buffer, jint size)
{
jbyte* bufferData = env->GetByteArrayElements(buffer, nullptr);

f3d::image* img =
new f3d::image(reinterpret_cast<std::byte*>(bufferData), static_cast<size_t>(size));
return reinterpret_cast<jlong>(img);
}

JNIEXPORT jlong JAVA_BIND(Image, nativeCreate)(
JNIEnv* env, jclass, jint width, jint height, jint channelCount, jint type)
{
Expand Down
5 changes: 5 additions & 0 deletions java/Image.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ public Image(int width, int height, int channelCount) {
this(width, height, channelCount, ChannelType.BYTE);
}

public Image(byte[] buffer, int size){
mNativeAddress = nativeCreateFromStream(buffer, size);
}

Image(long nativeAddress) {
mNativeAddress = nativeAddress;
}
Expand Down Expand Up @@ -233,5 +237,6 @@ public void delete() {

private static native long nativeCreateFromFile(String filePath);
private static native long nativeCreate(int width, int height, int channelCount, int type);
private static native long nativeCreateFromStream(byte[] buffer, int size);
private static native void nativeDestroy(long nativeAddress);
}
6 changes: 5 additions & 1 deletion java/testing/TestImage.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static void main(String[] args) {
img1.save(tmpPath + "test.jpg", Image.SaveFormat.JPG);

img1.saveBuffer();
img1.saveBuffer(Image.SaveFormat.PNG);
byte[] img1Buffer = img1.saveBuffer(Image.SaveFormat.PNG);

img1.setMetadata("key1", "value1");
img1.getMetadata("key1");
Expand All @@ -47,8 +47,12 @@ public static void main(String[] args) {
.setMetadata("author", "F3D")
.save(tmpPath + "chained.png");

Image img4 = new Image(img1Buffer, img1Buffer.length);
img4.compare(img1);

img1.delete();
img2.delete();
img3.delete();
img4.delete();
}
}
1 change: 1 addition & 0 deletions library/public/image.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class F3D_EXPORT image
image& operator=(const image& img) noexcept;
image(image&& img) noexcept;
image& operator=(image&& img) noexcept;
image(std::byte* byte, std::size_t size);
Copy link
Member

Choose a reason for hiding this comment

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

document separately and add doc about the behavior, espacially the exceptions

///@}

///@{ @name Operators
Expand Down
10 changes: 10 additions & 0 deletions library/src/config.cxx
Copy link
Member

Choose a reason for hiding this comment

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

this should not be commited, also do not configure in source to avoid this.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include "config.h"

namespace f3d::detail
{
const std::string LibVersion = ""; // Version is synchronized with F3D
const std::string LibVersionFull = ""; // Version is synchronized with F3D
const std::string LibBuildSystem = "Darwin ";
const std::string LibBuildDate = "";
const std::string LibCompiler = "AppleClang 17.0.0.17000604";
}
68 changes: 68 additions & 0 deletions library/src/image.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <vtkImageReader2Collection.h>
#include <vtkImageReader2Factory.h>
#include <vtkJPEGWriter.h>
#include <vtkMemoryResourceStream.h>
#include <vtkPNGReader.h>
#include <vtkPNGWriter.h>
#include <vtkPointData.h>
Expand All @@ -33,6 +34,7 @@
#include <sstream>
#include <string>
#include <unordered_map>
#include <iostream> //debug
Copy link
Member

Choose a reason for hiding this comment

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

to remove


namespace fs = std::filesystem;

Expand Down Expand Up @@ -258,6 +260,72 @@ image& image::operator=(image&& img) noexcept
return *this;
}

image::image(std::byte* buffer, std::size_t size)
: Internals(new image::internals())
{
detail::init::initialize();

try
{
if (buffer == nullptr)
{
delete this->Internals;
throw read_exception("Cannot read image from buffer: Buffer is empty");
}

vtkNew<vtkMemoryResourceStream> stream;
stream->SetBuffer(buffer, size);

vtkNew<vtkImageReader2Collection> collection;
vtkImageReader2Factory::GetRegisteredReaders(collection);
collection->InitTraversal();
vtkImageReader2* reader = collection->GetNextItem();

int bestScore = -1;
vtkImageReader2* bestReader = nullptr;

while (reader != nullptr)
{
int score = reader->CanReadFile(stream);
if (score > bestScore)
{
bestReader = reader;
bestScore = score;
}

reader = collection->GetNextItem();
}
Comment on lines +287 to +297
Copy link
Member

Choose a reason for hiding this comment

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

there is no API to do this for you in VTK ?

Copy link
Author

Choose a reason for hiding this comment

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

there is no API to do this for you in VTK ?

I'm looking at vtkImageReader2 docs, and I don't see an API that does this

I'm also looking at the relevant commits in vtk and there is CanReadFile(stream) support for the different readers but not a function that consolidates and finds best reader based on the stream buffer

Copy link
Member

Choose a reason for hiding this comment

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

I think I forgot to add it, ill take a look, but lets of course keep it here for now.


if (bestScore <= 0)
{
delete this->Internals;
throw read_exception("Cannot read image from buffer");
}

bestReader->SetStream(stream);
bestReader->Update();
this->Internals->Image = bestReader->GetOutput();

vtkPNGReader* pngReader = vtkPNGReader::SafeDownCast(bestReader);
if (pngReader != nullptr)
{
this->Internals->ReadPngMetadata(pngReader);
}

if (!this->Internals->Image)
{
delete this->Internals;
throw read_exception("Cannot read image from buffer");
}
}

catch (const fs::filesystem_error& ex)
{
delete this->Internals;
throw read_exception(std::string("Cannot read image from buffer: ") + ex.what());
}
}

//----------------------------------------------------------------------------
std::vector<std::string> image::getSupportedFormats()
{
Expand Down
11 changes: 11 additions & 0 deletions library/testing/TestSDKImage.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ int TestSDKImage([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
test("check 32-bits HDR image channel type size", hdrImg.getChannelTypeSize(), 4u);
hdrImg.save(tmpDir + "/TestSDKImage32hdr.tif", f3d::image::SaveFormat::TIF);

// check reading stream
std::vector<unsigned char> buffer = generated.saveBuffer();
std::byte* bufferData = reinterpret_cast<std::byte*>(buffer.data());
f3d::image bufferImage(bufferData, buffer.size());
// std::cerr << generated.compare(bufferImage) << "\n";
test("check loading stream from image reader", generated.compare(bufferImage), 0.0); // not 1e-14?

// // check reading invalid stream
test.expect<f3d::image::read_exception>(
"read invalid image stream", [&]() { f3d::image invalidImgStream(nullptr, 10); });

#if F3D_MODULE_EXR
// check reading EXR
f3d::image exrImg(testingDir + "/data/kloofendal_43d_clear_1k.exr");
Expand Down
5 changes: 5 additions & 0 deletions python/F3DPythonBindings.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ PYBIND11_MODULE(pyf3d, module)
.def(py::init<>())
.def(py::init<const std::filesystem::path&>())
.def(py::init<unsigned int, unsigned int, unsigned int, f3d::image::ChannelType>())
.def(py::init([](py::bytes buffer, size_t size){
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.def(py::init([](py::bytes buffer, size_t size){
.def(py::init([](py::bytes buffer){

Unlike in C which uses a raw pointer, the length can and should be retrieved from the Python buffer object, see setImageBytes above.

I believe this also applies to Java and WebAssembly

Copy link
Author

Choose a reason for hiding this comment

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

Unlike in C which uses a raw pointer, the length can and should be retrieved from the Python buffer object, see setImageBytes above.

I believe this also applies to Java and WebAssembly

Would there ever be a situation where the user wants to read only part of the buffer?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would there ever be a situation where the user wants to read only part of the buffer?

I don't think so, in python I'd slice the buffer with buffer[:n] to keep only the first n bytes. IMO, if we were concerned about allowing to read only a part of a bigger buffer (for performance/memory reasons) we'd need to have something like f(buffer, start, end) or f(buffer, start, len).

From what I can tell this whole extra size/end parameter thing is only because the technical limitation in C were pointers represent the start of the memory region and nothing else. @Meakk am I missing something?

Copy link
Member

Choose a reason for hiding this comment

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

That's right, let's drop the size argument, otherwise it will look odd from Python

std::string bufferStr = buffer;
std::byte* byte = reinterpret_cast<std::byte*>(bufferStr.data());
return f3d::image(byte, size);
}))
.def(py::self == py::self)
.def(py::self != py::self)
.def_static("supported_formats", &f3d::image::getSupportedFormats)
Expand Down
7 changes: 7 additions & 0 deletions python/testing/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ def test_save_buffer(f3d_engine: f3d.Engine):
assert buffer.startswith(b"\x89PNG")
assert img._repr_png_() == buffer

def test_loading_stream(f3d_engine: f3d.Engine):
img = f3d_engine.window.render_to_image(True)
buffer = img.save_buffer(f3d.Image.SaveFormat.PNG)

stream_image = f3d.Image(buffer, len(buffer))
assert stream_image == img


def test_formats(f3d_engine: f3d.Engine):
formats = f3d.Image.supported_formats()
Expand Down
1 change: 1 addition & 0 deletions webassembly/F3DEmscriptenBindings.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ EMSCRIPTEN_BINDINGS(f3d)
.constructor<>()
.constructor<const std::string&>()
.constructor<unsigned int, unsigned int, unsigned int, f3d::image::ChannelType>()
.constructor<std::byte* buffer, std::size_t size>()
.function("equals", &f3d::image::operator==)
.function(
"getNormalizedPixel", +[](const f3d::image& img, int x, int y) -> emscripten::val
Expand Down
Loading