Skip to content

Commit

Permalink
PlatformImage: Fix OOB access in MIRROR_ROT270, do subimage faster
Browse files Browse the repository at this point in the history 
The fun part of low-level optimizations, is that these kinds of
bugs get a lot more common, since Java's internal type and format
handling aren't here to save the day. Also, we can use a databuffer
to create a new subImage, so it is probably a bit faster now as
well.
  • Loading branch information
@AShiningRay
AShiningRay committed Dec 7, 2024
1 parent 0f2f8c9 commit d2e2ffb
Showing 1 changed file with 95 additions and 98 deletions.
193 changes: 95 additions & 98 deletions src/org/recompile/mobile/PlatformImage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
import java.awt.Graphics2D;
import java.awt.RenderingHints;
import java.awt.image.BufferedImage;
import java.awt.image.DataBuffer;
import java.awt.image.DataBufferInt;
import java.awt.image.DataBuffer;
import java.awt.image.DataBufferInt;

public class PlatformImage extends javax.microedition.lcdui.Image
{
Expand Down Expand Up @@ -195,30 +195,29 @@ public PlatformImage(int[] rgb, int Width, int Height, boolean processAlpha)

public PlatformImage(Image image, int x, int y, int Width, int Height, int transform)
{
// Create Image From Sub-Image, Transformed //
BufferedImage sub = new BufferedImage(Width, Height, BufferedImage.TYPE_INT_ARGB);

// Get the original pixel data from the source image
final int[] sourcePixels = ((DataBufferInt) image.platformImage.canvas.getRaster().getDataBuffer()).getData();

// Create an array to hold the sub-image pixel data
final int[] subPixels = new int[Width * Height];

// Copy pixel data directly
for (int j = 0; j < Height; j++)
{
System.arraycopy(sourcePixels, (y + j) * image.platformImage.canvas.getWidth() + x, subPixels, j * Width, Width);
}

// Set the pixel data for the new sub-image
sub.getRaster().setDataElements(0, 0, Width, Height, subPixels);

canvas = transformImage(sub, transform);
BufferedImage sub = new BufferedImage(Width, Height, BufferedImage.TYPE_INT_ARGB);

// Get the raw pixel data from the source image, and the new sub image
final int[] sourceData = ((DataBufferInt) image.platformImage.canvas.getRaster().getDataBuffer()).getData();
final int[] subData = ((DataBufferInt) sub.getRaster().getDataBuffer()).getData();

// Copy pixel data directly to the subimage's databuffer.
for (int j = 0; j < Height; j++)
{
int sourceRow = (y + j) * image.platformImage.canvas.getWidth() + x;
int subRow = j * Width;

// Copy pixel rows from the source image to the new sub-image
System.arraycopy(sourceData, sourceRow, subData, subRow, Math.min(Width, image.platformImage.canvas.getWidth() - x));
}

canvas = transformImage(sub, transform);;

createGraphics();

width = (int)canvas.getWidth();
height = (int)canvas.getHeight();

width = (int) canvas.getWidth();
height = (int) canvas.getHeight();
platformImage = this;
}

Expand All @@ -228,15 +227,15 @@ public void getRGB(int[] rgbData, int offset, int scanlength, int x, int y, int
if (width <= 0 || height <= 0) { return; } // No pixels to copy
if (x < 0 || y < 0 || x + width > canvas.getWidth() || y + height > canvas.getHeight())
{
throw new IllegalArgumentException("getRGB Requested area exceeds bounds of the image");
throw new IllegalArgumentException("getRGB Requested area exceeds bounds of the image");
}
if (Math.abs(scanlength) < width)
{
throw new IllegalArgumentException("scanlength must be >= width");
}

// Temporary array to hold the raw pixel data
int[] tempData = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
int[] tempData = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
// Copy the data into rgbData, taking scanlength into account
for (int row = 0; row < height; row++)
{
Expand All @@ -247,30 +246,30 @@ public void getRGB(int[] rgbData, int offset, int scanlength, int x, int y, int
}
}

public int getARGB(int x, int y)
{
if (x < 0 || y < 0 || x >= canvas.getWidth() || y >= canvas.getHeight())
{
throw new IllegalArgumentException("Requested area exceeds bounds of the image");
}

// Get the raw pixel data array directly from the canvas
int[] pixels = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
return pixels[y * canvas.getWidth() + x];
public int getARGB(int x, int y)
{
if (x < 0 || y < 0 || x >= canvas.getWidth() || y >= canvas.getHeight())
{
throw new IllegalArgumentException("Requested area exceeds bounds of the image");
}

// Get the raw pixel data array directly from the canvas
int[] pixels = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
return pixels[y * canvas.getWidth() + x];
}

public int getPixel(int x, int y) { return getARGB(x, y); }

public int getPixel(int x, int y) { return getARGB(x, y); }

public void setPixel(int x, int y, int color)
{
if (x < 0 || y < 0 || x >= canvas.getWidth() || y >= canvas.getHeight())
{
throw new IllegalArgumentException("Requested area exceeds bounds of the image");
}

// Get the raw pixel data array directly from the canvas
int[] pixels = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
pixels[y * canvas.getWidth() + x] = color;
if (x < 0 || y < 0 || x >= canvas.getWidth() || y >= canvas.getHeight())
{
throw new IllegalArgumentException("Requested area exceeds bounds of the image");
}

// Get the raw pixel data array directly from the canvas
int[] pixels = ((DataBufferInt) canvas.getRaster().getDataBuffer()).getData();
pixels[y * canvas.getWidth() + x] = color;
}

public static final BufferedImage transformImage(final BufferedImage image, final int transform)
Expand All @@ -284,123 +283,121 @@ public static final BufferedImage transformImage(final BufferedImage image, fina
BufferedImage transimage = null;
if(transform == Sprite.TRANS_ROT90 || transform == Sprite.TRANS_ROT270 || transform == Sprite.TRANS_MIRROR_ROT90 || transform == Sprite.TRANS_MIRROR_ROT270)
{
transimage = new BufferedImage(height, width, image.getType()); // Non-Math.PI rotations require width and height to be swapped
transimage = new BufferedImage(height, width, image.getType()); // Non-Math.PI rotations require width and height to be swapped
}
else { transimage = new BufferedImage(width, height, image.getType()); }
else { transimage = new BufferedImage(width, height, image.getType()); }

// We know the data is of TYPE_INT_ARGB, so just get it directly instead of checking for its type
final int[] sourceData = ((DataBufferInt)image.getRaster().getDataBuffer()).getData();
final int[] targetData = ((DataBufferInt)transimage.getRaster().getDataBuffer()).getData();
// We know the data is of TYPE_INT_ARGB, so just get it directly instead of checking for its type
final int[] sourceData = ((DataBufferInt)image.getRaster().getDataBuffer()).getData();
final int[] targetData = ((DataBufferInt)transimage.getRaster().getDataBuffer()).getData();

switch (transform)
{
case Sprite.TRANS_ROT90:
for (int y = 0; y < height; y++)
{
int targetPos = (height - 1 - y);
int targetPos = (height - 1 - y);
for (int x = 0; x < width; x++)
{
targetData[targetPos + x * height] = sourceData[y * width + x];
{
targetData[targetPos + x * height] = sourceData[y * width + x];
}
}
//dumpImage(image, "");
//dumpImage(transimage, "_rot90");
break;
break;

case Sprite.TRANS_ROT180:
for (int y = 0; y < height; y++)
{
int targetPos = (height - 1 - y) * width;
int targetPos = (height - 1 - y) * width;
for (int x = 0; x < width; x++)
{
targetData[targetPos + (width - 1 - x)] = sourceData[y * width + x];
targetData[targetPos + (width - 1 - x)] = sourceData[y * width + x];
}
}
//dumpImage(image, "");
//dumpImage(transimage, "_rot180");
break;
break;

case Sprite.TRANS_ROT270:
for (int y = 0; y < height; y++)
{
for (int x = 0; x < width; x++)
{
targetData[y + (width - 1 - x) * height] = sourceData[y * width + x];
targetData[y + (width - 1 - x) * height] = sourceData[y * width + x];
}
}
//dumpImage(image, "");
//dumpImage(transimage, "_rot270");
break;
break;

case Sprite.TRANS_MIRROR:
/*
* Even though sorting an entire column would be faster from a pure algorithmic perspective (like processing
* a whole row at once is on TRANS_MIRROR_ROT180), image data tends to be stored so that each row is contiguous
* in memory, which makes its access oftentimes MUCH faster than for columns, which could negate the performance
* benefits of column access entirely and then some.
*
* This transform is such a case. Making operations on columns, and eliminating that inner row loop actually
* results in far worse performance since columns would be accessed way more often. So the next best thing is
* only working on half of the image's width, making two pixel assignments on each inner loop iteration from
* the image's edges to the center, then checking if the width is odd, to just copy the pixel in the middle
* as it won't change.
*/
int tempPixel;
* Even though sorting an entire column would be faster from a pure algorithmic perspective (like processing
* a whole row at once is on TRANS_MIRROR_ROT180), image data tends to be stored so that each row is contiguous
* in memory, which makes its access oftentimes MUCH faster than for columns, which could negate the performance
* benefits of column access entirely and then some.
*
* This transform is such a case. Making operations on columns, and eliminating that inner row loop actually
* results in far worse performance since columns would be accessed way more often. So the next best thing is
* only working on half of the image's width, making two pixel assignments on each inner loop iteration from
* the image's edges to the center, then checking if the width is odd, to just copy the pixel in the middle
* as it won't change.
*/
int tempPixel;
for (int y = 0; y < height; y++)
{
int targetRow = y * width;
for (int x = 0; x < width / 2; x++)
{
int targetRow = y * width;
for (int x = 0; x < width / 2; x++)
{
tempPixel = sourceData[targetRow + x];
targetData[targetRow + (width - 1 - x)] = tempPixel;
targetData[targetRow + x] = sourceData[targetRow + (width - 1 - x)];
tempPixel = sourceData[targetRow + x];
targetData[targetRow + (width - 1 - x)] = tempPixel;
targetData[targetRow + x] = sourceData[targetRow + (width - 1 - x)];
}

// If image width is odd, copy the middle pixel directly as there's no need to swap anything.
if (width % 2 != 0) { targetData[targetRow + (width/2)] = sourceData[targetRow + (width/2)]; }

// If image width is odd, copy the middle pixel directly as there's no need to swap anything.
if (width % 2 != 0) { targetData[targetRow + (width/2)] = sourceData[targetRow + (width/2)]; }
}
//dumpImage(image, "");
//dumpImage(transimage, "_mirror");
break;
break;

case Sprite.TRANS_MIRROR_ROT90:
for (int y = 0; y < height; y++)
{
int targetPos = (height - 1 - y) * width;
int targetPos = (height - 1 - y) * width;
for (int x = 0; x < width; x++)
{
targetData[targetPos + (width - 1 - x)] = sourceData[y * width + x];
targetData[targetPos + (width - 1 - x)] = sourceData[y * width + x];
}
}
}
//dumpImage(image, "");
//dumpImage(transimage, "_mirror90");
break;
break;

case Sprite.TRANS_MIRROR_ROT180: // Basically mirror vertically (an arrow pointing up will then point down).
for (int y = 0; y < height; y++) // Due to this, we copy entire rows at once instead of going pixel by pixel
{
System.arraycopy(sourceData, y * width, targetData, (height - 1 - y) * width, width);
case Sprite.TRANS_MIRROR_ROT180: // Basically mirror vertically (an arrow pointing up will then point down).
for (int y = 0; y < height; y++) // Due to this, we copy entire rows at once instead of going pixel by pixel
{
System.arraycopy(sourceData, y * width, targetData, (height - 1 - y) * width, width);
}
//dumpImage(image, "");
//dumpImage(transimage, "_mirror180");
break;
break;


case Sprite.TRANS_MIRROR_ROT270:
for (int y = 0; y < height; y++)
{
int targetPos = (width - 1 - y);
for (int x = 0; x < width; x++)
{
targetData[x * height + targetPos] = sourceData[y * width + x];
targetData[(width - 1 - x) * height + y] = sourceData[y * width + x];
}
}
//dumpImage(image, "");
//dumpImage(transimage, "_mirror270");
break;
break;
}

return transimage;

return transimage;
}

// TODO: Turn this into a setting. Being able to dump image data would be nice.
Expand Down

0 comments on commit d2e2ffb

Please sign in to comment.