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

Loading half data directly #11

Closed
andrewfb opened this issue Jun 15, 2015 · 7 comments
Closed

Loading half data directly #11

andrewfb opened this issue Jun 15, 2015 · 7 comments

Comments

@andrewfb
Copy link
Contributor

Hi - thanks for all your work on this library. I'm working with @richardeakin to explore integrating it with Cinder. If I'm not misreading it, it looks like there's no way to load image data as half even when that is the format on disk. Is this something you'd be open to adding? Essentially we're trying to do a fp16 .exr -> OpenGL texture pipeline without any intermediate conversion to or from float.

@syoyo
Copy link
Owner

syoyo commented Jun 16, 2015

Hello Andrew.

Yes, currently tinyexr doesn't output half image directly(tinyexr widen half pixel to float type).
Its rather easy to output half image as is, for example by adding preferred pixel type flag in LoadMultiChannelEXR.

@syoyo
Copy link
Owner

syoyo commented Jun 19, 2015

In cdf93e6, I have added internal_pixel_types and internal_images field, which stores pixelType in .EXR and original fp16 value for half pixel image channel, respectively.

For other pixel type(i.e. FLOAT and UINT), internal_images[c] is set to NULL.

@andrewfb @richardeakin could you please try to use these field for fp16 texture?

@andrewfb
Copy link
Contributor Author

I think this would address our immediate use case, but I wonder if there might be a more general solution. It seems to me TinyEXR has as a first priority making the standard RGBA case easy, with f32 being the most general type. However most of the makings for a generalized EXR read-write solution are already here; specifically reading f16 and uint types, as well as arbitrary channels. I think this could be achieved pretty easily, though it may require a 2-function API. As a suggestion:

EXRImage image;
ParseEXRFromMemory( &image, rawExrData ); // inits 'image'
LoadEXR( &image );
FreeExrImage( &image );

This of course is not an improvement in its own right; it would simply return float data by default. But it would allow something like this:

EXRImage image;
ParseEXRFromMemory( &image, rawExrData );
for( size_t c = 0; c < image.num_channels; ++c )
    image.requested_pixel_types[c] = TINYEXR_PIXELTYPE_HALF;
LoadEXR( &image );
half *channel0Data = (half*)image.images[0];
FreeExrImage( &image );

In this case, we've requested half data regardless of what's stored internally. The flow I would propose is that ParseEXRFrom(Memory|File) reads all of the data, but does no decompression, byte-swapping or conversion on it. That way the file handle can be closed immediately. Then LoadEXR() does all of the decompression / conversion necessary based on the data types in requested_pixel_types. This would also allow a user to ignore a channel by setting its requested type to TINYEXR_PIXELTYPE_NONE.

To my mind, something like this is the most generalized solution, and wouldn't require massive changes. We'd be up for collaborating on an experimental implementation to this effect if it's in line with your goals for TinyEXR. Thanks again for your work on it.

@richardeakin
Copy link
Contributor

The other nice thing about a 2 step process is that it facilitates async loading, where you can retrieve header info, size etc synchronously up front, then issue the actual data load async, separately.

@syoyo
Copy link
Owner

syoyo commented Jun 23, 2015

I have added ParseEXR*** and requested_pixel_types feature in loading/saving EXR.

f62d0e8

Please take a look of this change. Usage is written in test.cc or README.md

@andrewfb
Copy link
Contributor Author

Thanks - this looks good from Cinder's perspective in terms of loading. I will experiment with saving in the next couple of days...

@syoyo
Copy link
Owner

syoyo commented Jun 24, 2015

BTW do you need EXR saving feature for Cinder? Note that saving is not yet perfect(e.g. lineOrder is missing). PR is also welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants