Skip to content

Invert DrawNode application for composability#2314

Merged
peppy merged 20 commits intoppy:masterfrom
smoogipoo:drawnode-composability
Apr 25, 2019
Merged

Invert DrawNode application for composability#2314
peppy merged 20 commits intoppy:masterfrom
smoogipoo:drawnode-composability

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