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,6 +4,8 @@
 // Copyright 2022 The Khronos Group Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+#include "imageio.h"
+
 #include <array>
 #include <cassert>
 #include <optional>
@@ -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"
 

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/ktx/command.cpp

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

tools/ktx/command.h

@@ -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
 };
 
 // -------------------------------------------------------------------------------------------------
@@ -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) {

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)