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

ScrollableControl.ScaleDockPadding seems to be broken? #2841

Open
hughbe opened this issue Feb 10, 2020 · 1 comment
Open

ScrollableControl.ScaleDockPadding seems to be broken? #2841

hughbe opened this issue Feb 10, 2020 · 1 comment

Comments

@hughbe
Copy link
Contributor

@hughbe hughbe commented Feb 10, 2020

Problem:

Looking at the implementation of `ScrollableControl.Scale{Core}:

internal void ScaleDockPadding(float dx, float dy)
{
    dockPadding?.Scale(dx, dy);
}

[EditorBrowsable(EditorBrowsableState.Never)]
protected override void ScaleCore(float dx, float dy)
{
    ScaleDockPadding(dx, dy);
    base.ScaleCore(dx, dy);
}

Where DockPadding.Scale is implemented as

internal void Scale(float dx, float dy) => _owner.Padding.Scale(dx, dy);

I would expect the scrollable control's padding to scale by dx and dy. However, the following tests demonstrate that no scaling is happening.

I think something may be weird in that calling Padding.Scale does not seem to affect the underlying Padding instance stored in Control

[WinFormsFact]
public void ScrollableControl_Scale_InvokeWithoutPaddingWithDockPadding_Success()
{
    using var control = new ScrollableControl();
    Assert.Equal(0, control.DockPadding.Left);
    Assert.Equal(0, control.DockPadding.Top);
    Assert.Equal(0, control.DockPadding.Right);
    Assert.Equal(0, control.DockPadding.Bottom);
    control.Scale(10, 20);

    Assert.Equal(0, control.DockPadding.Left);
    Assert.Equal(0, control.DockPadding.Top);
    Assert.Equal(0, control.DockPadding.Right);
    Assert.Equal(0, control.DockPadding.Bottom);
    Assert.Equal(Padding.Empty, control.Padding);
}

[WinFormsFact]
public void ScrollableControl_Scale_InvokeWithoutPaddingWithoutDockPadding_Success()
{
    using var control = new ScrollableControl();
    control.Scale(10, 20);
    Assert.Equal(0, control.DockPadding.Left);
    Assert.Equal(0, control.DockPadding.Top);
    Assert.Equal(0, control.DockPadding.Right);
    Assert.Equal(0, control.DockPadding.Bottom);
    Assert.Equal(Padding.Empty, control.Padding);
}

[WinFormsFact]
public void ScrollableControl_Scale_InvokeWithPaddingWithDockPadding_Success()
{
    using var control = new ScrollableControl
    {
        Padding = new Padding(1, 2, 3, 4)
    };
    control.Scale(10, 20);
    Assert.Equal(1, control.DockPadding.Left);
    Assert.Equal(2, control.DockPadding.Top);
    Assert.Equal(3, control.DockPadding.Right);
    Assert.Equal(4, control.DockPadding.Bottom);
    Assert.Equal(new Padding(1, 2, 3, 4), control.Padding);
}

[WinFormsFact]
public void ScrollableControl_Scale_InvokeWithPaddingWithoutDockPadding_Success()
{
    using var control = new ScrollableControl
    {
        Padding = new Padding(1, 2, 3, 4)
    };
    Assert.Equal(1, control.DockPadding.Left);
    Assert.Equal(2, control.DockPadding.Top);
    Assert.Equal(3, control.DockPadding.Right);
    Assert.Equal(4, control.DockPadding.Bottom);
    control.Scale(10, 20);
    Assert.Equal(1, control.DockPadding.Left);
    Assert.Equal(2, control.DockPadding.Top);
    Assert.Equal(3, control.DockPadding.Right);
    Assert.Equal(4, control.DockPadding.Bottom);
    Assert.Equal(new Padding(1, 2, 3, 4), control.Padding);
}
@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Feb 10, 2020

Its because its a struct, _owner.Padding.Scale(dx, dy); will return a copy of the owners Padding from the _owner.Padding property, so Scale(dx, dy) will operate on a copy of the original struct which will be immediately discarded.

Looking at reference source this seems to be broken in Desktop at as well, might have never been working. If you fix it you have to make sure any callers within WinForms can deal with the change (they might have had their own workarounds - or maybe the whole infrastructure never worked?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.