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

Fix autosize invalidations occurring with BypassAutoSIzeAxes #1554

Merged
merged 2 commits into from May 18, 2018

Conversation

2 participants
@smoogipoo
Contributor

smoogipoo commented May 16, 2018

In the case of, e.g.:

container1 = Container
    AutoSizeAxes = Axes.Both
    Children =
        {{ other children }}
        container2 = Container
            AutoSizeAxes = Axes.Both // Want to maintain a size
            BypassAutoSizeAxes = Axes.Both // But we don't want the parent to size to us

If the size of container2 was changed, it will propagate a DrawSize invalidation to container1 which will cause container1 to recompute its autosize.
When container1 recomputes its autosize, it finds that container2 has BypassAutoSizeAxes, so skips it.

This extra computation is unnecessary.

@smoogipoo smoogipoo added this to the May 2018 milestone May 16, 2018

@@ -254,7 +254,7 @@ protected internal virtual bool RemoveInternal(Drawable drawable)
drawable.IsAlive = false;
if (AutoSizeAxes != Axes.None)

This comment has been minimized.

@Tom94

Tom94 May 16, 2018

Collaborator

I'd rather have an && drawable.BypassAutoSizeAxes ... term here and in the other call to InvalidateFromChild.

InvalidateFromChild may be used in other cases (since it is virtual) where we do not want the early return if autosize should be bypassed.

This comment has been minimized.

@smoogipoo

smoogipoo May 18, 2018

Contributor

This will break the autosize case, down the call stack:

Width
    -> Invalidate
        -> Parent.InvalidateFromChild

As well as FlowContainer cases, since those invalidate layout based on this call.

CompositeDrawable's autosize calculation should always early return under this condition, since the autosize calculation won't update anything. The way I've implemented it doesn't affect overriders.

@Tom94

Tom94 approved these changes May 18, 2018

@Tom94 Tom94 merged commit 90c4252 into ppy:master May 18, 2018

2 checks passed

CodeFactor No issues found.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment