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

Invert DrawNode application for composability #2314

Merged
merged 20 commits into from
Apr 25, 2019

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Apr 2, 2019

I want DrawNodes to be composable for use with a future BufferedDrawNode.

The end goal is for the following to be possible:

Drawable:
    CreateDrawNode() => new DrawNode();

Path:
    CreateDrawNode() => new BufferedDrawNode(new PathDrawNode())

Which will basically draw the path into a framebuffer. This is not really well supported by DrawNodes right now due to Drawable.ApplyDrawNode() requiring that you know the type of DrawNode given by CreateDrawNode(), which is not always possible.
Furthermore, ApplyDrawNode() would also have to fill the state of BufferedDrawNode.

In this PR:

Drawable

- ApplyDrawNode(DrawNode node)

The ApplyDrawNode() method has been removed.

DrawNode

+ ctor(Drawable source)
+ ApplyState()

The constructor provides the source from which the state for the DrawNode is copied.
The ApplyState() method serves the same purpose as ApplyDrawNode(), except in reverse - copying states from the Drawable to ourselves.

Breaking Changes

Drawable.ApplyDrawNode() has been removed

The direction of application of DrawNode states has been inversed.

Previously:

class MyCustomDrawable : Drawable
{
    private bool state;

    protected override DrawNode CreateDrawNode() => new CustomDrawNode();

    protected override void ApplyDrawNode(DrawNode node)
    {
        base.ApplyDrawNode(node);

        var n = (CustomDrawNode)n;
        
        n.State = state; 
    }

    private class CustomDrawNode : DrawNode
    {
        public bool State;
    }
}

Now:

class MyCustomDrawable : Drawable
{
    private bool state;

    protected override DrawNode CreateDrawNode() => new CustomDrawNode(this);

    private class CustomDrawNode : DrawNode
    {
        protected new MyCustomDrawable Source => (MyCustomDrawable)base.Source;

        private bool state;

        public CustomDrawNode(MyCustomDrawable source)
            : base(source)
        {
        }

        public override void ApplyState()
        {
            base.ApplyState();

            state = Source.state;
        }
    }
}

The namespace of EdgeEffectParameters and EdgeEffectType has changed

They now reside in osu.Framework.Graphics.Effects.

# Conflicts:
#	osu.Framework/Graphics/Containers/BufferedContainerDrawNode.cs
#	osu.Framework/Graphics/Lines/PathDrawNode.cs
@smoogipoo
Copy link
Contributor Author

Blocking because this has broken volume meter.

@smoogipoo smoogipoo removed the blocked label Apr 9, 2019
@peppy
Copy link
Member

peppy commented Apr 14, 2019

On a first pass this is looking quite good to me.

peppy
peppy previously approved these changes Apr 14, 2019
# Conflicts:
#	osu.Framework/Graphics/Audio/WaveformGraph.cs
@peppy peppy removed the request for review from Tom94 April 25, 2019 06:38
@peppy peppy merged commit 87900e8 into ppy:master Apr 25, 2019
@smoogipoo smoogipoo deleted the drawnode-composability branch May 21, 2021 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants