Skip to content

Commit

Permalink
Paper PR: Optimise color distance check in MapPalette by removing flo…
Browse files Browse the repository at this point in the history
…ating point math
  • Loading branch information
Dreeam-qwq committed Aug 8, 2024
1 parent 8cfb6f0 commit dbfeb87
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Barnaby <[email protected]>
Date: Sat, 29 Jun 2024 12:04:48 +0100
Subject: [PATCH] Paper PR: Optimise color distance check in MapPalette by
removing floating point math

Original license: GPLv3
Original project: https://github.com/PaperMC/Paper
Paper pull request: https://github.com/PaperMC/Paper/pull/11000

diff --git a/src/main/java/org/bukkit/map/MapPalette.java b/src/main/java/org/bukkit/map/MapPalette.java
index 7d4172dbe5247f4cbe776334d99c4e4c40ac7dbb..1cda69ab051a327694215b6eb75e02ce3273beeb 100644
--- a/src/main/java/org/bukkit/map/MapPalette.java
+++ b/src/main/java/org/bukkit/map/MapPalette.java
@@ -31,14 +31,19 @@ public final class MapPalette {
}

private static double getDistance(@NotNull Color c1, @NotNull Color c2) {
- double rmean = (c1.getRed() + c2.getRed()) / 2.0;
- double r = c1.getRed() - c2.getRed();
- double g = c1.getGreen() - c2.getGreen();
+ // Paper start - Optimize color distance calculation by removing floating point math
+ int rsum = c1.getRed() + c2.getRed(); // Use sum instead of mean for no division
+ int r = c1.getRed() - c2.getRed();
+ int g = c1.getGreen() - c2.getGreen();
int b = c1.getBlue() - c2.getBlue();
- double weightR = 2 + rmean / 256.0;
- double weightG = 4.0;
- double weightB = 2 + (255 - rmean) / 256.0;
+ // All weights are 512x their original to avoid floating point division
+ int weightR = 1024 + rsum;
+ int weightG = 2048;
+ int weightB = 1024 + (255*2 - rsum);
+
+ // Division by 256 here is unnecessary as this won't change the result of the sort
return weightR * r * r + weightG * g * g + weightB * b * b;
+ // Paper end
}

@NotNull
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Barnaby <[email protected]>
Date: Sat, 29 Jun 2024 12:06:51 +0100
Subject: [PATCH] Paper PR: Reduce work done in CraftMapCanvas.drawImage by
limiting size of image and using System.arraycopy instead of for loops and
use bitwise operations to do bounds checks.

Original license: GPLv3
Original project: https://github.com/PaperMC/Paper
Paper pull request: https://github.com/PaperMC/Paper/pull/11000

diff --git a/src/main/java/org/bukkit/craftbukkit/map/CraftMapCanvas.java b/src/main/java/org/bukkit/craftbukkit/map/CraftMapCanvas.java
index ff59f759669620795ef355c988b664bdcda39f52..a5e98571d6d83390761c11e28a0bc3c4415799cd 100644
--- a/src/main/java/org/bukkit/craftbukkit/map/CraftMapCanvas.java
+++ b/src/main/java/org/bukkit/craftbukkit/map/CraftMapCanvas.java
@@ -91,12 +91,41 @@ public class CraftMapCanvas implements MapCanvas {

@Override
public void drawImage(int x, int y, Image image) {
- byte[] bytes = MapPalette.imageToBytes(image);
- for (int x2 = 0; x2 < image.getWidth(null); ++x2) {
- for (int y2 = 0; y2 < image.getHeight(null); ++y2) {
- this.setPixel(x + x2, y + y2, bytes[y2 * image.getWidth(null) + x2]);
+ // Paper start - Reduce work done by limiting size of image and using System.arraycopy
+ int width = 128 - x;
+ int height = 128 - y;
+ if (image.getHeight(null) < height)
+ height = image.getHeight(null);
+
+ // Create a subimage if the image is larger than the max allowed size
+ java.awt.image.BufferedImage temp;
+ if (image.getWidth(null) >= width && image instanceof java.awt.image.BufferedImage bImage) {
+ // If the image is larger than the max allowed size, get a subimage, otherwise use the image as is
+ if (image.getWidth(null) > width || image.getHeight(null) > height) {
+ temp = bImage.getSubimage(0, 0, width, height);
+ } else {
+ temp = bImage;
}
+ } else {
+ temp = new java.awt.image.BufferedImage(width, height, java.awt.image.BufferedImage.TYPE_INT_ARGB);
+ java.awt.Graphics2D graphics = temp.createGraphics();
+ graphics.drawImage(image, 0, 0, null);
+ graphics.dispose();
}
+
+ byte[] bytes = MapPalette.imageToBytes(temp);
+
+ // Since we now control the size of the image, we can safely use System.arraycopy
+ // If x is 0, we can just copy the entire image as width is 128 and height is <=(128-y)
+ if (x == 0) {
+ System.arraycopy(bytes, 0, this.buffer, y * 128, width * height);
+ return;
+ }
+
+ for (int y2 = 0; y2 < height; ++y2) {
+ System.arraycopy(bytes, 0, this.buffer, (y + y2) * 128 + x, width);
+ }
+ // Paper end
}

@Override
diff --git a/src/main/java/org/bukkit/craftbukkit/map/CraftMapRenderer.java b/src/main/java/org/bukkit/craftbukkit/map/CraftMapRenderer.java
index aef19cfbecb4ddfc8dc71c4f3b2a011364c12dc2..e30c851acf49a425cd4fd409a6d5bbb2ff836e0e 100644
--- a/src/main/java/org/bukkit/craftbukkit/map/CraftMapRenderer.java
+++ b/src/main/java/org/bukkit/craftbukkit/map/CraftMapRenderer.java
@@ -23,8 +23,10 @@ public class CraftMapRenderer extends MapRenderer {
@Override
public void render(MapView map, MapCanvas canvas, Player player) {
// Map
- for (int x = 0; x < 128; ++x) {
- for (int y = 0; y < 128; ++y) {
+ // Paper start - Swap inner and outer loops here to (theoretically) improve cache locality
+ for (int y = 0; y < 128; ++y) {
+ for (int x = 0; x < 128; ++x) {
+ // Paper end
canvas.setPixel(x, y, this.worldMap.colors[y * 128 + x]);
}
}

0 comments on commit dbfeb87

Please sign in to comment.