From c36c5fcf0f38f1df1334672ea6ec988d32cc691f Mon Sep 17 00:00:00 2001 From: homm Date: Sun, 3 Jul 2016 04:33:14 +0300 Subject: [PATCH 1/3] fix access tests: clean up global variable Image.USE_CFFI_ACCESS after tests do not run tests twice via `test_put` and `test_get` --- Tests/{test_cffi.py => test_image_access.py} | 106 ++++++++++++++++--- Tests/test_image_getpixel.py | 53 ---------- Tests/test_image_putpixel.py | 50 --------- 3 files changed, 92 insertions(+), 117 deletions(-) rename Tests/{test_cffi.py => test_image_access.py} (61%) delete mode 100644 Tests/test_image_getpixel.py delete mode 100644 Tests/test_image_putpixel.py diff --git a/Tests/test_cffi.py b/Tests/test_image_access.py similarity index 61% rename from Tests/test_cffi.py rename to Tests/test_image_access.py index 02d1ff7d3..c34cf9707 100644 --- a/Tests/test_cffi.py +++ b/Tests/test_image_access.py @@ -1,7 +1,6 @@ from helper import unittest, PillowTestCase, hopper try: - import cffi from PIL import PyAccess except ImportError: # Skip in setUp() @@ -9,13 +8,99 @@ except ImportError: from PIL import Image -from test_image_putpixel import TestImagePutPixel -from test_image_getpixel import TestImageGetPixel -Image.USE_CFFI_ACCESS = True +class AccessTest(PillowTestCase): + # initial value + _init_cffi_access = Image.USE_CFFI_ACCESS + _need_cffi_access = False + + @classmethod + def setUpClass(cls): + Image.USE_CFFI_ACCESS = cls._need_cffi_access + + @classmethod + def tearDownClass(cls): + Image.USE_CFFI_ACCESS = cls._init_cffi_access + + +class TestImagePutPixel(AccessTest): + def test_sanity(self): + im1 = hopper() + im2 = Image.new(im1.mode, im1.size, 0) + + for y in range(im1.size[1]): + for x in range(im1.size[0]): + pos = x, y + im2.putpixel(pos, im1.getpixel(pos)) + + self.assert_image_equal(im1, im2) + + im2 = Image.new(im1.mode, im1.size, 0) + im2.readonly = 1 + + for y in range(im1.size[1]): + for x in range(im1.size[0]): + pos = x, y + im2.putpixel(pos, im1.getpixel(pos)) + + self.assertFalse(im2.readonly) + self.assert_image_equal(im1, im2) + + im2 = Image.new(im1.mode, im1.size, 0) + + pix1 = im1.load() + pix2 = im2.load() + + for y in range(im1.size[1]): + for x in range(im1.size[0]): + pix2[x, y] = pix1[x, y] + + self.assert_image_equal(im1, im2) + + +class TestImageGetPixel(AccessTest): + @staticmethod + def color(mode): + bands = Image.getmodebands(mode) + if bands == 1: + return 1 + else: + return tuple(range(1, bands + 1)) + + def check(self, mode, c=None): + if not c: + c = self.color(mode) + + # check putpixel + im = Image.new(mode, (1, 1), None) + im.putpixel((0, 0), c) + self.assertEqual( + im.getpixel((0, 0)), c, + "put/getpixel roundtrip failed for mode %s, color %s" % (mode, c)) + + # check inital color + im = Image.new(mode, (1, 1), c) + self.assertEqual( + im.getpixel((0, 0)), c, + "initial color failed for mode %s, color %s " % (mode, c)) + + def test_basic(self): + for mode in ("1", "L", "LA", "I", "I;16", "I;16B", "F", + "P", "PA", "RGB", "RGBA", "RGBX", "CMYK", "YCbCr"): + self.check(mode) + + def test_signedness(self): + # see https://github.com/python-pillow/Pillow/issues/452 + # pixelaccess is using signed int* instead of uint* + for mode in ("I;16", "I;16B"): + self.check(mode, 2**15-1) + self.check(mode, 2**15) + self.check(mode, 2**15+1) + self.check(mode, 2**16-1) class TestCffiPutPixel(TestImagePutPixel): + _need_cffi_access = True def setUp(self): try: @@ -23,11 +108,9 @@ class TestCffiPutPixel(TestImagePutPixel): except ImportError: self.skipTest("No cffi") - def test_put(self): - self.test_sanity() - class TestCffiGetPixel(TestImageGetPixel): + _need_cffi_access = True def setUp(self): try: @@ -35,12 +118,9 @@ class TestCffiGetPixel(TestImageGetPixel): except ImportError: self.skipTest("No cffi") - def test_get(self): - self.test_basic() - self.test_signedness() - -class TestCffi(PillowTestCase): +class TestCffi(AccessTest): + _need_cffi_access = True def setUp(self): try: @@ -144,5 +224,3 @@ class TestCffi(PillowTestCase): if __name__ == '__main__': unittest.main() - -# End of file diff --git a/Tests/test_image_getpixel.py b/Tests/test_image_getpixel.py deleted file mode 100644 index 965233f94..000000000 --- a/Tests/test_image_getpixel.py +++ /dev/null @@ -1,53 +0,0 @@ -from helper import unittest, PillowTestCase - -from PIL import Image - -Image.USE_CFFI_ACCESS = False - - -def color(mode): - bands = Image.getmodebands(mode) - if bands == 1: - return 1 - else: - return tuple(range(1, bands+1)) - - -class TestImageGetPixel(PillowTestCase): - - def check(self, mode, c=None): - if not c: - c = color(mode) - - # check putpixel - im = Image.new(mode, (1, 1), None) - im.putpixel((0, 0), c) - self.assertEqual( - im.getpixel((0, 0)), c, - "put/getpixel roundtrip failed for mode %s, color %s" % (mode, c)) - - # check inital color - im = Image.new(mode, (1, 1), c) - self.assertEqual( - im.getpixel((0, 0)), c, - "initial color failed for mode %s, color %s " % (mode, color)) - - def test_basic(self): - for mode in ("1", "L", "LA", "I", "I;16", "I;16B", "F", - "P", "PA", "RGB", "RGBA", "RGBX", "CMYK", "YCbCr"): - self.check(mode) - - def test_signedness(self): - # see https://github.com/python-pillow/Pillow/issues/452 - # pixelaccess is using signed int* instead of uint* - for mode in ("I;16", "I;16B"): - self.check(mode, 2**15-1) - self.check(mode, 2**15) - self.check(mode, 2**15+1) - self.check(mode, 2**16-1) - - -if __name__ == '__main__': - unittest.main() - -# End of file diff --git a/Tests/test_image_putpixel.py b/Tests/test_image_putpixel.py deleted file mode 100644 index 418e9703c..000000000 --- a/Tests/test_image_putpixel.py +++ /dev/null @@ -1,50 +0,0 @@ -from helper import unittest, PillowTestCase, hopper - -from PIL import Image - -Image.USE_CFFI_ACCESS = False - - -class TestImagePutPixel(PillowTestCase): - - def test_sanity(self): - - im1 = hopper() - im2 = Image.new(im1.mode, im1.size, 0) - - for y in range(im1.size[1]): - for x in range(im1.size[0]): - pos = x, y - im2.putpixel(pos, im1.getpixel(pos)) - - self.assert_image_equal(im1, im2) - - im2 = Image.new(im1.mode, im1.size, 0) - im2.readonly = 1 - - for y in range(im1.size[1]): - for x in range(im1.size[0]): - pos = x, y - im2.putpixel(pos, im1.getpixel(pos)) - - self.assertFalse(im2.readonly) - self.assert_image_equal(im1, im2) - - im2 = Image.new(im1.mode, im1.size, 0) - - pix1 = im1.load() - pix2 = im2.load() - - for y in range(im1.size[1]): - for x in range(im1.size[0]): - pix2[x, y] = pix1[x, y] - - self.assert_image_equal(im1, im2) - - # see test_image_getpixel for more tests - - -if __name__ == '__main__': - unittest.main() - -# End of file From cedaaad1cfcf2545d53ea98061c59752fec3c304 Mon Sep 17 00:00:00 2001 From: homm Date: Sun, 3 Jul 2016 05:03:25 +0300 Subject: [PATCH 2/3] keep pointer to im object to prevent dereferencing --- PIL/PyAccess.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/PIL/PyAccess.py b/PIL/PyAccess.py index c9cbd7014..9f7827ce8 100644 --- a/PIL/PyAccess.py +++ b/PIL/PyAccess.py @@ -54,6 +54,9 @@ class PyAccess(object): self.xsize = vals['xsize'] self.ysize = vals['ysize'] + # Keep pointer to im object to prevent dereferencing. + self._im = img.im + # Debugging is polluting test traces, only useful here # when hacking on PyAccess # logger.debug("%s", vals) From af75f7ab40f0778a82dbc93f887837c82f02aa8e Mon Sep 17 00:00:00 2001 From: homm Date: Mon, 4 Jul 2016 13:42:45 +0300 Subject: [PATCH 3/3] test for reference counting --- Tests/test_image_access.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Tests/test_image_access.py b/Tests/test_image_access.py index c34cf9707..40f648f27 100644 --- a/Tests/test_image_access.py +++ b/Tests/test_image_access.py @@ -221,6 +221,17 @@ class TestCffi(AccessTest): # im = Image.new('I;32B', (10, 10), 2**10) # self._test_set_access(im, 2**13-1) + # ref https://github.com/python-pillow/Pillow/pull/2009 + def test_reference_counting(self): + size = 10 + + for _ in range(10): + # Do not save references to the image, only to the access object + px = Image.new('L', (size, 1), 0).load() + for i in range(size): + # pixels can contain garbarge if image is released + self.assertEqual(px[i, 0], 0) + if __name__ == '__main__': unittest.main()