Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix drawing slider with touch inserts a random control point at beginning #30263

Merged
merged 5 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuEditor.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,86 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System.Linq;
using NUnit.Framework;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input;
using osu.Framework.Testing;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Rulesets.UI;
using osu.Game.Screens.Edit.Components.RadioButtons;
using osu.Game.Screens.Edit.Compose.Components;
using osu.Game.Tests.Visual;
using osuTK;
using osuTK.Input;

namespace osu.Game.Rulesets.Osu.Tests.Editor
{
[TestFixture]
public partial class TestSceneOsuEditor : EditorTestScene
{
protected override Ruleset CreateEditorRuleset() => new OsuRuleset();

[Test]
public void TestTouchInputAfterTouchingComposeArea()
{
AddStep("tap circle", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "HitCircle")));

// this input is just for interacting with compose area
AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));

AddStep("move current time", () => InputManager.Key(Key.Right));

AddStep("tap to place circle", () => tap(this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(10, 10))));
AddAssert("circle placed correctly", () =>
{
var circle = (HitCircle)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
Assert.Multiple(() =>
{
Assert.That(circle.Position.X, Is.EqualTo(10f).Within(0.01f));
Assert.That(circle.Position.Y, Is.EqualTo(10f).Within(0.01f));
});

return true;
});

AddStep("tap slider", () => tap(this.ChildrenOfType<EditorRadioButton>().Single(b => b.Button.Label == "Slider")));

// this input is just for interacting with compose area
AddStep("tap playfield", () => tap(this.ChildrenOfType<Playfield>().Single()));

AddStep("move current time", () => InputManager.Key(Key.Right));

AddStep("hold to draw slider", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(50, 20)))));
AddStep("drag to draw", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, this.ChildrenOfType<Playfield>().Single().ToScreenSpace(new Vector2(200, 50)))));
AddAssert("selection not initiated", () => this.ChildrenOfType<DragBox>().All(d => d.State == Visibility.Hidden));
AddStep("end", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));
AddAssert("slider placed correctly", () =>
{
var slider = (Slider)EditorBeatmap.HitObjects.Single(h => h.StartTime == EditorClock.CurrentTimeAccurate);
Assert.Multiple(() =>
{
Assert.That(slider.Position.X, Is.EqualTo(50f).Within(0.01f));
Assert.That(slider.Position.Y, Is.EqualTo(20f).Within(0.01f));
Assert.That(slider.Path.ControlPoints.Count, Is.EqualTo(2));
Assert.That(slider.Path.ControlPoints[0].Position, Is.EqualTo(Vector2.Zero));

// the final position may be slightly off from the mouse position when drawing, account for that.
Assert.That(slider.Path.ControlPoints[1].Position.X, Is.EqualTo(150).Within(5));
Assert.That(slider.Path.ControlPoints[1].Position.Y, Is.EqualTo(30).Within(5));
});

return true;
});
}

private void tap(Drawable drawable) => tap(drawable.ScreenSpaceDrawQuad.Centre);

private void tap(Vector2 position)
{
InputManager.BeginTouch(new Touch(TouchSource.Touch1, position));
InputManager.EndTouch(new Touch(TouchSource.Touch1, position));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Input;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osu.Game.Rulesets.Edit;
Expand Down Expand Up @@ -392,6 +393,29 @@ public void TestSliderDrawingLinear()
assertFinalControlPointType(3, null);
}

[Test]
public void TestSliderDrawingViaTouch()
{
Vector2 startPoint = new Vector2(200);

AddStep("move mouse to a random point", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(Vector2.Zero)));
AddStep("begin touch at start point", () => InputManager.BeginTouch(new Touch(TouchSource.Touch1, InputManager.ToScreenSpace(startPoint))));

for (int i = 1; i < 20; i++)
addTouchMovementStep(startPoint + new Vector2(i * 40, MathF.Sin(i * MathF.PI / 5) * 50));

AddStep("release touch at end point", () => InputManager.EndTouch(new Touch(TouchSource.Touch1, InputManager.CurrentState.Touch.GetTouchPosition(TouchSource.Touch1)!.Value)));

