Creating new Widgets by inheritance

Started by gupas, 27 July 2013, 10:33:09

gupas

Hello,

I defined an Interface class like this:
class Interface
    : public tgui::Grid
{
public:
    typedef tgui::SharedWidgetPtr<Interface> Ptr;

private:
    tgui::Picture::Ptr m_logo;
    std::vector<tgui::Label::Ptr> m_menuItems;

public:
    Interface();
    ~Interface();
};


I can create one instance of this using: Interface::Ptr interface(gui); in main().
I need to get the parent of the interface in its constructor. However, when the constructor runs, parent is not yet initialized...
So it is not possible to make Widgets constructor taking Parent in first parameter?

Interface::Interface()
    : tgui::Grid()
    , m_logo(*this)
{
    // Get parent size (normally)
    sf::Vector2f parentSize;
    tgui::Gui* guiParent = dynamic_cast<tgui::Gui*>(getParent());
    if (guiParent)
        parentSize = sf::Vector2f(guiParent->getWindow()->getSize());
    else if (getParent())
        parentSize = static_cast<tgui::ContainerWidget*>(getParent())->getSize();

    // Init logo
    m_logo->load("data/images/logo.png");

    // Init items
    tgui::Button::Ptr label(*this);
    label->setTextSize(30);
    label->setTextColor(sf::Color(255, 255, 255));

    tgui::Button::Ptr label2;

    label2 = label.clone();
    label2->setText("Introduction");
    add(label2);
    m_menuItems.push_back(label2);
    label2 = label.clone();
    label2->setText("Nouvelle partie");
    m_menuItems.push_back(label2);
    label2 = label.clone();
    label2->setText("Options");
    m_menuItems.push_back(label2);
    label2 = label.clone();
    label2->setText("Aide");
    m_menuItems.push_back(label2);
    label2 = label.clone();
    label2->setText("Crédits");
    m_menuItems.push_back(label2);
    label2 = label.clone();
    label2->setText("Quitter");
    m_menuItems.push_back(label2);

    // Add widget to the grid.
    addRow(200/*parentSize.y/3*/);
    addToRow(m_logo/*, tgui::Vector4u(0,0,0,0), Layout::Center*/);

    for (auto it = m_menuItems.begin(); it != m_menuItems.end(); ++it) {
        addRow(30);
        addToRow(*it);
    }
    updateWidgets(); // Not sure if it is needed but I tried with it and without it.
}



This should display a menu with a logo and below, a list of menu items. However, I have not any Label (menu item) displayed. Only the logo is shown, but it is not at the center of its tile. Do you have any idea of why?

Thanks for your answer.

texus

#1
Some widgets are created inside tgui without a parent, so they don't always need one. A widget can normally function fine without a parent.

You could add a "void initialize(tgui::Container *const container)" function to your class. It will be called when the widget is added to the container, which is right after your contructor finishes in this case.

Or you could override the "void setSize(float width, float height)" function. By the time you would call that function, the parent will already be initialized.

I'm not sure if you are going to add more code, but in your current code the 'parentSize' doesn't do much. The parameter in addRow was actually added for creating empty rows. Once you add a widget to the row, the height of the row will become the height of that widget.

Grid should be rewritten in the future too (so that you can just write something like "grid[2][3] = button"), but I already have so much to do that this won't come soon.

For the reason you only see your logo...
You are creating buttons instead of labels. Buttons require a background image and you thus need to use the load function before they will show up. If you only need text then you should use tgui::Label instead.

Edit: That's not really the issue here. There is no parent yet, so there is no font yet. Without a font, the labels can't draw anything.
You should move the creation of the labels to the initialize function.
void initialize(tgui::Container *const container)
{
    setGlobalFont(container->getGlobalFont());

    // Create the labels here
}


Another option is to create the labels like now, but set the font in the initialize method
Code (cpp) Select
void initialize(tgui::Container *const container)
{
    for all labels in m_menuItems:
        label->setTextFont(container->getGlobalFont());
}


Edit2:
You should also ask yourself if you really need an interface that inherits from Grid.
If you don't need this design then it could be better to place the tgui::Grid::Ptr as a member of Interface instead of using inheritance. That way you have your own constructor that can have any parameter (and thus has anything it needs) and can add the widgets to the grid without problems.

gupas

