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

Fix legacy app input from pipes on Windows. #749

Merged
merged 8 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 2 additions & 1 deletion tools/imageio/exr.imageio/exrinput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// Copyright 2022 The Khronos Group Inc.
// SPDX-License-Identifier: Apache-2.0

#include "imageio.h"

#include <array>
#include <cassert>
#include <optional>
Expand All @@ -14,7 +16,6 @@
#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
13 changes: 11 additions & 2 deletions tools/imageio/imageinput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ ImageInput::open(const _tstring& filename,
ImageInput::Creator createFunction = nullptr;
const _tstring* fn;
const _tstring sn("stdin");
bool doBuffer = true;

if (filename.compare("-")) {
// Regular file.
Expand Down Expand Up @@ -72,10 +73,18 @@ ImageInput::open(const _tstring& filename,
#if defined(_WIN32)
// Set "stdin" to have binary mode. There is no way to this via cin.
(void)_setmode( _fileno( stdin ), _O_BINARY );
#endif
// Windows shells set the FILE_SYNCHRONOUS_IO_NONALERT option when
// creating pipes. Cygwin since 3.4.x does the same thing, a change
// which affects anything dependent on it, e.g. Git for Windows
// (since 2.41.0) and MSYS2. When this option is set, cin.seekg(0)
// erroneously returns success. Always buffer.
doBuffer = true;
#else
// Can we seek in this cin?
std::cin.seekg(0);
if (std::cin.fail()) {
doBuffer = std::cin.fail();
#endif
if (doBuffer) {
// Can't seek. Buffer stdin. This is a potentially large buffer.
// Must avoid copy. If use stack variable for ss, it's streambuf
// will also be on the stack and lost after this function exits
Expand Down
4 changes: 2 additions & 2 deletions tools/imageio/imageio.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
//! @brief Create plugin maps.
//!

#include "imageio.h"

#include <algorithm>
#include <cctype>
#include <iomanip>
Expand All @@ -24,8 +26,6 @@

#include <stdarg.h>

#include "imageio.h"

#define PLUGENTRY(name) \
ImageInput* name##InputCreate(); \
ImageOutput* name##OutputCreate(); \
Expand Down
4 changes: 2 additions & 2 deletions tools/imageio/imageoutput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
//! @brief ImageOutput class implementation
//!

#include "imageio.h"

#include <algorithm>
#include <cctype>
#include <iomanip>
Expand All @@ -24,8 +26,6 @@

#include <stdarg.h>

#include "imageio.h"

std::unique_ptr<ImageOutput>
ImageOutput::create(const _tstring& filename)
{
Expand Down
3 changes: 1 addition & 2 deletions tools/imageio/jpg.imageio/jpginput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@
* @author Mark Callow.
*/

#include "stdafx.h"
#include "imageio.h"

#include <iostream>
#include <sstream>
#include <stdexcept>

#include "imageio.h"
#include "encoder/jpgd.h"

using namespace jpgd;
Expand Down
3 changes: 1 addition & 2 deletions tools/imageio/png.imageio/pnginput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@
* @author Mark Callow
*/

#include "stdafx.h"
#include "imageio.h"

#include <array>
#include <iterator>
#include <sstream>
#include <stdexcept>

#include "imageio.h"
#include "lodepng.h"
#include <KHR/khr_df.h>
#include "dfd.h"
Expand Down
3 changes: 1 addition & 2 deletions tools/imageio/png.imageio/pngoutput.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@
* @author Mark Callow
*/

#include "stdafx.h"
#include "imageio.h"

#include <iterator>
#include <sstream>
#include <stdexcept>

#include "imageio.h"
#include "lodepng.h"
#include <KHR/khr_df.h>
#include "dfd.h"
Expand Down
49 changes: 49 additions & 0 deletions tools/ktx/command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include <fmt/ostream.h>
#include <fmt/printf.h>

#if defined(_WIN32) && defined(DEBUG)
#include <windows.h> // For functions used by launchDebugger
#endif

// -------------------------------------------------------------------------------------------------

Expand All @@ -38,7 +41,53 @@ void Command::parseCommandLine(const std::string& name, const std::string& desc,
} catch (const cxxopts::exceptions::parsing& ex) {
fatal_usage("{}.", ex.what());
}

#if defined(_WIN32) && defined(DEBUG)
if (args["ld"].as<bool>())
launchDebugger();
#endif
}

#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 Command::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

std::string version(bool testrun) {
return testrun ? STR(KTX_DEFAULT_VERSION) : STR(KTX_VERSION);
Expand Down
9 changes: 8 additions & 1 deletion tools/ktx/command.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ class Command : public Reporter {

virtual void initOptions(cxxopts::Options& /*opts*/) { }
virtual void processOptions(cxxopts::Options& /*opts*/, cxxopts::ParseResult& /*args*/) { };
#if defined(_WIN32) && defined(DEBUG)
bool launchDebugger();
#endif
};

// -------------------------------------------------------------------------------------------------
Expand All @@ -162,7 +165,11 @@ struct OptionsGeneric {
opts.add_options()
("h,help", "Print this usage message and exit")
("v,version", "Print the version number of this program and exit")
("testrun", "Indicates test run. If enabled the tool will produce deterministic output whenever possible");
("testrun", "Indicates test run. If enabled the tool will produce deterministic output whenever possible")
#if defined(_WIN32) && defined(DEBUG)
("ld", "Launch debugger on startup.")
#endif
;
}

void process(cxxopts::Options& opts, cxxopts::ParseResult& args, Reporter& report) {
Expand Down
20 changes: 16 additions & 4 deletions tools/ktx2check/ktx2check.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ struct issue {
};

#define WARNING 0x00010000
#if defined(ERROR) // windows.h defines this and is included by ktxapp.h.
#undef ERROR
#endif
#define ERROR 0x00100000
#define FATAL 0x01000000

Expand Down Expand Up @@ -1104,7 +1107,6 @@ ktxValidator::usage()
ktxApp::usage();
}


int _tmain(int argc, _TCHAR* argv[])
{

Expand Down Expand Up @@ -1147,17 +1149,27 @@ ktxValidator::validateFile(const _tstring& filename)
// of this method.
ifstream ifs;
stringstream buffer;
bool doBuffer;

if (filename.compare(_T("-")) == 0) {
#if defined(_WIN32)
/* Set "stdin" to have binary mode */
(void)_setmode( _fileno( stdin ), _O_BINARY );
#endif
// Windows shells set the FILE_SYNCHRONOUS_IO_NONALERT option when
// creating pipes. Cygwin since 3.4.x does the same thing, a change
// which affects anything dependent on it, e.g. Git for Windows
// (since 2.41.0) and MSYS2. When this option is set, cin.seekg(0)
// erroneously returns success. Always buffer.
doBuffer = true;
#else
// Can we seek in this cin?
cin.seekg(0);
if (cin.fail()) {
doBuffer = cin.fail();
#endif
if (doBuffer) {
// Read entire file into a stringstream so we can seek.
buffer << std::cin.rdbuf();
buffer << cin.rdbuf();
buffer.seekg(0, ios::beg);
isp = &buffer;
} else {
isp = &cin;
Expand Down
10 changes: 1 addition & 9 deletions tools/ktxsc/ktxsc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,7 @@
// Copyright 2019-2020 Mark Callow
// SPDX-License-Identifier: Apache-2.0

#if defined(_WIN32)
// <windows.h> must appear before "scapp.h" for error-free mingw/gcc11 build.
// _CRT_SECURE_NO_WARNINGS must be defined before <windows.h> and <iostream>
// so we can't rely on the definition included by "scapp.h".
#define _CRT_SECURE_NO_WARNINGS
#define WINDOWS_LEAN_AND_MEAN
#include <windows.h>
#endif
#include "scapp.h"

#include <cstdlib>
#include <errno.h>
Expand All @@ -22,7 +15,6 @@
#include <ktx.h>

#include "argparser.h"
#include "scapp.h"
#include "version.h"

#if defined(_MSC_VER)
Expand Down
Loading