Merge pull request #32572 from peppy/storyboard-optimise-noop-alpha-transforms

Fix storyboards with no-op alpha operations causing extended drawable lifetimes

osu.Game.Tests/Beatmaps/Formats/LegacyStoryboardDecoderTest.cs

@@ -135,6 +135,24 @@ namespace osu.Game.Tests.Beatmaps.Formats
             }
         }
 
+        [Test]
+        public void TestNoopFadeTransformIsIgnoredForLifetime()
+        {
+            var decoder = new LegacyStoryboardDecoder();
+
+            using (var resStream = TestResources.OpenResource("noop-fade-transform-is-ignored-for-lifetime.osb"))
+            using (var stream = new LineBufferedReader(resStream))
+            {
+                var storyboard = decoder.Decode(stream);
+
+                StoryboardLayer background = storyboard.Layers.Single(l => l.Depth == 3);
+                Assert.AreEqual(2, background.Elements.Count);
+
+                Assert.AreEqual(1500, background.Elements[0].StartTime);
+                Assert.AreEqual(1500, background.Elements[1].StartTime);
+            }
+        }
+
         [Test]
         public void TestOutOfOrderStartTimes()
         {
@@ -288,6 +306,29 @@ namespace osu.Game.Tests.Beatmaps.Formats
             }
         }
 
+        [Test]
+        public void TestVideoWithCustomFadeIn()
+        {
+            var decoder = new LegacyStoryboardDecoder();
+
+            using var resStream = TestResources.OpenResource("video-custom-alpha-transform.osb");
+            using var stream = new LineBufferedReader(resStream);
+
+            var storyboard = decoder.Decode(stream);
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(storyboard.GetLayer(@"Video").Elements, Has.Count.EqualTo(1));
+                Assert.That(storyboard.GetLayer(@"Video").Elements.Single(), Is.InstanceOf<StoryboardVideo>());
+                Assert.That(storyboard.GetLayer(@"Video").Elements.Single().StartTime, Is.EqualTo(-5678));
+                Assert.That(((StoryboardVideo)storyboard.GetLayer(@"Video").Elements.Single()).Commands.Alpha.Single().StartTime, Is.EqualTo(1500));
+                Assert.That(((StoryboardVideo)storyboard.GetLayer(@"Video").Elements.Single()).Commands.Alpha.Single().EndTime, Is.EqualTo(1600));
+
+                Assert.That(storyboard.EarliestEventTime, Is.Null);
+                Assert.That(storyboard.LatestEventTime, Is.Null);
+            });
+        }
+
         [Test]
         public void TestVideoAndBackgroundEventsDoNotAffectStoryboardBounds()
         {

osu.Game.Tests/Resources/noop-fade-transform-is-ignored-for-lifetime.osb

@@ -0,0 +1,8 @@
+[Events]
+//Storyboard Layer 0 (Background)
+Sprite,Background,TopCentre,"img.jpg",320,240
+ F,0,1000,1000,0,0 // should be ignored
+ F,0,1500,1600,0,1
+Sprite,Background,TopCentre,"img.jpg",320,240
+ F,0,1000,1000,0,0 // should be ignored
+ F,0,1500,1600,1,1

osu.Game.Tests/Resources/video-custom-alpha-transform.osb

@@ -0,0 +1,5 @@
+osu file format v14
+
+[Events]
+Video,-5678,"Video.avi",0,0
+ F,0,1500,1600,0,1

osu.Game/Storyboards/StoryboardSprite.cs

@@ -24,35 +24,48 @@ namespace osu.Game.Storyboards
 
         public readonly StoryboardCommandGroup Commands = new StoryboardCommandGroup();
 
-        public double StartTime
+        public virtual double StartTime
         {
             get
             {
-                // To get the initial start time, we need to check whether the first alpha command to exist (across all loops) has a StartValue of zero.
-                // A StartValue of zero governs, above all else, the first valid display time of a sprite.
+                // Users that are crafting storyboards using raw osb scripting or external tools may create alpha events far before the actual display time
+                // of sprites.
                 //
-                // You can imagine that the first command of each type decides that type's start value, so if the initial alpha is zero,
-                // anything before that point can be ignored (the sprite is not visible after all).
-                var alphaCommands = new List<(double startTime, bool isZeroStartValue)>();
+                // To make sure lifetime optimisations work as efficiently as they can, let's locally find the first time a sprite becomes visible.
+                var alphaCommands = new List<StoryboardCommand<float>>();
 
-                var command = Commands.Alpha.FirstOrDefault();
-                if (command != null) alphaCommands.Add((command.StartTime, command.StartValue == 0));
+                foreach (var command in Commands.Alpha)
+                {
+                    alphaCommands.Add(command);
+                    if (visibleAtStartOrEnd(command))
+                        break;
+                }
 
                 foreach (var loop in loopingGroups)
                 {
-                    command = loop.Alpha.FirstOrDefault();
-                    if (command != null) alphaCommands.Add((command.StartTime, command.StartValue == 0));
+                    foreach (var command in loop.Alpha)
+                    {
+                        alphaCommands.Add(command);
+                        if (visibleAtStartOrEnd(command))
+                            break;
+                    }
                 }
 
                 if (alphaCommands.Count > 0)
                 {
-                    var firstAlpha = alphaCommands.MinBy(t => t.startTime);
+                    // Special care is given to cases where there's one or more no-op transforms (ie transforming from alpha 0 to alpha 0).
+                    // - If a 0->0 transform exists, we still need to check it to ensure the absolute first start value is non-visible.
+                    // - After ascertaining this, we then check the first non-noop transform to get the true start lifetime.
+                    var firstAlpha = alphaCommands.MinBy(c => c.StartTime);
+                    var firstRealAlpha = alphaCommands.Where(visibleAtStartOrEnd).MinBy(c => c.StartTime);
 
-                    if (firstAlpha.isZeroStartValue)
-                        return firstAlpha.startTime;
+                    if (firstAlpha!.StartValue == 0 && firstRealAlpha != null)
+                        return firstRealAlpha.StartTime;
                 }
 
                 return EarliestTransformTime;
+
+                bool visibleAtStartOrEnd(StoryboardCommand<float> command) => command.StartValue > 0 || command.EndValue > 0;
             }
         }
 

osu.Game/Storyboards/StoryboardVideo.cs

@@ -14,9 +14,11 @@ namespace osu.Game.Storyboards
         {
             // This is just required to get a valid StartTime based on the incoming offset.
             // Actual fades are handled inside DrawableStoryboardVideo for now.
-            Commands.AddAlpha(Easing.None, offset, offset, 0, 0);
+            StartTime = offset;
         }
 
+        public override double StartTime { get; }
+
         public override Drawable CreateDrawable() => new DrawableStoryboardVideo(this);
     }
 }