Skip to content

Commit

Permalink
Backport 43 fixes for input from pipes on Windows and issue #727 (#760)
Browse files Browse the repository at this point in the history
* Fix legacy app input from pipes on Windows. From commit 3e7fd0a.

   Was always broken for pipes created by Windows' shells (PowerShell
   and Command). Became broken in cygwin 3.4.x for pipes created by
   cygwin/MSYS2/Git for Windows shells when cygwin started mimicking
   the way Windows shells create pipes. The cause is that setting the
   `FILE_SYNCHRONOUS_IO_NONALERT` option when creating a pipe
   causes `cin.seekg(0)` on the pipe to return success when in fact seek
   is not supported.

   Add launchDebugger function and --ld option to invoke it to `ktxApp`
   class and ktx tool as helpers for debugging pipes on Windows. These
   are only added when _WIN32 and DEBUG are defined.

* Do target_type changes only in toktx. They were being done in both
  `toktx` and the `imageinput` plugin. Fixes #727. From commit
  2cf053c.

  Changes included with above fix:
  - Move image-related utilities and image.hpp from `ktxtools` to `imageio`
     target for sharing between new and legacy tools.
  - Change `toktx` and `unittests` to use the updated version of image.hpp.

* Changes to make build work with latest compilers in CI services:
   - Fix mismatched `new[]` and `delete` in `glloadtests`. From commit
     3e7fd0a
   - Warning fixes for clang 14 and cmake 3.26.
   - Make other_include a SYSTEM include for loadtests (#762) to stop
     warnings in assimp files. Clang 16.0.5 has a new warning it raises on
     some of the assimp headers. From commit 0feeda6.
  • Loading branch information
MarkCallow authored Sep 7, 2023
1 parent 170e758 commit 1b9378e
Show file tree
Hide file tree
Showing 51 changed files with 2,394 additions and 1,183 deletions.
14 changes: 10 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,18 @@ macro(commom_lib_settings lib write)
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lib/basisu/zstd>
$<INSTALL_INTERFACE:lib/basisu/zstd>

$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/other_include>
$<INSTALL_INTERFACE:other_include>

$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/utils>
$<INSTALL_INTERFACE:utils>
)

target_include_directories(
${lib}
SYSTEM
PRIVATE
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/other_include>
$<INSTALL_INTERFACE:other_include>
)

if( LIB_TYPE STREQUAL STATIC )
target_compile_definitions(${lib} PUBLIC KHRONOS_STATIC)
endif()
Expand Down Expand Up @@ -681,7 +686,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
# version. Also future proofing for when xcode catches up.
set_source_files_properties(
${BASISU_ENCODER_CXX_SRC}
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter"
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter;-Wno-deprecated-copy-with-user-provided-copy"
)
set_source_files_properties(
lib/basisu/transcoder/basisu_transcoder.cpp
Expand Down Expand Up @@ -935,6 +940,7 @@ else()
endif()

# FMT
set(FMT_SYSTEM_HEADERS ON)
add_subdirectory(other_projects/fmt)

# Tools
Expand Down
2 changes: 0 additions & 2 deletions cmake/cputypetest.cmake
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Copyright 2016, Simon Werta (@webmaster128).
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 2.8.12)

