Re: [dev] [st] [PATCH] fix screen clearing in tresize

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Mon, 18 Aug 2014 18:47:50 +0200

> If defaultbg != 0, the alt screen is cleared with the wrong
> color. The bug is fixed by avoiding to load a cursor that
> might be 0 initialized.

This patch also change the loop, and this change is not
related to the bug you want to fix. Please send a patch
for every issue.


> - orig = term.line;

Good point, this variable was a stupid way of making a
loop of two iterations.

> - c = term.c;

This sentence was here to avoid lost the current position of
the cursor, because tcursor(CURSOR_LOAD) changes the position
and we don't want this movement here. The call to tcursor is
only because we can have different cursors attributes in each
screen, and these attributes are used for erasing.


> - do {
> + for(i = 1; i <= 2; i++) {

I would prefer a loop like:

        for(i = 0; i < 2; i++)

because is more idiomatic and i is not used in the loop
so they are equivalent.

> - tcursor(CURSOR_LOAD);
> - } while(orig != term.line);
> - term.c = c;
> + }

I don't see why removing term.c = c fix any problem, because
term.c had a correct value, because in tnew:

        tnew(int col, int row) {
                term = (Term){ .c = { .attr =
                                        { .fg = defaultfg,
                                          .bg = defaultbg } } };
                tresize(col, row);
                term.numlock = 1;

                treset();
        }

I think the problem is that the saved attributes of cursors
are wrong at the beginning of the program. tnew is called
at the beginning of main:

        setlocale(LC_CTYPE, "");
        XSetLocaleModifiers("");
        tnew(cols? cols : 1, rows? rows : 1);
        xinit();
        selinit();
        run();

        return 0;
}


and wa can see that in tcursor:

        tcursor(int mode) {
                static TCursor c[2];
                bool alt = IS_SET(MODE_ALTSCREEN);

                if(mode == CURSOR_SAVE) {
                        c[alt] = term.c;
                } else if(mode == CURSOR_LOAD) {
                        term.c = c[alt];
                        tmoveto(c[alt].x, c[alt].y);
                }
        }

c[0] and c[1] are not initialized here, and if default fg and
bg are different of 0, then the first time that we call
tcursor(CURSOR_LOAD) the attributes of term.c are wrong. and
it means that in the loop of tresize we are using wrong values:

        do {
                if(mincol < col && 0 < minrow) {
                        tclearregion(mincol, 0, col - 1, minrow - 1);
                }
                if(0 < col && minrow < row) {
                        tclearregion(0, minrow, col - 1, row - 1);
                }
                tswapscreen();
                tcursor(CURSOR_LOAD);
        }

The problem is fixed for next calls because the call to reset at
the end of tnew has:

        tclearregion(0, 0, term.col-1, term.row-1);
        tmoveto(0, 0);
        tcursor(CURSOR_SAVE);

But only for main screen, and it means that the saved cursor
attributes of alternate screen are still wrong.

So, a correct patch to fix this problem should set correct values
on the declaration of c[2] in tcursor.


Regards,

-- 
Roberto E. Vargas Caballero
Received on Mon Aug 18 2014 - 18:47:50 CEST

This archive was generated by hypermail 2.3.0 : Mon Aug 18 2014 - 19:00:04 CEST