Calculation of image offsets for performing cropping

Question

I am using the java.awt.Rectangle class to construct subsets of a GEOTIFF file. In order to do this I would need to specify the x,y offsets, height and width of each subset image. In my particular case I would need to crop the original GEOTIFF image bottom to top. The java.awt.Rectangle class specifies that the origin point is to be found in the upper left hand corner.

Please let me know if the calculation of the image offsets are correct and/or there is a better way of calculating the image offsets. The crop(r) method is a proprietary method used to crop subsets of the original image.

int width = 40;
int height = 34;
int cellSize = 3600;
int xOffset = 0;
int yOffset = 0;
int pixelWidth = cellSize * width;
int pixelHeight = cellSize * height;
for (int i = 0 ; i < width; i++)
{
    for (int j = 0; j < height; j++)
    {
        yOffset = pixelHeight - cellSize *(j+1) ;
        Rectangle r = new Rectangle(xOffset,
                                    yOffset,
                                    cellSize,
                                    cellSize);
        crop(r);
    }
    xOffset = pixelWidth - cellSize * (i+1);
}

Answer 1

int width = 40;
int height = 34;
int cellSize = 3600;
int xOffset = 0;
int yOffset = 0;
int pixelWidth = cellSize * width;
int pixelHeight = cellSize * height;

I would find this easier to follow in a different order:

final int CELL_SIZE = 3600;

final int WIDTH = 40;
final int HEIGHT = 34;

final int PIXEL_WIDTH = CELL_SIZE * WIDTH;
final int PIXEL_HEIGHT = CELL_SIZE * HEIGHT;

int xOffset = 0;
int yOffset = 0;

Now I see a natural progression of values and have the variables separate from the constants. The variables are also closest to where they are changed.

I changed the values that never change to final to reflect that.

The ALL_CAPS notation is common for constants.

for (int i = 0 ; i < width; i++)
    {
    for (int j = 0; j < height; j++)
        {
        yOffset = pixelHeight - cellSize *(j+1) ;
        Rectangle r = new Rectangle(xOffset,
                                    yOffset,
                                    cellSize,
                                    cellSize);
        crop(r);
        }
      xOffset = pixelWidth - cellSize * (i+1);
     }

You can write this as

for (int xOffset = PIXEL_WIDTH - CELL_SIZE; xOffset >= 0; xOffset -= CELL_SIZE)
{
    for (int yOffset = PIXEL_HEIGHT - CELL_SIZE; yOffset >= 0; yOffset -= CELL_SIZE)
    {
        Rectangle r = new Rectangle(xOffset,
                                    yOffset,
                                    CELL_SIZE,
                                    CELL_SIZE);
        crop(r);
    }
}

Then you don't need i or j at all and you don't have to calculate the offsets further.

I also changed the indent of the brackets to match the more common practice of lining up with the for declarations rather than the block contents.

You may want to do this in the other order:

for (int xOffset = 0; xOffset < PIXEL_WIDTH; xOffset += CELL_SIZE)
{
    for (int yOffset = 0; yOffset < PIXEL_HEIGHT; yOffset += CELL_SIZE)
    {

As that is easier to read. Not sure how much order matters.

If you need the original order exactly, you may need an extra loop. The original order started with an xOffset of 0 and then went to PIXEL_WIDTH - CELL_SIZE.

If you are committed to the i and j method, consider starting them at 1, as so

for (int i = 1 ; i <= width; i++)
    {
    for (int j = 1; j <= height; j++)
        {
        yOffset = pixelHeight - cellSize * j;

Then you don't have to add 1 to i and j each time.