Quote from: texus on 27 July 2013, 10:54:53
You could add a "void initialize(tgui::Container *const container)" function to your class. It will be called when the widget is added to the container, which is right after your contructor finishes in this case.
Ok. I think I will try with this function.

Quote
Or you could override the "void setSize(float width, float height)" function. By the time you would call that function, the parent will already be initialized.
By the way, I see (in documentation) that Grid::setSize() "currently does nothing". Normal?

Quote
I'm not sure if you are going to add more code, but in your current code the 'parentSize' doesn't do much. The parameter in addRow was actually added for creating empty rows. Once you add a widget to the row, the height of the row will become the height of that widget.
Yes, some more code using parentSize should come. For logo, I wanted having it at the middle of the top third of the screen. So, finally, I should modify borders in addToRow() function instead of height in addRow().

Quote
Grid should be rewritten in the future too (so that you can just write something like "grid[2][3] = button"), but I already have so much to do that this won't come soon.
Depending on how I will have free time to work on my project, it is possible that I need this and start some work on it. I will inform you when (and if) I start. If I do this, I will make a Pull Request on github.

Quote
For the reason you only see your logo...
You are creating buttons instead of labels. Buttons require a background image and you thus need to use the load function before they will show up. If you only need text then you should use tgui::Label instead.

Edit: That's not really the issue here. There is no parent yet, so there is no font yet. Without a font, the labels can't draw anything.
You should move the creation of the labels to the initialize function.
Another option is to create the labels like now, but set the font in the initialize method
Oh yes, the font... (For Buttons, that was a try I made (but did not work) and I forget to replace by Label again).

Quote
Edit2:
You should also ask yourself if you really need an interface that inherits from Grid.
If you don't need this design then it could be better to place the tgui::Grid::Ptr as a member of Interface instead of using inheritance. That way you have your own constructor that can have any parameter (and thus has anything it needs) and can add the widgets to the grid without problems.
My first reason was that I don't need to create "accessors" functions to (un)focus, hide, show, (...) child Widgets. But yes, I will think about it.

Thanks

texus

QuoteBy the way, I see (in documentation) that Grid::setSize() "currently does nothing". Normal?
Yes, its normal. I recently decided that every widget requires a setSize and getSize function. Although getSize is defined for every widget, some widgets don't have a meaning for setSize.
What would happen if setSize was called on a Grid? Should it be cropped and all widgets outide this area be invisible? Or should the widgets inside the grid be resized so that together the widgets fill the size? To avoid making a decision right away, I just implemented an empty setSize method. Before this, there wasn't a setSize function at all, so its not like something is missing that was previously there.

QuoteDepending on how I will have free time to work on my project, it is possible that I need this and start some work on it. I will inform you when (and if) I start. If I do this, I will make a Pull Request on github.
Its actually quite a big change, its not just adding a brackets operator. The "grid[2][3] = button" was just a simplified version. In reality I was thinking more of "grid[2][3] = tgui::Grid::Cell(button, tgui::Borders(...), tgui::Layout::Centered)" and a lot of internal changes. So if you would make the changes that you need and contribute them then thats very kind, but I will probably make a lot of changes to it afterwards anyway.

And with my current definition of "not soon", it still means that I could be working on this next week.
I got plenty of time these days, but I have soo many things to write that I just don't know where to start. And because this change is a bit smaller than write a completely new form builder or rewriting the entire TextBox class, I might find more motivation to work on Grid instead.

gupas

For setSize(), I thought of a function which:
-resize to a minimal width or height if the specified width or height is too small. So all widgets will fit.
-when the size is bigger than the "minimal required size", internal margins of all cells are recalculated to fit the whole space of the Grid in the best way. This is very interesting and powerful.
I hope you understand what I want to say. To resume, IMHO, Grid::setSize() (and probably Grid in general) should be heavily inspired of Qt QGridLayout. This can lead to a very powerful widget in TGUI.

Moreover, currently, if the (auto-sized) grid need a size greater than its parent size to be correctly displayed, how this work?

QuoteIts actually quite a big change, its not just adding a brackets operator. The "grid[2][3] = button" was just a simplified version. In reality I was thinking more of "grid[2][3] = tgui::Grid::Cell(button, tgui::Borders(...), tgui::Layout::Centered)" and a lot of internal changes. So if you would make the changes that you need and contribute them then thats very kind, but I will probably make a lot of changes to it afterwards anyway.
You probably have considered many possibilities but, to be sure I understand, when you will do "grid[2][3] = ..." you will need to already have grid of a minimal size of (3, 4)?
Why not something like grid->addWidget(myWidget, 2, 3, tgui::Borders(...), tgui::Layout::Centered) (always inspired by Qt)? With this, you can automatically resize the grid to the number of cells needed. No?
But, yes, nonetheless, I understand that there are a lot of changes to do.

