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

Problem with MESH transform /w transparent background #254

Closed
nikmolnar opened this issue Jun 20, 2013 · 8 comments

Comments

Projects
None yet
3 participants
@nikmolnar
Copy link
Contributor

commented Jun 20, 2013

This may apply to other transformations as well, but I've specifically observed it using the MESH transform.

The problem is that when performing multiple transformations, "garbage" from previous transformations begins appearing in the image, as shown here:
image

In the above image, the image in the middle is the desired result... everything around it should be transparent, but is instead filled with the results of previous transform operations.

I was eventually able to fix this by using memset to clear the memory allocated by ImagingNewBlock in Storage.c.

I'd be happy to submit a fix for this, but I have two questions:

  1. Is this the appropriate solution?
  2. How would I write a test for this? It happens inconsistently, and the severity seems to vary between systems (e.g., on a micro AWS instance with 512MB of memory, it happens frequently, but on my development machine with 12GB of memory, I haven't observed it once).

Thanks,
_Nik

@wiredfool

This comment has been minimized.

Copy link
Member

commented Jun 20, 2013

First glance, it looks like a bug from using uninitialized memory. If ImagingNewBlock isn't clearing the memory, and it's allocating it from previously allocated and recently freed memory, then you'd get just this sort of thing. And I'd certainly expect it more on a machine with more memory pressure.

As for a test, I'd start with posting what you did to make that image. I'd think that if the non-transformed region contains anything other than whatever the background should be initialized to, then we'd have a test failure, though admittedly, a passed test isn't always going to mean that the bug is fixed. But it will be useful to run on resource constrained machines to stress test.

@aclark4life

This comment has been minimized.

Copy link
Member

commented Jun 29, 2013

@nikmolnar Got anything we can include in 2.1.0?

@nikmolnar

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2013

@aclark4life Sorry, I haven't had a chance to come back to this yet. When's 2.1.0 due?

@aclark4life

This comment has been minimized.

Copy link
Member

commented Jul 1, 2013

Today, else you can target 2.2.0 in 3 months.

@nikmolnar

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2013

Ok, I'll aim for 2.2.0. Thanks.

@aclark4life

This comment has been minimized.

Copy link
Member

commented Sep 28, 2013

@nikmolnar Time to hit the mark! 2.2.0 goes out in 3 days, else 2.3.0 or Future for this one

@nikmolnar

This comment has been minimized.

Copy link
Contributor Author

commented Sep 29, 2013

I can submit a fix (it's a one liner) but I haven't had time to implement a test for the problem yet. It doesn't look like there are any tests for transform at the moment, so I'll need to create a new one. Would you like me to submit the fix on its own or wait until I can submit it with an appropriate test?

@wiredfool

This comment has been minimized.

Copy link
Member

commented Sep 30, 2013

Send the fix, someone else might be able to pull together a test.

nikmolnar pushed a commit to nikmolnar/Pillow that referenced this issue Sep 30, 2013

@nikmolnar nikmolnar referenced this issue Sep 30, 2013

Merged

Fix for #254 #348

aclark4life added a commit that referenced this issue Sep 30, 2013

@aclark4life aclark4life closed this Oct 2, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.