Automatic Layouting (horizontal, vertical, grid, ...)

[CrazyEddie]

What do you think about layoutAfter? Is it worth the hassle? It might speed things up for really long sequential layouts but it complicates things.

On the whole, I think keeping things as simple as possible is the better choice. Though if you want to leave it in, it's fine

I will need a new event on CEGUI::Window - EventMarginChanged. Should I split it to EventMarginTopChange, etc? Probably not worth the hassle. I am planning to define margin as 4 UDim, because URect doesn't really fit.

Yeah, I think one new event would be fine here, and four UDims would be ok for storing the margins.

I've been looking into the clipping issue. I've identified at least one definite issue, and some other things that look inconsistent. I've gotten to the point where I'm not especially happy with much of the stuff surrounding these calculations, and so I need to work through it from scratch to ensure that everything is as it should be, and in order to figure out precisely which combinations are causing the issues. None of this has anything to do with the new code; I've reproduced the issues in unmodified CEGUI code - so they have been lurking a while. I was hoping this would be a quick fix, but this is not the case - but on the positive side, at least this issue is found and can be addressed

CE.

[Kulik]

Hooray, issue was found I tried to play with the clipping, but didn't really find anything wrong I suggest adding something like getSizeForChildRelativeSizing() (extremely bad name, but you get the idea). getUnclippedRect() could be used for something other that child relative sizing and it could cause problems in the future.

So...
Got rid of layoutAfter, it looks much cleaner now
Got rid of LayoutParameters and added margin properties to CEGUI::Window
Cleaned vertical and horizontal container a bit

Created some initial grid code - demonstration here http://www.youtube.com/watch?v=PLOOcMq3VvQ - it's a bit messy, I will have to do a lot of cleaning, but it works and that's what matters. I got away with just mapping the grid to d_children so I don't use any other structure. When grid size is set, I populate the container with "dummy" DefaultWindows (and mark them as auto windows). Whenever I add new window to the layout container, it replaces a dummy window or other window that's in the grid cell that we are adding to. I have 2 auto ordering methods implemented - left to right and top to bottom + I made it easy to add more methods.

I currently use setGridDimensions(width, height) and it only works when there is nothing in the grid before it's called. I have to upgrade it to support shrinking and growing the grid and that's going to be a lot of headache inducing work but it's necessary for tools and convenient manipulation with the grid.

So before this can get into CEGUI I have to sort these out:
1) rearranging windows in sequential layouts - necessary for tools. I want to support insertion after and before specific window as well by windows pointer, name of the position in the grid (same as getChildByIdx)
2) setGridDimensions has to support shrinking growing whilst only removing/destroying what would later be outside the grid.
3) I added margins to CEGUI::Window, but I haven't yet figured how to add it to CEGUIWindowsProperties.h (4 properties vs 1 property?)

The patch in it's current state (messy ) - http://harcov.czenet.com/kulik/CEGUI/layouting2.patch

[CrazyEddie]

Ok, I have committed a fix for the clipping issue. This partly consists of new functions to allow separation of the clipping areas from the areas used for positioning and sizing child content, as well as cleaning up and fixing some existing things a little bit.

For the work you're doing here, it does mean that some modifications are required (sorry!). Basically, the existing LayoutContainer::getUnclippedInnerRect_impl function is changed to simply fetch the parent area, like so:

Code: Select all

Rect LayoutContainer::getUnclippedInnerRect_impl(void) const
{
    return d_parent ?
        d_parent->getUnclippedInnerRect() :
        Window::getUnclippedInnerRect_impl();
}

And a new virtual function override of Window::getClientChildWindowContentArea_impl is needed, which would look like this:

Code: Select all

Rect LayoutContainer::getClientChildWindowContentArea_impl() const
{
   if (!d_parent)
        return Window::getClientChildWindowContentArea_impl();
    else
        return Rect(getUnclippedOuterRect().getPosition(),
                    d_parent->getUnclippedInnerRect().getSize());
}

