Add support for the Windows OS

[jaimecbernardo]

This pull request adds support for running react-native-sketch-canvas on react-native-windows. All current features are implemented. It also adds instructions on how to use it on Windows to the README.

[creambyemute]

https://github.com/creambyemute/react-native-sketch-canvas contains this PR + 1 iOS Fix and iOS Memory Leak fixes for anyone interested!

[jaimecbernardo]

I've added commits to:

• Fix a bug when a null directory was passed to the loaded image.
• Update the example to react-native 0.63 and add code for the windows platform.
• Add tests that use the example app on Windows.
• Add a Github action that runs those tests on Windows.

[creambyemute]

@jaimecbernardo Hey, I've just made it to the point where RN-SketchCanvas is used in our project and I think I've found a error/bug for the windows implementation when changing display scale (which is usually not 100% on surface devices)

How to reproduce:

• Set scale of display in settings to 200% or 150%
• Open App, open SketchCanvas
• Drawings are at the wrong place, not where I touched

If the scale setting is set to 100% it works fine.

[jaimecbernardo]

Hi @creambyemute , Thank you for reporting this. I'm looking into it.

[jaimecbernardo]

Turns out the JavaScript side was applying a scale according to the pixel ratio, but Windows should work with Device Independent Pixels already. I've added a commit to fix this issue. @creambyemute , I've added this commit to the PR I have open in your fork as well: https://github.com/creambyemute/react-native-sketch-canvas/pull/2 Please let me know if it fixes your issue.

[creambyemute]

@jaimecbernardo Oh great, I'll try it out soon 👍 .

[jaimecbernardo]

Hi @terrylinla , What would be needed for this PR to be merged and released? 😄 Thank you, in advance.

[creambyemute]

Hey @jaimecbernardo, I have been testing it a bit more and the scaling problem is fixed but I have a question about saving: The picture is always saved within the Drive:\Users\{user}\Pictures\{folder}\ folder.

• The same is happening in Android, so that is working as expected.
• On iOS it's using the Temporary Directory if a folder + fileName is set when saving. Application/User has access to the Temporary directory so u can move the stuff out of it.

Now to the problem: I have tried to move the saved picture on RN-Windows with react-native-fs from the Pictures Library to Drive:\Users\{user}\AppData\Local\Packages\myApp\LocalState\{AppUsername}\pictures\{fileName}.jpg which doesn't work.

On Android and iOS we are doing the same and there it is working fine. I guess it's because it's saved inside the Pictures Library Folder of Windows?

How would I move the saved image from Pictures Library to AppData/Local/Packages/myApp ? Wouldn't it make sense to place it somewhere where the Application has access to so it can do something with it like upload/move/modify/copy/delete?

Edit: It seems that the windows implementation of react-native-fs crashes when using the .exists(...)method on the Pictures Library folder or a file within it.

• .readFile(...) seems to work.
• .copyFile(...) seems to work
• .moveFile(...)seems to work
• .exists(...) crashes

[jaimecbernardo]

Hi @creambyemute ,

The logic for the module seems to use whatever specific place for images the OS provides, like a Camera Roll, which would be the Pictures library in Windows.

The application does seem to have access to the Pictures library, if it's able to save the picture and then run readFile, copyFile and moveFile successfully. Thank you for opening the issue in react-native-fs.

[creambyemute]

@jaimecbernardo Yeah all good. Took me a bit to figure out it was the .exists method from the rn-fs implementation that bugs out.

Have a nice week :-)

[burtonrodman]

@terrylinla any chance of getting this merged or possibly transferring the repo to someone who is willing to maintain it?

[creambyemute]

Hey @jaimecbernardo I have been using the branch now for some time and found one odd behaviour on windows that I can replicate when using a localSourceImage/BackgroundImage to draw onto and later on trying to delete the old image.

My UseCase is: Take Photo from camera or import image from FilePicker which is then move to appPackage/userFolder. The user can then edit the image and save it, which results in a new image so we'd want to delete the old one.

ImageEditor

<SketchCanvas
    localSourceImage={this.props.localSourceImage} // Image to edit
    ref={this.imageEditorRef}
    style={styles.imageEditor}
    onSketchSaved={this.onImageSaved}
/>

onPressSaveImage = () => {
    if (this.imageEditorRef.current instanceof SketchCanvas) {
        this.imageEditorRef.current.save("jpg", false, "someTmpFolderName", "someTmpFileName", true, false, true);
    }
};

onImageSaved = (success: boolean, path: string) => {
    if (this.props.onImageSaved && success) {
        this.props.onImageSaved(path);
    }
};

Parent Component

onImageSaved = async (path: string) => {
    // Delete the old image (the localSourceImage in this case)
    // This step fails when providing localSourceImage although the ImageEditor component has been unmounted

    // Move the new image (path) to appPackage/userFolder
};

When I try to delete the old image that was provided as localSourceImage for editing I get an error The process can't access the file because it's in use by another process (translated from german :P may not be 100% accurate).

Any idea what might be going on there?

[jaimecbernardo]

Hi @creambyemute,

It's possible that CanvasBitmap::LoadAsync is leaving the file handle open in the resulting CanvasBitmap object.

One possibility would be to have a memory buffer containing the files contents, though it means using more memory. I'll experiment with that change. Have you by any chance observed what the behavior is in the mobile platforms?

[creambyemute]

@jaimecbernardo We're using the same code as outlined above on iOS/Android without any problems cleaning up the old image (localSourceImage) after saving.

Haven't looked at the iOS/Android implementation on how they exactly load the provided image, if they dispose it afterwards or create a copy of it to work on.

Edit: I had a quick look at the iOS implementation and it seems to create an in-memory copy of the provided localSourceImage.

