Handle begy/begx and leny/lenx

[vysheng]

Currently using begy/begx or leny/lenx results in wrong behavior or segmentation faults.

I tested it on kitty and foot:

• asking notcurses to allocate a plane always results in the whole image being rendered
• supplying a properly sized plane results in SIGSEGV on foot and only the top of the image being rendered on kitty

The reason for this behavour is missing handling of these fields' values. I see two possible solutions to fix this behavior:

1. crop the image in functions like ncvisual_blit and leave everything else as it is
2. fix all functions, so that they check the values of corresponding fields

I would say, that the variant (1) is less intrusive, so I implemented it. No tests ever used non-zero values for begx/begy, so the tests weren't broken.

[dankamongmen]

taking a look at this, although the failure cases you describe seem like they would have been seen elsewhere. can you please provide some triggering code, so i can make it into a unit test?

[vysheng]

Ok, this is a simple program, that tries to render the middle of an image. There are two modes: "simple" to allocate a new plane by hand and "child" to ask notcurses to do it.

Observed behavior:

1. Kitty: in simple mode renders first lines of image (essentially ignoring begy field), in "child" mode shows the complete image
2. Foot: in simple mode works, but sometimes crashes, in "child" mode renders the whole image. I think, that it crashes, when the font is too small, but I am not sure.

This is my code:

#include <notcurses/notcurses.h>
#include <cmath>
#include <iostream>

void wait_sleep(struct notcurses *nc) {
  while (true) {
    struct timespec ts = {.tv_sec = 600, .tv_nsec = 0};
    struct ncinput ni;
    auto res = notcurses_get(nc, &ts, &ni);
    if (!res) {
      continue;
    }

    if (ni.evtype != NCTYPE_PRESS && ni.evtype != NCTYPE_REPEAT) {
      continue;
    }

    notcurses_stop(nc);
    exit(2);
  }
}

int main(int argc, char **argv) {
  if (argc != 3) {
    std::cerr << "notcurses-test <mode> <file>\n";
    return 3;
  }
  std::string mode = argv[1];
  struct notcurses_options curses_opts{.termtype = NULL,
                                       .loglevel = NCLOGLEVEL_WARNING,
                                       .margin_t = 0,
                                       .margin_r = 0,
                                       .margin_b = 0,
                                       .margin_l = 0,
                                       .flags = 0};
  auto nc = notcurses_init(&curses_opts, nullptr);

  auto baseplane = notcurses_stdplane(nc);
  struct ncvgeom geom;
  memset(&geom, 0, sizeof(geom));
  ncvisual_geom(nc, nullptr, nullptr, &geom);
  if (geom.cdimy <= 0 || geom.cdimx <= 0) {
    std::cerr << "failed to get pixel configuration\n";
    notcurses_stop(nc);
    return 2;
  }

  auto cdimy = geom.cdimy;
  auto cdimx = geom.cdimx;

  std::cerr << "dimy=" << cdimy << " dimx=" << cdimx;

  auto visual = ncvisual_from_file(argv[2]);
  if (!visual) {
    std::cerr << "failed to open image file\n";
    notcurses_stop(nc);
    return 2;
  }

  if (ncvisual_resize(visual, 20 * cdimy, 40 * cdimx) < 0) {
    std::cerr << "failed to resize image\n";
    notcurses_stop(nc);
    return 2;
  }

  if (mode == "simple") {
    struct ncplane_options plane_opts{.y = 0,
                                      .x = 0,
                                      .rows = 10,
                                      .cols = 40,
                                      .userptr = nullptr,
                                      .name = nullptr,
                                      .resizecb = nullptr,
                                      .flags = NCPLANE_OPTION_FIXED,
                                      .margin_b = 0,
                                      .margin_r = 0};
    auto tmp_plane = ncplane_create(baseplane, &plane_opts);
    if (!tmp_plane) {
      std::cerr << "failed to create image plane\n";
      notcurses_stop(nc);
      return 2;
    }
    struct ncvisual_options opts = {.n = tmp_plane,
                                    .scaling = NCSCALE_NONE,
                                    .y = 0,
                                    .x = 0,
                                    .begy = cdimy * 5,
                                    .begx = 0,
                                    .leny = cdimy * 10,
                                    .lenx = cdimx * 40,
                                    .blitter = NCBLIT_PIXEL,
                                    .flags = 0,
                                    .transcolor = 0,
                                    .pxoffy = 0,
                                    .pxoffx = 0};
    auto visual_plane = ncvisual_blit(nc, visual, &opts);
    if (!visual_plane) {
      notcurses_stop(nc);
      return 2;
    }

    notcurses_render(nc);
    wait_sleep(nc);
  } else if (mode == "full") {
    struct ncvisual_options opts = {.n = baseplane,
                                    .scaling = NCSCALE_NONE,
                                    .y = 0,
                                    .x = 0,
                                    .begy = 0,
                                    .begx = 0,
                                    .leny = 0,
                                    .lenx = 0,
                                    .blitter = NCBLIT_PIXEL,
                                    .flags = NCVISUAL_OPTION_CHILDPLANE,
                                    .transcolor = 0,
                                    .pxoffy = 0,
                                    .pxoffx = 0};
    auto visual_plane = ncvisual_blit(nc, visual, &opts);
    if (!visual_plane) {
      notcurses_stop(nc);
      return 2;
    }

    notcurses_render(nc);
    wait_sleep(nc);
  } else if (mode == "child") {
    struct ncvisual_options opts = {.n = baseplane,
                                    .scaling = NCSCALE_NONE,
                                    .y = 0,
                                    .x = 0,
                                    .begy = cdimy * 5,
                                    .begx = 0,
                                    .leny = cdimy * 10,
                                    .lenx = cdimx * 40,
                                    .blitter = NCBLIT_PIXEL,
                                    .flags = NCVISUAL_OPTION_CHILDPLANE,
                                    .transcolor = 0,
                                    .pxoffy = 0,
                                    .pxoffx = 0};
    auto visual_plane = ncvisual_blit(nc, visual, &opts);
    if (!visual_plane) {
      notcurses_stop(nc);
      return 2;
    }

    notcurses_render(nc);
    wait_sleep(nc);
  } else {
    std::cerr << "unknown mode " << mode;
    notcurses_stop(nc);
    return 3;
  }
}