Or in other words. The code that used to be in getUnclippedInnerRect_impl is moved to getClientChildWindowContentArea_impl, and getUnclippedInnerRect_impl returns the area from the parent (similar updates were required in CEGUIScrolledContainer.cpp).

I did not try out the new patch from above yet, initially because fixing this clipping issue was priority one, and now because the patch will need the minor updates from above (sorry, again!).

I hope you find the fix working, and that it did not break anything else.

CE

[Kulik]

The fix worked! No more clipping bugs Thanks a lot.

[Kulik]

To avoid lots of repetitive code, I have created CEGUI::SequentialLayoutContainer that will implement all the reordering and manipulation of vertical and horizontal layouts. moveChildWindowToPosition(Window* wnd, size_t position) already works, now I must do addChildWindowToPosition, removeChildWindowFromPosition, moveChildWindowForward, moveChildWindowBackward and all the String vs Window* variants.

I will do the same for Grid but the method will be moveChildWindowToPosition(Window* wnd, size_t gridX, size_t gridY)... Also moveChildWindowUp, Left, Right and Down might be useful especially for tools.

I've got some hit box bugs with FrameWindow not wanting to be sized smaller than it's min size, but that's probably understandable.
Why does FrameWindow not alter it's size when it's rolled up? Is there any reason for that or just that it isn't necessary? If it did, I could just alter it's looknfeel and I can get my collapsible box for free

[Jamarr]

I have not looked at this code, but I am wondering how these layouts are going to be constrained? Are you constraining their size to their parents min/max size? Also, it looks like the scrollable-window with grid-layout approach could be used to (finally) replace the current MCL with a window-based implementation.

Also, with regards to margin-properties, why not allow optional properties? If a window has no margin-left/top/right/bottom property then just interpret that has having 0 margins. And so these windows would not require any additional memory for properties they are not using (or are implying a default value for).

[Kulik]

OK, I have finished the patch and need some code review I tried to stick to CEGUI's coding style but there may be some deviations, habits are hard to break. I made it really easy to add new sequential layouts if somebody wanted them - something like SoupLayoutContainer would probably be nice, it would fit Windows as left and as top as possible without overlapping other window.

Even changing grid's dimensions works with stuff already inside the grid. It gets remapped Plus lots of new rearranging methods added to grid and sequential.

What I am not sure about:
const float absWidth = getChildWindowContentArea().getWidth();
const float absHeight = getChildWindowContentArea().getHeight();
if (colSizes[x].asAbsolute(absWidth) < size.d_x.asAbsolute(absWidth))
...

This could compares UDims and takes absWidth and absHeight to convert them to absolutes, is this right? How do we trigger relayout when ChildWindowContentArea().getXXX() changes? Currently it works automatically, but I want to make sure it will continue to do so

What isn't ready:
Margin properties. I don't know how to do this cleanly. Should I create static Margin PropertyHelper::stringToMargin(const String& str) or void PropertyHelper::stringToMargin(const String& str, UDim& top, UDim& left, UDim& bottom, UDim& right); This should be a quick thing to do though...

TODO?:
Events like LayoutingStarted, LayoutingEnded? Should be quick as well

Here is the patch: http://harcov.czenet.com/kulik/CEGUI/layouting3.patch

@Jamarr: I am not constraining them. They use their parent's size to convert child windows relative sizes to absolute. When layouting ends, I set layout container's size to the overall size of all the widgets inside including margins. As for the optional property, yes I guess it will be possible to do it this way. But it would slow layouting down a bit... GridLayoutContainer maps to 1 dimensional vector, so resizing is a bit costly at the moment. For MCL it would probably require some other lightweight structure that would allow faster row addding/removing.

[Jamarr]

@Jamarr: I am not constraining them. They use their parent's size to convert child windows relative sizes to absolute. When layouting ends, I set layout container's size to the overall size of all the widgets inside including margins.

