From af1a6d048d8dae46418b9bbb10111008a0d177f9 Mon Sep 17 00:00:00 2001
From: Richard Antalik <richardantalik@gmail.com>
Date: Thu, 11 Jul 2024 07:38:02 +0200
Subject: [PATCH] VSE: Simplify outline parameters definition and usage

Function `strip_data_outline_params_set()` was simplified, so setting
color and outline parameters are not mixed and overwriting as code flows
and so the function is better readable.

Shader code is changed, so that when strip overlaps other strip, it gets
2 px red outline regardless of whether it is active or selected. This
makes it more consistent when strip is not active or selected.

Pull Request: https://projects.blender.org/blender/blender/pulls/124442
---
 .../sequencer_timeline_draw.cc                | 42 +++++++++----------
 .../gpu_shader_sequencer_strips_frag.glsl     | 10 +++--
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc b/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc
index 4632bef5cd0..7676c71d9f5 100644
--- a/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc
+++ b/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc
@@ -1340,46 +1340,46 @@ static void strip_data_outline_params_set(const StripDrawContext &strip,
                                           const TimelineDrawContext *timeline_ctx,
                                           SeqStripDrawData &data)
 {
-  const bool selected = strip.seq->flag & SELECT;
   const bool active = strip.is_active_strip;
-  uchar col[4];
+  const bool selected = strip.seq->flag & SELECT;
+  uchar4 col{0, 0, 0, 255};
 
   if (selected) {
-    UI_GetThemeColor3ubv(active ? TH_SEQ_ACTIVE : TH_SEQ_SELECTED, col);
+    UI_GetThemeColor3ubv(TH_SEQ_SELECTED, col);
+    data.flags |= GPU_SEQ_FLAG_SELECTED;
   }
-  else {
+  if (active) {
+    if (selected) {
+      UI_GetThemeColor3ubv(TH_SEQ_ACTIVE, col);
+    }
+    else {
+      UI_GetThemeColorShade3ubv(TH_SEQ_ACTIVE, -40, col);
+    }
+    data.flags |= GPU_SEQ_FLAG_ACTIVE;
+  }
+  if (!selected && !active) {
     /* Color for unselected strips is a bit darker than the background. */
     UI_GetThemeColorShade3ubv(TH_BACK, -40, col);
   }
-  col[3] = 255;
+
+  const eSeqOverlapMode overlap_mode = SEQ_tool_settings_overlap_mode_get(timeline_ctx->scene);
+  const bool use_overwrite = overlap_mode == SEQ_OVERLAP_OVERWRITE;
+  const bool overlaps = (strip.seq->flag & SEQ_OVERLAP) && (G.moving & G_TRANSFORM_SEQ);
+
   /* Outline while translating strips:
    *  - Slightly lighter.
    *  - Red when overlapping with other strips. */
-  const eSeqOverlapMode overlap_mode = SEQ_tool_settings_overlap_mode_get(timeline_ctx->scene);
   if (G.moving & G_TRANSFORM_SEQ) {
-    if ((strip.seq->flag & SEQ_OVERLAP) && (overlap_mode != SEQ_OVERLAP_OVERWRITE)) {
+    if (overlaps && !use_overwrite) {
       col[0] = 255;
       col[1] = col[2] = 33;
+      data.flags |= GPU_SEQ_FLAG_OVERLAP;
     }
     else if (selected) {
       UI_GetColorPtrShade3ubv(col, col, 70);
     }
   }
 
-  const bool overlaps = (strip.seq->flag & SEQ_OVERLAP) && (G.moving & G_TRANSFORM_SEQ);
-  if (overlaps) {
-    data.flags |= GPU_SEQ_FLAG_OVERLAP;
-  }
-
-  if (selected) {
-    data.flags |= GPU_SEQ_FLAG_SELECTED;
-  }
-  else if (active && !overlaps) {
-    /* If the strips overlap when retiming, don't replace the red outline. */
-    /* A subtle highlight outline when active but not selected. */
-    UI_GetThemeColorShade3ubv(TH_SEQ_ACTIVE, -40, col);
-    data.flags |= GPU_SEQ_FLAG_ACTIVE;
-  }
   data.col_outline = color_pack(col);
 }
 
diff --git a/source/blender/gpu/shaders/gpu_shader_sequencer_strips_frag.glsl b/source/blender/gpu/shaders/gpu_shader_sequencer_strips_frag.glsl
index e5d70bf68e5..5f572730a9d 100644
--- a/source/blender/gpu/shaders/gpu_shader_sequencer_strips_frag.glsl
+++ b/source/blender/gpu/shaders/gpu_shader_sequencer_strips_frag.glsl
@@ -193,12 +193,16 @@ void main()
 
     /* Active, but not selected strips get a thin inner line. */
     bool active_strip = (strip.flags & GPU_SEQ_FLAG_ACTIVE) != 0;
-    /* When moving the retiming keys, the strip might overlap even if it isn't selected. */
-    bool overlaps = (strip.flags & GPU_SEQ_FLAG_OVERLAP) != 0;
-    if ((active_strip && !selected) || overlaps) {
+    if (active_strip && !selected) {
       col = add_outline(sdf, 1.0, 2.0, col, col_outline);
     }
 
+    /* 2px outline for all overlapping strips. */
+    bool overlaps = (strip.flags & GPU_SEQ_FLAG_OVERLAP) != 0;
+    if (overlaps) {
+      col = add_outline(sdf, 1.0, 3.0, col, col_outline);
+    }
+
     /* Outer 1px outline for all strips. */
     col = add_outline(sdf, 0.0, 1.0, col, unpackUnorm4x8(context_data.col_back));
   }