Create options/sources for JPEG/PNG support #407
Conversation
|
Still testing with my gallery images, but seems like the code can be reviewed for now... If there is a coding convention, please let me know. I will read it and update the code. |
|
It appears that the -srgb flag in texconv does not actually set the png to use srgb color space. From the code it appears that this PR does that -cool! |
|
I have a few comments: First, ArchLinux (https://archlinux.org/packages/extra/x86_64/libjpeg-turbo/) and the MSYS2 (https://www.msys2.org/) mingw-w64 distributions use libjpeg-turbo (https://libjpeg-turbo.virtualgl.org/) . That does NOT have the Second, I get the following warning with GCC 13.2.0: You should remove. the " and Third, pkg-config support was recently added so you need to add the following lines in CMakeLists at around line 390: This will help some users handle the direct dependencies. |
|
Nice check @JPeterMugaas . You made what I need to complete this PR. JCS_BG_RGB
But not in modern versions like 2.1.91 or 3.0.0 I will try detection with |
|
I have the modern 3.0.1 version of jpeg-turbo. The code I have in Auxiliary/DirectXTexJPEG.cpp is: I think that is the cleanest solution to the issue with LibJPEG versions. |
|
I have a question about ZLib as a dependency. Is ZLIB a DIRECT dependency where this library links directly to zlib or is it an INDIRECT dependency where this library links to libpng and libpng links to zlib? I ask because we do NOT want to list INDIRECT dependencies in our pkg-config file because those can change between library versions (see: #426 ) and how the library was built (this library is a good example of that). The pkg-config program solves INDIRECT dependencies provided everybody writes their pkg-config metadata files CORRECTLY. For the curious, https://people.freedesktop.org/~dbn/pkg-config-guide.html and its FAQ section at https://people.freedesktop.org/~dbn/pkg-config-guide.html#faq provide excellent information about the pkg-config program and associated metadata files. |
|
Understood. I will check if it works in both Mozilla JPEG and IJG JPEG. In short, I wanted to be simple for |
|
I wonder if |
|
Indeed they are private dependencies and that's why I'm going to try
`find_dependency` in CMake.
Both toolchains need to be synchronized.
…On Wed, Dec 27, 2023 at 11:10 AM J. Peter Mugaas ***@***.***> wrote:
I wonder if find_package(ZLIB REQUIRED) and ZLIB::ZLIB are necessary at
all. You just would need to directly link to libpng with something like target_link_libraries(${PROJECT_NAME}
PUBLIC PNG::PNG). libpng.pc lists zlib as a private dependency and I have
seen .pc files that list libpng but not zlib.
—
Reply to this email directly, view it on GitHub
<#407 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADLSYE7QGLGSTZDNWTOICKDYLN7PVAVCNFSM6AAAAAA65M6W46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRZHA4DANRWGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
*Profile*Facebook: fb.com/luncliff
GitHub: github.com/luncliff
LinkedIn: linkedin.com/in/dong-ha-park-171481229/
|
|
In the meantime, I suggest that you change CMakeLists.txt, around line 393 line to: |
|
I guess they are NOT from the new code
…On Wed, Dec 27, 2023, 16:52 J. Peter Mugaas ***@***.***> wrote:
Another matter, I got this warning message compiling your Texdiag program
using Clang 17.0.6:
[25/26] Building CXX object CMakeFiles/texdiag.dir/Texdiag/texdiag.cpp.obj
C:/msys64/home/jpmugaas/directxtex/Texdiag/texdiag.cpp:1799:25: warning: 106 enumeration values not handled in switch: 'DXGI_FORMAT_UNKNOWN', 'DXGI_FORMAT_R32G32B32A32_TYPELESS', 'DXGI_FORMAT_R32G32B32A32_FLOAT'... [-Wswitch]
1799 | switch (image.format)
| ^~~~~~~~~~~~
1 warning generated.
With GNU GCC 13.2.0, I get:
-- Build files have been written to: C:/msys64/home/jpmugaas/directxtex/testbuild/box
[25/26] Building CXX object CMakeFiles/texdiag.dir/Texdiag/texdiag.cpp.obj
In file included from C:/msys64/home/jpmugaas/directxtex/Texdiag/texdiag.cpp:51:
C:/msys64/home/jpmugaas/directxtex/DirectXTex/DirectXTex.h:903:106: warning: ignoring attributes on template argument 'void(const DirectX::XMVECTOR*, size_t, size_t)' {aka 'void(const DirectX::XMVECTOR*, long long unsigned int, long long unsigned int)'} [-Wignored-attributes]
903 | _In_ std::function<void __cdecl(_In_reads_(width) const XMVECTOR* pixels, size_t width, size_t y)> pixelFunc);
|
^
C:/msys64/home/jpmugaas/directxtex/DirectXTex/DirectXTex.h:906:106: warning: ignoring attributes on template argument 'void(const DirectX::XMVECTOR*, size_t, size_t)' {aka 'void(const DirectX::XMVECTOR*, long long unsigned int, long long unsigned int)'} [-Wignored-attributes]
906 | _In_ std::function<void __cdecl(_In_reads_(width) const XMVECTOR* pixels, size_t width, size_t y)> pixelFunc);
|
^
C:/msys64/home/jpmugaas/directxtex/DirectXTex/DirectXTex.h:911:80: warning: ignoring attributes on template argument 'void(DirectX::XMVECTOR*, const DirectX::XMVECTOR*, size_t, size_t)' {aka 'void(DirectX::XMVECTOR*, const DirectX::XMVECTOR*, long long unsigned int, long long unsigned int)'} [-Wignored-attributes]
911 | _In_reads_(width) const XMVECTOR* inPixels, size_t width, size_t y)> pixelFunc,
| ^
C:/msys64/home/jpmugaas/directxtex/DirectXTex/DirectXTex.h:916:80: warning: ignoring attributes on template argument 'void(DirectX::XMVECTOR*, const DirectX::XMVECTOR*, size_t, size_t)' {aka 'void(DirectX::XMVECTOR*, const DirectX::XMVECTOR*, long long unsigned int, long long unsigned int)'} [-Wignored-attributes]
916 | _In_reads_(width) const XMVECTOR* inPixels, size_t width, size_t y)> pixelFunc,
| ^
[26/26] Linking CXX executable bin\CMake\texdiag.exe
To compile this, I added these lines to the main CMakeLists.txt file
around line 410:
#--- texddiag program
if(WIN32)
add_executable(texdiag
Texdiag/texdiag.cpp
Texdiag/texdiag.rc
Texdiag/settings.manifest)
target_link_libraries(texdiag PRIVATE ${PROJECT_NAME} ole32.lib version.lib)
source_group(texdiag REGULAR_EXPRESSION *.*)
if(MINGW)
target_link_options(texdiag PRIVATE -municode)
endif()
if(BC_USE_OPENMP)
target_link_libraries(texdiag PRIVATE OpenMP::OpenMP_CXX)
endif()
if(ENABLE_OPENEXR_SUPPORT)
target_include_directories(texdiag PRIVATE Auxiliary)
target_link_libraries(texdiag PRIVATE ${OPENEXR_ILMIMF_LIBRARY})
target_compile_options(texdiag PRIVATE -DUSE_OPENEXR)
endif()
if(ENABLE_LIBJPEG_SUPPORT)
target_include_directories(texdiag PRIVATE Auxiliary)
target_link_libraries(texdiag PRIVATE JPEG::JPEG)
target_compile_definitions(texdiag PRIVATE USE_LIBJPEG)
endif()
if(ENABLE_LIBPNG_SUPPORT)
target_include_directories(texdiag PRIVATE Auxiliary)
target_link_libraries(texdiag PRIVATE PNG::PNG)
target_compile_definitions(texdiag PRIVATE USE_LIBPNG)
endif()
if(WINDOWS_STORE)
target_compile_definitions(texdiag PRIVATE WINAPI_FAMILY=WINAPI_FAMILY_APP)
endif()
endif()
—
Reply to this email directly, view it on GitHub
<#407 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADLSYE44QR3TQFOJFA5KR6DYLPHUJAVCNFSM6AAAAAA65M6W46VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZQGA2DGMBXGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
No, the CMakeLists.txt code I added was just for local testing so I didn't check it into a branch. You can add it to your own CMakeLists.txt file. |
|
I see. Fixing build warnings of texdiag can be done in another PR. It's out of this PR's purpose. |
|
My appologies for that posting mistake. Enough said. |
|
Thanks for the hard work on this. I'll work though it and do some formatting changes and start working on test coverge. |
|
My test media is hosted in and I hit a number of failures. Maybe you can take a look?
|
|
I've pushed code review changes back and rebased for the latest main. You should delete your working branch and repull. |
|
/azp run DirectXTex-GitHub-CMake,DirectXTex-GitHub-CMake-Dev17,DirectXTex-GitHub-MinGW,DirectXTex-GitHub-CMake-Xbox,DirectXTex-GitHub-CMake-Xbox-Dev17,DirectXTex-GitHub-WSL,DirectXTex-GitHub-WSL-11 |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Short note for the files.
No much idea about the zlib memory issue. I'm going to debug it in Windows together |
* Sort headers * Add `[[noreturn]]` attribute to `OnJPEGError`
|
Started a doc page: https://github.com/microsoft/DirectXTex/wiki/Using-JPEG-PNG-OSS |
|
@luncliff Looks like the only failure I'm getting now is: If you don't want to support CMYK that's fine, but it would ideally return |
|
/azp run DirectXTex-GitHub-CMake,DirectXTex-GitHub-CMake-Dev17,DirectXTex-GitHub-MinGW,DirectXTex-GitHub-CMake-Xbox,DirectXTex-GitHub-CMake-Xbox-Dev17,DirectXTex-GitHub-WSL,DirectXTex-GitHub-WSL-11 |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run DirectXTex-GitHub-CMake,DirectXTex-GitHub-CMake-Dev17,DirectXTex-GitHub-MinGW,DirectXTex-GitHub-CMake-Xbox,DirectXTex-GitHub-CMake-Xbox-Dev17,DirectXTex-GitHub-WSL,DirectXTex-GitHub-WSL-11 |
|
Azure Pipelines successfully started running 3 pipeline(s). |
+1 for |
|
/azp run DirectXTex-GitHub-CMake,DirectXTex-GitHub-CMake-Dev17,DirectXTex-GitHub-MinGW,DirectXTex-GitHub-CMake-Xbox,DirectXTex-GitHub-CMake-Xbox-Dev17,DirectXTex-GitHub-WSL,DirectXTex-GitHub-WSL-11 |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Resolve #351
This provides opt-in support for using libpng and/or libjpeg. This is intended for platforms that lack WIC (i.e. WIndows Subsystem for Linux).
texassemblesupports libPNG, libJPEG buildtexconvsupports libPNG, libJPEG buildtexdiagsupports libPNG, libJPEG buildDXGI_FORMATDXGI_FORMATFILE*) based implementation for the following functionsJPEG
DirectXTexJPEG.h/cppGetMetadataFromJPEGFileLoadFromJPEGFileSaveToJPEGFilePNG
DirectXTexPNG.h/cppGetMetadataFromPNGFileLoadFromPNGFileSaveToPNGFileReferences
Existing OpenEXR implementation