QuoteAnd with my current definition of "not soon", it still means that I could be working on this next week.
That's why, if I start this, I want to inform you. But I don't think I will do anything related to this until (at least) the end of the next week. So if you start something, I won't do nothing of course.

texus

I really should take a look at other gui's like qt and gtk, because so far I have never looked there for inspiration. I just did it my own way so far, or in a way suggested by users.

QuoteGrid::setSize() (and probably Grid in general) should be heavily inspired of Qt QGridLayout. This can lead to a very powerful widget in TGUI.
The current idea is to implement decent layouts in v0.7.
The only two big changes that should still be done in v0.6 are TextBox and the new form builder. Other than this there are just some small changes and then v0.6 will be finished. After that I will start working on layouts.

QuoteMoreover, currently, if the (auto-sized) grid need a size greater than its parent size to be correctly displayed, how this work?
Nothing special happens, it is just drawn as usually. It doesn't matter if a part is drawn outside of the screen. And child windows use clipping to avoid that widgets draw outsize of them.

QuoteYou probably have considered many possibilities but, to be sure I understand, when you will do "grid[2][3] = ..." you will need to already have grid of a minimal size of (3, 4)?
My idea was actually to automatically resize the grid if it wasn't big enough. I wanted you to be able to set grid[1][0] even before you set grid[0][0] so that you just don't have to care about how many rows or columns there are: grid will take care of it. If you had a picture in [ 0 ][ 1 ] then it would be placed on position (0, 0) until you put a widget in [ 0 ][ 0 ] and then the picture will just move. It just requires some extra code in the brackets operator.

gupas

Quote from: texus on 27 July 2013, 14:26:52
I really should take a look at other gui's like qt and gtk, because so far I have never looked there for inspiration. I just did it my own way so far, or in a way suggested by users.
It is a really impressive work to have done tgui without having already used any other GUI lib. Wow!

Quote
QuoteGrid::setSize() (and probably Grid in general) should be heavily inspired of Qt QGridLayout. This can lead to a very powerful widget in TGUI.
The current idea is to implement decent layouts in v0.7.
The only two big changes that should still be done in v0.6 are TextBox and the new form builder. Other than this there are just some small changes and then v0.6 will be finished. After that I will start working on layouts.
So I am waiting v0.7 ;) More seriously, for v0.6, Grid with a better way for adding widgets and a setSize function would be really great (and enough for this 0.6 version, IMHO).

Quote
QuoteMoreover, currently, if the (auto-sized) grid need a size greater than its parent size to be correctly displayed, how this work?
Nothing special happens, it is just drawn as usually. It doesn't matter if a part is drawn outside of the screen. And child windows use clipping to avoid that widgets draw outsize of them.
That is what I wanted to point out:
QuoteWhat would happen if setSize was called on a Grid? Should it be cropped and all widgets outide this area be invisible? Or should the widgets inside the grid be resized so that together the widgets fill the size?
Finally, in this case, it is cropped even if you don't have a setSize() function. So, I think, keep setSize() without implementation is useless because of this, and provide an implementation (following the behaviour I suggested in my last post for example) can be very interesting.
And if you really don't want a such function, you should at least provide a way to be able to choose for each grid if you want it to fit the whole parent or to be as the current grid.

Quote
QuoteYou probably have considered many possibilities but, to be sure I understand, when you will do "grid[2][3] = ..." you will need to already have grid of a minimal size of (3, 4)?
My idea was actually to automatically resize the grid if it wasn't big enough.
The problem of automatically resize the grid using "grid[2][3] = ..." is that if you try (only) to access to widget at (2,3), and if it doesn't exist you will create it (similar to the std::map [] operator). Or am I wrong?
That is why I proposed a function like grid->addWidget(myWidget, 2, 3, ...); So you can have a better control of this.

