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