That makes sense. So then it basically retains the existing CEGUI behavior.

As for the optional property, yes I guess it will be possible to do it this way. But it would slow layouting down a bit...

Why would it slow it down? You have to do a lookup to fetch the property-value either way. Adding a single conditional would not be noticeable, and in cases where the margin is not used you do not have to fetch the value from the property nor do you have to convert it from a string to a numeric. So if anything, it would be faster and more memory efficient for windows without margins, and unnoticeable for windows with them.

[Kulik]

Why would it slow it down? You have to do a lookup to fetch the property-value either way. Adding a single conditional would not be noticeable, and in cases where the margin is not used you do not have to fetch the value from the property nor do you have to convert it from a string to a numeric. So if anything, it would be faster and more memory efficient for windows without margins, and unnoticeable for windows with them.

It's basically window->getLeftMargin() vs PropertyHelper::stringToUDim(window->getProperty("LeftMargin")). The left approach returns a const reference, the right one has to lookup a property by string, return it as a string, then create UDim and place converted values into it.

But it's debatable whether it would slow things down marginally or not, it probably wouldn't be noticeable...

[CrazyEddie]

From what I saw previously, the coding style is pretty close - I generally run contributed code through a code beautifier and then hand tweak for line length anyway - but having a uniform style is good for the initial inspection

The base value used for UDIm conversion should be the pixel width/height of the appropriate area (client/non-client) on the parent. In this case, since getChildWIndowContentArea for LayoutContainer fetches the parent area, it should be correct (I think, sometimes I confuse myself). The layout trigger should continue to work because it's based on the resize event (right?)

With regards to the margin properties... You could go for individual UDim properties for each edge, if you think it would be better or more useful to have a single property (I do!), what would the property value string look like?

Events are possible, there's always an overhead to pay when firing them, so it's good to consider how and when those events would be useful.

With regards to margin properties on Windows taking more space and performance of accessing them etc... The way I see it, the margins are stored locally in Window as UDims with get/set functions, so you're almost never paying the property lookup overhead - usually only when loading a layout. The property objects themselves would be static and contain no state, so they're shared for all windows and take up no space other than the entry in the std::map (PropertySet), I'm not sure how much of an overhead we could / should consider that (there's a whole other discussion right there, actually - one I'm happy to have elsewhere, since 99% of the properties registered on each window will never be accessed; there may be ways we can eliminate that waste in the future).

I've had a cursory glance over the text of the patch - it looks good! I'll give it a test-drive tomorrow. Thanks for your efforts with this, maybe we can get 0.7.2 out sometime soon!!

CE.

[Kulik]

OK, I am still thinking whether to introduce new type UMargin that would basically be the same thing as URect but with edges instead of points. Having URect as margin would probably confuse people. This would mean having UMargin d_margin inside CEGUI::Window and just setMargin getMargin. Plus only one property and I would add PropertyHelper::convertStringToUMargin(..).

I am subscribed to child windows resize but not for layout container resize. Should I subscribe to layout container's parent Resize?

Plus Jamarr suggested Grid layout could be used for new MCL, that is true but it would probably need few tweaks. Resizing the grid now is costly, because everything simply maps to d_children and d_children has to be constructed from scratch each resize, I could probably get around this by implementing a special case where only height is increased, it would be much faster, but I still don't like the solution. If I did some other lightweight struct independent of d_children, I could really speed resizing up and then it could be used for the new MCL. Plus I could get rid of the dummies then. Right now, every addRow call would take a lot of time. I will do some research first though... Plus MCL sizes all of it's elements in one row the same, grid layout container doesn't do that, it simply pushes the next row past the biggest window in previous row.

[Jamarr]

With regards to margin properties on Windows taking more space and performance of accessing them etc... The way I see it, the margins are stored locally in Window as UDims with get/set functions, so you're almost never paying the property lookup overhead - usually only when loading a layout. The property objects themselves would be static and contain no state, so they're shared for all windows and take up no space other than the entry in the std::map (PropertySet), I'm not sure how much of an overhead we could / should consider that (there's a whole other discussion right there, actually - one I'm happy to have elsewhere, since 99% of the properties registered on each window will never be accessed; there may be ways we can eliminate that waste in the future

