Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating the native code to rust #5

Open
dhikshithrm opened this issue Aug 4, 2023 · 17 comments
Open

Migrating the native code to rust #5

dhikshithrm opened this issue Aug 4, 2023 · 17 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@dhikshithrm
Copy link
Member

One fell swoop for (#1 & #2 ),

#1, Rust and flutter_rust_bridge has support for web through WASM, probably there is a way to target current C code to compile for WASM. there is no official implementation for WASM in ffi or ffigen.

#2, this issues arose because MSVC does not support c99 standard which includes the Variable Length Arrays(VLA's), i'm not a expert not them to try and fight the compiler to get this feature working, i tried alloaca, but still no result.

I'm thinking Implementing encoder and the decoder in Rust will solve the above two issues, so i think i'm gonna need some time to learn rust ( heard good things, but did not work with rust before).

Any help in migrating to rust will be of a great contribution to this project.

Thank you.

@dhikshithrm dhikshithrm added enhancement New feature or request help wanted Extra attention is needed labels Aug 4, 2023
@dhikshithrm dhikshithrm pinned this issue Aug 4, 2023
@Sufilevy
Copy link

Sufilevy commented Aug 4, 2023

I will be happy to help!

@geofmureithi
Copy link

How is flutter's support for WASM?
I am willing to help if you need any input regarding WASM.

@Sufilevy
Copy link

Sufilevy commented Aug 5, 2023

From what I know, Flutter is only now getting full support for WASM, so right now it's not perfect.

@geofmureithi
Copy link

@Sufilevy
I think you are confusing WASM as a compilation target and executing WASM modules.
Tools like wasmtime are already available in the dart ecosystem.
https://github.com/juancastillo0/wasm_run#usage

@dhikshithrm
Copy link
Member Author

Thanks for your attention.

Problem: #1 Web support is missing, and #2 MSVC compiler does not support c99 standard, which this native part uses a feature of c99 (VLA's).

Solution Proposal: Use rust on the native part, and use flutter_rust_bridge which is cross platform.

#1, Rust and flutter_rust_bridge has support for web through WASM, probably there is a way to target current C code to compile for WASM. there is no official implementation for WASM in ffi or ffigen.

as mentioned above flutter_rust_bridge is an awesome community package that is cross platform and supports web ( so i'm assuming it's targeting rust for WASM ).

the help required is with migrating native part from C to rust, and making it work with flutter_rust_bridge on all platforms (incl. Web & Windows).

here is a quick video on how to use integrate rust and flutter on youtube using flutter_rust_bridge, docs for the web setup.

I'm confident if we follow the docs for flutter_rust_bridge we can achieve cross-platform with web-support using WASM, because i've seen some other packages on pub.dev do such a thing.

If you decide to help this project, please ask any further questions on this thread.

note: I'm not an expert in rust or wasm, but i'll try to get you help in any way possible.

@geofmureithi
Copy link

@geofmureithi
Copy link

Let me try and play around and see how it goes. My Rust and WASM is solid but its long since I used Flutter.
Let me get back on Monday hopefully after playing around.

@dhikshithrm
Copy link
Member Author

dhikshithrm commented Aug 5, 2023

Thanks @geofmureithi, considering your Rust knowledge, i'd suggest to implement these 5 functions in Rust by referencing how it's done in ./src/blurhash_ffi.c implementation with similar params and return values as mentioned in the comments above each function definition.

/*
	encode : Returns the blurhash string for the given image
	This function returns a string containing the BlurHash. This memory is managed by the function, and you should not free it.
	It will be overwritten on the next call into the function, so be careful!
	Parameters : 
		`xComponents` - The number of components in the X direction. Must be between 1 and 9. 3 to 5 is usually a good range for this.
		`yComponents` - The number of components in the Y direction. Must be between 1 and 9. 3 to 5 is usually a good range for this.
		`width` - The width in pixels of the supplied image.
		`height` - The height in pixels of the supplied image.
		`rgb` - A pointer to the pixel data. This is supplied in RGB order, with 3 bytes per pixels.
		`bytesPerRow` - The number of bytes per row of the RGB pixel data.
*/

FFI_PLUGIN_EXPORT
const char *blurHashForPixels(int xComponents, int yComponents, int width, int height, uint8_t *rgb, size_t bytesPerRow);

/*
	decode : Returns the pixel array of the result image given the blurhash string,
	Parameters : 
		blurhash : A string representing the blurhash to be decoded.
		width : Width of the resulting image
		height : Height of the resulting image
		punch : The factor to improve the contrast, default = 1
		nChannels : Number of channels in the resulting image array, 3 = RGB, 4 = RGBA
	Returns : A pointer to memory region where pixels are stored in (H, W, C) format
*/
FFI_PLUGIN_EXPORT 
uint8_t * decode(const char * blurhash, int width, int height, int punch, int nChannels);

/*
	decodeToArray : Decodes the blurhash and copies the pixels to pixelArray,
					This method is suggested if you use an external memory allocator for pixelArray.
					pixelArray should be of size : width * height * nChannels
	Parameters :
		blurhash : A string representing the blurhash to be decoded.
		width : Width of the resulting image
		height : Height of the resulting image
		punch : The factor to improve the contrast, default = 1
		nChannels : Number of channels in the resulting image array, 3 = RGB, 4 = RGBA
		pixelArray : Pointer to memory region where pixels needs to be copied.
	Returns : int, -1 if error 0 if successful
*/
FFI_PLUGIN_EXPORT 
int decodeToArray(const char * blurhash, int width, int height, int punch, int nChannels, uint8_t * pixelArray);

/*
	isValidBlurhash : Checks if the Blurhash is valid or not.
	Parameters :
		blurhash : A string representing the blurhash
	Returns : bool (true if it is a valid blurhash, else false)
*/
FFI_PLUGIN_EXPORT
bool isValidBlurhash(const char * blurhash); 

/*
	freePixelArray : Frees the pixel array
	Parameters :
		pixelArray : Pixel array pointer which will be freed.
	Returns : void (None)
*/
FFI_PLUGIN_EXPORT
void freePixelArray(uint8_t * pixelArray);

@dhikshithrm
Copy link
Member Author

dhikshithrm commented Aug 5, 2023

notice how blurHashForPixels function takes in rgb param as - A pointer to the pixel data. This is supplied in RGB order, with 3 bytes per pixels.

this plugin (blurhash_ffi) does the work for converting a flutter Image into 3 bytes per pixel array in RGB order before sending it to the ffi, so that the native part gets how it's expected, so that's why i asked to keep the function signatures similar to that of the current one.

after that we can integrate the new rust code with flutter_rust_bridge and we don't have to change any of the client code inside the package so that there won't be any breaking changes for the users of blurhash_ffi.

@sakchhams
Copy link

Interesting idea, conviniently there is a blurhash crate that should do everything for us.

One thing, however, it doesn't look like we can pass pointers around with flutter_rust_bridge.

@geofmureithi
Copy link

@sakchhams take a look at https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/frb_example/with_flutter/rust/src/bridge_generated.rs
This generated code should handle exchange of data between host and guest.

@f-person
Copy link

Hi! Doesn't the package use the original C implementation (https://github.com/woltapp/blurhash)? If that's the case, I fail to see how migrating the interface to Rust would help solve the issues mentioned above since you'd still be compiling some C code (unless you want to rewrite the original implementation in Rust?). Am I missing something?

@dhikshithrm
Copy link
Member Author

dhikshithrm commented Sep 20, 2023

Yes, the package uses the original C implementation from WoltApp's repo, we are not trying to migrate the interface, we are trying to rewrite the algorithm in Rust (not by using FFI in Rust) because the original C implementation is using Variable Length Array (VLA's) in the algorithm which is a part of C99 standard which are not supported in MSVC Compiler which is necessary for Windows support.

so using the original C implementation with FFI bridge is giving compilation errors in Windows #2, that's why we are looking to migrate to Rust, I hope I made it clear.

Rust's toolchain makes it easier for it to be cross-platform and also opens a way for WASM to make this package truly cross-platform enabling it to run on the web, windows which are currently un-supported.

@f-person
Copy link

Got it. Thanks for the explanation!
Yup, that'd be great, indeed :). I was unsure if you wanted to rewrite it all in Rust™ and wanted to ensure I'm not missing something :).

@dhikshithrm
Copy link
Member Author

Just Saw your YouTube channel & your notes app video, though I don't take notes, I thought it was a cool idea, keep up the good work.

@f-person
Copy link

Thank you! I'll be using this plugin in the app once I implement attachments (in the works!) :)

@dhikshithrm
Copy link
Member Author

found this rust implementation from the woltapp's blurhash project readme, probably gonna just use this.

need to figure out the wasm part tho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants