Skip to content

Commit

Permalink
Fix potential seg fault during pixel op
Browse files Browse the repository at this point in the history
  • Loading branch information
@BloCamLimb
BloCamLimb committed Nov 19, 2024
1 parent bf7fb76 commit cf0ce9c
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 52 deletions.
146 changes: 94 additions & 52 deletions core/src/main/java/icyllis/modernui/graphics/Bitmap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,11 @@ public int getPixelARGB(int x, int y) {
if (getColorType() == ColorInfo.CT_UNKNOWN) {
return 0;
}
return mPixmap.getColor(x, y);
try {
return mPixmap.getColor(x, y);
} finally {
Reference.reachabilityFence(this);
}
}

/**
Expand Down Expand Up @@ -685,7 +689,11 @@ public float[] getColor4f(int x, int y, @NonNull @Size(4) float[] dst) {
dst[i] = 0.0f;
}
} else {
mPixmap.getColor4f(x, y, dst);
try {
mPixmap.getColor4f(x, y, dst);
} finally {
Reference.reachabilityFence(this);
}
}
return dst;
}
Expand Down Expand Up @@ -714,7 +722,11 @@ public void setColor4f(int x, int y, @NonNull @Size(4) float[] src) {
}
checkOutOfBounds(x, y);
if (getColorType() != ColorInfo.CT_UNKNOWN) {
mPixmap.setColor4f(x, y, src);
try {
mPixmap.setColor4f(x, y, src);
} finally {
Reference.reachabilityFence(this);
}
}
}

Expand Down Expand Up @@ -761,14 +773,18 @@ public void getPixels(@NonNull @ColorInt int[] dst, int offset, int stride,
Arrays.fill(dst, index, index + width, 0);
}
} else {
var dstInfo = new ImageInfo(width, height,
ColorInfo.CT_BGRA_8888_NATIVE, ColorInfo.AT_UNPREMUL,
ColorSpace.get(ColorSpace.Named.SRGB));
var dstPixmap = new Pixmap(dstInfo, dst,
Unsafe.ARRAY_INT_BASE_OFFSET + (long) offset << 2,
stride << 2);
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
try {
var dstInfo = new ImageInfo(width, height,
ColorInfo.CT_BGRA_8888_NATIVE, ColorInfo.AT_UNPREMUL,
ColorSpace.get(ColorSpace.Named.SRGB));
var dstPixmap = new Pixmap(dstInfo, dst,
Unsafe.ARRAY_INT_BASE_OFFSET + (long) offset << 2,
stride << 2);
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
} finally {
Reference.reachabilityFence(this);
}
}
}

Expand Down Expand Up @@ -806,14 +822,18 @@ public void setPixels(@NonNull @ColorInt int[] src, int offset, int stride,
}
checkOutOfBounds(dstX, dstY, width, height, offset, stride, src.length);
if (getColorType() != ColorInfo.CT_UNKNOWN) {
var srcInfo = new ImageInfo(width, height,
ColorInfo.CT_BGRA_8888_NATIVE, ColorInfo.AT_UNPREMUL,
ColorSpace.get(ColorSpace.Named.SRGB));
var srcPixmap = new Pixmap(srcInfo, src,
Unsafe.ARRAY_INT_BASE_OFFSET + (long) offset << 2,
stride << 2);
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
try {
var srcInfo = new ImageInfo(width, height,
ColorInfo.CT_BGRA_8888_NATIVE, ColorInfo.AT_UNPREMUL,
ColorSpace.get(ColorSpace.Named.SRGB));
var srcPixmap = new Pixmap(srcInfo, src,
Unsafe.ARRAY_INT_BASE_OFFSET + (long) offset << 2,
stride << 2);
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
} finally {
Reference.reachabilityFence(this);
}
}
}

Expand Down Expand Up @@ -859,14 +879,18 @@ public void getPixels(@NonNull @Size(multiple = 4) float[] dst, int offset, int
Arrays.fill(dst, index, index + (width << 2), 0.0f);
}
} else {
var dstInfo = new ImageInfo(width, height,
ColorInfo.CT_RGBA_F32, getAlphaType(),
getColorSpace());
var dstPixmap = new Pixmap(dstInfo, dst,
Unsafe.ARRAY_FLOAT_BASE_OFFSET + (long) offset << 4,
stride << 4);
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
try {
var dstInfo = new ImageInfo(width, height,
ColorInfo.CT_RGBA_F32, getAlphaType(),
getColorSpace());
var dstPixmap = new Pixmap(dstInfo, dst,
Unsafe.ARRAY_FLOAT_BASE_OFFSET + (long) offset << 4,
stride << 4);
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
} finally {
Reference.reachabilityFence(this);
}
}
}

Expand Down Expand Up @@ -906,14 +930,18 @@ public void setPixels(@NonNull @Size(multiple = 4) float[] src, int offset, int
}
checkOutOfBounds(dstX, dstY, width, height, offset, stride, src.length >> 2);
if (getColorType() != ColorInfo.CT_UNKNOWN) {
var srcInfo = new ImageInfo(width, height,
ColorInfo.CT_RGBA_F32, getAlphaType(),
getColorSpace());
var srcPixmap = new Pixmap(srcInfo, src,
Unsafe.ARRAY_FLOAT_BASE_OFFSET + (long) offset << 4,
stride << 4);
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
try {
var srcInfo = new ImageInfo(width, height,
ColorInfo.CT_RGBA_F32, getAlphaType(),
getColorSpace());
var srcPixmap = new Pixmap(srcInfo, src,
Unsafe.ARRAY_FLOAT_BASE_OFFSET + (long) offset << 4,
stride << 4);
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
} finally {
Reference.reachabilityFence(this);
}
}
}

Expand Down Expand Up @@ -956,13 +984,17 @@ public void getPixels(@NonNull Bitmap dst, int dstX, int dstY,
checkOutOfBounds(srcX, srcY, width, height);
dst.checkOutOfBounds(dstX, dstY, width, height);
var dstRect = new Rect2i(dstX, dstY, dstX + width, dstY + height);
if (getColorType() == ColorInfo.CT_UNKNOWN) {
dst.getPixmap().clear(new float[]{0, 0, 0, 0}, dstRect);
} else {
var dstPixmap = dst.getPixmap().makeSubset(dstRect);
assert dstPixmap != null;
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
try {
if (getColorType() == ColorInfo.CT_UNKNOWN) {
dst.getPixmap().clear(new float[]{0, 0, 0, 0}, dstRect);
} else {
var dstPixmap = dst.getPixmap().makeSubset(dstRect);
assert dstPixmap != null;
boolean res = mPixmap.readPixels(dstPixmap, srcX, srcY);
assert res;
}
} finally {
Reference.reachabilityFence(this);
}
}

Expand Down Expand Up @@ -1005,13 +1037,17 @@ public void setPixels(@NonNull Bitmap src, int srcX, int srcY,
checkOutOfBounds(srcX, srcY, width, height);
src.checkOutOfBounds(dstX, dstY, width, height);
var srcRect = new Rect2i(srcX, srcY, srcX + width, srcY + height);
if (getColorType() == ColorInfo.CT_UNKNOWN) {
mPixmap.clear(new float[]{0, 0, 0, 0}, srcRect);
} else {
var srcPixmap = src.getPixmap().makeSubset(srcRect);
assert srcPixmap != null;
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
try {
if (getColorType() == ColorInfo.CT_UNKNOWN) {
mPixmap.clear(new float[]{0, 0, 0, 0}, srcRect);
} else {
var srcPixmap = src.getPixmap().makeSubset(srcRect);
assert srcPixmap != null;
boolean res = mPixmap.writePixels(srcPixmap, dstX, dstY);
assert res;
}
} finally {
Reference.reachabilityFence(this);
}
}

Expand Down Expand Up @@ -1064,6 +1100,7 @@ public boolean copyPixelsToBuffer(@NonNull java.nio.Buffer dst, int rowBytes,
return mPixmap.readPixels(dstPixmap, srcX, srcY);
} finally {
Reference.reachabilityFence(dst);
Reference.reachabilityFence(this);
}
}

Expand Down Expand Up @@ -1120,6 +1157,7 @@ public boolean copyPixelsFromBuffer(@NonNull java.nio.Buffer src, int rowBytes,
return mPixmap.writePixels(srcPixmap, dstX, dstY);
} finally {
Reference.reachabilityFence(src);
Reference.reachabilityFence(this);
}
}

Expand Down Expand Up @@ -1162,9 +1200,13 @@ public boolean clear(@NonNull @Size(4) float[] color, @Nullable Rect area) {
if (isImmutable()) {
throw new IllegalStateException("Cannot clear immutable bitmaps");
}
return mPixmap.clear(color, area == null ? null : new Rect2i(
area.left, area.top, area.right, area.bottom
));
try {
return mPixmap.clear(color, area == null ? null : new Rect2i(
area.left, area.top, area.right, area.bottom
));
} finally {
Reference.reachabilityFence(this);
}
}

/**
Expand Down
68 changes: 68 additions & 0 deletions core/src/test/java/icyllis/modernui/test/TestHighConcurrencyBitmap.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Modern UI.
* Copyright (C) 2024 BloCamLimb. All rights reserved.
*
* Modern UI is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* Modern UI is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with Modern UI. If not, see <https://www.gnu.org/licenses/>.
*/

