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

Internalise TextureGL and rewrite Texture to take its role #5330

Merged
merged 33 commits into from
Aug 5, 2022

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Aug 2, 2022

This PR is very big and nigh impossible to split out due to the amount of dependencies between TextureGL and Texture. Hopefully the diff should be pretty easy to go through, and I've done my best to separate the process via commits.

  • TextureGL* classes merged into one.
  • Opacity calculation and subtexturing has been moved to Texture.
  • Texture no longer exposes the underlying native texture.
  • Textures must be created through the IRenderer.

The rest should be explainable via the breaking changes below:

vNext

WrapMode and Opacity enums have been re-namespaced

Their new location is osu.Framework.Graphics.Textures.

Texture.WhitePixel has been moved to IRenderer

It is no longer provided as a static member. Instead, resolve an IRenderer and access IRenderer.WhitePixel.

public class MyDrawable : Drawable
{
+   [BackgroundDependencyLoader]
+   private void load(IRenderer renderer)
+   {
+       texture ??= renderer.WhitePixel;
+   }

    private Texture texture;

    public Texture Texture
    {
-       get => texture ?? Texture.WhitePixel;
+       get => texture;
        set => texture = value;
    }
}

TextureStore, FontStore and LargeTextureStore construction parameters have changed

  • Requires an IRenderer parameter (via dependency injection).
  • Filter mode parameter changed from All to TextureFilteringMode.

One common use case is to provide a new texture store from inside a derived Game, for which the following change is required:

public class TestGame : osu.Framework.Game
{
    private DependencyContainer dependencies;

    protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) =>
        dependencies = new DependencyContainer(base.CreateChildDependencies(parent));

    [BackgroundDependencyLoader]
    private void load()
    {
-       var largeStore = new LargeTextureStore(Host.CreateTextureLoaderStore(new NamespacedResourceStore<byte[]>(Resources, @"Textures")), All.Nearest);
+       var largeStore = new LargeTextureStore(Host.Renderer, Host.CreateTextureLoaderStore(new NamespacedResourceStore<byte[]>(Resources, @"Textures")), TextureFilteringMode.Nearest);
        largeStore.AddTextureSource(Host.CreateTextureLoaderStore(new OnlineStore()));
        dependencies.Cache(largeStore);
    }
}

Texture cannot be used to create textures

Texture must now be created via IRenderer.

public class MyDrawable : Drawable
{
-   private readonly Texture texture;

-   public MyDrawable()
-   {
-       texture = new Texture(100, 100);
-       texture.SetData(...);
-   }
+   private Texture texture;

+   [BackgroundDependencyLoader]
+   private void load(IRenderer renderer)
+   {
+       texture = renderer.CreateTexture(100, 100);
+       texture.SetData(...);
+   }
}

DummyRenderer may be used for cases where textures need to be created and neither an IRenderer nor GameHost is accessible:

[TestFixture]
public class MyTestFixture
{
    public void CreateATexture()
    {
-       Texture texture = new Texture(1, 1);
+       Texture texture = new DummyRenderer().CreateTexture(1, 1);
    }
}

Texture drawing methods moved from DrawNode to IRenderer extension methods

Appearing alongside IRenderer is the new RendererExtensions class providing extension helper methods for common drawing procedures.

An example of the type of change required DrawNode:

public class MyDrawNode : DrawNode
{
    public override void Draw(IRenderer renderer)
    {
        base.Draw(renderer);

-       DrawQuad(renderer.WhitePixel, ...);
+       renderer.DrawQuad(renderer.WhitePixel, ...);
    }
}

The following methods have been moved to this class:

DrawTriangle()
DrawQuad()
DrawClipped()
DrawFrameBuffer()

TextureGL is no longer accessible

Properties such as TextureGL.BypassTextureUploadQueueing have been moved to Texture itself, and Texture can be used for all drawing procedures.

During rendering, textures may now be bound via an integer sampling unit.

public class MyDrawable : Drawable
{
    private Texture texture;

    [BackgroundDependencyLoader]
    private void load(IRenderer renderer)
    {
        texture = renderer.CreateTexture(100, 100);
        texture.BypassUploadQueueing = true;
        texture.SetData(...);
    }
}

public class MyDrawNode : DrawNode
{
    private Texture texture;

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

-       texture.TextureGL.Bind();
-       texture.TextureGL.Bind(TextureUnit.Texture1);
+       texture.Bind();
+       texture.Bind(1);

        // Note: The texture binding above isn't required in either case for the call below.
-       DrawQuad(texture.TextureGL, ...);
+       renderer.DrawQuad(texture, ...);
    }
}

osu.Framework/Graphics/OpenGL/Buffers/FrameBuffer.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Rendering/INativeTexture.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Rendering/INativeTexture.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Textures/TextureRegion.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Textures/TextureAtlas.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Textures/TextureAtlas.cs Outdated Show resolved Hide resolved
osu.Framework/Graphics/Textures/Texture.cs Outdated Show resolved Hide resolved
@peppy peppy requested a review from frenzibyte August 3, 2022 08:02
Comment on lines +258 to +261
/// <param name="area">A display-space area of this texture to compute the UV coordinates of.<br/>
/// The full area is used if not provided: (0, 0, <see cref="DisplayWidth"/>, <see cref="DisplayHeight"/>).</param>
/// <returns>The UV coordinates of <paramref name="area"/> in this texture.</returns>
public virtual RectangleF GetTextureRect(RectangleF? area = null)
Copy link
Contributor Author

@smoogipoo smoogipoo Aug 3, 2022

Choose a reason for hiding this comment

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

One thing I forgot to mention is that I had a lot of trouble getting this GetTextureRect() method to work due to working in two "coordinate spaces".

In this PR, I've tried to use the terminology "texture-space" and "display-space" to describe which coordinate space the user or reviewer of code should be thinking of.

  • "texture-space" is in coordinates (0, 0, Width, Height)
  • "display-space" is in coordinates (0, 0, DisplayWidth, DisplayHeight)

This method here is one example, as well as:

As well as... GetTextureRect() returns as "UV coordinates" which is typically known to be normalised coordinates in texture space (i.e. what's passed to the GPU).

I'd be glad to change it if anyone has better suggestions.

@peppy peppy merged commit 49b29cf into ppy:master Aug 5, 2022
Copy link
Member

@frenzibyte frenzibyte left a comment

Choose a reason for hiding this comment

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

Looks good on a post-merge read.

@smoogipoo smoogipoo deleted the irenderer-textures 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