Skip to content

Commit

Permalink
Backport 43 fixes for input from pipes on Windows and issue KhronosGr…
Browse files Browse the repository at this point in the history
…oup#727 (KhronosGroup#760)

* 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 KhronosGroup#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 (KhronosGroup#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 f24e8eb commit ce8b030
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 11 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
67 changes: 63 additions & 4 deletions utils/ktxapp.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

#include "stdafx.h"

#if defined (_WIN32)
#define _CRT_SECURE_NO_WARNINGS
#define WINDOWS_LEAN_AND_MEAN
#include <windows.h>
#endif

#include <stdarg.h>
#if (_MSVC_LANG >= 201703L || __cplusplus >= 201703L)
#include <algorithm>
Expand Down Expand Up @@ -87,8 +93,12 @@ class ktxApp {
virtual int main(int argc, _TCHAR* argv[]) = 0;
virtual void usage() {
cerr <<
" -h, --help Print this usage message and exit.\n"
" -v, --version Print the version number of this program and exit.\n";
" -h, --help Print this usage message and exit.\n"
" -v, --version Print the version number of this program and exit.\n"
#if defined(_WIN32) && defined(DEBUG)
" --ld Launch Visual Studio deugger at start up.\n"
#endif
;
};

protected:
Expand All @@ -97,8 +107,9 @@ class ktxApp {
_tstring outfile;
int test;
int warn;
int launchDebugger;

commandOptions() : test(false), warn(1) { }
commandOptions() : test(false), warn(1), launchDebugger(0) { }
};

ktxApp(std::string& version, std::string& defaultVersion,
Expand Down Expand Up @@ -259,7 +270,7 @@ class ktxApp {
listName.erase(0, relativize ? 2 : 1);

FILE *lf = nullptr;
#ifdef _WIN32
#if defined(_WIN32)
_tfopen_s(&lf, listName.c_str(), "r");
#else
lf = _tfopen(listName.c_str(), "r");
Expand Down Expand Up @@ -352,6 +363,10 @@ class ktxApp {
}
}
}
#if defined(_WIN32) && defined(DEBUG)
if (options.launchDebugger)
launchDebugger();
#endif
}

virtual bool processOption(argparser& parser, int opt) = 0;
Expand All @@ -366,6 +381,47 @@ class ktxApp {
cerr << endl;
}

#if defined(_WIN32) && defined(DEBUG)
// For use when debugging stdin with Visual Studio which does not have a
// "wait for executable to be launched" choice in its debugger settings.
bool launchDebugger()
{
// Get System directory, typically c:\windows\system32
std::wstring systemDir(MAX_PATH + 1, '\0');
UINT nChars = GetSystemDirectoryW(&systemDir[0],
static_cast<UINT>(systemDir.length()));
if (nChars == 0) return false; // failed to get system directory
systemDir.resize(nChars);

// Get process ID and create the command line
DWORD pid = GetCurrentProcessId();
std::wostringstream s;
s << systemDir << L"\\vsjitdebugger.exe -p " << pid;
std::wstring cmdLine = s.str();

// Start debugger process
STARTUPINFOW si;
ZeroMemory(&si, sizeof(si));
si.cb = sizeof(si);

PROCESS_INFORMATION pi;
ZeroMemory(&pi, sizeof(pi));

if (!CreateProcessW(NULL, &cmdLine[0], NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) return false;

// Close debugger process handles to eliminate resource leak
CloseHandle(pi.hThread);
CloseHandle(pi.hProcess);

// Wait for the debugger to attach
while (!IsDebuggerPresent()) Sleep(100);

// Stop execution so the debugger can take over
DebugBreak();
return true;
}
#endif

_tstring name;
_tstring& version;
_tstring& defaultVersion;
Expand All @@ -378,6 +434,9 @@ class ktxApp {
{ "help", argparser::option::no_argument, NULL, 'h' },
{ "version", argparser::option::no_argument, NULL, 'v' },
{ "test", argparser::option::no_argument, &options.test, 1},
#if defined(_WIN32) && defined(DEBUG)
{ "ld", argparser::option::no_argument, &options.launchDebugger, 1},
#endif
// -NSDocumentRevisionsDebugMode YES is appended to the end
// of the command by Xcode when debugging and "Allow debugging when
// using document Versions Browser" is checked in the scheme. It
Expand Down
7 changes: 6 additions & 1 deletion utils/stdafx.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@

#pragma once

#define _CRT_SECURE_NO_WARNINGS // For _WIN32. Must be before <stdio.h>.
#if defined(_WIN32)
// _CRT_SECURE_NO_WARNINGS must be defined before <windows.h>,
// <stdio.h> and and <iostream>
#define _CRT_SECURE_NO_WARNINGS
#endif

#include <assert.h>
#include <stdio.h>
#ifdef _WIN32
Expand Down

0 comments on commit ce8b030

Please sign in to comment.