feat: flip + rotation #25

rodgomesc · 2024-01-29T10:57:18Z

No description provided.

mrousavy

Hey - this is a really interesting feature!

I know this is a draft PR, but I couldn't help and already leave a few thoughts on this, hope you don't mind 🙈

android/src/main/cpp/ResizePlugin.cpp

mrousavy · 2024-01-29T12:22:11Z

android/src/main/cpp/ResizePlugin.cpp

+      if (_rotatedBuffer == nullptr || _rotatedBuffer->getDirectSize() != argbSize) {
+          _rotatedBuffer = allocateBuffer(argbSize, "_rotatedBuffer");
+      }
+
+      FrameBuffer destination = {
+              .width = rotatedWidth,
+              .height = rotatedHeight,
+              .pixelFormat = PixelFormat::ARGB,
+              .dataType = DataType::UINT8,
+              .buffer = _rotatedBuffer,
+      };


I wonder if we can avoid the copy and do this in-memory? The size is always the same, only width height and bytesPerRow changes, no?

In vImage on iOS it can work in-memory. Might not be the case here for libyuv tho..

android/src/main/cpp/ResizePlugin.cpp

android/src/main/cpp/ResizePlugin.h

src/index.ts

mrousavy · 2024-01-29T12:27:26Z

Also JFYI; I will only merge PRs that work the same on both iOS and Android, so we need iOS support here as well. I can work on that at some point, but right now I think I have other priorities (VisionCamera Android preview stretching)

rodgomesc · 2024-03-11T11:32:21Z

.gitignore

@@ -13,6 +13,7 @@ jsconfig.json
 #
 build/
 *.pbxuser
+.xcode.env.local


This file should not be sent to GitHub, as Xcode creates it at build time

True, but it might break builds if it gets generated and the user doesn't know about it. So I tend to leave it in gitignore, because then the user sees that this file was generated (in git status / GitHub desktop overview)

mrousavy

Heyo Rodrigo - this is amazing, thank you so much for this PR!

I have a few code nitpicks first to make sure this is as error-proof as possible, and as nice-to-use as possible :)

mrousavy · 2024-03-11T11:56:35Z

.gitignore

@@ -13,6 +13,7 @@ jsconfig.json
 #
 build/
 *.pbxuser
+.xcode.env.local


True, but it might break builds if it gets generated and the user doesn't know about it. So I tend to leave it in gitignore, because then the user sees that this file was generated (in git status / GitHub desktop overview)

mrousavy · 2024-03-11T11:57:12Z

android/src/main/cpp/ResizePlugin.cpp

@@ -150,12 +150,78 @@ FrameBuffer ResizePlugin::cropARGBBuffer(vision::FrameBuffer frameBuffer, int x,
  return destination;
 }

