Grid::performLayout can change centred child Component dimensions

General JUCE discussion
1

I’m migrating some code from JUCE 6 to 7 and have hit a change in behaviour.

I’m using Grid::performLayout to centre a component of even-numbered dimensions inside a component with odd-numbered dimensions.

Until this JUCE 7.0.3 commit, though the centred bounds (left, right, top & bottom) fall between integers they all would have been rounded in the same direction, preserving the size of the child component - but with this commit the dimensions of the child component (if previously an integer) will be enlarged.

According to the commit message, this has been done to preserve the behaviour of very small controls, but it has unfortunate side effects for us.

Would it be possible to opt-out of this behaviour? Preserving the sizes of components by default would seem to be a desirable property where possible even when their centred position needs to be rounded to the nearest pixel.

Here’s some code to illustrate the problem:

    juce::Component myChild;
    myChild.setBounds (100, 100, 100, 100);

    juce::GridItem item {myChild};
    item.alignSelf = juce::GridItem::AlignSelf::center;
    item.justifySelf = juce::GridItem::JustifySelf::center;
    item.width = myChild.getWidth();
    item.height = myChild.getHeight();

    juce::Grid grid;
    grid.templateColumns = {juce::Grid::Fr{1}};
    grid.templateRows = {juce::Grid::Fr{1}};
    grid.items = {item};

    grid.performLayout ({0, 0, 200, 200});

    // these will succeed...
    jassert(myChild.getWidth() == 100);
    jassert(myChild.getHeight() == 100);

    grid.performLayout ({0, 0, 199, 199});

    // ... but these will fail
    jassert(myChild.getWidth() == 100);
    jassert(myChild.getHeight() == 100);

Any advice you can offer would be appreciated!

Steve.

2

can’t you set minWidth/maxWidth to be the same as width ?

3

Great suggestion, thanks! I just gave it a try and it seems to have no effect though. This is the code now in case I’m missing something:

    juce::Component myChild;
    myChild.setBounds (100, 100, 100, 100);

    juce::GridItem item {myChild};
    item.alignSelf = juce::GridItem::AlignSelf::center;
    item.justifySelf = juce::GridItem::JustifySelf::center;
    item.width = myChild.getWidth();
    item.height = myChild.getHeight();
    item.minWidth = myChild.getWidth();
    item.minHeight = myChild.getHeight();
    item.maxWidth = myChild.getWidth();
    item.maxHeight = myChild.getHeight();

    juce::Grid grid;
    grid.templateColumns = {juce::Grid::Fr{1}};
    grid.templateRows = {juce::Grid::Fr{1}};
    grid.items = {item};

    grid.performLayout ({0, 0, 200, 200});

    // these will succeed...
    jassert(myChild.getWidth() == 100);
    jassert(myChild.getHeight() == 100);

    grid.performLayout ({0, 0, 199, 199});

    // ... but these will fail
    jassert(myChild.getWidth() == 100);
    jassert(myChild.getHeight() == 100);
4

Set justifyContents and alignItems of the grid itself to center and not stretch ?

5

Made sure nothing was set to stretch as follows:

    grid.alignItems = juce::Grid::AlignItems::center;
    grid.alignContent = juce::Grid::AlignContent::center;
    grid.justifyItems = juce::Grid::JustifyItems::center;
    grid.justifyContent = juce::Grid::JustifyContent::center;

but the dimensions of the child still end up at 101.

6

Maybe a bug but i m not proficient with Grid to say for sure
I would try FlexBox see if the same issue arises

7

Thanks for your help Olivier. Unfortunately we have existing products relying on the our use of Grid here, so it’s not something we can swap out easily. Hopefully someone on the JUCE team will take pity on us and take a look!

8

@attila I think you were the author of this change, could you comment?

9

I’m looking into a solution right now that will maintain absolutely specified Grid item sizes. I’ll update the thread as the work progresses.

10

It’s reasonable to expect that Component sizes would be preserved, also GridItem’s with an absolute Px size should also be correctly sized/rounded.

It seems a bit more involved change is needed to ensure this and avoid regressions. I think I have what would be reasonable behaviour, but given the large number of possible corner cases, could you take a look please to see if this approach produces any artifacts with your UI?

I’m sharing a patch relative to our current develop branch.

grid1px.patch (15.2 KB)

11

Thanks Attila, I’ll try this and let you know how it looks here.

12

Hi @attila, I’ve tried this out and it seems to resolve the scaling issue I noted above, thanks! :slight_smile:

However, I noticed some controls on our UI changed positions compared to JUCE 7.0.2, so I ran some tests with combinations of even- and odd-sized grid and child to see what was going on. I’ve included my test code below which is commented to show the behaviour in JUCE 7.0.2 (before the commit which changed the behaviour), JUCE 7.0.3 (after that same commit), and develop branch with the latest patch testing specifically for cases where an float position could round up or down to the nearest integer.

