Ver Fonte

Added option to customize the image diff tool (#652)

Added option to customize the image diff tool
executable via settings registry key:
`O3DE/External/DiffTool`

If the regsitry key is not defined then
it will default to the previously hardcoded platform
defaults.

Signed-off-by: galibzon <[email protected]>
galibzon há 1 ano atrás
pai
commit
4388c9c831

+ 10 - 7
Gem/Code/Source/Platform/Linux/Utils_Linux.cpp

@@ -20,10 +20,14 @@ namespace AtomSampleViewer
             return true;
         }
 
-        bool RunDiffTool(const AZStd::string& filePathA, const AZStd::string& filePathB)
+        AZStd::string GetDefaultDiffToolPath_Impl()
+        {
+            return AZStd::string("/usr/bin/bcompare");
+        }
+
+        bool RunDiffTool_Impl(const AZStd::string& diffToolPath, const AZStd::string& filePathA, const AZStd::string& filePathB)
         {
             bool result = true;
-            AZStd::string executablePath = "/usr/bin/bcompare";
 
             // Fork a process to run Beyond Compare app
             pid_t childPid = fork();
@@ -31,15 +35,14 @@ namespace AtomSampleViewer
             if (childPid == 0)
             {
                 // In child process
-                char* args[] = { const_cast<char*>(executablePath.c_str()), const_cast<char*>(filePathA.c_str()),
+                char* args[] = { const_cast<char*>(diffToolPath.c_str()), const_cast<char*>(filePathA.c_str()),
                                  const_cast<char*>(filePathB.c_str()), static_cast<char*>(0) };
-                execv(executablePath.c_str(), args);
+                execv(diffToolPath.c_str(), args);
 
                 AZ_Error(
                     "RunDiffTool", false,
-                    "RunDiffTool: Unable to launch Beyond Compare %s : errno = %s . Make sure you have installed Beyond Compare command "
-                    "line tools.",
-                    executablePath.c_str(), strerror(errno));
+                    "RunDiffTool: Unable to launch Diff Tool %s : errno = %s .",
+                    diffToolPath.c_str(), strerror(errno));
 
                 _exit(errno);
             }

+ 12 - 8
Gem/Code/Source/Platform/Mac/Utils_Mac.cpp

@@ -21,21 +21,25 @@ namespace AtomSampleViewer
             return true;
         }
 
