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

Add IRenderer and adjust DrawNode signature #5324

Merged
merged 5 commits into from
Jul 30, 2022

Conversation

smoogipoo
Copy link
Contributor

For a view of the full changes: smoogipoo#88

  • Adds the IRenderer interface.
  • Adds IRenderer to the DrawNode method signatures.
  • Moves default quad batch stuff to GLWrapper for now. This isn't the final location, but is needed for the time being due to the way TextureGLSingle works.
  • Re-namespaces ClearInfo/MaskingInfo/DepthInfo drawing structs.

vNext

IRenderer added as parameter to DrawNode

- DrawNode.Draw(Action<TexturedVertex2D> vertexAction);
+ DrawNode.Draw(IRenderer renderer);
- DrawNode.DrawOpaqueInterior(Action<TexturedVertex2D> vertexAction);
+ DrawNode.DrawOpaqueInterior(IRenderer renderer);

In places where the vertexAction parameter was used, pass null instead.

ClearInfo, MaskingInfo, DepthInfo have been re-namespaced

The new namespace is osu.Framework.Graphics.Rendering.

@peppy
Copy link
Member

peppy commented Jul 30, 2022

No issues with the code changes in this PR, but:

In places where the vertexAction parameter was used, pass null instead

Do you have an example of a case like this? I couldn't find any in the diff, and I'm not sure what the null is. Is it implying a null IRenderer is provided in some edge case? At first I thought this wouldn't even be valid but then noticed nullability isn't turned on (might be something to do as part of this pass?)

@smoogipoo
Copy link
Contributor Author

smoogipoo commented Jul 30, 2022

The vertexAction parameter doesn’t exist anymore, replaced by internal handling (see the PushQuadBatch/PopQuadBatch). It’s the same thing just that the consumer doesn’t have access to that quad batch anymore.
The DrawQuad vertexAction parameter is already default-null, so you can either provide null or remove the parameter altogether.

Examples of this are in the osu!-side PR, where I’ve removed the parameter.

@peppy
Copy link
Member

peppy commented Jul 30, 2022

Hmm, even looking at the osu! side changes I can't see where this advice would apply. Can you link an example line?

@smoogipoo
Copy link
Contributor Author

https://github.com/ppy/osu/pull/19461/files#diff-029bbf29cd3db400bca8db562da31f5f7c289ce2fde9fe9033a90234131fb76dR115-R117 (ParticleExplosion.cs)

@frenzibyte frenzibyte merged commit 50533b0 into ppy:master Jul 30, 2022
@smoogipoo smoogipoo deleted the irenderer-base branch September 11, 2023 02:25
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.

3 participants