Closed efstathiosntonas closed 5 months ago
Hi there, the warning is meant to be a harmless indication that the crop dimensions are bigger than the size the image is occupying in the screen, it does not affect the functionality as far as I am concerned, however before I release the new version with the fix I'd like to know if you had success cropping the image after the small >=
change, because everything seems working fine on my end.
hi Glazzes, yes, after the small fix everything is fine. Thanks for the update.
/offtopic: It seems that sometimes the image won’t load or if it loads I get a weird blurred image instead. It seems like it’s one render behind, the moment I make a small change and fast refresh kicks in, the image shows up. (I’m using the cropper). Also on android, if I don’t set image height/ width among flex:1 the preview image quality is degraded by a lot.
I saw the same problems with the image in the issue, sadly that's something I've no control over, I tried the image and it was rendered properly using {width: '100%', height: '100%'}
, also remember image scaling behaviour on Android depends on what RN thinks is best, so set Image's resizeMethod
property to scale
.
Give it a try and let me know.
Just released version 1.2.2, give it a try!
@Glazzes just installed 1.2.2, I appliedresizeMethod='scale'
and Android seems fine, now it started on iOS.
See how the image (same image as above) looks like on both Platforms, it seems like full rendering in the image is somehow "stopped" half way there. I'm using the latest reanimated@3.11.0, gh@2.16.2:
I get the crash again on Android only in a different area this time, same image as the issue, same snippet as above:
@efstathiosntonas Could you hand me the code of what you're currently building? This way I can investigate further, btw remove flex: 1
from styles, it's either flex or width and height, not both.
@Glazzes here are the components:
ImageCropper
import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from "react";
import { Image, View } from "react-native";
import { useIsFocused } from "@react-navigation/native";
import { Action, FlipType, manipulateAsync } from "expo-image-manipulator";
import { OnboardingScreensProps } from "navigation/types";
import { useTranslation } from "react-i18next";
import { createStyleSheet, UnistylesRuntime, useStyles } from "react-native-unistyles";
import {
CropZoom,
CropZoomType,
ScaleMode,
useImageResolution
} from "react-native-zoom-toolkit";
import { CustomHeader } from "@components/BaseTexts";
import Button from "@components/Buttons/Button";
import { CloseButtonHeader } from "@components/NavigationBars/NavigationBar/components";
import CropOverlay from "@flows/camera/screens/CropImage/CropOverlay";
import { ms } from "@utils/size-matters";
import { dimensions, isIOS } from "@utils/utils";
const cropSize = ms(dimensions.width * 0.8);
const ImageCropper = ({ route, navigation }: OnboardingScreensProps<"ImageCropper">) => {
const { data, onAddMedia } = route.params;
const { styles, theme } = useStyles(stylesheet);
const { t } = useTranslation(["profile_tab", "common"]);
const ref = useRef<CropZoomType>(null);
const imageUri = data.uri;
const isFocused = useIsFocused();
const { resolution } = useImageResolution({ uri: imageUri });
useEffect(() => {
if (isFocused) {
UnistylesRuntime.navigationBar.setColor("#000000");
UnistylesRuntime.statusBar.setColor("#000000");
}
return () => {
UnistylesRuntime.navigationBar.setColor(theme.colors.neutrals["0"]);
UnistylesRuntime.statusBar.setColor(theme.colors.neutrals["0"]);
};
}, [isFocused, theme.colors.neutrals]);
const renderOverlay = useCallback(() => {
return <CropOverlay />;
}, []);
const done = useCallback(
(image: any) => {
onAddMedia(image);
navigation.goBack();
},
[navigation, onAddMedia]
);
useLayoutEffect(() => {
navigation.setOptions({
headerLeft: () => (
<CloseButtonHeader
accessibilityHint={t("crop_image_screen.accessibility.buttons.close.hint")}
accessibilityLabel={t("crop_image_screen.accessibility.buttons.close.label")}
isCropImage
onPress={() => navigation.goBack()}
/>
)
});
}, [navigation, styles.doneText, t, theme.white]);
const setCrop = useCallback(() => {
if (ref.current === null) {
return;
}
const cropResult = ref.current.crop(200);
const actions: Action[] = [];
if (cropResult.resize !== undefined) {
actions.push({ resize: cropResult.resize });
}
if (cropResult.context.flipHorizontal) {
actions.push({ flip: FlipType.Horizontal });
}
if (cropResult.context.flipVertical) {
actions.push({ flip: FlipType.Vertical });
}
if (cropResult.context.rotationAngle !== 0) {
actions.push({ rotate: cropResult.context.rotationAngle });
}
actions.push({ crop: cropResult.crop });
manipulateAsync(data?.uri?.[0]?.uri ? data.uri[0].uri : data.uri, actions).then(
(manipulationResult) => {
done(manipulationResult.uri);
}
);
}, [data.uri, done]);
if (resolution === undefined) {
return null;
}
return (
<View style={styles.container}>
<CustomHeader
color="#fff"
hx="h4"
style={styles.header}
text={t("crop_image_screen.move_and_zoom_to_reframe")}
weight="eb"
/>
<View style={styles.innerContainer}>
<CropZoom
OverlayComponent={renderOverlay}
cropSize={{ width: cropSize, height: 390 }}
ref={ref}
resolution={resolution}
scaleMode={ScaleMode.BOUNCE}
>
<Image source={{ uri: imageUri }} style={styles.image} resizeMethod="scale" />
</CropZoom>
</View>
<Button
hint="test"
onPress={setCrop}
size="m"
style={styles.button}
title="Add photo"
type="secondary"
/>
</View>
);
};
const stylesheet = createStyleSheet((theme, runtime) => ({
container: {
flex: 1,
backgroundColor: "#000000"
},
header: {
marginHorizontal: theme.spacing.s16,
marginTop: theme.spacing.mvs24,
zIndex: 9999
},
innerContainer: {
marginHorizontal: theme.spacing.s16,
marginTop: theme.spacing.mvs24,
flex: 1
},
cropButtonWrapper: {
bottom: isIOS ? runtime.insets.bottom + theme.spacing.mvs4 : theme.spacing.mvs24,
flex: 1,
flexDirection: "row",
justifyContent: "space-around",
left: 0,
marginTop: theme.spacing.mvs4,
position: "absolute",
right: 0
},
cropViewContainer: {
paddingBottom: theme.spacing.mvs14,
borderRadius: theme.spacing.mvs20,
borderColor: "#D9D9D9",
borderWidth: theme.spacing.s2,
height: theme.spacing.mvs370
},
doneText: {
color: theme.white,
...theme.typography.paragraph.large.semibold
},
image: {
width: "100%",
height: "100%"
},
button: {
alignSelf: "stretch",
marginBottom: runtime.insets.bottom,
marginHorizontal: theme.spacing.s16,
zIndex: 99999
}
}));
export default ImageCropper;
CropOverlay
import React from "react";
import Svg, { Defs, Mask, Rect } from "react-native-svg";
import { dimensions } from "@utils/utils";
const { width, height } = dimensions;
const CropOverlay: React.FC = () => {
const overlayWidth = width * 0.8;
const overlayHeight = 390;
const overlayX = (width - overlayWidth) / 2;
const overlayY = (height - overlayHeight) / 2;
const borderRadius = 20;
return (
<Svg height={height} width={width}>
<Defs>
<Mask height={height} id="mask" width={width} x="0" y="0">
<Rect fill="white" height={height} width={width} x={0} y={0} />
<Rect
fill="black"
height={overlayHeight}
rx={borderRadius}
ry={borderRadius}
width={overlayWidth}
x={overlayX}
y={overlayY}
/>
</Mask>
</Defs>
<Rect
fill="rgba(0, 0, 0, 0.8)" // Overlay color and opacity
height={height}
mask="url(#mask)"
width={width}
x={0}
y={0}
/>
<Rect
fill="none"
height={overlayHeight}
rx={borderRadius}
ry={borderRadius}
stroke="#9D9A98"
strokeWidth={4}
width={overlayWidth}
x={overlayX}
y={overlayY}
/>
</Svg>
);
};
export default CropOverlay;
ms function:
import { PixelRatio } from "react-native";
import memoize from "fast-memoize";
import { dimensions } from "@utils/utils";
const { width, height } = dimensions;
const [shortDimension, longDimension] =
width < height ? [width, height] : [height, width];
const guidelineBaseWidth = 390;
const guidelineBaseHeight = 844;
const scale = memoize((size) =>
PixelRatio.roundToNearestPixel((shortDimension / guidelineBaseWidth) * size)
);
const verticalScale = memoize((size) =>
PixelRatio.roundToNearestPixel((longDimension / guidelineBaseHeight) * size)
);
const moderateScale = memoize((size, factor = 0.5) =>
PixelRatio.roundToNearestPixel(size + (scale(size) - size) * factor)
);
const moderateVerticalScale = memoize((size, factor = 0.5) =>
PixelRatio.roundToNearestPixel(size + (verticalScale(size) - size) * factor)
);
export const s = scale;
export const vs = verticalScale;
export const ms = moderateScale;
export const mvs = moderateVerticalScale;
ps. I think it would be nice to add the rectangle SVG too among the circle one in the example, not everyone wants to bloat the app with 6MB by using Skia 😅
ps2: even though the preview image is blurred, the cropping works fine and the cropped image is fine too.
It turns out that the moment I set the width to cropSize={{ width: 390, height: 390 }}
the blur went away. Seems like a calculation issue (or these 2 values needs to be the same as per example), the ms(width * 0.8)
produces 315.6666666666667
(iPhone 15 simulator) and something goes off.
What I tried:
@efstathiosntonas Hi there, just to give you an update, dealing with square content has proven to be more annonying than I expected, it may take a few days for me to figure out the math to make the square content fit properly in crop sizes which aspect ratio is not one, I was working on it yesterday and it did work as I expected but it broke the cropper entirely, so yeah pretty much that for the moment.
Bare with me.
no worries, keep it up! These maths are hard as f 😄
@efstathiosntonas Hi there, just to give you an update on how this thing is going, last days have been the wrost of my life in terms of mental health so you'll have to wait for a little longer.
Sizing right now is working really good, better than before, however the cropper is still fully broken when using the fixedWidth
argument, pretty much this is how they look now for all three different aspect ratios, while using at the same time the three different aspect ratios on the imgaes, one per each ratio.
Square | Portrait | Landscape |
---|---|---|
thanks for the heads up!
no worries at all, mental health is above all!
I just finished the problem with the cropper entirely this time (I hope), the previous implementation for the cropping of "uneven" aspect ratios has been fully rewritten, just like you saw in the videos in my last comment, the warning is no longer neccesary and in general it comes to a shorter implementation leading to a small size improvement aswell.
Before plubishing I'd like you to give try a it first, in theory you need to go to your project's node-modules/react-native-zoom-toolkit
folder and replace lib
and src
folders with the contents of the tarball below
lib.tar.gz
There's also an issue which comes with these new changes (at the moment), the order of the actions for this work has changed to the following one:
const actions: Action[] = [];
if (cropResult.context.flipHorizontal) {
actions.push({ flip: FlipType.Horizontal });
}
if (cropResult.context.flipVertical) {
actions.push({ flip: FlipType.Vertical });
}
if (cropResult.context.rotationAngle !== 0) {
actions.push({ rotate: cropResult.context.rotationAngle });
}
if (cropResult.resize !== undefined) {
actions.push({ resize: cropResult.resize });
}
actions.push({ crop: cropResult.crop });
Let me know how it goes!
@Glazzes thank you for all your effort in this one.
It is much better but I still encounter the blurred image issue. The first image from Library loads fine and I'm able to crop.
When I pick a second image then the image is blurred. When I tap save button to crop it throws this error from ImageManipulator
, using const cropSize = dimensions.width * 0.85;
:
Error: Invalid crop options has been passed. Please make sure the requested crop rectangle is inside source image
(^^ previously it was saving the image). In previous version it could get unstuck from the blurry image if I trigger a Fast Refresh, now it won't work.
ImageManipulator crop checks on ios:
let rect = crop.toRect()
let isOutOfBounds = rect.origin.x > image.size.width
|| rect.origin.y > image.size.height
|| rect.width > image.size.width
|| rect.height > image.size.height
guard !isOutOfBounds else {
throw ImageInvalidCropException()
}
this is the cropResult, image original width is 2000px, on resize it's 2994. Problem is on crop.ts
width * fixer
:
{"context": {"flipHorizontal": false, "flipVertical": false, "rotationAngle": 0}, "crop": {"height": 1999.1726027397262, "originX": 163.2466661946392, "originY": -4.102269362667166, "width": 2000}, "resize": {"height": 1997, "width": 2994}
I copied the src/lib
to /react-native-zoom-toolkit
, killed metro and runned it again with --reset-cache
.
edit: I've also noticed this weird behavior:
When the image is blurry, if I remove the source={{ uri: imageUri }}
from the Image
component and fast refresh then the image is still blurry. Seems like something is frozen and keeps reference, maybe reanimated/gh 🤔 ?
@efstathiosntonas That's really an odd behaviour, could you please hand me the exact same numbers you're using for the cropszie property? I just tested with a 2000x2000 picture and it was cropped fine.
@Glazzes been using these 2 images on iOS Simulator 17.5 and rn 0.74.2, reanimated@3.12.0, rngh@2.16.2, expo-image-manipulator@12.0.5:
gist with the code I use: https://gist.github.com/efstathiosntonas/7727b8e84f221a534446e51249bd269b
flow: I pick the image from Library and I navigate to the ImageCropper screen. The moment it mounts the image is blurry. It's a hit or miss, sometimes they load fine some other they don't :/
I switched to expo-image and the blur is gone. That's really weird, do you think rn Image cache might be causing this?
@efstathiosntonas Whatever I tell you may be a lie as I'm not sure why it happens, all I know is that RN has a lot non sense stuff I will never be able to comprehend, one just has to flow with them against one's desires, I'm happy the blur went it away nonetheless.
In terms of the crash I made a few adjustments to the lib, everything seems fine here lib.tar.gz , the order was fixed to be the one in the docs again https://glazzes.github.io/react-native-zoom-toolkit/guides/cropzoomexpo.html#crop-method, sorry for the inconvenient, give it a try.
will test it in a while and update.
Have you tried using a fixedWidth in the cropResult? ie:
const cropResult = ref.current.crop(2000)
will test it in a while and update.
Have you tried using a fixedWidth in the cropResult? ie:
const cropResult = ref.current.crop(2000)
Yes, with and without the fixedWidth
parameter.
@Glazzes everything works as a charm. Thank you for your effort on this one, it was really tricky 😅
@efstathiosntonas It's good to know it works, even the simplest ideas can turn into tricky and insufferable thoughts when some level of Math is involved in particular for a self taught like myself, however your issue has been quite fun to work with, I would have not thought of changing the way the image renders without looking at what you're attempting to create, so for that all I can say is thanks.
I think the new version may be published by the end of this day, with this outcome I consider this issue to be closed for good now.
I avoid this kind of math like the plague, my mental health is above all hahaha! Glad I could help to make this library even better, even without touching the code 🤪
I think you should add extensive comments in the code, I tried to dive into the code but after seeing the complexity I quietly backed up
Yeah, that's something to work on the near future, however it may be a difficukt to get right as I've been forced to apply a lot of tricks to achieve simple things which are difficult to achieve by a simple usage of GH, for instance: When you're pinching using any of the components offered by this library, what's really being pinched is not even being animated and its always one step behind of what you do.
Summary
Hi, thank you for creating this module.
I'm encountering this error when the image is a square eg. 1280x1280:
When I change the height:
https://github.com/Glazzes/react-native-zoom-toolkit/blob/2d34962f1f741243a4b6797d2fe834854e218a8d/src/commons/utils/getCropRotatedSize.ts#L23-L27
to:
aspectRatio >= 1 ? maxHeight : undefined
The image loads fine and I get this warning:
I tried it with this image:
Click me
![7515495b-982d-44d2-9931-5a8bbbf27532](https://github.com/Glazzes/react-native-zoom-toolkit/assets/717975/8e798de8-6bf0-49ce-a771-7068cfbd97bd)I'm not doing something fancy, just followed the common /example
ps. I'm also not rotating or trying to rotate the image.
Environment
rn 0.74.1
Steps to Reproduce
No response