osbook_day09c：FrameBuffer::Copyの実装について

[momo3159]

FrameBuffer::Copyの実装 において、コピー元のアドレスの初期がconst uint8_t* src_buf = src.config_.frame_buffer;と設定されています。 この場合、1回目のmemcpyでは、常にsrc.config_.frame_bufferの先頭からbytes_per_copy_lineだけコピーされる形になると思います。

一方で、図9.15（FrameBuffer::Copy()で範囲外だった場合の動作）にもあるように、先頭からコピーされない場合もあるかと思います。

そのため、dst_bufと同様にコピー元のアドレスについても計算が必要かと考えましたがいかがでしょうか。 （私の思い違いであれば申し訳ないです。）

修正前

Error FrameBuffer::Copy(Vector2D<int> pos, const FrameBuffer& src) {
  const auto dst_width = config_.horizontal_resolution;
  const auto dst_height = config_.vertical_resolution;
  const auto src_width = config_.horizontal_resolution;
  const auto src_height = config_.vertical_resolution;

  const auto copy_start_dst_x = std::max(0, pos.x);
  const auto copy_start_dst_y = std::max(0, pos.y);
  const auto copy_end_dst_x  = std::min(pos.x + src_width, dst_width);
  const auto copy_end_dst_y = std::min(pos.y + src_height, dst_height);

  const auto bytes_per_pixel = (bits_per_pixel + 7) / 8;
  const auto bytes_per_copy_line = 
    (copy_end_dst_x - copy_start_dst_x) * bytes_per_pixel;

  uint8_t* dst_buf = 
    config_.frame_buffer + (copy_start_dst_y * config_.pixels_per_scan_line + copy_start_dst_x) * bytes_per_pixel;
  uint8_t* src_buf = src.config_.frame_buffer;

  for (int dy=0;dy<copy_end_dst_y - copy_start_dst_y;dy++) {
    memcpy(dst_buf, src_buf, bytes_per_copy_line);
    dst_buf += bytes_per_pixel * config_.pixels_per_scan_line;
    src_buf += bytes_per_pixel * src.config_.pixels_per_scan_line;
  }

  return MAKE_ERROR(Error::kSuccess);
}

修正後

Error FrameBuffer::Copy(Vector2D<int> pos, const FrameBuffer& src) {
  const auto dst_width = config_.horizontal_resolution;
  const auto dst_height = config_.vertical_resolution;
  const auto src_width = config_.horizontal_resolution;
  const auto src_height = config_.vertical_resolution;

  const auto copy_start_dst_x = std::max(0, pos.x);
  const auto copy_start_dst_y = std::max(0, pos.y);
  const auto copy_end_dst_x  = std::min(pos.x + src_width, dst_width);
  const auto copy_end_dst_y = std::min(pos.y + src_height, dst_height);

  const auto bytes_per_pixel = (bits_per_pixel + 7) / 8;
  const auto bytes_per_copy_line = 
    (copy_end_dst_x - copy_start_dst_x) * bytes_per_pixel;

  uint8_t* dst_buf = 
    config_.frame_buffer + (copy_start_dst_y * config_.pixels_per_scan_line + copy_start_dst_x) * bytes_per_pixel;
  uint8_t* src_buf = 
    src.config_.frame_buffer + (std::abs(copy_start_dst_y-pos.y) * src.config_.pixels_per_scan_line + std::abs(copy_start_dst_x-pos.x))  * bytes_per_pixel;

  for (int dy=0;dy<copy_end_dst_y - copy_start_dst_y;dy++) {
    memcpy(dst_buf, src_buf, bytes_per_copy_line);
    dst_buf += bytes_per_pixel * config_.pixels_per_scan_line;
    src_buf += bytes_per_pixel * src.config_.pixels_per_scan_line;
  }

  return MAKE_ERROR(Error::kSuccess);
}

[uchan-nos]

osbook_day10d に含まれるこちらのコミット https://github.com/uchan-nos/mikanos/commit/e4c0690d4a17350e3fd8edc52240d2f94091ffeb によって、修正されています。

[uchan-nos]

確かに @momo3159 さんのご指摘の通り、osbook_day09c の時点においてはバグがある気がします。

基本的にタグの内容は変更しない方針のため、ご指摘のバグは修正しません。 お手元で、src の左端が画面外になるような状況でバグが発現するかを確かめ、修正案がうまく動くかを検証するなどして、バグをお楽しみください。