Invalidation does not always propagate to children when it needs to. about osu-framework HOT 12 CLOSED

@Tom94 is this still an issue?

from osu-framework.

Yup, it still is. Note, that currently this is not a correctness problem, but rather makes invalidations slightly more inefficient than they could be. More specifically: if something like Color is invalidated, then the entire DrawInfo gets invalidated; potentially for the subtree if it depends on the DrawInfo of its parent.

from osu-framework.

Just for documentation, there are a few special cases coded in to handle the fallout effects of this:

baba269#diff-74fa55675af99d2ab8d5ed0f726caf41R332

and

from osu-framework.

Now that the framework progressed quite a bit further than when this issue was initially opened I think the general problem is clearer: this issue surfaces whenever the same underlying Cached<> is invalidated by different invalidation types. The solution to break this problem apart is to split DrawInfo further into individual components as done with color in #1836.

from osu-framework.

An another case is illustrated in #1803 FillFlow2 and FillFlow2Simplified.

from osu-framework.

I think this invariant should be maintained:

• If a Cached (Say A) is invalid then other Cached (Say B) which depends on A should be invalid.

Parent:
  Cached<TypeA> Abacking;
  TypeA A => Abacking.IsValid ? Abacking.Value : (Abacking.Value = ComputeA());
  TypeA ComputeA() => ...;
Child:
  Cached<TypeB> Bbacking;
  TypeB B => Bbacking.IsValid ? Bbacking.Value : (Bbacking.Value = ComputeB());
  TypeB ComputeB() => someFunc(Parent.A).
  invariant: !Bbacking.IsValid || Parent.Abacking.IsValid;

from osu-framework.

Suppose there are two invalidations, I1 and I2.
Suppose there are two drawables, X and Y with X containing Y.

Suppose X has one cached value, C1, and Y has one cached value, C2.

Suppose X responds to both I1 and I2 and invalidates C1.
Suppose Y responds to I2 and invalidates C2.

1. X invalidates with I1, it invalidates C1, propagates to Y, which then proceeds to do nothing.
2. X invalidates with I2, it sees that C1 is already invalid, so it doesn't propagate. Y cannot respond to I2 because X has determined it unnecessary to be propagated.

It seems like you're suggesting that C2 should depend on C1, but I have not mentioned anything about the relationship between C1 and C2 - for all we know they may be completely independent of each other.

Rather the issue is that I1 and I2 both touch C1.

from osu-framework.

That case can be implemented as follows:

1. There are cached properies CI1 and CI2.
2. X.C1 depends on both X.CI1 and X.CI2.
3. Y.C2 depends on X.CI2.

from osu-framework.

Here's another case of this happening:

public class TestCaseScratch : TestCase
{
    public TestCaseScratch()
    {
        Child = new TestContainer
        {
            RelativeSizeAxes = Axes.Both,
            Size = new Vector2(0.5f),
            Child = new FillFlowContainer
            {
                RelativeSizeAxes = Axes.Both,
                Direction = FillDirection.Horizontal,
                Children = new Drawable[]
                {
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Width = 0.5f,
                        Colour = Color4.Red
                    },
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Width = 0.5f,
                        Colour = Color4.Green
                    },
                }
            },
        };
    }

    private class TestContainer : CompositeDrawable
    {
        public Drawable Child
        {
            set => childContainer.Child = value;
        }

        private readonly Container childContainer;

        public TestContainer()
        {
            InternalChild = childContainer = new Container();
        }

        protected override void Update()
        {
            base.Update();

            childContainer.Size = DrawSize;
        }
    }
}

B.Invalidate(DrawSize) doesn't get propagated on resize.

from osu-framework.

public class TestSceneFillFlowContainer : TestScene
{
    public TestSceneFillFlowContainer()
    {
        TestContainer testContainer;
        Add(testContainer = new TestContainer
        {
            RelativeSizeAxes = Axes.Both,
            Child = new FillFlowContainer
            {
                RelativeSizeAxes = Axes.Both,
                Height = 0.25f,
                Children = new[]
                {
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Colour = Color4.Beige,
                        Width = 0.2f,
                    },
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Colour = Color4.Bisque,
                        Width = 0.2f,
                    },
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Colour = Color4.Aquamarine,
                        Width = 0.2f,
                    },
                    new Box
                    {
                        RelativeSizeAxes = Axes.Both,
                        Colour = Color4.Cornsilk,
                        Width = 0.2f,
                    },
                }
            }
        });

        AddSliderStep("Adjust scale", 0.5, 1.5, 1.0, b => testContainer.AdjustScale((float)b));
        AddStep("Invalidate layout", () => testContainer.Invalidate());
    }

    private class TestContainer : Container<Drawable>
    {
        public void AdjustScale(float scale = 1.0f)
        {
            this.ScaleTo(new Vector2(scale));
            this.ResizeTo(new Vector2(1 / scale));
        }
    }
}

from osu-framework.

Going to bump this up from candidate since we're reaching more and more cases where this is an issue.

from osu-framework.

As a bit of an update, I've started working on this issue in https://github.com/smoogipoo/osu-framework/tree/invalidation-rework .

I've come up with the following structure:

Drawable A:
    - Layout L_A
    - Drawable B:
        - Layout L_B
        - Drawable C
    - Drawable B_1

Invalidate(A, type)

1. If { A, L_A, B, B_1 } have already been invalidated with all of the given invalidation type flags, stop execution, otherwise:
2. Invalidate the layout of A.
3. Recursively invalidate the layout dependencies { L_A, B, B_1 }.

Validate(L_B, type)

1. If { L_B } has already been validated with all of the given invalidation type flags, stop execution.
2. Validate { L_B }.
3. Recursively validate the dependent layout { B }.

Explanation

For a given subtree, we only care about two states:

1. The entire subtree is invalidated.
2. Any of the subtree is validated.

During the validation stage, we make sure that (2) is satisfied. Since we only care about any being validated we can avoid the O(N^2) complexity by validating the dependents only once for any particular invalidation type.

During the invalidation stage, since the previous validation has propagated upwards, we know when a particular hierarchy will need a validation at some point. This will fix this issue, and will also allow us to optimise our invalidations a bit more (see the original commit mentioned in the OP).

There is one edge case that I've yet to tackle in my WIP branch, which is when a layout is conditionally invalidated (e.g. SpriteText or DrawColourInfo for size changes).

from osu-framework.