Right. I forgot that properties are linked to member variables and accessed directly in code. So it would be slower, and unnecessary, to check for the existence of the property first.

[CrazyEddie]

Ok. I have had a chance to look over the 3rd patch earlier... On the whole I think everything is fine, there's a couple of minor things...

The files for the LayoutContainer and SequentialLayoutContainer should ideally be under the elements subdirectory - this will happen by magic anyway once I commit the patch, so don't worry about it now!
We need to add properties for the grid container grid dimensions (suggest changing the parameter type to CEGUI::Size to make use of the existing property helper for that, and also for the auto ordering option. Once those are available, it may be necessary to play around with writing and loading some xml to make sure that what comes out in XML produces the same results when read back in.

To come back to the margin properties for Window, I agree that the way URect is set up it's not logical for the margin type. It would be good to come up with a name other than UMargin for the new type, since it might be useful as a general type for other things where left, right, top, bottom is more logical the the min/max thing in URect. The only thing is, I can't come up with a better name! (UBox?) Having said this, the four UDims are still fine with me, I'll let you make the final decision here

Should I subscribe to layout container's parent Resize?

No, I don't think so. If the child content has a relatively sized component, you hear about it anyway via the existing subscription, and if it's pure absolute sized, it doesn't matter either way (because the size will never change, unless changed explicitly, and in that case the regular event will fire).

I'm not sure if I've missed any points raised in any of the above posts, if I have feel free to list them!

CE.

[Kulik]

Well the one thing that is still undecided is whether it's OK to have the grid mapped to d_children. If we introduced some lightweight structure next to d_children, no DefaultWindow dummies would be required, plus it would be much faster to resize the grid. The grid was originally planned to be something that's sized only once, then elements are added and then it just layouts. But Jamarr got me thinking that it could be used for MCL in future and MCL requires something faster, because you could be adding and removing rows frequently. I know something like setting the grid larger than it should be might do the job as well, but I am still not satisfied with my awkward grid to idx mapping solution Perhaps a list of rows would be more suitable for this. I'll think of something more generic though, because list of rows would still be slow in case of width change.

I think introducing something like UBox would be better than 4 UDims, everything would be cleaner and more conscise. However UBox isn't the right name probably, but I can't think of anything better either UMargin sounds sketchy as well...

About the elements directory, I only placed concrete classes there, abstract classes are in cegui/include and cegui/src, LayoutContainer and SequentialLayoutContainer are both abstract. But now I see no other CEGUI::Window derived classes are there, so you're right, I missed that

The problem with CEGUI::Size is, that it's 2 floats. I need two unsigned integers. But I guess I could probably round them or something...

I've encountered a weird glitch that even though GridLayout set it's size correctly, ScrollablePane didn't update the scrollbar so you couldn't scroll all the way to the bottom. Can't reproduce it now though Really weird...

Yeah and why aren't CEGUI::FrameWindows changing their size when they rollup? I know it's not necessary for rendering or anything, but with layouts it would make sense (even though I must confess having frame windows inside frame windows isn't probably the best use case of them). I could create CollapsibleBoxes with just looknfeel.

[Jamarr]

If you do not see an obvious/easy implementation for making the grid MCL-capable then, as you implied perhaps that is best left for the "future". Having layout containers alone will be awesome, and you should focus on that aspect first and foremost. After we have a solid, implemented layout system then perhaps improving the speed of the grid-layout and re-implementing the MCL in terms of the grid could be the /next/ step.

As far as the best term to define a rectangular area having left, top, right, bottom properties - I cannot think of a term more fitting than "box" Also, as an incentive, the W3C calls it's margin, padding, border, and content areas the "Box Model."