QuoteI wanted you to be able to set grid[1][0] even before you set grid[0][0] so that you just don't have to care about how many rows or columns there are: grid will take care of it. If you had a picture in [ 0 ][ 1 ] then it would be placed on position (0, 0) until you put a widget in [ 0 ][ 0 ] and then the picture will just move. It just requires some extra code in the brackets operator.
I am not sure you should move widgets when they are placed. You can want to have a widget at (0,1) and an other at (1,0). If you move the first at (0,0), you will not have the expected layout.

texus

Your idea for the setSize function is one hand better than clipping, but on the other hand, setSize(getSize()) will not give the same grid like you may expect. All borders will be made the same size while before you could have some widgets without borders and other widgets with big borders.

Anyway, I'm sure you'll find other widgets that don't stay identical on setSize(getSize()) so thats not an issue to me. Having an empty setSize function is even worse. So I'll try to implement your idea for setSize.

QuoteThe problem of automatically resize the grid using "grid[2][3] = ..." is that if you try (only) to access to widget at (2,3), and if it doesn't exist you will create it (similar to the std::map [] operator). Or am I wrong?
That is why I proposed a function like grid->addWidget(myWidget, 2, 3, ...); So you can have a better control of this.
It would indeed be similar to std::map's bracket operator.
In the end, I don't think it really matters. With the brackets operator, you will indeed create extra space which contains a nullptr. Program will crash if you try to use the nullptr. With the addWidget way, requesting a widget (either with [] operator or with getWidget function) would result in perhaps an exception being thrown.
So I see it like this. Both methods are fatal if you try to use a cell that didn't contain anything. You could catch the error but I don't see any usage for that. And if you really needed to check if some cell was empty then it was probably with the idea to add a widget there anyway if there isn't one. Which means it doesn't matter to reserve the extra space already when accessing a cell.
So I basically see those methods as equivalent and this is thus just a design issue. Some would prefer writing the brackets, others might prefer using the function. But I spend too many words on this already, and actually I don't even care which of the two methods is used.

QuoteI am not sure you should move widgets when they are placed. You can want to have a widget at (0,1) and an other at (1,0). If you move the first at (0,0), you will not have the expected layout.
The assumption in my example was that the grid was empty. Obviously when there are widgets in the first column (lets say with a max width of 50 and no borders), widgets added to the second column will have a left of 50.

gupas

Quote from: texus on 27 July 2013, 16:04:18
All borders will be made the same size while before you could have some widgets without borders and other widgets with big borders.
It was not the idea. If a widget have a margin of 0 and another a size of 30, if the size of the grid is changed (increased by 30 for example), they should have margin like 15 and 45 and not like 30 and 30.

Quote
So I basically see those methods as equivalent and this is thus just a design issue. Some would prefer writing the brackets, others might prefer using the function. But I spend too many words on this already, and actually I don't even care which of the two methods is used.
Right, finally both methods seem to be equivalent.

Quote
QuoteI am not sure you should move widgets when they are placed. You can want to have a widget at (0,1) and an other at (1,0). If you move the first at (0,0), you will not have the expected layout.
The assumption in my example was that the grid was empty. Obviously when there are widgets in the first column (lets say with a max width of 50 and no borders), widgets added to the second column will have a left of 50.
If you speak at a final (graphical) level you are right, but if you speak at the code (algorithm) level (that I assumed), you are wrong. If you add a widget to the second column (even if grid is empty), you should leave it at the second column (and the first column will have a width of 0 if there is no widget). Now if you add a widget to the first column, you can leave the widget of the second column untouched (in term of position in the grid), only the (x, y) pixels position will be changed.

texus

QuoteIt was not the idea. If a widget have a margin of 0 and another a size of 30, if the size of the grid is changed (increased by 30 for example), they should have margin like 15 and 45 and not like 30 and 30.
That is indeed better.

QuoteIf you speak at a final (graphical) level you are right, but if you speak at the code (algorithm) level (that I assumed), you are wrong. If you add a widget to the second column (even if grid is empty), you should leave it at the second column (and the first column will have a width of 0 if there is no widget). Now if you add a widget to the first column, you can leave the widget of the second column untouched (in term of position in the grid), only the (x, y) pixels position will be changed.
That was the idea, I was talking at a graphical level.

texus

If you want a setSize function then you'll have to write it yourself, I give up.

I did replace the addRow and addToRow with an addWidget function though. And I've also added a getWidget function.

gupas

I will have a look on the setSize function in the next days.

gupas

Now, it's done!  ;)
But yes, it was a bit long...

texus

Everything seems to be working great, thanks a lot!