Skip to content

Commit

Permalink
fix: 4219 - check if new picture is big enough before server upload (#…
Browse files Browse the repository at this point in the history
…4224)

Impacted files:
* `app_en.arb`: added 2 labels for a "too small image" dialog!
* `background_task_image.dart`: now we check before server call if the image is big enough
* `crop_page.dart`: now we check if the image is big enough before new image upload
* `pubspec.lock`: wtf
* `pubspec.yaml`: upgraded crop_image for controller bug fix
  • Loading branch information
monsieurtanuki authored Jun 24, 2023
1 parent 95c3a67 commit 57eff45
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 18 deletions.
60 changes: 54 additions & 6 deletions packages/smooth_app/lib/background/background_task_image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ class BackgroundTaskImage extends BackgroundTaskUpload {
return result;
}

// cf. https://github.com/openfoodfacts/smooth-app/issues/4219
// TODO(monsieurtanuki): move to off-dart
static const int minimumWidth = 640;
static const int minimumHeight = 160;

static bool isPictureBigEnough(final num width, final num height) =>
width >= minimumWidth || height >= minimumHeight;

/// Adds the background task about uploading a product image.
static Future<void> addTask(
final String barcode, {
Expand Down Expand Up @@ -213,19 +221,54 @@ class BackgroundTaskImage extends BackgroundTaskUpload {
/// Conversion factor to `int` from / to UI / background task.
static const int cropConversionFactor = 1000000;

/// Returns true if a cropped operation is needed - after having performed it.
Future<bool> _crop(final File file) async {
/// Returns true if a crop operation is needed - after having performed it.
///
/// Returns false if no crop operation is needed.
/// Returns null if the image (cropped or not) is too small.
Future<bool?> _crop(final File file) async {
final ui.Image full = await loadUiImage(await File(fullPath).readAsBytes());
if (cropX1 == 0 &&
cropY1 == 0 &&
cropX2 == cropConversionFactor &&
cropY2 == cropConversionFactor &&
rotationDegrees == 0) {
if (!isPictureBigEnough(full.width, full.height)) {
return null;
}
// in that case, no need to crop
return false;
}
final ui.Image full = await loadUiImage(
await File(fullPath).readAsBytes(),
);

Size getCroppedSize() {
final Rect cropRect = getResizedRect(
Rect.fromLTRB(
cropX1.toDouble(),
cropY1.toDouble(),
cropX2.toDouble(),
cropY2.toDouble(),
),
1 / cropConversionFactor,
);
switch (CropRotationExtension.fromDegrees(rotationDegrees)!) {
case CropRotation.up:
case CropRotation.down:
return Size(
cropRect.width * full.height,
cropRect.height * full.width,
);
case CropRotation.left:
case CropRotation.right:
return Size(
cropRect.width * full.width,
cropRect.height * full.height,
);
}
}

final Size croppedSize = getCroppedSize();
if (!isPictureBigEnough(croppedSize.width, croppedSize.height)) {
return null;
}
final ui.Image cropped = await CropController.getCroppedBitmap(
crop: getResizedRect(
Rect.fromLTRB(
Expand Down Expand Up @@ -253,7 +296,12 @@ class BackgroundTaskImage extends BackgroundTaskUpload {
Future<void> upload() async {
final String path;
final String croppedPath = _getCroppedPath();
if (await _crop(File(croppedPath))) {
final bool? neededCrop = await _crop(File(croppedPath));
if (neededCrop == null) {
// TODO(monsieurtanuki): maybe something more refined when we dismiss the picture, like alerting the user, though it's not supposed to happen anymore from upstream.
return;
}
if (neededCrop) {
path = croppedPath;
} else {
path = fullPath;
Expand Down
22 changes: 22 additions & 0 deletions packages/smooth_app/lib/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,28 @@
"@crop_page_action_local": {
"description": "Action being performed on the crop page"
},
"crop_page_too_small_image_title": "The image is too small!",
"@crop_page_too_small_image_title": {
"description": "Title of a dialog warning the user that the image is too small for upload"
},
"crop_page_too_small_image_message": "The minimum size in pixels for picture upload is {expectedMinWidth}x{expectedMinHeight}. The current picture is {actualWidth}x{actualHeight}.",
"@crop_page_too_small_image_message": {
"description": "Message of a dialog warning the user that the image is too small for upload",
"placeholders": {
"expectedMinWidth": {
"type": "int"
},
"expectedMinHeight": {
"type": "int"
},
"actualWidth": {
"type": "int"
},
"actualHeight": {
"type": "int"
}
}
},
"crop_page_action_server": "Preparing a call to the server…",
"@crop_page_action_server": {
"description": "Action being performed on the crop page"
Expand Down
78 changes: 69 additions & 9 deletions packages/smooth_app/lib/pages/crop_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:smooth_app/data_models/continuous_scan_model.dart';
import 'package:smooth_app/database/dao_int.dart';
import 'package:smooth_app/database/local_database.dart';
import 'package:smooth_app/generic_lib/design_constants.dart';
import 'package:smooth_app/generic_lib/dialogs/smooth_alert_dialog.dart';
import 'package:smooth_app/generic_lib/loading_dialog.dart';
import 'package:smooth_app/helpers/database_helper.dart';
import 'package:smooth_app/helpers/image_compute_container.dart';
Expand Down Expand Up @@ -243,6 +244,61 @@ class _CropPageState extends State<CropPage> {
}

Future<File?> _saveFileAndExitTry() async {
final AppLocalizations appLocalizations = AppLocalizations.of(context);

// only for new image upload we have to check the minimum size.
if (widget.imageId == null) {
// Returns the size of the resulting cropped image.
Size getCroppedSize() {
switch (_controller.rotation) {
case CropRotation.up:
case CropRotation.down:
return Size(
_controller.crop.width * _image.width,
_controller.crop.height * _image.height,
);
case CropRotation.left:
case CropRotation.right:
return Size(
_controller.crop.width * _image.height,
_controller.crop.height * _image.width,
);
}
}

final Size croppedSize = getCroppedSize();
if (!BackgroundTaskImage.isPictureBigEnough(
croppedSize.width,
croppedSize.height,
)) {
final int width = croppedSize.width.floor();
final int height = croppedSize.height.floor();
await showDialog<void>(
context: context,
builder: (BuildContext context) => SmoothAlertDialog(
title: appLocalizations.crop_page_too_small_image_title,
body: Text(
appLocalizations.crop_page_too_small_image_message(
BackgroundTaskImage.minimumWidth,
BackgroundTaskImage.minimumHeight,
width,
height,
),
),
actionsAxis: Axis.vertical,
positiveAction: SmoothActionButton(
text: appLocalizations.okay,
onPressed: () => Navigator.of(context).pop(),
),
),
);
return null;
}
}

if (!mounted) {
return null;
}
final LocalDatabase localDatabase = context.read<LocalDatabase>();
final DaoInt daoInt = DaoInt(localDatabase);
final int sequenceNumber =
Expand All @@ -255,7 +311,7 @@ class _CropPageState extends State<CropPage> {
);

setState(
() => _progress = AppLocalizations.of(context).crop_page_action_server,
() => _progress = appLocalizations.crop_page_action_server,
);
if (widget.imageId == null) {
// in this case, it's a brand new picture, with crop parameters.
Expand Down Expand Up @@ -312,21 +368,26 @@ class _CropPageState extends State<CropPage> {
return croppedFile;
}

Future<void> _saveFileAndExit() async {
Future<bool> _saveFileAndExit() async {
setState(
() => _progress = AppLocalizations.of(context).crop_page_action_saving,
);
try {
final File? file = await _saveFileAndExitTry();
_progress = null;
if (!mounted) {
return;
}
if (file == null) {
setState(() {});
if (mounted) {
setState(() {});
}
return false;
} else {
Navigator.of(context).pop<File>(file);
if (mounted) {
Navigator.of(context).pop<File>(file);
}
return true;
}
} catch (e) {
return false;
} finally {
_progress = null;
}
Expand Down Expand Up @@ -420,8 +481,7 @@ class _CropPageState extends State<CropPage> {
}

try {
await _saveFileAndExit();
return true;
return _saveFileAndExit();
} catch (e) {
if (mounted) {
// not likely to happen, but you never know...
Expand Down
4 changes: 2 additions & 2 deletions packages/smooth_app/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ packages:
dependency: "direct main"
description:
name: crop_image
sha256: "0b5b939106eb71cbb66d2b97e56d9ec3469d88b12324143439bf0911e4c9bf11"
sha256: "2c70294bdcb0f2fd8b7d660535b314eb5d2477000057e1f30db8295d1588c422"
url: "https://pub.dev"
source: hosted
version: "1.0.6"
version: "1.0.10"
cross_file:
dependency: transitive
description:
Expand Down
2 changes: 1 addition & 1 deletion packages/smooth_app/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ dependencies:
flutter_native_splash: 2.2.19
image: ^4.0.17
auto_size_text: 3.0.0
crop_image: 1.0.6
crop_image: 1.0.10
shared_preferences: 2.1.1
intl: 0.18.0
collection: 1.17.1
Expand Down

0 comments on commit 57eff45

Please sign in to comment.