Bug on video.cpp compute_rects

[tanoxyz]

I was tweaking the source and making the window resizable I noticed that the function computer_rects fails at odd dimensions generating wrong behaviors and crashes.

Here:

  int dw=src->w*2-dst->w;
  /* the src width is too big */
  if (dw>0)
  {
    src->w-=dw/2;
    src->x+=dw/4;

    dst->x=0;
    dst->w=vid->w;
  }

And here:

  int dh=src->h*2-dst->h;
  /* the src height is too big */
  if (dh>0)
  {
    src->h-=dh/2;
    src->y+=dh/4;

    dst->y=0;
    dst->h=vid->h;
  }

To solve this I added the modulus on the new src dimension calculation src->w-=dw/2+dw%2; and src->h-=dh/2+%2;.

I hope it can be helpful.

[ColinPitrat]

Thanks.

This method lacks some documentation and testing, I'll try to improve this.

[ColinPitrat]

I'd be interested by your findings into trying to make the window resizable.

[ColinPitrat]

I added tests for what I thought was the problem (rect becoming bigger than the surface they refer too in some cases) but the test is actually passing. Do you have more info about the crash?

Could you provide me the fully patch of your changes to the code so that I can try to reproduce?

[tanoxyz]

In reality isn't a bug, works correctly if the window isn't resizable.

If you have a screen with odd dimensions, if you do this src->w-=dw/2; you may end with a scr->w larger than the real screen causing segmentation faults.

dw=INTERNAL_SCREEN_WIDTH * SCALE - REAL_SCREEN_WIDTH;
INTERNAL_SCREEN_WIDTH -= dw/2;

If REAL_SCREEN_WIDTH is odd the, dw will be odd causing truncation making the INTERNAL_SCREEN_WIDTH 1 pixel width more.

I think that my solution also was dumb, if the internal screen is bigger than the real screen simply:

INTERNAL_SCREEN_WIDTH = REAL_SCREEN_WIDTH;

I got a resizable window for the tvx2 rendering that automatically changes the scale (always integer (x1, x2, x3, x4...)) for the more near to the actual window size.

I messed up the whole project but if you have interest on it, When I will have enough time, I can clean it and open a pull request.

[ColinPitrat]

I'd be interested in it yes. This bug may have been why the window are not resizable. It really looks like the code was made to support it and then a hack that forces it to be 2*CPC_VISIBLE_SCR_WIDTH/HEIGHT has been added because it was not working properly.

Your description of the problem matches my initial understanding, but then I'd expect my test to fail:

     void ExpectRectInSurface(SDL_Rect *r, SDL_Surface *s) {
       EXPECT_LE(r->x + r->w, s->w);
       EXPECT_LE(r->y + r->h, s->h);
     }

For the case BiggerPub with an odd offset.

[tanoxyz]

I think this will trigger the error.

pub = CreateSurface(CPC_VISIBLE_SCR_WIDTH, CPC_VISIBLE_SCR_HEIGHT);
vid = CreateSurface(13, 13);
Surface src, dst;
compute_rects(&src, &dst);
assert(src.w *2<= dst.w);
assert(src.h*2 <= dst.h);

[ColinPitrat]

Ah yes indeed, the issue is between src & dst rectangles, not between the rectangle and the surface. This makes sense and I do manage to reproduce the error.

[ColinPitrat]

Extended the test and fixed the code (by just incrementing dw or dh when they are >0).