PSFilterPdn (01-18-2024)

[null54]

1 hour ago, Rick Brewster said:

That appears to be the same crash reported in the post above yours by @user.by

 

It is, and I know what is causing it.

 

For some reason ILLink is removing System.IDisposable from the trimmed runtime, despite the fact that I set the option in the project file that should make it ignore the main executable and any assemblies that are not marked as compatible with trimming. The main executable uses IDisposable in a few places, so I have no idea why ILLink thinks it can be removed.

It was working when I built 2.0.6, but it obviously no longer does.

[Rick Brewster]

That might be something that folks on the C# Discord Server can help figure out.

[null54]

Release version 2.0.8, which fixes a crash when running 32-bit filters.

[user.by]

1 hour ago, null54 said:

Release version 2.0.8, which fixes a crash when running 32-bit filters.

First time thank you for your works, I just say as information, i have some plugin problems. I am using Curve plugin (http://www.mehdiplugins.com/english/curves.htm)

It's work v2.0.2 and earlier versions but dont work 2.0.3 and later versions. Like this

 

first pic v2.0.8 - second pic 2.0.2

[null54]

11 hours ago, user.by said:

It's work v2.0.2 and earlier versions but dont work 2.0.3 and later versions. Like this

 

Version 2.0.3 had a lot of changes to the image storage and rendering code, I converted it all from GDI+ to WIC/Direct2D.

As a workaround you can delete the 64-bit version of that filter, the 32-bit version is working correctly.

 

I am not sure what is going on with the 64-bit version, it looks like a possible bug in the pixel copying or resizing code.

[user.by]

9 hours ago, null54 said:

 

Version 2.0.3 had a lot of changes to the image storage and rendering code, I converted it all from GDI+ to WIC/Direct2D.

As a workaround you can delete the 64-bit version of that filter, the 32-bit version is working correctly.

 

I am not sure what is going on with the 64-bit version, it looks like a possible bug in the pixel copying or resizing code.

Thank you for concern, you know better, but when i change PSFilterPdn.dll all versions are working.

[null54]

It looks like Paint.NET is padding the stride of the WIC bitmaps depending on the dimensions. This does not occur when using WIC directly.

I still don't know how this is causing the code to produce a corrupt image. the loops should handle different stride values when copying the data.

[Rick Brewster]

10 hours ago, null54 said:

It looks like Paint.NET is padding the stride of the WIC bitmaps depending on the dimensions. This does not occur when using WIC directly.

I still don't know how this is causing the code to produce a corrupt image. the loops should handle different stride values when copying the data.

 

It depends on circumstances. Bitmap allocations are pooled/recycled, and I also try to pool/recycle Direct2D device contexts along with the bitmaps they were wrapped around (via IDirect2DFactory.CreateBitmapRenderTarget(), which has special/private performance advantages vs. other paths). In order to reuse these bitmaps and their DCs better, I do some trickery with the strides to reduce the need to recreate device contexts (which aren't cheap, so this improves perf by quite a good amount). See BitmapUtil.GetOptimalStrideChecked() for more.

 

Is the "corrupt" bitmap being drawn by the 8BF plugin? It may not be doing the right thing when it comes to stride -- a lot of code out there incorrectly assumes that stride==width*sizeof(pixel), which almost always works ...

[null54]

8 hours ago, Rick Brewster said:

Is the "corrupt" bitmap being drawn by the 8BF plugin?

 

Not directly, PSFilterPdn is feeding it bad data when the image uses those stride tricks. But I have no idea where the underlying bug is located, everything I have checked looks correct.

The data copy loops I am using would be similar to:

using (IBitmapLock<ColorBgra32> bitmapLock = scaledSurface!.Lock(bounds, BitmapLockMode.Read))
{
    for (int y = 0; y < bounds.Height; y++)
    {
        uint* src = (uint*)((byte*)bitmapLock.Buffer + (nuint)((nint)y * bitmapLock.BufferStride));
        byte* dst = (byte*)dstScan0 + ((y + padding.top) * destStride) + padding.left;		
        ImageRow.Load(src, bitmapLock.BufferStride, dst, destStride, bounds.Width, channelIndex, numberOfPlanes);
    }
}

The source BufferStride is equal to whatever PDN sets it to, and the destination buffer stride is equal to width x numberOfPlanes.

[Rick Brewster]

3 hours ago, null54 said:

byte* dst = (byte*)dstScan0 + ((y + padding.top) * destStride) + padding.left;

padding.left is not being multiplied by the bytes-per-pixel?

 

Also there are methods in PaintDotNet.Imaging.BitmapUtil that will make your life much easier for calculations like these, like GetRowPointer and GetPointPointer

[null54]

7 hours ago, Rick Brewster said:

padding.left is not being multiplied by the bytes-per-pixel?

 

That was a bug, but unfortunately not the cause of this issue.

Padding is a Photoshop API feature that its filters can use to virtually extend the canvas by the requested number of pixels, it was presumably intended to make writing convolution filters easier. Instead of adjusting the algorithm to handle the image edges a filter developer could have the host extend the canvas by replicating those pixels for n rows/columns.

[Rick Brewster]

Hmm. If you've got a link to the code in question, I can take a look and see if I can figure something out

[null54]

4 minutes ago, Rick Brewster said:

Hmm. If you've got a link to the code in question, I can take a look and see if I can figure something out

 

Copy to 8bf filter loop: https://github.com/0xC0000054/PSFilterPdn/blob/4066ed29118dfff69994361d38bc0a0a45cdcf7e/src/PSApi/LoadPsFilter.cs#L1788

ImageRow load method: https://github.com/0xC0000054/PSFilterPdn/blob/4066ed29118dfff69994361d38bc0a0a45cdcf7e/src/PSApi/ImageRow.cs#L41

[Rick Brewster]

I can't seem to figure it out from looking at the code 🤔

 

But from looking at the screenshots, my guess is there's (at least) a bug with how the per-row output pointer is being calculated. Notice how it has a transparent chunk at the bottom. This could happen if the destination stride was not taken into account, and if that stride was a bit larger than the width*bytesPerPixel. It looks like maybe about 10% wasn't filled in? So maybe the output is ~900px wide, but the buffer is ~1024px wide (with 3600 byte stride).

 

I can't really discern a specific pattern in the output pixels -- it's obviously not just the left portion of the image, which sometimes happens if bytesPerPixel is incorrectly applied (e.g. only filling or utilizing the left 25%, or something like that)

[Rick Brewster]

@null54 you can do some additional debugging by making your own implementation of IBitmap[Source]. Derive from BitmapSourceBase<TPixel> and then also implement IBitmap<TPixel>. In your implementation you can use whatever allocator you want, e.g. NativeMemory, so you can employ whatever stride you need to help you narrow down where the problem is.

 

[null54]

21 minutes ago, Rick Brewster said:

Derive from BitmapSourceBase<TPixel> and then also implement IBitmap<TPixel>. In your implementation you can use whatever allocator you want, e.g. NativeMemory, so you can employ whatever stride you need to help you narrow down where the problem is.

 

Could that also be used as a workaround for this issue?

So that instead of calling PDN's IImagingFactory.CreateBitmap, I have my own IBitmap<TPixel> class that ensures the bitmap has the correct stride.

[Rick Brewster]

I could also look into adding that in an extra overload, like factory.CreateBitmap(..., CreateBitmapOptions.RequireContiguousAllocation)

 

There isn't yet a non-internal implementation of IBitmap[Source] that I know of, so I'm not yet ready to endorse and support it. It's not that I'm disallowing it, there's just a few weird corner cases in the component model and I need to make sure the potential sharp edges are rounded off.

 

For the moment, let's just see if it helps narrow down what the problem is. Then we can figure out if 5.0.13 should have that extra overload. And/or we make sure that implementing IBitmap[Source] via BitmapSourceBase<TPixel> is both possible (which it is right now) and fully supported (i.o.w. I won't regret it later for some reason I can't foresee right now).

[Rick Brewster]

In other words I prefer you to not need to implement IBitmap yourself. Having a flag for controlling this behavior does sound reasonable.