String corruption with VipsForeignLoad / VipsForeignSave when using in Unity

[jsm174]

When trying to use NetVips in Unity (2020.3.12f1), we are seeing string corruption issues with methods that call VipsForeignLoad / VipsForeignSave:

For example:

public Image GetImage()
{
   return Image.NewFromFile("/Users/jmillard/Desktop/test.png");
}

errors with:

VipsException: unable to load from file /Users/jmillard/Desktop/test.png
VipsForeignLoad: file "/Users/jmillard/Desktop/test.png�Le���" does not exist

and

var path = texture.GetUnityFilename(folder);
Debug.Log($"Writing to {path}");
var im = texture.GetImage();
File.WriteAllBytes(path, im.WriteToBuffer(".png"));

errors with:

VipsException: unable to write to buffer
VipsForeignSave: ".pngallrolling3" is not a known buffer format

We are seeing this on MacOS and Windows. (Did not try on Linux yet)

[kleisauke]

IIRC, I had similar issues with older versions of Mono in combination with Span<T>. According to this post, Unity's fork of Mono (prior to version 2021.2.0a18) is about two years behind the latest upstream code, which might possibly be the cause.

Given that information, and that Unity 2021.2 is still an alpha version, I think it makes more sense to avoid using Span<T> throughout the codebase (hopefully Unity will move to CoreCLR in the near future). Commit 70119ea on the unity branch is supposed to fix this. If you want to test this, you can use the nightly version of NetVips. Add the https://ci.appveyor.com/nuget/net-vips feed in the <packageSources> section of your NuGet.config:

<packageSources>
  <add key="netvips-nightly" value="https://ci.appveyor.com/nuget/net-vips" />
</packageSources>

And update NetVips to 2.0.0 (build number 276 - prerelease). If that works, I'll try to integrate that in NetVips 2.0.1.

[jsm174]

Thank you for such a fast reply on this!

Unfortunately, that did not work. I'm positive I did it correctly, as I triple checked the plugins folder (and restarted unity):

Same random corruption:

[kleisauke]

Oh curious, let me try to reproduce it on my Windows PC. Do you have a test branch somewhere that I can git checkout?

The screenshots and changes (i.e. System.Memory removal, updated deps, etc.) looks OK, NetVips 2.0.0 is 179712 bytes and build number 276 should indeed be 179200 bytes. Perhaps the example Unity project didn't install VPE from disk?

[jsm174]

ok. sorry in advance for instructions that are a little long.

git clone https://github.com/freezy/VisualPinball.Engine
cd VisualPinball.Engine
git checkout refactor/scene-netvips-test
cd ..
git clone https://github.com/VisualPinball/VisualPinball.Unity.Hdrp
cd VisualPinball.Unity.Hdrp
git checkout refactor/scene-netvips-test

2. create new HDRP project in unity

3. in the package manager set up a new scope registry:

Name: Visual Pinball Engine
URL: https://registry.visualpinball.org
Scope(s): org.visualpinball
Enable Preview Packages [X]
(Apply)

4. In the package manager, add package from disk, and add the VisualPinball.Engine project

5. In the package manager, add package from disk, and add the VisualPinball.Engine.HDRP project

They should show up flagged with "local" in a box:

6. Create a new scene "Basic Indoors (HDRP)"

7. In the Visual Pinball Menu, Select Import VPX

8. Import the file exampleTable.vpx

You can download that here (it's 19mb)

If all that worked, you should see the error in the console:

[kleisauke]

Thanks for these instructions, I could reproduce it on Windows. It seems that the Mono version provided by Unity doesn't properly null-terminate strings(?), I was able to fix/workaround this with commit 95a9627 (which should be available as nightly version 2.0.0 build number 277).

I'll investigate further why this only occurs on Mono and not on .NET Core/Framework, in the meantime you might also be interested in these changes:
freezy/VisualPinball.Engine@refactor/scene-netvips-test...kleisauke:refactor/scene-netvips-test

[freezy]

@kleisauke your reactivity and efficiency never ceases to amaze me :)

@jsm174 lemme know if I should merge that branch!

Thanks to both of you!

[jsm174]

@kleisauke - awesome. Thank you again!

@freezy - sure we can merge the branch listed above!

[kleisauke]

Thanks for the kind words. After further investigation, it looks like I assumed that passing a byte array via P/Invoke will always add a null-terminator, which turned out to be incorrect, although it did work in most cases.

I've merged the above mentioned commits to the master branch, expect a new release soon.

[kleisauke]

NetVips v2.0.1 is now available. Thanks for reporting this!