From f0306ca66094c310d70c0d315a2d6d24bc8e1bbc Mon Sep 17 00:00:00 2001
From: tildearrow <me@tildearrow.org>
Date: Fri, 8 Sep 2023 01:38:23 -0500
Subject: [PATCH] GUI: sample editor usability fixes

---
 src/gui/sampleEdit.cpp | 67 ++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/src/gui/sampleEdit.cpp b/src/gui/sampleEdit.cpp
index 20fe69960..3c7faa617 100644
--- a/src/gui/sampleEdit.cpp
+++ b/src/gui/sampleEdit.cpp
@@ -195,6 +195,7 @@ void FurnaceGUI::drawSampleEdit() {
       ImGui::Separator();
 
       String warnLoop, warnLoopMode, warnLoopPos;
+      String warnLoopStart, warnLoopEnd;
       String warnLength, warnRate;
 
       bool isChipVisible[DIV_MAX_CHIPS];
@@ -213,9 +214,21 @@ void FurnaceGUI::drawSampleEdit() {
         switch (e->song.system[i]) {
           case DIV_SYSTEM_SNES:
             if (sample->loop) {
-              if (sample->loopStart&15 || sample->loopEnd&15) {
-                SAMPLE_WARN(warnLoopPos,"SNES: loop must be a multiple of 16");
+              if (sample->loopStart&15) {
+                int tryWith=(sample->loopStart+8)&(~15);
+                if (tryWith>(int)sample->samples) tryWith-=16;
+                String alignHint=fmt::sprintf("SNES: loop start must be a multiple of 16 (try with %d)",tryWith);
+                SAMPLE_WARN(warnLoopStart,alignHint);
               }
+              if (sample->loopEnd&15) {
+                int tryWith=(sample->loopEnd+8)&(~15);
+                if (tryWith>(int)sample->samples) tryWith-=16;
+                String alignHint=fmt::sprintf("SNES: loop end must be a multiple of 16 (try with %d)",tryWith);
+                SAMPLE_WARN(warnLoopEnd,alignHint);
+              }
+            }
+            if (sample->samples&15) {
+              SAMPLE_WARN(warnLength,"SNES: sample length will be padded to multiple of 16");
             }
             if (dispatch!=NULL) {
               MAX_RATE("SNES",dispatch->chipClock/8.0);
@@ -302,8 +315,11 @@ void FurnaceGUI::drawSampleEdit() {
             break;
           case DIV_SYSTEM_AMIGA:
             if (sample->loop) {
-              if (sample->loopStart&1 || sample->loopEnd&1) {
-                SAMPLE_WARN(warnLoopPos,"Amiga: loop must be a multiple of 2");
+              if (sample->loopStart&1) {
+                SAMPLE_WARN(warnLoopStart,"Amiga: loop start must be a multiple of 2");
+              }
+              if (sample->loopEnd&1) {
+                SAMPLE_WARN(warnLoopEnd,"Amiga: loop end must be a multiple of 2");
               }
             }
             if (sample->samples>131070) {
@@ -342,8 +358,11 @@ void FurnaceGUI::drawSampleEdit() {
             break;
           case DIV_SYSTEM_C219:
             if (sample->loop) {
-              if (sample->loopStart&1 || sample->loopEnd&1) {
-                SAMPLE_WARN(warnLoopPos,"C219: loop must be a multiple of 2");
+              if (sample->loopStart&1) {
+                SAMPLE_WARN(warnLoopStart,"C219: loop start must be a multiple of 2");
+              }
+              if (sample->loopEnd&1) {
+                SAMPLE_WARN(warnLoopEnd,"C219: loop end must be a multiple of 2");
               }
             }
             if (sample->samples>131072) {
@@ -643,7 +662,7 @@ void FurnaceGUI::drawSampleEdit() {
           }
           popWarningColor();
 
-          pushWarningColor(!warnLoopPos.empty());
+          pushWarningColor(!warnLoopPos.empty() || !warnLoopStart.empty());
           ImGui::AlignTextToFramePadding();
           ImGui::Text("Start");
           ImGui::SameLine();
@@ -663,13 +682,23 @@ void FurnaceGUI::drawSampleEdit() {
           if (ImGui::IsItemActive()) {
             keepLoopAlive=true;
           }
-          if (ImGui::IsItemHovered() && (!warnLoopPos.empty() || sample->depth==DIV_SAMPLE_DEPTH_BRR)) {
-            if (sample->depth==DIV_SAMPLE_DEPTH_BRR) {
-              SAMPLE_WARN(warnLoopPos,"changing the loop in a BRR sample may result in glitches!");
+          if (ImGui::IsItemHovered() && (!warnLoopPos.empty() || !warnLoopStart.empty() || sample->depth==DIV_SAMPLE_DEPTH_BRR)) {
+            if (ImGui::BeginTooltip()) {
+              if (sample->depth==DIV_SAMPLE_DEPTH_BRR) {
+                ImGui::Text("changing the loop in a BRR sample may result in glitches!");
+              }
+              if (!warnLoopStart.empty()) {
+                ImGui::Text("%s",warnLoopStart.c_str());
+              }
+              if (!warnLoopPos.empty()) {
+                ImGui::Text("%s",warnLoopPos.c_str());
+              }
+              ImGui::EndTooltip();
             }
-            ImGui::SetTooltip("%s",warnLoopPos.c_str());
           }
+          popWarningColor();
 
+          pushWarningColor(!warnLoopPos.empty() || !warnLoopEnd.empty());
           ImGui::AlignTextToFramePadding();
           ImGui::Text("End");
           ImGui::SameLine();
@@ -689,11 +718,19 @@ void FurnaceGUI::drawSampleEdit() {
           if (ImGui::IsItemActive()) {
             keepLoopAlive=true;
           }
-          if (ImGui::IsItemHovered() && (!warnLoopPos.empty() || sample->depth==DIV_SAMPLE_DEPTH_BRR)) {
-            if (sample->depth==DIV_SAMPLE_DEPTH_BRR) {
-              SAMPLE_WARN(warnLoopPos,"changing the loop in a BRR sample may result in glitches!");
+          if (ImGui::IsItemHovered() && (!warnLoopPos.empty() || !warnLoopEnd.empty() || sample->depth==DIV_SAMPLE_DEPTH_BRR)) {
+            if (ImGui::BeginTooltip()) {
+              if (sample->depth==DIV_SAMPLE_DEPTH_BRR) {
+                ImGui::Text("changing the loop in a BRR sample may result in glitches!");
+              }
+              if (!warnLoopEnd.empty()) {
+                ImGui::Text("%s",warnLoopEnd.c_str());
+              }
+              if (!warnLoopPos.empty()) {
+                ImGui::Text("%s",warnLoopPos.c_str());
+              }
+              ImGui::EndTooltip();
             }
-            ImGui::SetTooltip("%s",warnLoopPos.c_str());
           }
           popWarningColor();
           ImGui::EndDisabled();