Bugzilla

OK. I constructed a simple example to show what I meant by that comment ... and I now realize it's wrong :-( Your proposed patch, Jeff, is all that's needed here. My original patch can be backed out. Though please do leave the `// Set the subimage's location in its parent` comment in place. It makes the original code's logic a lot clearer.

Say you have image data for an image. The image's height (in pixels) is `height` and its width is `width`. Each line of data has `bytes_per_pixel * width` bytes. To keep things simple, let's say the `stride` for the whole image is also `bytes_per_pixel * width`. The image's data is stored in a single, continuous block starting at `image_data`.

Let's say you want to get the data corresponding to a subimage of the original image, where the subimage's `subimage_x` and `subimage_y` coordinates are offsets in `height` and `width`. (Let's also say that the subimage is entirely contained in the original image.)

If `subimage_x` is `0`, it's very straightforward. The subimage's data is a also continuous block starting at `image_data + subimage_y * image_stride`.

But if `subimage_x` is (say) `1`, its data is no longer in a continuous block inside `image_data`. However (and this is where I was wrong) the subimage's `stride` does stay the same as that of the original image. When you copy data for the subimage starting at `aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat)`, the copy will include some data not in the subimage. But as long as the code which displays the subimage knows to skip the first `1 * bytes_per_pixel` (aka `aRect.X() * BytesPerPixel(aFormat)`) bytes of each line, it will be able to do so correctly.

OK. I constructed a simple example to show what I meant by that comment ... and I now realize it's wrong :-( Your proposed patch, Jeff, is all that's needed here. My original patch can be backed out. Though please do leave the `// Set the subimage's location in its parent` comment in place. It makes the original code's logic a lot clearer.

Say you have image data for an image. The image's height (in pixels) is `height` and its width is `width`. Each line of data has `bytes_per_pixel * width` bytes. To keep things simple, let's say the `stride` for the whole image is also `bytes_per_pixel * width`. The image's data is stored in a single, continuous block starting at `image_data`.

Let's say you want to get the data corresponding to a subimage of the original image, where the subimage's `subimage_x` and `subimage_y` coordinates are offsets in `height` and `width`. (Let's also say that the subimage is entirely contained in the original image.)

If `subimage_x` is `0`, it's very straightforward. The subimage's data is also a continuous block starting at `image_data + subimage_y * image_stride`.

But if `subimage_x` is (say) `1`, its data is no longer in a continuous block inside `image_data`. However (and this is where I was wrong) the subimage's `stride` does stay the same as that of the original image. When you copy data for the subimage starting at `aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat)`, the copy will include some data not in the subimage. But as long as the code which displays the subimage knows to skip the first `1 * bytes_per_pixel` (aka `aRect.X() * BytesPerPixel(aFormat)`) bytes of each line, it will be able to do so correctly.

OK. I constructed a simple example to show what I meant by that comment ... and I now realize it's wrong :-( Your proposed patch, Jeff, is all that's needed here. My original patch can be backed out. Though please do leave the `// Set the subimage's location in its parent` comment in place. It makes the original code's logic a lot clearer.

Say you have image data for an image. The image's height (in pixels) is `height` and its width is `width`. Each line of data has `bytes_per_pixel * width` bytes. To keep things simple, let's say the `stride` for the whole image is also `bytes_per_pixel * width`. The image's data is stored in a single, continuous block starting at `image_data`.

Let's say you want to get the data corresponding to a subimage of the original image, where the subimage's `subimage_x` and `subimage_y` coordinates are offsets in `height` and `width`. (Let's also say that the subimage is entirely contained in the original image.)

If `subimage_x` is `0`, it's very straightforward. The subimage's data is also a continuous block starting at `image_data + subimage_y * image_stride`.

But if `subimage_x` is (say) `1`, its data is no longer in a continuous block inside `image_data`. However (and this is where I was wrong) the subimage's `stride` does stay the same as that of the original image. When you copy data for the subimage starting at `aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat)`, the copy will include some data not in the subimage. But as long as the code which displays the subimage knows to skip the first `1 * bytes_per_pixel` (aka `aRect.X() * BytesPerPixel(aFormat)`) bytes of each line, it will be able to do so correctly. And it will do this if it uses the superimage's `stride` to jump from the start of subimage data for one line to the start of subimage data for the next line.

OK. I constructed a simple example to show what I meant by that comment ... and I now realize it's wrong :-( Your proposed patch, Jeff, is all that's needed here. My original patch can be backed out. Though please do leave the `// Set the subimage's location in its parent` comment in place. It makes the original code's logic a lot clearer.

Say you have image data for an image. The image's height (in pixels) is `height` and its width is `width`. Each line of data has `bytes_per_pixel * width` bytes. To keep things simple, let's say the `stride` for the whole image is also `bytes_per_pixel * width`. The image's data is stored in a single, continuous block starting at `image_data`.

Let's say you want to get the data corresponding to a subimage of the original image, where the subimage's `subimage_x` and `subimage_y` coordinates are offsets in `height` and `width`. (Let's also say that the subimage is entirely contained in the original image.)

If `subimage_x` is `0`, it's very straightforward. The subimage's data is also a continuous block starting at `image_data + subimage_y * image_stride`.

But if `subimage_x` is (say) `1`, its data is no longer in a continuous block inside `image_data`. However (and this is where I was wrong) the subimage's `stride` can stay the same as that of the original image. When you copy data for the subimage starting at `aData + aRect.Y() * aStride + aRect.X() * BytesPerPixel(aFormat)`, the copy will include some data not in the subimage. But as long as the code which displays the subimage knows to skip the first `1 * bytes_per_pixel` (aka `aRect.X() * BytesPerPixel(aFormat)`) bytes of each line, it will be able to do so correctly. And it will do this if it uses the superimage's `stride` to jump from the start of subimage data for one line to the start of subimage data for the next line.