Menu

#7089 GUI: Crash in vector renderer when opening/closing console

*None
closed-fixed
None
5
2016-03-26
2016-03-26
No

When opening/closing the console repeatedly, it sometimes crashes. (See attached backtrace)

The issue happens in drawInteriorRoundedSquareAlg(). When it gets a small enough h, it goes through the the h>0 test.

h is then modified a few lines later:
h -= 2*Base::_strokeWidth;

Here it turns to 0 and causes the crash.

This can be easily guarded with another if(h<=0){return;} at that point.

However, I think there's another bug behind this:
strokeWidth is usually 0 when the console slides, so h shouldn't change by the above snipept.
In my crash (and tests where I slowed down the slide and printed the values), _strokeWidth was always 1. I'm not sure what caused it to change to 1.

1 Attachments

Discussion

  • Ori Avtalion

    Ori Avtalion - 2016-03-26
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -7,7 +7,7 @@
    
     Here it turns to 0 and causes the crash.
    
    -This can be easily guarded with another if(){return} at that point.
    +This can be easily guarded with another `if(h<=0){return;}` at that point.
    
     However, I think there's another bug behind this:
     strokeWidth is usually 0 when the console slides, so h shouldn't change by the above snipept.
    
     
  • Johannes Schickel

    I think it might be best to change the early check to assure w and h are bigger than the values subtracted at a later point. Or simply move the check further down below (if that doesn't cause other issues).

     
  • Ori Avtalion

    Ori Avtalion - 2016-03-26

    The problem is with the scroll bar in the console, which uses a stroke >= 0.

    To consistently get the error, type several "help" commands to get a scrollbar, before closing the console. You have to slow down the console sliding so it will always try to render the last few frames at the top of the screen:

    --- a/gui/console.cpp
    +++ b/gui/console.cpp
    @@ -227,7 +227,7 @@ void ConsoleDialog::handleTickle() {
            if (_slideMode != kNoSlideMode) {
                    const float tmp = (float)(g_system->getMillis() - _slideTime) / kConsoleSlideDownDuration;
                    if (_slideMode == kUpSlideMode) {
    -                       _y = (int)(_h * (0.0 - tmp));
    +                       _y -= 1;
                    } else {
                            _y = (int)(_h * (tmp - 1.0));
                    }
    

    So, do you think a good solution to move the if (w <= 0 || h <= 0) { return; }?
    I don't like the idea of performing the calculation inside the test itself - IMO it should be kept simple.

     
  • Ori Avtalion

    Ori Avtalion - 2016-03-26
    • status: open --> closed-fixed
    • assigned_to: Ori Avtalion
     
  • Ori Avtalion

    Ori Avtalion - 2016-03-26

    Fixed in 9b69b86f2972c93395ce6ee88f5566a31a05ce0d