set(cputypetest_code "
//
// https://gist.github.com/webmaster128/e08067641df1dd784eb195282fd0912f
Expand Down
10 changes: 4 additions & 6 deletions tests/ktx2check-tests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,10 @@ add_test( NAME ktx2check-test-stdin-read
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/testimages
)

if(NOT WIN32) # Disable due to bug in Git for Windows 2.41.0.windows.1 pipe.
add_test( NAME ktx2check-test-pipe-read
COMMAND ${BASH_EXECUTABLE} -c "cat color_grid_uastc_zstd.ktx2 | $<TARGET_FILE:ktx2check>"
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/testimages
)
endif()
add_test( NAME ktx2check-test-pipe-read
COMMAND ${BASH_EXECUTABLE} -c "cat color_grid_uastc_zstd.ktx2 | $<TARGET_FILE:ktx2check>"
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/testimages
)

add_test( NAME ktx2check-test-invalid-face-count-and-padding
COMMAND ktx2check invalid_face_count_and_padding.ktx2
Expand Down
16 changes: 13 additions & 3 deletions tests/loadtests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,17 @@ PUBLIC
appfwSDL
$<TARGET_PROPERTY:ktx,INCLUDE_DIRECTORIES>
${PROJECT_SOURCE_DIR}/lib
${PROJECT_SOURCE_DIR}/other_include
${PROJECT_SOURCE_DIR}/utils
common
geom
)

target_include_directories(
appfwSDL
SYSTEM PUBLIC
${PROJECT_SOURCE_DIR}/other_include
)

if(${KTX_FEATURE_LOADTEST_APPS} MATCHES "OpenGL")
add_library( GLAppSDL STATIC
appfwSDL/GLAppSDL.cpp
Expand All @@ -136,13 +141,18 @@ if(${KTX_FEATURE_LOADTEST_APPS} MATCHES "OpenGL")
target_include_directories(
GLAppSDL
PUBLIC
${PROJECT_SOURCE_DIR}/other_include
appfwSDL
$<TARGET_PROPERTY:appfwSDL,INCLUDE_DIRECTORIES>
common
glloadtests
glloadtests/utils
)

target_include_directories(
GLAppSDL
SYSTEM PRIVATE
$<TARGET_PROPERTY:appfwSDL,INTERFACE_SYSTEM_INCLUDE_DIRECTORIES>
)

if(OPENGL_FOUND)
target_include_directories(
GLAppSDL
Expand Down
7 changes: 6 additions & 1 deletion tests/loadtests/glloadtests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,17 @@ function( create_gl_target target version sources common_resources test_images
target_include_directories(
${target}
PRIVATE
$<TARGET_PROPERTY:appfwSDL,INTERFACE_INCLUDE_DIRECTORIES>
$<TARGET_PROPERTY:GLAppSDL,INTERFACE_INCLUDE_DIRECTORIES>
$<TARGET_PROPERTY:ktx,INCLUDE_DIRECTORIES>
$<TARGET_PROPERTY:objUtil,INTERFACE_INCLUDE_DIRECTORIES>
)

target_include_directories(
${target}
SYSTEM PRIVATE
${PROJECT_SOURCE_DIR}/other_include
)

set_target_properties(${target} PROPERTIES
CXX_VISIBILITY_PRESET ${STATIC_APP_LIB_SYMBOL_VISIBILITY}
)
Expand Down
5 changes: 5 additions & 0 deletions tests/loadtests/vkloadtests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ PRIVATE
vkloadtests/utils
)

target_include_directories(vkloadtests
SYSTEM PRIVATE
${PROJECT_SOURCE_DIR}/other_include
)

target_link_libraries(vkloadtests
ktx
${KTX_ZLIB_LIBRARIES}
Expand Down
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_10x5_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_12x10_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_12x12_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_4x4_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x5_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_kodim17_fast.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_kodim17_fastest.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_kodim17_medium.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_posy.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_6x6_posz.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_8x6_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_8x8_posx.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/astc_mipmap_ldr_cubemap_6x6.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/testimages/cimg5293_uastc.ktx2
Git LFS file not shown
4 changes: 2 additions & 2 deletions tests/testimages/cimg5293_uastc_zstd.ktx2
Git LFS file not shown
4 changes: 2 additions & 2 deletions tests/testimages/cubemap_goldengate_uastc_rdo4_zstd5_rd.ktx2
Git LFS file not shown
4 changes: 2 additions & 2 deletions tests/testimages/cubemap_yokohama_basis_rd.ktx2
Git LFS file not shown
4 changes: 2 additions & 2 deletions tests/testimages/ktx_document_basis.ktx2
Git LFS file not shown
4 changes: 2 additions & 2 deletions tests/testimages/ktx_document_uastc_rdo4_zstd5.ktx2
Git LFS file not shown
18 changes: 17 additions & 1 deletion tests/tests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ add_subdirectory(transcodetests)
add_subdirectory(streamtests)

add_executable( unittests
unittests/unittests.cc
unittests/image_unittests.cc
unittests/unittests.cc
unittests/wthelper.h
tests.cmake
)
Expand All @@ -45,16 +45,32 @@ PRIVATE
$<TARGET_PROPERTY:ktx,INCLUDE_DIRECTORIES>
${PROJECT_SOURCE_DIR}/lib
${PROJECT_SOURCE_DIR}/tools
${PROJECT_SOURCE_DIR}/tools/imageio
loadtests/common
)

target_include_directories(
unittests
SYSTEM
PRIVATE
${PROJECT_SOURCE_DIR}/other_include
)

target_link_libraries(
unittests
gtest
ktx
fmt::fmt
${CMAKE_THREAD_LIBS_INIT}
)

set_target_properties(
unittests
PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED YES
)

add_executable( texturetests
texturetests/texturetests.cc
unittests/wthelper.h
Expand Down
10 changes: 5 additions & 5 deletions tests/toktx-tests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -241,20 +241,20 @@ if (NOT ${CPU_ARCHITECTURE} STREQUAL "arm64" )
gencmpktx( astc_mipmap_ldr_6x6_posx astc_mipmap_ldr_6x6_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 6x6 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_6x6_posz astc_mipmap_ldr_6x6_posz.ktx2 ../srcimages/Yokohama3/posz.jpg "--test --encode astc --astc_blk_d 6x6 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_6x6_posy astc_mipmap_ldr_6x6_posy.ktx2 ../srcimages/Yokohama3/posy.jpg "--test --encode astc --astc_blk_d 6x6 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_6x6_kodim17_fastest astc_mipmap_ldr_6x6_kodim17_fastest.ktx2 ../srcimages/kodim17.png "--test --encode astc --astc_blk_d 6x6 --genmipmap --astc_quality fastest " "" "" )
gencmpktx( astc_mipmap_ldr_6x6_kodim17_fast astc_mipmap_ldr_6x6_kodim17_fast.ktx2 ../srcimages/kodim17.png "--test --encode astc --astc_blk_d 6x6 --genmipmap --astc_quality fast " "" "" )
endif()
gencmpktx( astc_mipmap_ldr_6x6_kodim17_fastest astc_mipmap_ldr_6x6_kodim17_fastest.ktx2 ../srcimages/kodim17.png "--test --encode astc --astc_blk_d 6x6 --genmipmap --astc_quality fastest " "" "" )
gencmpktx( astc_mipmap_ldr_6x6_kodim17_fast astc_mipmap_ldr_6x6_kodim17_fast.ktx2 ../srcimages/kodim17.png "--test --encode astc --astc_blk_d 6x6 --genmipmap --astc_quality fast " "" "" )
gencmpktx( astc_mipmap_ldr_6x6_kodim17_medium astc_mipmap_ldr_6x6_kodim17_medium.ktx2 ../srcimages/kodim17.png "--test --encode astc --astc_blk_d 6x6 --genmipmap --astc_quality medium " "" "" )

if (NOT ${CPU_ARCHITECTURE} STREQUAL "arm64" )
gencmpktx( astc_mipmap_ldr_4x4_posx astc_mipmap_ldr_4x4_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 4x4 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_6x5_posx astc_mipmap_ldr_6x5_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 6x5 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_8x6_posx astc_mipmap_ldr_8x6_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 8x6 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_10x5_posx astc_mipmap_ldr_10x5_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 10x5 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_8x8_posx astc_mipmap_ldr_8x8_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 8x8 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_12x10_posx astc_mipmap_ldr_12x10_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 12x10 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_12x12_posx astc_mipmap_ldr_12x12_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 12x12 --genmipmap" "" "" )
endif()
gencmpktx( astc_mipmap_ldr_8x8_posx astc_mipmap_ldr_8x8_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 8x8 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_12x10_posx astc_mipmap_ldr_12x10_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 12x10 --genmipmap" "" "" )
gencmpktx( astc_mipmap_ldr_12x12_posx astc_mipmap_ldr_12x12_posx.ktx2 ../srcimages/Yokohama3/posx.jpg "--test --encode astc --astc_blk_d 12x12 --genmipmap" "" "" )

gencmpktx( astc_ldr_4x4_FlightHelmet_baseColor astc_ldr_4x4_FlightHelmet_baseColor.ktx2 ../srcimages/FlightHelmet_baseColor.png "--test --encode astc --astc_blk_d 4x4" "" "")
gencmpktx( astc_ldr_6x5_FlightHelmet_baseColor astc_ldr_6x5_FlightHelmet_baseColor.ktx2 ../srcimages/FlightHelmet_baseColor.png "--test --encode astc --astc_blk_d 6x5" "" "")
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/image_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#endif

#include "gtest/gtest.h"
#include "toktx/image.hpp"
#include "image.hpp"

namespace {

Expand Down
4 changes: 2 additions & 2 deletions tests/webgl/libktx-webgl/ktx_app_basis.ktx2
Git LFS file not shown
2 changes: 1 addition & 1 deletion tests/webgl/libktx-webgl/ktx_document_uastc_rdo5.ktx2
Git LFS file not shown
13 changes: 11 additions & 2 deletions tools/imageio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ set( PLUGIN_HEADERS

add_library( imageio STATIC
formatdesc.h
image.hpp
imageinput.cc
imageio.cc
imageio.h
imageio_utility.h
imageoutput.cc
${PROJECT_SOURCE_DIR}/lib/astc-encoder/Source/tinyexr.h
${PROJECT_SOURCE_DIR}/lib/basisu/encoder/jpgd.cpp
Expand All @@ -43,11 +45,18 @@ target_include_directories(
PUBLIC
.
PRIVATE
${PROJECT_SOURCE_DIR}/lib/astc-encoder/Source
$<TARGET_PROPERTY:objUtil,INTERFACE_INCLUDE_DIRECTORIES>
)

target_include_directories(
imageio
SYSTEM
PUBLIC
${PROJECT_SOURCE_DIR}/other_include
PRIVATE
${PROJECT_SOURCE_DIR}/lib/astc-encoder/Source
${PROJECT_SOURCE_DIR}/lib/basisu
${PROJECT_SOURCE_DIR}/lib/dfdutils
${PROJECT_SOURCE_DIR}/other_include
)

target_compile_definitions(
Expand Down
11 changes: 8 additions & 3 deletions tools/imageio/exr.imageio/exrinput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,18 @@
// Copyright 2022 The Khronos Group Inc.
// SPDX-License-Identifier: Apache-2.0

// TEXR is not defined in tinyexr.h. Current GitHub tinyexr master uses
// assert. The version in astc-encoder must be old.
#include "imageio.h"

#include <array>
#include <cassert>
#include <optional>
#include <string_view>
#include <vector>
// TEXR_ASSERT is not defined in tinyexr.h. Current GitHub tinyexr master
// uses assert. The version in astc-encoder must be old.
#define TEXR_ASSERT(x) assert(x)
#define TINYEXR_IMPLEMENTATION
#include "tinyexr.h"
#include "imageio.h"
#include <KHR/khr_df.h>
#include "dfd.h"

Expand Down Expand Up @@ -175,6 +176,10 @@ void ExrInput::open(ImageSpec& newspec) {
newspec = spec();
}

/// @brief Read an entire image into contiguous memory performing conversions
/// to @a requestFormat.
///
/// Supported conversions are half->[half,float,uint], float->float, and uint->uint.
void ExrInput::readImage(void* outputBuffer, size_t bufferByteCount,
uint32_t subimage, uint32_t miplevel,
const FormatDescriptor& requestFormat) {
Expand Down
Loading

0 comments on commit 1b9378e

Please sign in to comment.