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

Cleanup DrawNode drawing methods #6073

Merged
merged 9 commits into from Dec 4, 2023

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Dec 4, 2023

Curious what people's thoughts are on this.

Right now, we have a dichotomy between the implementations of Draw() and DrawOpaqueInternal() where one batches everything including children inside Draw() but the other exposes DrawOpaqueInternalSubtree() for this purpose.

This results in other weirdness, like Draw() being public and DrawOpaqueInternal() being protected. As a consumer of the framework, there is no way to correctly implement DrawOpaqueInternal() if needing to draw a child, prior to this PR.

My eventual goal is to be able to wrap both of these draw functions in some way that indicates to the renderer what DrawNode is currently being drawn, for example something like...

class DrawNode
{
    protected internal static void DrawOther(DrawNode node, IRenderer renderer)
    {
        using (renderer.BeginDrawNode(node))
            node.Draw();
    }
}

This is a breaking change.

vNext

DrawNode.Draw() has been made protected

In order to match the signature of DrawOpaqueInternal(), Draw() has been made protected.

DrawNodes which draw others from within themselves need to make use of helper methods:

- public override void Draw(IRenderer renderer)
+ protected override void Draw(IRenderer renderer)
{
    base.Draw(renderer);

-   other.Draw(renderer);
+   DrawOther(other, renderer);
}

Copy link
Sponsor Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Fine to my eye.

@bdach bdach changed the title Cleanup DrawNode drawing methods Cleanup DrawNode drawing methods Dec 4, 2023
@bdach bdach enabled auto-merge December 4, 2023 11:25
@bdach bdach merged commit f7b43b5 into ppy:master Dec 4, 2023
20 of 21 checks passed
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.

None yet

3 participants