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

Layout bugs due to incorrect component measurements #5

Open
Videogamers0 opened this issue Apr 22, 2023 · 0 comments
Open

Layout bugs due to incorrect component measurements #5

Videogamers0 opened this issue Apr 22, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@Videogamers0
Copy link
Owner

Videogamers0 commented Apr 22, 2023

There are currently 2 bugs related to how MGComponent<TElementType> instances are measured:


1. The component measuring logic is not robust enough to account for cases where multiple components of the same control must share their measurement with the control's Content

Suppose you created a control called MGHeaderFooterPresenter which was structured something like this (but using MGComponent<TElementType> to apply the layout instead of actually using an MGDockPanel):

<DockPanel>
    <ContentPresenter Name="HeaderPresenter" Dock="Top" />
    <ContentPresenter Name="FooterPresenter" Dock="Bottom" />
    <ContentPresenter Name="ContentPresenter" />
</DockPanel>

If the HeaderPresenter and FooterPresenter are both components of the control, the total width of the control would need to be:

int Width = Max(HeaderWidth, FooterWidth, ContentWidth);

MGComponent<TElementType> allows a component to share its width with the control's Content, but does NOT allow it to share width with other components inside that same control. So you end up with an incorrect measurement that's something like this:
int Width = Max(Sum(components that share width with content), ContentWidth);

Currently, the only control that this seems to affect is MGListBox which has 2 components, 1 for the title header, and 1 for the rows of content. MGListBox intentionally does not measure the title's width when measuring itself in order to avoid this bug (which in turn means the MGListBox might not be wide enough in cases where the title would normally be wider than the rows)


2. The component measuring logic incorrectly handles Padding

If the component does not use the owner's Padding for its measurements (MGComponentBase.UsesOwnersPadding = false, such as for the tab headers of an MGTabControl where the Padding of the MGTabControl only affects the content region), then the measurement logic may incorrectly add the Padding to the end result in cases where it shouldn't.

Suppose we're measuring an MGTabControl where the tab headers are docked to the top. If the tab headers are 100px wide, the tab content is 50px wide, and the Padding is 20px, the total width should be:

int Width = Max(100, 50+20);

But the Padding is being automatically applied to the result before any components are measured, so we currently end up with:

int Padding = 20;
int UnpaddedWidth = Max(100, 50);
int TotalWidth = Padding + UnpaddedWidth;

Meaning we get an incorrect result in cases where the component shares width with the content, AND the component is wider than the content, AND the component does not respect the control's Padding.

To demonstrate this issue, create a window with the following layout:

<Window xmlns="clr-namespace:MGUI.Core.UI.XAML;assembly=MGUI.Core"
        SizeToContent="WidthAndHeight" WindowStyle="None">
    <TabControl TabHeaderPosition="Left" Padding="20">
        <TabItem Header="Tab Header">
            <Rectangle Fill="Red" Height="10" Width="10" />
        </TabItem>
    </TabControl>
</Window>

Result:

component bug

The TabControl ends up being 42px too tall. (40px of the Padding's Top+Bottom, and 2px of the BorderThickness Top+Bottom)


To fix these bugs, the following methods should be refactored:

MGElement.MeasureSelf(Size, Thickness)
MGElement.UpdateLayout(Rectangle)
MGComponentBase.Arrange(Thickness)

After fixing these bugs, we should also modify MGListBox<TITemType>'s constructor to set the title component's ConsumesLeftSpace to true since listboxes currently don't account for the title bar's width when measuring their entire width:

Before bugfix:

this.TitleComponent = new(TitleBorder, true, false, false, true, false, false, false,
    (AvailableBounds, ComponentSize) => ApplyAlignment(AvailableBounds, HorizontalAlignment.Stretch, VerticalAlignment.Top, ComponentSize.Size));

After bugfix:

this.TitleComponent = new(TitleBorder, true, false, true, true, false, false, false,
    (AvailableBounds, ComponentSize) => ApplyAlignment(AvailableBounds, HorizontalAlignment.Stretch, VerticalAlignment.Top, ComponentSize.Size));
@Videogamers0 Videogamers0 added the bug Something isn't working label Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant