From 57eff4524e81d27ae1d993eccb77b7ab90d80425 Mon Sep 17 00:00:00 2001 From: monsieurtanuki Date: Sat, 24 Jun 2023 19:10:14 +0200 Subject: [PATCH] fix: 4219 - check if new picture is big enough before server upload (#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 --- .../lib/background/background_task_image.dart | 60 ++++++++++++-- packages/smooth_app/lib/l10n/app_en.arb | 22 ++++++ packages/smooth_app/lib/pages/crop_page.dart | 78 ++++++++++++++++--- packages/smooth_app/pubspec.lock | 4 +- packages/smooth_app/pubspec.yaml | 2 +- 5 files changed, 148 insertions(+), 18 deletions(-) diff --git a/packages/smooth_app/lib/background/background_task_image.dart b/packages/smooth_app/lib/background/background_task_image.dart index f72be4164f8..23fc5b20cdf 100644 --- a/packages/smooth_app/lib/background/background_task_image.dart +++ b/packages/smooth_app/lib/background/background_task_image.dart @@ -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 addTask( final String barcode, { @@ -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 _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 _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( @@ -253,7 +296,12 @@ class BackgroundTaskImage extends BackgroundTaskUpload { Future 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; diff --git a/packages/smooth_app/lib/l10n/app_en.arb b/packages/smooth_app/lib/l10n/app_en.arb index c8a8ae96697..e36ef0cb43b 100644 --- a/packages/smooth_app/lib/l10n/app_en.arb +++ b/packages/smooth_app/lib/l10n/app_en.arb @@ -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" diff --git a/packages/smooth_app/lib/pages/crop_page.dart b/packages/smooth_app/lib/pages/crop_page.dart index e9b8f79db6c..4e4c7d0342b 100644 --- a/packages/smooth_app/lib/pages/crop_page.dart +++ b/packages/smooth_app/lib/pages/crop_page.dart @@ -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'; @@ -243,6 +244,61 @@ class _CropPageState extends State { } Future _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( + 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(); final DaoInt daoInt = DaoInt(localDatabase); final int sequenceNumber = @@ -255,7 +311,7 @@ class _CropPageState extends State { ); 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. @@ -312,21 +368,26 @@ class _CropPageState extends State { return croppedFile; } - Future _saveFileAndExit() async { + Future _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); + if (mounted) { + Navigator.of(context).pop(file); + } + return true; } + } catch (e) { + return false; } finally { _progress = null; } @@ -420,8 +481,7 @@ class _CropPageState extends State { } try { - await _saveFileAndExit(); - return true; + return _saveFileAndExit(); } catch (e) { if (mounted) { // not likely to happen, but you never know... diff --git a/packages/smooth_app/pubspec.lock b/packages/smooth_app/pubspec.lock index 4f026e42c6f..8e6bc3fcdef 100644 --- a/packages/smooth_app/pubspec.lock +++ b/packages/smooth_app/pubspec.lock @@ -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: diff --git a/packages/smooth_app/pubspec.yaml b/packages/smooth_app/pubspec.yaml index 2622b74c450..80d2c92e8c6 100644 --- a/packages/smooth_app/pubspec.yaml +++ b/packages/smooth_app/pubspec.yaml @@ -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