+FrameBuffer ResizePlugin::flipARGBBuffer(FrameBuffer frameBuffer, bool flip) {


Let's rename this entire thing (function and flip parameter) to "mirror" instead of "flip". Let's use the term mirror everywhere in the codebase.

android/src/main/cpp/ResizePlugin.cpp

mrousavy · 2024-03-11T11:59:36Z

android/src/main/cpp/ResizePlugin.cpp

@@ -277,8 +343,8 @@ FrameBuffer ResizePlugin::convertBufferToDataType(FrameBuffer frameBuffer, DataT
 }

 jni::global_ref<jni::JByteBuffer> ResizePlugin::resize(jni::alias_ref<JImage> image, int cropX, int cropY, int cropWidth, int cropHeight,
-                                                       int scaleWidth, int scaleHeight, int /* PixelFormat */ pixelFormatOrdinal,
-                                                       int /* DataType */ dataTypeOrdinal) {
+                                                       int scaleWidth, int scaleHeight, int rotation, bool flip,


re my previous comment; rotation should also be rotationOrdinal - it should be an Enum in Kotlin.

And rename flip to mirror again

android/src/main/cpp/ResizePlugin.h

android/src/main/java/com/visioncameraresizeplugin/ResizePlugin.kt

mrousavy · 2024-03-11T12:03:12Z

example/src/App.tsx


 export default function App() {
  const permission = useCameraPermission();
-  const device = useCameraDevice('back');
+  const device = useCameraDevice('front');


please revert

mrousavy · 2024-03-11T12:03:20Z

example/src/App.tsx

@@ -35,6 +35,8 @@ export default function App() {
      },
      pixelFormat: 'rgba',
      dataType: 'uint8',
+      rotation: ROTATION.R90,
+      flip: true,


flip -> mirror

mrousavy · 2024-03-11T12:03:40Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "vision-camera-resize-plugin",
-  "version": "2.0.1",
+  "version": "2.1.0",


Please revert

mrousavy · 2024-03-11T12:06:24Z

src/index.ts

+export enum ROTATION {
+  R0 = 0,
+  R90 = 90,
+  R180 = 180,
+  R270 = 270,
+}


Yea, no let's not use JS enums here. Check out how I built the pixelFormat, this is a TS union instead of a JS enum. JS Enums just aren't that flexible.
I'm also not a big fan of the names here.

Let's try to do something like

rotate: '0deg' | '90deg' | '180deg' | '270deg'

Or

orientation: 'portrait' | 'landscape-right' | 'portrait-upside-down' | 'landscape-left'

where the latter defines the target orientation. As in, it makes sure to rotate the frame so it will later be in that exact orientation, no matter what input orientation it currently is in. Get what I mean?
Like if it currently is in portrait, I want it to be in landscape-left, it will rotate by however degrees it should to get to that orientation.

I think that makes more sense, no?

rodgomesc · 2024-03-11T12:11:05Z

True, but it might break builds if it gets generated and the user doesn't know about it. So I tend to leave it in gitignore, because then the user sees that this file was generated (in git status / GitHub desktop overview)

sorry i'm a bit confuse on what's your suggestion here, i'm my case the xcode build only finished successfully after i deleted this file

mrousavy · 2024-03-11T12:13:28Z

True, but it might break builds if it gets generated and the user doesn't know about it. So I tend to leave it in gitignore, because then the user sees that this file was generated (in git status / GitHub desktop overview)

sorry i'm a bit confuse on what's your suggestion here, i'm my case the xcode build only finished successfully after i deleted this file

Yea so this is a it weird - it seems like CocoaPods will generate this file, with an invalid path to node.

Once you try to build the Xcode project after installing pods, it will fail.

When the file is in .gitignore, the user (or developer; one of us) will never know why the build failed, it will be hard to pinpoint it to that specific file.

If we instead leave this file in git, we will notice that this file was created because it will show up in git status, then we'll be like ahh yea we need to delete that.

'ya know? It's kind of a weird problem but better to leave it in than silently fail

rodgomesc · 2024-03-11T14:02:04Z

When the file is in .gitignore, the user (or developer; one of us) will never know why the build failed, it will be hard to pinpoint it to that specific file.

the error (image bellow) is not a easy spot, and not descriptive at all i spent at least 40 min trying to figure out why my build was failing, would be nice if we can avoid this from happing with anyone else so i'm clarifying some things to make sure we are on the same page:

leaving the xcode.env.local with your node location will fail the build for everyone else except for you
deleting it will build the app successfuly for you and everyone else
react-native template itself has it on gitignore
as peer docs this file overrides the local config

if you want to reproduce this

replace the xcode.env.local with my node location and try to build ios app

export NODE_BINARY=/Users/rodrigogomes/.nvm/versions/node/v18.2.0/bin/node

rodgomesc · 2024-03-12T13:04:58Z

full disclosure: I don't want to be a nuisance here, I'm only insisting because I think what's happening isn't about you agreeing or disagreeing with the xcode.env.local thing, but rather about us still not being on the same page regarding what's going on

mrousavy · 2024-03-12T13:48:31Z

Yes I know that this file makes the build break - I tend to not put it into the gitignore so I can always see in my git status that it has been created. In reality, this should not be created I think. This is a pod install side-effect. But we can leave it in, no worries.

mrousavy requested changes Jan 29, 2024

View reviewed changes

rodgomesc force-pushed the feat/rotation branch 2 times, most recently from 92d73e2 to 083914f Compare January 31, 2024 12:18

rodgomesc changed the title ~~wip: implement android rotation support~~ feat: flip + rotation Mar 11, 2024

rodgomesc force-pushed the feat/rotation branch 6 times, most recently from 13093a9 to 690f60c Compare March 11, 2024 11:21

rodgomesc added 2 commits March 11, 2024 08:23

feat: implement android rotation support

0119bf1

feat: implement flip

211ed42

rodgomesc force-pushed the feat/rotation branch 2 times, most recently from 19d8e85 to ab2f2bc Compare March 11, 2024 11:25

rodgomesc commented Mar 11, 2024

View reviewed changes

rodgomesc requested a review from mrousavy March 11, 2024 11:34

rodgomesc marked this pull request as ready for review March 11, 2024 11:35

rodgomesc force-pushed the feat/rotation branch from a77ff1e to ab460af Compare March 11, 2024 11:39

mrousavy requested changes Mar 11, 2024

View reviewed changes

rodgomesc force-pushed the feat/rotation branch 8 times, most recently from 3ed2cf6 to 39c8c85 Compare March 11, 2024 13:57

rodgomesc requested a review from mrousavy March 11, 2024 13:58

feat: implement flip and rotate for ios

42c2344

rodgomesc force-pushed the feat/rotation branch from 39c8c85 to 42c2344 Compare March 11, 2024 14:33

fix: Format

afda1b0

mrousavy merged commit 9280118 into mrousavy:main Mar 12, 2024
7 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: flip + rotation #25

feat: flip + rotation #25

rodgomesc commented Jan 29, 2024

mrousavy left a comment

mrousavy Jan 29, 2024

mrousavy commented Jan 29, 2024

rodgomesc Mar 11, 2024 •

edited

Loading

mrousavy Mar 11, 2024

mrousavy left a comment

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

mrousavy Mar 11, 2024

rodgomesc commented Mar 11, 2024

mrousavy commented Mar 11, 2024

rodgomesc commented Mar 11, 2024 •

edited

Loading

rodgomesc commented Mar 12, 2024

mrousavy commented Mar 12, 2024

feat: flip + rotation #25

feat: flip + rotation #25

Conversation

rodgomesc commented Jan 29, 2024

mrousavy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrousavy commented Jan 29, 2024

rodgomesc Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrousavy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodgomesc commented Mar 11, 2024

mrousavy commented Mar 11, 2024

rodgomesc commented Mar 11, 2024 • edited Loading

rodgomesc commented Mar 12, 2024

mrousavy commented Mar 12, 2024

rodgomesc Mar 11, 2024 •

edited

Loading

rodgomesc commented Mar 11, 2024 •

edited

Loading