In summary, the patch seems to ensure the child size is maintained in all cases (which is great), but is different to JUCE 7.0.2 which did not preserve the size of an odd-sized child correctly in an even-sized grid.

I think now the sizes are maintained in all cases we can tweak positions on our UI to restore it to its JUCE 7.0.2 state, but I just wanted to flag this in case you feel further changes are warranted before committing this.

    auto createGridItem = [] (auto& child)
    {
        juce::GridItem item {child};
        item.alignSelf = juce::GridItem::AlignSelf::center;
        item.justifySelf = juce::GridItem::JustifySelf::center;
        item.width = float (child.getWidth());
        item.height = float (child.getHeight());
        item.minWidth = float (child.getWidth());
        item.minHeight = float (child.getHeight());
        item.maxWidth = float (child.getWidth());
        item.maxHeight = float (child.getHeight());
        return item;
    };

    auto createGrid = [] (auto& item)
    {
        juce::Grid grid;
        grid.templateColumns = {juce::Grid::Fr {1}};
        grid.templateRows = {juce::Grid::Fr {1}};
        grid.items = {item};
        grid.alignItems = juce::Grid::AlignItems::center;
        grid.alignContent = juce::Grid::AlignContent::center;
        grid.justifyItems = juce::Grid::JustifyItems::center;
        grid.justifyContent = juce::Grid::JustifyContent::center;
        return grid;
    };

    {
        juce::Component evenChild;
        evenChild.setBounds (0, 0, 8, 8);

        auto& child = evenChild;
        auto item = createGridItem (child);
        auto grid = createGrid (item);
        grid.performLayout ({0, 0, 16, 16});

        // JUCE 7.0.2 succeeds
        // JUCE 7.0.3 succeeds
        // JUCE develop + patch succeeds - same as JUCE 7.0.2
        jassert (child.getBoundsInParent() == juce::Rectangle<int> (4, 4, 8, 8));

        grid.performLayout ({0, 0, 15, 15});

        // JUCE 7.0.2 succeeds, returning (4, 4, 8, 8)
        // JUCE 7.0.3 fails, returning (3, 3, 9, 9)
        // JUCE develop + patch succeeds, returning (4, 4, 8, 8) - same as JUCE 7.0.2
        jassert (child.getBoundsInParent() == juce::Rectangle<int> (3, 3, 8, 8)
                 || child.getBoundsInParent() == juce::Rectangle<int> (4, 4, 8, 8));
    }

    {
        juce::Component oddChild;
        oddChild.setBounds (0, 0, 7, 7);

        auto& child = oddChild;
        auto item = createGridItem (child);
        auto grid = createGrid (item);
        grid.performLayout ({0, 0, 16, 16});

        // JUCE 7.0.2 fails, returning (4, 4, 8, 8)
        // JUCE 7.0.3 fails, returning (4, 4, 8, 8)
        // JUCE succeeds, returning (5, 5, 7, 7) - but this is a change from JUCE 7.0.2
        jassert (child.getBoundsInParent() == juce::Rectangle<int> (4, 4, 7, 7)
                 || child.getBoundsInParent() == juce::Rectangle<int> (5, 5, 7, 7));

        grid.performLayout ({0, 0, 15, 15});

        // JUCE 7.0.2 succeeds
        // JUCE 7.0.3 succeeds
        // JUCE develop + patch succeeds - same as JUCE 7.0.2
        jassert (child.getBoundsInParent() == juce::Rectangle<int> (4, 4, 7, 7));
    }
1 Like
13

Thank you for testing the changes. It’s true that they can visibly change GUI layouts by changing Component sizes by a pixel, so they constitute breaking changes and will be marked as such.

They will soon land on develop, I’ll update the thread when that happens.

14

The change is now out on develop

15

Now my layout using juce::Grid is completely broken. Before, the absolute size was sometimes not 100% correct, but now the performLayout function exceeds the supplied rectangle, not only by one errant pixel but by a larger amount (roughly 16 pixels in my case).

I use this line to layout

grid.performLayout ( c.getLocalBounds ().reduced ( 16 ) );

If I remove the “reduced ( 16 )”, everything gets layout correctly. It seems the new layout algorithm doesn’t like it if the rectangle is not zero-based.

17

Thank you for reporting this. I’ll see if there’s unreasonable/unbrowserlike behaviour relating to this, and will rectify it.

18

The problem is definitely real and a fix is on it’s way. Given how small layout algorithm changes can visibly affect complex GUIs, I’m sharing the fix here until it finds its way to develop.

Besides the bug reported by reFX, there’s another necessary tweak, which might be affecting you.

grid1pxfix.patch (1.3 KB)

2 Likes
19

Thanks Attila. I’ll apply this locally and run some tests as soon as I can and will feed back whatever I find.

20

After a quick test it seems to be working again.

1 Like
21

Yes, it seems to work fine now.

1 Like