See dealloc (in my fork https://github.com/creambyemute/react-native-sketch-canvas/blob/master/ios/RNSketchCanvas/RNSketchCanvas/RNSketchCanvas.m) and openSketchFile (present in original and my fork) methods

[jaimecbernardo]

Hi @creambyemute , Please try this fix for the file handle being kept open issue: https://github.com/creambyemute/react-native-sketch-canvas/pull/3

[creambyemute]

Hi @jaimecbernardo

I just tested the change locally and it works. I also checked the memory consumption and it always went down to the initial consumption (before opening SketchCanvas) after saving.

Looks good to me! Thx a lot

[dd-apt]

Hello @creambyemute / @jaimecbernardo , I found your code from the RNW 0.64 release blog, and am currently attempting to integrate this fork - https://github.com/creambyemute/react-native-sketch-canvas

Using the simple SketchCanvas component (since we are building our own UI) and for the most part things seem to work great! However, not having any luck using the getBase64 method to actually extract an image from the canvas.

I get the following error returned to the callback parameter / method -

HRESULT -2147417842: The application called an interface that was marshalled for a different thread.

Any help / advice you can provide is much appreciated. Happy to provide any other you may need as well. Thank you in advance!

[jaimecbernardo]

Hi @dd-apt , Thank you for reporting it. I believe there was some changes in threading in RNW 0.64, which might be causing this. I'll look at it this coming week.

[dd-apt]

@jaimecbernardo thanks for the quick response and your efforts to bring Windows support to this module, much appreciated!

[jaimecbernardo]

Hi @dd-apt , I've pushed a fix and opened this PR: https://github.com/creambyemute/react-native-sketch-canvas/pull/4 Please give it a try.

[dd-apt]

@jaimecbernardo thanks so much for the quick fix!

[dd-apt]

@jaimecbernardo we ran into another issue, although still investigating. Everything works fine locally in the development / debug build, however as soon as we do a release build and get to a screen with the SketchCanvas component, it shows a blank white screen. Our error boundary doesn't seem to catch anything. Was wondering if you are able to reproduce this (x86 Release build), or can provide any direction on tracking down what the issue could be? Thanks again!

Update: we were able to get an error, but unsure how helpful it is -

[jaimecbernardo]

Hi @dd-apt ,

Thank you for reporting this. One of the things that happens when you switch to Release is that it'll use ChakraCore instead of v8, so it might be related to that.

I'll take a better look at this a bit later today or next week. One of the current workarounds I can think of is using 0.63 for now since the module seemed to work without issues there.

[dd-apt]

@jaimecbernardo ah, yeah we have run into discrepancies between the two engines' behaviors previously, so that would make sense....

Let me know if there's any other information I can provide to help w/ troubleshooting!

[jaimecbernardo]

Hi @dd-apt , I've narrowed this down to some native module features that seem to no longer work when buliding for Release in 0.64. I've opened an issue here: https://github.com/microsoft/react-native-windows/issues/7537 Unfortunately, the module depends on these features, so the only workaround I currently have is to keep your project on react-native-windows 0.63.

[jaimecbernardo]

Hi @dd-apt , The great react-native-windows suggested using the new method to access Constants and Commands in the code: https://github.com/microsoft/react-native-windows/issues/7537#issuecomment-813571373

This seems to have fixed it for my reproduction code so I'll try to modernize this module's JavaScript code. I'll push a fix and let you know.

[dd-apt]

Thank you for the update @jaimecbernardo and for looking into the fix, glad to hear you were able to track it down!

[jaimecbernardo]

Hi @dd-apt , I've pushed the fix to https://github.com/creambyemute/react-native-sketch-canvas/pull/4 , but there seem to be other underlying issues, which I'll investigate. On Release, real time drawing is broken for me. It looks like dispatchCommand is only sent to native code when the UI updates for some reason. This may or may not affect you.

[jaimecbernardo]

I've opened a new issue for the UIManager.dispatchViewManagerCommand problem in Release: https://github.com/microsoft/react-native-windows/issues/7543

[jaimecbernardo]

Hi @dd-apt , The fix in https://github.com/creambyemute/react-native-sketch-canvas/pull/4 should now work for the latest react-native-windows 0.64.4, which fixed https://github.com/microsoft/react-native-windows/issues/7543

[dd-apt]

@jaimecbernardo thanks for the heads up!

[creambyemute]

Hey @jaimecbernardo I have found another issue regarding the Windows Display Scaling Setting and this time saving an image with rn-sketch-canvas.

The problem is, that only a portion (1/4 on 200% scale?) of the backgroundImage (localSourceImage) is included in the saved image and the actual drawing may also not be included (depending on where u draw).

It's easily reproducable and I have created a repro example: https://github.com/creambyemute/rnSketchCanvasW10Test

How to reproduce:

• Set display scale in Windows 10 Settings to > 100%
• Use SketchCanvas with a localSourceImage as background to draw on and includeImage = true in save

  sketchRef?.save(
  'jpg',
  false,
  'rnSketchTest',
  String(Math.ceil(Math.random() * 100000000)),
  true,
  false,
  true // or cropToImageSize false, doesn't matter
  );

• Have a look at the image, it's cropped
• Try the same with 100%, image is not cropped

[jaimecbernardo]

Hi @creambyemute , Thanks for reporting this. I'll take a look.

[jaimecbernardo]

Hi @creambyemute , I could reproduce the issue. Thanks for the repo. It was very helpful in reproducing and testing a fix. Please give the fix a try and let me know: https://github.com/creambyemute/react-native-sketch-canvas/pull/5

[creambyemute]

@jaimecbernardo That line fixed it with and without localSourceImage usage. Thanks a lot!