Fix legacy app input from pipes on Windows. (#749)

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 mimicing 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.

tests/ktx2check-tests.cmake

@@ -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

tools/imageio/exr.imageio/exrinput.cc

@@ -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"
 

tools/imageio/imageinput.cc

@@ -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.
@@ -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

tools/imageio/imageio.cc

@@ -12,6 +12,8 @@
 //! @brief Create plugin maps.
 //!
 
+#include "imageio.h"
+
 #include <algorithm>
 #include <cctype>
 #include <iomanip>
@@ -24,8 +26,6 @@
 
 #include <stdarg.h>
 
-#include "imageio.h"
-
 #define PLUGENTRY(name) \
  ImageInput* name##InputCreate(); \
  ImageOutput* name##OutputCreate(); \

tools/imageio/imageoutput.cc

@@ -12,6 +12,8 @@
 //! @brief ImageOutput class implementation
 //!
 
+#include "imageio.h"
+
 #include <algorithm>
 #include <cctype>
 #include <iomanip>
@@ -24,8 +26,6 @@
 
 #include <stdarg.h>
 
-#include "imageio.h"
-
 std::unique_ptr<ImageOutput>
 ImageOutput::create(const _tstring& filename)
 {

tools/imageio/jpg.imageio/jpginput.cc

@@ -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;

tools/imageio/png.imageio/pnginput.cc

@@ -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"

tools/imageio/png.imageio/pngoutput.cc

@@ -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"

tools/ktx2check/ktx2check.cpp

@@ -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
 
@@ -1104,7 +1107,6 @@ ktxValidator::usage()
  ktxApp::usage();
 }
 
-
 int _tmain(int argc, _TCHAR* argv[])
 {
 
@@ -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;

tools/ktxsc/ktxsc.cpp

@@ -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>
@@ -22,7 +15,6 @@
 #include <ktx.h>
 
 #include "argparser.h"
-#include "scapp.h"
 #include "version.h"
 
 #if defined(_MSC_VER)

utils/ktxapp.h

@@ -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>
@@ -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:
@@ -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,
@@ -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");
@@ -352,6 +363,10 @@ class ktxApp {
  }
  }
  }
+#if defined(_WIN32) && defined(DEBUG)
+ if (options.launchDebugger)
+ launchDebugger();
+#endif
  }
 
  virtual bool processOption(argparser& parser, int opt) = 0;
@@ -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;
@@ -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

utils/stdafx.h

@@ -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