Problem with the FlexBox class


#1

Hi !

I’m working on an application that used FlexBox, and I might be wrong, but I think I just found an issue with it. The issue appears when the amount of FlexItem contained in the FlexBox is equal to the amount of row needed (for instance, 5 items taking 5 rows, 1 row per item), and the issue is that the last FlexItem added to the FlexBox isn’t resized, or placed at the right place.
I’ve done my digging, and here’s what I found :
All items are being added to ItemStates array, but not in the lineItems HeapBlocks. It’s because of the

if (++row >= numItems)
    break;

in the void initialiseItems() method, in the juice_FlexBox.cpp.

So, it causes the FlexItem to be always drawn to the 0,0 coordinates, with his initially given width and height, and being ignored during the performLayout.

I tried to make the simplest demonstration possible for it, and the closest to the demo, and here it is :

Main.cpp

/*
  ==============================================================================

    This file was auto-generated!

    It contains the basic startup code for a Juce application.

  ==============================================================================
*/

#include "../JuceLibraryCode/JuceHeader.h"
#include "MainComponent.h"


//==============================================================================
class FlexBoxIssueApplication  : public JUCEApplication
{
public:
    //==============================================================================
    FlexBoxIssueApplication() {}

    const String getApplicationName() override       { return ProjectInfo::projectName; }
    const String getApplicationVersion() override    { return ProjectInfo::versionString; }
    bool moreThanOneInstanceAllowed() override       { return true; }

    //==============================================================================
    void initialise (const String& commandLine) override
    {
        // This method is where you should put your application's initialisation code..

        mainWindow = new MainWindow (getApplicationName());
    }

    void shutdown() override
    {
        // Add your application's shutdown code here..

        mainWindow = nullptr; // (deletes our window)
    }

    //==============================================================================
    void systemRequestedQuit() override
    {
        // This is called when the app is being asked to quit: you can ignore this
        // request and let the app carry on running, or call quit() to allow the app to close.
        quit();
    }

    void anotherInstanceStarted (const String& commandLine) override
    {
        // When another instance of the app is launched while this one is running,
        // this method is invoked, and the commandLine parameter tells you what
        // the other instance's command-line arguments were.
    }

    //==============================================================================
    /*
        This class implements the desktop window that contains an instance of
        our MainContentComponent class.
    */
    class MainWindow    : public DocumentWindow
    {
    public:
        MainWindow (String name)  : DocumentWindow (name,
                                                    Colours::lightgrey,
                                                    DocumentWindow::allButtons)
        {
            setUsingNativeTitleBar (true);
            setContentOwned (new MainContentComponent(), true);
            setResizable(true, false);

            centreWithSize (getWidth(), getHeight());
            setVisible (true);
        }

        void closeButtonPressed() override
        {
            // This is called when the user tries to close this window. Here, we'll just
            // ask the app to quit when this happens, but you can change this to do
            // whatever you need.
            JUCEApplication::getInstance()->systemRequestedQuit();
        }

        /* Note: Be careful if you override any DocumentWindow methods - the base
           class uses a lot of them, so by overriding you might break its functionality.
           It's best to do all your work in your content component instead, but if
           you really have to override any DocumentWindow methods, make sure your
           subclass also calls the superclass's method.
        */

    private:
        JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (MainWindow)
    };

private:
    ScopedPointer<MainWindow> mainWindow;
};

//==============================================================================
// This macro generates the main() routine that launches the app.
START_JUCE_APPLICATION (FlexBoxIssueApplication)

MainComponent.h

#ifndef MAINCOMPONENT_H_INCLUDED
#define MAINCOMPONENT_H_INCLUDED

#include "../JuceLibraryCode/JuceHeader.h"


struct blankPanel   : public juce::Component
{
    blankPanel (juce::Colour col, FlexItem& item)  : colour (col), flexItem (item){
        
    }
    
    void paint (Graphics& g) override
    {
        auto r = getLocalBounds();
        
        g.setColour (colour);
        g.fillRect (r);
        
        g.setColour (Colours::black);
        g.drawFittedText ("w: " + String (r.getWidth()) + newLine + "h: " + String (r.getHeight()),
                          r.reduced (4), Justification::bottomRight, 2);
    }

    juce::Colour colour;
    FlexItem& flexItem;

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (blankPanel)
};

struct MainContentComponent : public juce::Component
{
public:
    MainContentComponent()
    {
        flexbox.flexWrap        = FlexBox::Wrap::wrap;
        flexbox.justifyContent  = FlexBox::JustifyContent::flexStart;
        flexbox.alignItems      = FlexBox::AlignItems::stretch;
        flexbox.alignContent    = FlexBox::AlignContent::stretch;
        flexbox.flexDirection   = FlexBox::Direction::row;
        
        addItem (Colours::orange);
        addItem (Colours::aqua);
        addItem (Colours::lightcoral);
        addItem (Colours::aquamarine);
        addItem (Colours::forestgreen);
        
        setSize (500, 600);
    }

    ~MainContentComponent()
    {
    }

    void addItem (Colour colour)
    {
        flexbox.items.add (FlexItem (400, 100).withFlex(1,1,1600));
        
        auto& flexItem = flexbox.items.getReference (flexbox.items.size() - 1);
        auto panel = new blankPanel (colour, flexItem);
        array.add(panel);
        flexItem.associatedComponent = panel;
        addAndMakeVisible (panel);
    }
    
    void paint (Graphics& g) override
    {
        g.fillAll (Colours::lightgrey);
    
        g.setColour (Colours::white);
        g.fillRect (getFlexBoxBounds());
    }

    void resized() override
    {
        flexbox.performLayout (getFlexBoxBounds());
    }

    Rectangle<float> getFlexBoxBounds() const
    {
        return getLocalBounds().toFloat();
    }

private:
    FlexBox flexbox;
    OwnedArray<blankPanel> array;
};

#endif

Do not hesitate to ask any questions !

Thank you in advance !

EDIT : Here’s a screenshot of what’s happening


Please test-drive our new FlexBox classes!
#2

Thank you for reporting this bug and also thank you for this really nice bug report!

This should now be fixed on develop. Let me know if this works for you. Sorry for the delay.


#3

Seems to work fine for me now, thanks :wink:
No problem for the delay !

I still have a suggestion, I was wondering if you could add the possibility of coding the itemBasis so it works with percentage. I don’t know if I’m really clear, so here’s an exemple : an item with a basis of 50% will always take half of the Flexbox width or height, and if the two next item are 25% and 25%, then the 3 of them will be an entire row or column, all the time (except if you put margin between them of course)
It’s a pretty useful feature included in the CSS Flexbox, and that would hep a lot I’m sure !