-        bool RunDiffTool(const AZStd::string& filePathA, const AZStd::string& filePathB)
+        AZStd::string GetDefaultDiffToolPath_Impl()
+        {
+            return AZStd::string("/usr/local/bin/bcompare");
+        }
+
+        bool RunDiffTool_Impl(const AZStd::string& diffToolPath, const AZStd::string& filePathA, const AZStd::string& filePathB)
         {
             bool result = true;
-            AZStd::string executablePath = "/usr/local/bin/bcompare";
-           
+
             //Fork a process to run Beyond Compare app
             pid_t childPid = fork();
-            
+
             if (childPid == 0)
             {
                 //In child process
-                char* args[] = { const_cast<char*>(executablePath.c_str()), const_cast<char*>(filePathA.c_str()), const_cast<char*>(filePathB.c_str()), static_cast<char*>(0)};
-                execv(executablePath.c_str(), args);
-                
-                AZ_TracePrintf("RunDiffTool", "RunDiffTool: Unable to launch Beyond Compare %s : errno = %s . Make sure you have installed Beyond Compare command line tools.", executablePath.c_str(), strerror(errno));
+                char* args[] = { const_cast<char*>(diffToolPath.c_str()), const_cast<char*>(filePathA.c_str()), const_cast<char*>(filePathB.c_str()), static_cast<char*>(0)};
+                execv(diffToolPath.c_str(), args);
+
+                AZ_TracePrintf("RunDiffTool", "RunDiffTool: Unable to launch Diff Tool %s : errno = %s .", diffToolPath.c_str(), strerror(errno));
                 std::cerr << strerror(errno) << std::endl;
 
                 _exit(errno);

+ 8 - 2
Gem/Code/Source/Platform/Windows/Utils_Windows.cpp

@@ -18,11 +18,17 @@ namespace AtomSampleViewer
             return true;
         }
 
-        bool RunDiffTool(const AZStd::string& filePathA, const AZStd::string& filePathB)
+        AZStd::string GetDefaultDiffToolPath_Impl()
+        {
+            return AZStd::string("C:\\Program Files\\Beyond Compare 4\\BCompare.exe");
+        }
+
+        bool RunDiffTool_Impl(const AZStd::string& diffToolPath, const AZStd::string& filePathA, const AZStd::string& filePathB)
         {
             bool result = false;
 
-            AZStd::wstring exeW = L"C:\\Program Files\\Beyond Compare 4\\BCompare.exe";
+            AZStd::wstring exeW;
+            AZStd::to_wstring(exeW, diffToolPath.c_str());
             AZStd::wstring filePathAW;
             AZStd::to_wstring(filePathAW, filePathA.c_str());
             AZStd::wstring filePathBW;

+ 58 - 0
Gem/Code/Source/Utils/Utils.cpp

@@ -7,6 +7,9 @@
  */
 #include <Utils/Utils.h>
 
+#include <AzCore/IO/SystemFile.h>
+#include <AzCore/Settings/SettingsRegistryMergeUtils.h>
+
 #include <AtomCore/Instance/InstanceDatabase.h>
 #include <Atom/RHI/RHISystemInterface.h>
 #include <Atom/RPI.Public/RenderPipeline.h>
@@ -258,5 +261,60 @@ namespace AtomSampleViewer
             return false;
         }
 
+        bool RunDiffTool(const AZStd::string& filePathA, const AZStd::string& filePathB)
+        {
+            // First let's try to use the user's favorite diff tool. 
+            static constexpr AZStd::string_view DiffToolPathKey = "/O3DE/External/DiffTool";
+
+            AZStd::string diffToolPath;
+            if (auto registry = AZ::SettingsRegistry::Get())
+            {
+                registry->Get(diffToolPath, DiffToolPathKey);
+            }
+
+            if (!diffToolPath.empty())
+            {
+                // Does the executable exist?
+                if (AZ::IO::SystemFile::Exists(diffToolPath.c_str()))
+                {
+                    return RunDiffTool_Impl(diffToolPath, filePathA, filePathB);
+                }
+
+                AZ_Warning(
+                    "ASV::RunDiffTool", false, "The user's diff tool <%s> doesn't exist. Will use the platform default <%s>.\n",
+                    diffToolPath.c_str(), GetDefaultDiffToolPath_Impl().c_str());
+
+            }
+
+            // If the key doesn't exist in the registry, or the user specified executable doesn't exist, then
+            // use the platform default.
+            diffToolPath = GetDefaultDiffToolPath_Impl();
+            if (!diffToolPath.empty())
+            {
+                // Does the executable exist?
+                if (AZ::IO::SystemFile::Exists(diffToolPath.c_str()))
+                {
+                    return RunDiffTool_Impl(diffToolPath, filePathA, filePathB);
+                }
+
+                AZ_Warning(
+                    "ASV::RunDiffTool", false,
+                    "The platform default diff tool <%s> doesn't exist.\n"
+                    "You can customize the tool path in the settings registry key: %s.\n",
+                    diffToolPath.c_str(),
+                    DiffToolPathKey.data());
+            }
+            else
+            {
+                AZ_Warning(
+                    "ASV::RunDiffTool", false,
+                    "No default diff tool has been defined for the current platform.\n"
+                    "You can customize the tool path in the settings registry key: %s.\n",
+                    DiffToolPathKey.data());
+            }
+
+            return false;
+        }
+
     } // namespace Utils
 } // namespace AtomSampleViewer

+ 6 - 0
Gem/Code/Source/Utils/Utils.h

@@ -66,8 +66,14 @@ namespace AtomSampleViewer
 
         void ReportScriptableAction(const char* formatStr, ...);
 
+        // Discovers, from the settings registry, the path to the diff tool and invokes
+        // the per-platform RunDiffTool_Impl.
         bool RunDiffTool(const AZStd::string& filePathA, const AZStd::string& filePathB);
 
+        // Customized per platform
+        AZStd::string GetDefaultDiffToolPath_Impl();
+        bool RunDiffTool_Impl(const AZStd::string& diffToolPath, const AZStd::string& filePathA, const AZStd::string& filePathB);
+
         AZ::Data::Instance<AZ::RPI::StreamingImage> GetSolidColorCubemap(uint32_t color);
 
         //! Provides a more convenient way to call AZ::IO::FileIOBase::GetInstance()->ResolvePath()