Add support for LoadFromEXRMemory as other formats#645
Add support for LoadFromEXRMemory as other formats#645vkaytsanov wants to merge 2 commits intomicrosoft:mainfrom
Conversation
|
Can you rebase this? |
vkaytsanov
force-pushed
the
EXRLoadFromMemory
branch
from
ba1573c to
cbb8f27
Compare
|
@walbourn rebased and rearranged the code a little bit to give better diff. Please note, that I haven't tested non-windows desktop systems for the template argument deduction |
| _Use_decl_annotations_ | ||
| HRESULT DirectX::LoadFromEXRFile(const wchar_t* szFile, TexMetadata* metadata, ScratchImage& image) | ||
| template <typename StreamType> | ||
| HRESULT LoadFromEXRCommon(StreamType stream, TexMetadata* metadata, ScratchImage& image) |
There was a problem hiding this comment.
This template is not public so it should be placed in an anonymous namespace.
_Use_decl_annotations_ is not valid here and instead it needs to be SAL annotated directly. In this case, it's sufficient to make it:
HRESULT LoadFromEXRCommon(StreamType stream, _Out_opt_ TexMetadata* metadata, ScratchImage& image)
There was a problem hiding this comment.
The diff was really bad when it was in the anonymous namespace. Can move it after approval with the removal of _Use_decl_annotations_
There was a problem hiding this comment.
Sure, you can wrap it in a namespace {} in place and then I'll clean it up at some future point.
| _Use_decl_annotations_ | ||
| HRESULT DirectX::LoadFromEXRFile(const wchar_t* szFile, TexMetadata* metadata, ScratchImage& image) | ||
| template <typename StreamType> | ||
| HRESULT LoadFromEXRCommon(StreamType stream, TexMetadata* metadata, ScratchImage& image) |
There was a problem hiding this comment.
Should stream be passed by reference here instead of by value?
There was a problem hiding this comment.
It was, however the EXR API expects constchar * which fails the posix builds with Error: /home/runner/work/DirectXTex/DirectXTex/Auxiliary/DirectXTexEXR.cpp:503:44: error: cannot bind non-const lvalue reference of type ‘const char*&’ to an rvalue of type ‘const char*’.
Instead, exploited the rvalue object stream for Win32. Could modify it with std::move for better visibillity?
|
Any plans to add support for loading mipmaps from tiled exrs? |
3 participants
Tested from memory and from file the following exr: https://polyhaven.com/a/qwantani_dusk_2_puresky
Changes:
Rename *Stream -> *FileStream
Add InputStream wrapper to read from memory
Cut-paste the whole LoadFromEXRFile try-catch into LoadFromEXRCommon
Remove the equal sign from COMBINED_OPENEXR_VERSION as I am exactly on this version and I don't have this virtual method, todo: implement it in the future