package icyllis.modernui.test;

import icyllis.modernui.graphics.Bitmap;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.infra.Blackhole;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.RunnerException;
import org.openjdk.jmh.runner.options.OptionsBuilder;

import java.util.Random;

@Fork(2)
@Threads(16)
@Warmup(iterations = 2, time = 1)
@Measurement(iterations = 5, time = 1)
@State(Scope.Thread)
public class TestHighConcurrencyBitmap {

public static void main(String[] args) throws RunnerException {
// you will get seg fault if there is no reachabilityFence() in the Bitmap class
new Runner(new OptionsBuilder()
.include(TestHighConcurrencyBitmap.class.getSimpleName())
.shouldFailOnError(true).shouldDoGC(true)
.jvmArgs("-Xmx128m", "-Dorg.lwjgl.system.allocator=system")
.build())
.run();
}

private static final Bitmap SRC;

static {
Bitmap bm = Bitmap.createBitmap(32, 32, Bitmap.Format.RGBA_F32);
Random r = new Random();
for (int i = 0; i < 32; i++) {
for (int j = 0; j < 32; j++) {
bm.setColor4f(i, j, new float[]{r.nextFloat(), r.nextFloat(), r.nextFloat(), r.nextFloat()});
}
}
SRC = bm;
}

@Benchmark
public static void testRWPixel(Blackhole blackhole) {
blackhole.consume(new Object());
@SuppressWarnings("resource")
var bm = Bitmap.createBitmap(32, 32, Bitmap.Format.RGBA_8888);
bm.setPixels(SRC, 0, 0, 0, 0, 32, 32);
blackhole.consume(new Object());
}
}

0 comments on commit cf0ce9c

Please sign in to comment.