ScrollablePane resizes recursively if a single child window exceeds the pane

Issue #1085 resolved
Lukas Meindl
created an issue

Reproducibility: always

Reproduction steps:

  1. Create FrameWindow

  2. Add a ScrollablePane

  3. Attach window to scrollablepane with relative size larger than 1

  4. Resize FrameWindow

See: http://cegui.org.uk/forum/viewtopic.php?f=10&t=7043

Comments (33)

  1. Yaron Cohen-Tal

    @Lukas Meindl: Is there any logic in using a relative size in a child of a scrollable pane? If there is it all, I'd expact it to b taken from the content area. But currently it's taken from the window size of the scrollable pane, which doesn't seem to me like anything useful.

  2. Lukas Meindl reporter

    @Yaron Cohen-Tal from what I can tell no one had a clear logic in mind for relative sizes of children which is why we got this bug now, so I am open to any suggestions.

    I am split about which size should be used as base. On one hand, the scrollable pane's window size seems more logical and calculatable. On the other hand, the content area is where the actual placement happens.

    If we use the scrollable pane's window size we got a fixed size (this one does not scale with the content after all) so this allows to solve our issue here pretty easily (if taking the dynamically added/removed scrollbar's into account). I am not sure how a 120% sized child of a scrollable pane would be handled if the content area is used as basis: if "correctly" handling this, this sounds like an infinite loop to me. How would you solve this?

  3. Yaron Cohen-Tal

    Ok, so the bug here happens like this: When the scrollable pane is resized, the following happens:

    1. Scollbar visibility changes.
    2. Scrollable pane's child is resized.
    3. Auto size is true, therefore the content area changes.
    4. Back to 1, and we've got an infinite loop.

    Note that the size of the scrollable pane's child is computed relative to the inner rect of the scrollable pane, that is, without the scrollbars. Therefore, when we're back to (1), the inner rect changes, and therefore the child is resized (again) (in (2)), which leads to an infinite loop.

    So even if u think that, for the scrollable pane's child, using a size relative to the window size of the scrollable pane is logical, I don't c what it can mean in such a scenario. Imo it can only have any sense when auto size is false.

  4. Lukas Meindl reporter

    From this I take that your view is that having relative areas in general is fine. But what is not fine, according to your suggestions, is combining them with scrollbars being visible. Imo, this is questionable though because it only fixes this very case. It sounds more like a workaround than a fix. What we are trying to fix is a rare edge-cases that occurs in the current setup, on the other hand the 99,9% of cases do not trigger an infinite loop and can have the scrollbar automatically hidden or shown.

    If we would go your path this would break previously functioning behaviour, therefore it cannot go into 0.8.X. I would prefer a fix that can be done in 0.8.X without changing current behaviour (except for the broken case obviously) but this is extremely tricky. We would have to: - Know that current edge case is hit - React by showing the scrollbar

  5. Yaron Cohen-Tal

    What I'm saying is, I think the main scenario is wanting to use absolute dimensions for the child. Wanting to using relative dimensions relative to the content area seems to me like a less common scenario, and wanting to use relative dimensions relative to the window size of the scrollable pane seems to me like an even less common secanrio. If I had to implement the scrollable pane from scratch, I'd prolly support only absolute dimensions and perhaps relative dimensions relative to the content area, and perhaps sometime, with a low priority, add an option to make the relative dimensions relative to the window size of the scrollable pane.

    I don't think that in the scenario we're talking about (i.e. if u use the combination of ( relative area && auto size && ! (both scrollbars are set to b always visible) ) ) then the bug occours only 0.1% of the times, as u suggest. If it were so, then either "gams" (the bug reporter) was extremely unlucky, or he's a gifted QA who does voluntery work for CEGUI, or there are so many ppl who have used cegui with this very scenario and would report if they had a problem, and so statistically one of them would prolly be "gams". So, no, I don't think the bug is that rare, and I don't know if it even requires to use a relative size greater than 1 (as u mention in the "Reproduction steps:" above). Don't forget that when a user drags a window's frame to resize it, the window is in practice resized many times, with every tiny movement of the mouse. It's enough that one of those sizes causes the bug.

    To solve this bug u need to identify exactly when we get an infinite loop. It seems not to be instantaneous: it'll prolly require some work. I doubt that work is worth for a scenario that seems to me uncommon in the 1st place. I did take into account that throwing an exception is only feasable in the API incombatible branch. So what I suggest to do in the ABI/API compatible branches is give a warning (in the cegui log) for that scenario. This bug can remain open, but I'd change its priority to "minor" - I believe there are issues that r better worth our time.

    In the API incombatible branch, I suggest to change the behaviour so that a relative dimension is taken relative to the content area. Then we of course must forbid to use a relative dimension combined with auto size set to true (in such a case - throw an exception). I'd then perhaps add a low priority issue (or use this same issue) about adding a posibility to choose if a relative dimension is taken from the content area or from the window size of the scrollable pane.

    If u don't like that solution, I'd just keep it relative to the window size of the scrollable pane, and throw an exception in the problematic scenario. And if u don't like that either - I suggest to keep the behaviour the same as what I suggest to do in the ABI/API compatible branches - that is, only write a warning to the cegui log, until some1, sometimes, perhaps, fixes this bug.

  6. Lukas Meindl reporter

    Sure, let us throw a warning on v0-8 and v0, imo that is good enough. When do you wanna check to throw a warning though? Whenever a child is added to the scrollable pane that has relative dimensions? What if the child is added and THEN the size/position is changed to relative? ;) That's a bit much checking for my taste. This is of course just a technicality now, as we seem to agree a warning is enough.

    Regarding default branch: I don't know what we can do there best. I am okay with going for exceptions as you said. the exception could be thrown when the actual auto-sizing happens, which makes this as water-tight as possible.

  7. Yaron Cohen-Tal

    When do you wanna check to throw a warning though? Whenever a child is added to the scrollable pane that has relative dimensions? What if the child is added and THEN the size/position is changed to relative? ;)

    Yes I had thought of that problem. I need to think about it.

    Regarding default branch: I don't know what we can do there best. I am okay with going for exceptions as you said. the exception could be thrown when the actual auto-sizing happens, which makes this as water-tight as possible.

    Don't u agree that taking a relative dimension relative to the content area is more useful than the current behavior? (beside the fact it will make the bug impossible to happen)

  8. Lukas Meindl reporter

    How does it make the bug impossible to happen?

    The content is still auto resized when the scrollbar is shown/hidden isn't it? Sounds like the same loop can occur with slightly different position&size values if you ask me

  9. Yaron Cohen-Tal

    What I meant is If we take a relative dimension relative to the content area then there's no logic in using auto size, so a combination of a relative dimension with auto size will b forbidden (again, perhaps throw an exception).

  10. Lukas Meindl reporter

    Then it should be relative to that for sure. These changes can go into default branch. Not v0-8 though so there we would need to know when we want to throw the exception, have you come up with a solution for that?

  11. Yaron Cohen-Tal

    In the ABI/API compatible branches I couldn't come up with a solution. If we write a warning to the log when the cegui user changes something that brings to the problematic scenario then we have the problem u mentioned earlier (the settings may be problematic temporarily but then the cegui user changes them again and they become valid). If we write a warning to the log somewhere we update the widget (like in ScrollablePane::configureScrollbars) then the log may get swamped.. So I'll eventually prolly do nothing but add a comment in the code..

  12. Lukas Meindl reporter

    Why not at least throw a log warning if something is added as child that has problematic settings? This won't destroy anything and won't spam in most cases.

  13. Lukas Meindl reporter

    Is that better than not throwing any warning though? I am not sure. If users are bothered by the warning they can set the child up properly before adding it to the scrollablepane.

  14. Yaron Cohen-Tal

    Well, then u demand the cegui user to set settings in a constrained order in order to avoid a warning, not something I'd wanna do.

    And btw auto-size is a property of the whole scrollable pane, not of a child.

  15. Log in to comment