ppy / osu-framework

A game framework written with osu! in mind.
MIT License
1.67k stars 420 forks source link

Texture wrap mode ignored when atlas already bound #6409

Closed Jumprocks1 closed 2 weeks ago

Jumprocks1 commented 2 weeks ago

Found on 2024.523.0, confirmed and tested on 2024.1025.0

When a texture is bound through Renderer.BindTexture, the wrap mode is ignored if the texture is a part of the currently bound atlas.

The logic for this is in: https://github.com/ppy/osu-framework/blob/0750044927b258228ae39ef52e443c5cd2ad634e/osu.Framework/Graphics/Rendering/Renderer.cs#L790-L839

Notice that if the INativeTexture is already bound, it skips setting the wrap mode. This is incorrect since a single INativeTexture (ie. an atlas) can have textures with different wrap modes in it.

There was already a special case added for TextureWhitePixel in https://github.com/ppy/osu-framework/pull/6350 but that does not fix the issue for other textures.

Here's a test case to reproduce the issue. Note that I had to downscale "sample-texture.png" from 512x512 to 256x256 since the 512x512 size does not allow fitting 2 of them in the 1024x1024 atlas.

// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using osu.Framework.Allocation;
using osu.Framework.Graphics.Primitives;
using osu.Framework.Graphics.Sprites;
using osu.Framework.Graphics.Textures;

namespace osu.Framework.Tests.Visual
{
    public partial class TestSceneSpriteWrap : FrameworkTestScene
    {
        [BackgroundDependencyLoader]
        private void load(TextureStore store)
        {
            float y = 0f;
            Sprite makeSprite(WrapMode wrapModeS)
            {
                var o = new Sprite
                {
                    Width = 100,
                    Height = 50,
                    Texture = store.Get("sample-texture-256", wrapModeS, WrapMode.None),
                    TextureRectangle = new RectangleF(0f, 0f, 0.5f, 1f),
                    Y = y
                };
                y += 50;
                return o;
            }
            // sets Renderer.CurrentWrapMode to ClampToBorder and binds the atlas for sample-texture-256
            Add(makeSprite(WrapMode.ClampToBorder));
            // this wrap mode will be ignored and drawn as ClampToBorder instead
            Add(makeSprite(WrapMode.Repeat));
            Add(new SpriteText { Y = y, Text = "Text to cause bind to different texture atlas" });
            y += 20;
            // We should expect this to look identical to the second sprite, but it ends up
            // looking different due to the texture bind order.
            // This sprite is drawn correctly.
            Add(makeSprite(WrapMode.Repeat));
        }
    }
}

image

This is my recommended fix, though I don't know much about the intricacies of the render code:

diff --git a/osu.Framework/Graphics/Rendering/Renderer.cs b/osu.Framework/Graphics/Rendering/Renderer.cs
index 8066d379e..01a8a1f39 100644
--- a/osu.Framework/Graphics/Rendering/Renderer.cs
+++ b/osu.Framework/Graphics/Rendering/Renderer.cs
@@ -818,7 +818,10 @@ public bool BindTexture(Texture texture, int unit, WrapMode? wrapModeS, WrapMode
         public bool BindTexture(INativeTexture texture, int unit = 0, WrapMode wrapModeS = WrapMode.None, WrapMode wrapModeT = WrapMode.None)
         {
             if (lastActiveTextureUnit == unit && lastBoundTexture[unit] == texture)
+            {
+                setWrapMode(wrapModeS, wrapModeT);
                 return true;
+            }

             FlushCurrentBatch(FlushBatchSource.BindTexture);
smoogipoo commented 2 weeks ago

Ouch. Your fix looks fine, feel free to push it + a test case alongside the one added in #6350.