[vysheng]

Also this is code, suitable for unit tests, that checks at least ncvisual_geom. Rcelly/rcellx are incorrect, i expect to get 10x40, but get 20x40

#include <notcurses/notcurses.h>
#include <cmath>
#include <iostream>

void wait_sleep(struct notcurses *nc) {
  while (true) {
    struct timespec ts = {.tv_sec = 600, .tv_nsec = 0};
    struct ncinput ni;
    auto res = notcurses_get(nc, &ts, &ni);
    if (!res) {
      continue;
    }

    if (ni.evtype != NCTYPE_PRESS && ni.evtype != NCTYPE_REPEAT) {
      continue;
    }

    notcurses_stop(nc);
    exit(2);
  }
}

int main(int argc, char **argv) {
  if (argc != 3) {
    std::cerr << "notcurses-test <mode> <file>\n";
    return 3;
  }
  std::string mode = argv[1];
  struct notcurses_options curses_opts{.termtype = NULL,
                                       .loglevel = NCLOGLEVEL_WARNING,
                                       .margin_t = 0,
                                       .margin_r = 0,
                                       .margin_b = 0,
                                       .margin_l = 0,
                                       .flags = 0};
  auto nc = notcurses_init(&curses_opts, nullptr);

  auto baseplane = notcurses_stdplane(nc);
  struct ncvgeom geom;
  memset(&geom, 0, sizeof(geom));
  ncvisual_geom(nc, nullptr, nullptr, &geom);
  if (geom.cdimy <= 0 || geom.cdimx <= 0) {
    std::cerr << "failed to get pixel configuration\n";
    notcurses_stop(nc);
    return 2;
  }

  auto cdimy = geom.cdimy;
  auto cdimx = geom.cdimx;

  std::cerr << "dimy=" << cdimy << " dimx=" << cdimx;

  auto visual = ncvisual_from_file(argv[2]);
  if (!visual) {
    std::cerr << "failed to open image file\n";
    notcurses_stop(nc);
    return 2;
  }

  if (ncvisual_resize(visual, 20 * cdimy, 40 * cdimx) < 0) {
    std::cerr << "failed to resize image\n";
    notcurses_stop(nc);
    return 2;
  }

  struct ncvisual_options opts = {.n = baseplane,
                                  .scaling = NCSCALE_NONE,
                                  .y = 0,
                                  .x = 0,
                                  .begy = cdimy * 5,
                                  .begx = 0,
                                  .leny = cdimy * 10,
                                  .lenx = cdimx * 40,
                                  .blitter = NCBLIT_PIXEL,
                                  .flags = NCVISUAL_OPTION_CHILDPLANE,
                                  .transcolor = 0,
                                  .pxoffy = 0,
                                  .pxoffx = 0};

  memset(&geom, 0, sizeof(geom));
  ncvisual_geom(nc, visual, &opts, &geom);

  if (geom.rcelly != 10 || geom.rcellx != 40) {
    std::cerr << "wrong geom in image: (" << geom.rcelly << "x" << geom.rcellx << ") instead of (10x40)\n";
    notcurses_stop(nc);
    return 2;
  }

  std::cerr << "OK\n";

  notcurses_stop(nc);
  return 3;
}

[vysheng]

I found out, that I only fixed code in visual.c, but there are similar places in direct.c ( ncdirectf_render and ncdirectf_geom, for example). Not sure, if begy/begx need to be supported in direct mode. If you want, I can fix direct.c in similar way. Not sure, whether you'll want to merge this patch.