assertPlaced(true);
assertLength(808, tolerance: 10);
assertControlPointCount(5);
assertFinalControlPointType(0, PathType.BSpline(4));
assertFinalControlPointType(1, null);
assertFinalControlPointType(2, null);
assertFinalControlPointType(3, null);
assertFinalControlPointType(4, null);
}

[Test]
public void TestPlacePerfectCurveSegmentAlmostLinearlyExterior()
{
Expand Down Expand Up @@ -492,6 +516,8 @@ public void TestPlacePerfectCurveSegmentCompleteArc()

private void addMovementStep(Vector2 position) => AddStep($"move mouse to {position}", () => InputManager.MoveMouseTo(InputManager.ToScreenSpace(position)));

private void addTouchMovementStep(Vector2 position) => AddStep($"move touch1 to {position}", () => InputManager.MoveTouchTo(new Touch(TouchSource.Touch1, InputManager.ToScreenSpace(position))));

private void addClickStep(MouseButton button)
{
AddStep($"click {button}", () => InputManager.Click(button));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private void refreshTool()
ensurePlacementCreated();
}

private void updatePlacementPosition()
private void updatePlacementTimeAndPosition()
{
var snapResult = Composer.FindSnappedPositionAndTime(InputManager.CurrentState.Mouse.Position, CurrentPlacement.SnapType);

Expand Down Expand Up @@ -329,8 +329,18 @@ protected override void Update()
if (Composer.CursorInPlacementArea)
ensurePlacementCreated();

// updates the placement with the latest editor clock time.
if (CurrentPlacement != null)
updatePlacementPosition();
updatePlacementTimeAndPosition();
}

protected override bool OnMouseMove(MouseMoveEvent e)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you measure what the performance overhead of this is, if any? Given input is polled at 1000hz I expect this will be a bit redundant / slower.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
// updates the placement with the latest mouse position.
if (CurrentPlacement != null)
updatePlacementTimeAndPosition();

return base.OnMouseMove(e);
}

protected sealed override SelectionBlueprint<HitObject> CreateBlueprintFor(HitObject item)
Expand Down Expand Up @@ -367,7 +377,7 @@ private void ensurePlacementCreated()
placementBlueprintContainer.Child = CurrentPlacement = blueprint;

// Fixes a 1-frame position discrepancy due to the first mouse move event happening in the next frame
updatePlacementPosition();
updatePlacementTimeAndPosition();

updatePlacementSamples();

Expand Down
28 changes: 26 additions & 2 deletions osu.Game/Tests/Visual/PlacementBlueprintTestScene.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

#nullable disable

using System;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input.Events;
using osu.Framework.Timing;
using osu.Game.Beatmaps;
using osu.Game.Rulesets.Edit;
Expand All @@ -24,6 +26,10 @@ public abstract partial class PlacementBlueprintTestScene : OsuManualInputManage
protected PlacementBlueprintTestScene()
{
base.Content.Add(HitObjectContainer = CreateHitObjectContainer().With(c => c.Clock = new FramedClock(new StopwatchClock())));
base.Content.Add(new MouseMovementInterceptor
{
MouseMoved = updatePlacementTimeAndPosition,
});
}

protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
Expand Down Expand Up @@ -83,10 +89,11 @@ public void Delete(HitObject hitObject)
protected override void Update()
{
base.Update();

CurrentBlueprint.UpdateTimeAndPosition(SnapForBlueprint(CurrentBlueprint));
updatePlacementTimeAndPosition();
}

private void updatePlacementTimeAndPosition() => CurrentBlueprint.UpdateTimeAndPosition(SnapForBlueprint(CurrentBlueprint));

protected virtual SnapResult SnapForBlueprint(HitObjectPlacementBlueprint blueprint) =>
new SnapResult(InputManager.CurrentState.Mouse.Position, null);

Expand All @@ -107,5 +114,22 @@ public override void Add(Drawable drawable)

protected abstract DrawableHitObject CreateHitObject(HitObject hitObject);
protected abstract HitObjectPlacementBlueprint CreateBlueprint();

private partial class MouseMovementInterceptor : Drawable
{
public Action MouseMoved;

public MouseMovementInterceptor()
{
RelativeSizeAxes = Axes.Both;
Depth = float.MinValue;
}

protected override bool OnMouseMove(MouseMoveEvent e)
{
MouseMoved?.Invoke();
return base.OnMouseMove(e);
}
}
}
}
Loading