Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2022, Gluon
* Copyright (c) 2016, 2026, Gluon
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -33,14 +33,10 @@
import javafx.beans.property.ReadOnlyObjectProperty;
import javafx.beans.property.ReadOnlyObjectWrapper;
import javafx.beans.property.SimpleObjectProperty;
import javafx.scene.SnapshotParameters;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.paint.Color;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.util.Optional;
import java.util.logging.Logger;

Expand Down Expand Up @@ -93,6 +89,7 @@ public class AndroidPicturesService implements PicturesService {

private static final ObjectProperty<File> imageFile = new SimpleObjectProperty<>();
private static final ReadOnlyObjectWrapper<Image> imageProperty = new ReadOnlyObjectWrapper<>();
private static final int MAX_IMAGE_DIMENSION = 1280;
private static ObjectProperty<Image> result;
private static boolean enteredLoop;

Expand Down Expand Up @@ -149,31 +146,28 @@ public ReadOnlyObjectProperty<Image> imageProperty() {
public static native void selectPicture();

// callback
public static void setResult(String filePath, int rotate) {
public static void setResult(String filePath) {
LOG.fine("Got photo file at: " + filePath);
File photoFile = new File(filePath);
imageFile.set(photoFile);
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getImageFile() is documented to return the original file created by takePhoto(true), but filePath is now the preprocessed cache copy when a photo is saved. Setting imageFile to this path changes the API behavior and can break callers expecting to share the original full-resolution file. Consider passing both original+display paths from native, or otherwise preserving imageFile as the original saved file path.

Copilot uses AI. Check for mistakes.
Image initialImage = null;
try {
initialImage = new Image(new FileInputStream(photoFile));
} catch (FileNotFoundException e) {
LOG.severe("GalleryActivity: file not found: " + e);

// Release the old image reference and try to free resources
imageProperty.setValue(null);
System.gc();
Comment on lines +160 to +163
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling System.gc() here is a forced full GC that is not guaranteed to reclaim the native/Prism resources you care about, and can introduce noticeable UI pauses (this callback can happen frequently when taking photos). It’s usually better to rely on dropping strong references (which you already do by clearing the property) and let the runtime decide when to collect; if explicit cleanup is needed, prefer an API-specific disposal mechanism rather than forcing GC.

Suggested change
// Release the old image reference and try to free resources
imageProperty.setValue(null);
System.gc();
// Release the old image reference and let the runtime reclaim resources as needed
imageProperty.setValue(null);

Copilot uses AI. Check for mistakes.

Image image = null;
try (FileInputStream fis = new FileInputStream(photoFile)) {
image = new Image(fis, MAX_IMAGE_DIMENSION, MAX_IMAGE_DIMENSION, true, true);
} catch (Exception e) {
LOG.severe("GalleryActivity: error loading image: " + e);
}
if (enteredLoop && (initialImage == null || rotate == 0)) {
result.set(initialImage);

final Image finalImage = image;
if (enteredLoop) {
result.set(finalImage);
}
final Image finalImage = initialImage;
Platform.runLater(() -> {
if (finalImage != null && rotate != 0) {
Image image = rotateImage(finalImage, rotate);
if (enteredLoop) {
result.set(image);
} else {
imageProperty.setValue(image);
}
} else {
imageProperty.setValue(finalImage);
}
imageProperty.setValue(finalImage);
if (enteredLoop) {
enteredLoop = false;
try {
Expand All @@ -184,18 +178,4 @@ public static void setResult(String filePath, int rotate) {
}
});
}

private static Image rotateImage(Image image, int rotate) {
if (image == null || rotate == 0) {
return image;
}
ImageView iv = new ImageView(image);
iv.setFitWidth(1280);
iv.setFitHeight(1280);
iv.setPreserveRatio(true);
iv.setRotate(rotate);
SnapshotParameters params = new SnapshotParameters();
params.setFill(Color.TRANSPARENT);
return iv.snapshot(params, null);
}
}
10 changes: 5 additions & 5 deletions modules/pictures/src/main/native/android/c/pictures.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2021, Gluon
* Copyright (c) 2020, 2026, Gluon
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -38,7 +38,7 @@ static jmethodID jPicturesServiceSelectPictureMethod;

void initializePicturesGraalHandles(JNIEnv *graalEnv) {
jGraalPicturesClass = (*graalEnv)->NewGlobalRef(graalEnv, (*graalEnv)->FindClass(graalEnv, "com/gluonhq/attach/pictures/impl/AndroidPicturesService"));
jGraalSendPhotoFileMethod = (*graalEnv)->GetStaticMethodID(graalEnv, jGraalPicturesClass, "setResult", "(Ljava/lang/String;I)V");
jGraalSendPhotoFileMethod = (*graalEnv)->GetStaticMethodID(graalEnv, jGraalPicturesClass, "setResult", "(Ljava/lang/String;)V");
}

void initializePicturesDalvikHandles() {
Expand Down Expand Up @@ -101,14 +101,14 @@ JNIEXPORT void JNICALL Java_com_gluonhq_attach_pictures_impl_AndroidPicturesServ
// From Dalvik to native //
///////////////////////////

JNIEXPORT void JNICALL Java_com_gluonhq_helloandroid_DalvikPicturesService_sendPhotoFile(JNIEnv *env, jobject service, jstring path, jint rotate) {
JNIEXPORT void JNICALL Java_com_gluonhq_helloandroid_DalvikPicturesService_sendPhotoFile(JNIEnv *env, jobject service, jstring path) {
if (isDebugAttach()) {
ATTACH_LOG_FINE("Send Photo File\n");
}
const char *pathChars = (*env)->GetStringUTFChars(env, path, NULL);
ATTACH_GRAAL();
jstring jpath = (*graalEnv)->NewStringUTF(graalEnv, pathChars);
(*graalEnv)->CallStaticVoidMethod(graalEnv, jGraalPicturesClass, jGraalSendPhotoFileMethod, jpath, rotate);
(*graalEnv)->CallStaticVoidMethod(graalEnv, jGraalPicturesClass, jGraalSendPhotoFileMethod, jpath);
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jpath is created as a local reference on the Graal JNI env (NewStringUTF) and is never deleted. In a loop (taking many photos), this can leak local refs and eventually overflow the local reference table. After CallStaticVoidMethod, explicitly DeleteLocalRef(graalEnv, jpath) (and consider also checking/clearing any pending exception).

Suggested change
(*graalEnv)->CallStaticVoidMethod(graalEnv, jGraalPicturesClass, jGraalSendPhotoFileMethod, jpath);
(*graalEnv)->CallStaticVoidMethod(graalEnv, jGraalPicturesClass, jGraalSendPhotoFileMethod, jpath);
(*graalEnv)->DeleteLocalRef(graalEnv, jpath);

Copilot uses AI. Check for mistakes.
DETACH_GRAAL();
// (*env)->ReleaseStringUTFChars(env, jpath, jpathChars);
(*env)->ReleaseStringUTFChars(env, path, pathChars);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2024, Gluon
* Copyright (c) 2020, 2026, Gluon
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -31,6 +31,9 @@
import android.app.Activity;
import android.content.Intent;
import android.database.Cursor;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Matrix;
import android.media.ExifInterface;
import android.media.MediaScannerConnection;
import android.net.Uri;
Expand All @@ -41,6 +44,7 @@
import android.util.Log;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
Expand All @@ -64,6 +68,10 @@ public class DalvikPicturesService {
private final String authority;
private String photoPath;

private static final int TARGET_SIZE = 1280;
private static final int JPEG_QUALITY = 85;
private static final byte[] DECODE_BUFFER = new byte[16 * 1024];

public DalvikPicturesService(Activity activity) {
this.activity = activity;
this.debug = Util.isDebug();
Expand Down Expand Up @@ -92,6 +100,7 @@ public void takePhoto(final boolean savePhoto) {
Log.v(TAG, "Permission verification failed: Camera disabled");
return;
}
clearCache();

Intent intent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE);

Expand Down Expand Up @@ -132,12 +141,16 @@ public void gotActivityResult (int requestCode, int resultCode, Intent intent) {
if (debug) {
Log.v(TAG, "Image file located at " + photoFile.getAbsolutePath() + " with rotation: " + imageRotation);
}
sendPhotoFile(photoFile.getAbsolutePath(), imageRotation);

if (savePhoto) {
// media scanner to rescan DIRECTORY_PICTURES after an image is saved/deleted
// Saved photos: keep the original file untouched in
// DIRECTORY_PICTURES, scan it into the gallery, and
// send a preprocessed cache copy for display.
MediaScannerConnection.scanFile(activity, new String[]{photoFile.toString()}, null, null);
photoFile = copyToCache(photoFile);
}
preprocessImage(photoFile, imageRotation);
sendPhotoFile(photoFile.getAbsolutePath());
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When savePhoto is true, this code switches photoFile to a cache copy and then sends that path to the Java layer. That means PicturesService.getImageFile() will now point at the display cache rather than the original saved image in DIRECTORY_PICTURES, which contradicts the PicturesService contract (it says the file returned for takePhoto(true) is the original). Consider sending both paths to the callback (original + display) or keeping imageFile set to the original while only using the cache copy for display loading.

Copilot uses AI. Check for mistakes.
} else {
Log.e(TAG, "Picture file doesn't exist for: " + photoFile.getAbsolutePath());
}
Expand All @@ -162,6 +175,8 @@ public void gotActivityResult (int requestCode, int resultCode, Intent intent) {
}

private void selectPicture() {
clearCache();

Comment on lines 179 to +181
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These clearCache() calls won’t actually clear all cached images created by this flow: copyFile() uses the gallery DISPLAY_NAME (often not ending in .jpg), and clearCache() only deletes files ending with .jpg. This can leave old cached images accumulating in the cache dir. Consider either (1) storing all display/preprocessed files under a known prefix (e.g., display_) and deleting by prefix, or (2) deleting all files created by this service regardless of extension (and also defensively handle listFiles() returning null).

Copilot uses AI. Check for mistakes.
Intent intent = new Intent(Intent.ACTION_GET_CONTENT);
intent.setFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
intent.setType("image/*");
Expand Down Expand Up @@ -193,7 +208,8 @@ public void gotActivityResult(int requestCode, int resultCode, Intent intent) {
if (debug) {
Log.v(TAG, "Image file located at " + cachePhotoFile.getAbsolutePath() + " with rotation: " + imageRotation);
}
sendPhotoFile(cachePhotoFile.getAbsolutePath(), imageRotation);
preprocessImage(cachePhotoFile, imageRotation);
sendPhotoFile(cachePhotoFile.getAbsolutePath());
}
}
}
Expand Down Expand Up @@ -232,7 +248,9 @@ private int getImageRotation(Uri uri) {
try {
ExifInterface ei;
if (Build.VERSION.SDK_INT > 23) {
ei = new ExifInterface(activity.getContentResolver().openInputStream(uri));
try (InputStream is = activity.getContentResolver().openInputStream(uri)) {
ei = new ExifInterface(is);
}
} else {
ei = new ExifInterface(uri.getPath());
}
Expand All @@ -258,7 +276,7 @@ private File copyFile(Uri uri) {
File selectedFile = new File(activity.getCacheDir(), getImageName(uri));
try (InputStream is = activity.getContentResolver().openInputStream(uri);
OutputStream os = new FileOutputStream(selectedFile)) {
byte[] buffer = new byte[8 * 1024];
byte[] buffer = new byte[32 * 1024];
int bytesRead;
while ((bytesRead = is.read(buffer)) != -1) {
os.write(buffer, 0, bytesRead);
Expand All @@ -271,7 +289,105 @@ private File copyFile(Uri uri) {
return selectedFile;
}

private File copyToCache(File source) {
File dest = new File(activity.getCacheDir(), "display_" + source.getName());
try (InputStream is = new FileInputStream(source); OutputStream os = new FileOutputStream(dest)) {
byte[] buffer = new byte[32 * 1024];
int bytesRead;
while ((bytesRead = is.read(buffer)) != -1) {
os.write(buffer, 0, bytesRead);
}
} catch (IOException ex) {
Log.e(TAG, "copyToCache failed: " + ex.getMessage());
return source; // fall back to original
}
return dest;
}

/**
* Scales and rotates the image file in place, with sub sampling and lower jpg quality,
* to reduce memory footprint
*/
private void preprocessImage(File imageFile, int rotation) {
try {
// 1. Read dimensions only
BitmapFactory.Options opts = new BitmapFactory.Options();
opts.inJustDecodeBounds = true;
opts.inTempStorage = DECODE_BUFFER;
BitmapFactory.decodeFile(imageFile.getAbsolutePath(), opts);

// 2. Calculate inSampleSize
int srcW = opts.outWidth;
int srcH = opts.outHeight;
if (rotation == 90 || rotation == 270) {
srcW = opts.outHeight;
srcH = opts.outWidth;
}
opts.inSampleSize = calculateInSampleSize(srcW, srcH, TARGET_SIZE);
opts.inJustDecodeBounds = false;
opts.inPreferredConfig = Bitmap.Config.RGB_565;
opts.inTempStorage = DECODE_BUFFER;

// 3. Load sub-sampled bitmap
Bitmap bitmap = BitmapFactory.decodeFile(imageFile.getAbsolutePath(), opts);
if (bitmap == null) {
Log.e(TAG, "preprocessImage: failed to decode bitmap");
return;
}

try {
// 4. Build a single Matrix for scale + rotate combined
Matrix matrix = new Matrix();
float scale = Math.min(
(float) TARGET_SIZE / bitmap.getWidth(),
(float) TARGET_SIZE / bitmap.getHeight());
if (scale < 1.0f) {
matrix.postScale(scale, scale);
}
if (rotation != 0) {
// Rotate around the center of the image
float cx = bitmap.getWidth() * Math.max(scale, 1.0f) / 2f;
float cy = bitmap.getHeight() * Math.max(scale, 1.0f) / 2f;
Comment on lines +347 to +353
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cx/cy are computed using Math.max(scale, 1.0f) even though scaling is only applied when scale < 1.0f. For images smaller than TARGET_SIZE (scale > 1), no scale is applied but the pivot becomes width*scale/2, so rotation happens around the wrong point and can produce shifted/cropped output. Pivot should be based on the bitmap’s actual dimensions (and, if scaling is applied, the pivot should be adjusted consistently with the applied transform).

Suggested change
if (scale < 1.0f) {
matrix.postScale(scale, scale);
}
if (rotation != 0) {
// Rotate around the center of the image
float cx = bitmap.getWidth() * Math.max(scale, 1.0f) / 2f;
float cy = bitmap.getHeight() * Math.max(scale, 1.0f) / 2f;
float appliedScale = scale < 1.0f ? scale : 1.0f;
if (appliedScale < 1.0f) {
matrix.postScale(appliedScale, appliedScale);
}
if (rotation != 0) {
// Rotate around the center of the image after any applied scaling
float cx = bitmap.getWidth() * appliedScale / 2f;
float cy = bitmap.getHeight() * appliedScale / 2f;

Copilot uses AI. Check for mistakes.
matrix.postRotate(rotation, cx, cy);
}

// 5. Apply combined transform
if (!matrix.isIdentity()) {
Bitmap transformed = Bitmap.createBitmap(bitmap, 0, 0,
bitmap.getWidth(), bitmap.getHeight(), matrix, true);
bitmap.recycle();
bitmap = transformed;
}

// 6. Write processed image back to file
try (FileOutputStream fos = new FileOutputStream(imageFile)) {
bitmap.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, fos);
}
if (debug) {
Log.v(TAG, "preprocessImage: wrote " + bitmap.getWidth() + "x" + bitmap.getHeight()
+ " (rotation=" + rotation + ") to " + imageFile.getName());
}
} finally {
bitmap.recycle();
}
} catch (Exception e) {
Log.e(TAG, "preprocessImage failed, falling back to original: " + e.getMessage());
}
}

private static int calculateInSampleSize(int width, int height, int targetSize) {
int inSampleSize = 1;
if (height > targetSize || width > targetSize) {
int halfH = height / 2;
int halfW = width / 2;
while ((halfH / inSampleSize) >= targetSize && (halfW / inSampleSize) >= targetSize) {
inSampleSize *= 2;
}
}
return inSampleSize;
}

// native
private native void sendPhotoFile(String filePath, int rotate);
private native void sendPhotoFile(String filePath);

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[
{
"name" : "com.gluonhq.attach.pictures.impl.AndroidPicturesService",
"methods":[{"name":"setResult","parameterTypes":["java.lang.String", "int"] }]
"methods":[{"name":"setResult","parameterTypes":["java.lang.String"] }]
}
]
Loading