Skip to content

bug fix: Qt wants data aligned to 32 bits - #1430

Merged
wiredfool merged 1 commit into
python-pillow:masterfrom
ericfrederich:qt_align
Sep 20, 2015
Merged

wiredfool merged 1 commit into
python-pillow:masterfrom
ericfrederich:qt_align

Conversation

@ericfrederich

@ericfrederich ericfrederich commented Sep 17, 2015

Copy link
Copy Markdown
Contributor

Images in Qt show up incorrectly if each line is not aligned to 32 bits.

It is pretty common for an image's lines to be 32-bit aligned by chance.
Obviously any 32-bit image will not have any problem.
For the bug to manifest itself you'd need...

  • a 1-bit image whose width is not a multiple of 32
  • an 8-bit image who width is not a multiple of 4

Testing more images now and added a 7x13 png test image

@ericfrederich

Copy link
Copy Markdown
Contributor Author

If this fix gets merged I can rebase #1429 on top of it since they both added the same tests.

Images in Qt show up incorrectly if each line is not aligned to 32 bits.

It is pretty common for an image's lines to be 32-bit alinged by chance.
Obviously any 32-bit image will not have any problem.
For the bug to manifest itself you'd need...
* a 1-bit image whose width is not a multiple of 32
* an 8-bit image who width is not a multiple of 4

Testing more images now and added a 7x13 png test image
@ericfrederich

Copy link
Copy Markdown
Contributor Author

I just re-based this off of master. If you need me to do anything else let me know. I think it should be merged.
I wrote a little PyQt viewer that views an image after being converted through various Pillow modes.
Here you can see that it was clearly broken before my fix and that it works afterwards.
Further, this pull request includes tests for this image now.

before fix

screenshot from 2015-09-18 16 07 20

after fix

screenshot from 2015-09-18 16 07 59

@wiredfool

Copy link
Copy Markdown
Member

Can you share the viewer?

@ericfrederich

Copy link
Copy Markdown
Contributor Author

@wiredfool sure, here's the viewer...
Its more complex than is needed to exhibit the bug and the fix to it though.
Really just displaying a monochrome Tests/images/transparent.png through Qt in any way should show the problem.

Without this fix transparent.png will mess up with mode '1' but will be fine with mode 'L' and 'P' because its width is a multiple of 4. If you use an image like 7x13.png you'll see modes '1', 'L', and 'P' mess up.

Just something I wrote to test some stuff.
Didn't make any attempts to have it work on anything except the machine I was working on (Python2 and Qt4 with PyQt). You'll probably have to make minor changes to get it to work w/ Python3 or Qt5 or Pyside, etc.

#!/usr/bin/env python

import sys
from collections import OrderedDict

from PyQt4.QtCore import *
from PyQt4.QtGui import *

from PIL import Image

modes = OrderedDict()

modes['RGB'  ] = '(3x8-bit pixels, true color)'
modes['RGBA' ] = '(4x8-bit pixels, true color with transparency mask)'
modes['1'    ] = '(1-bit pixels, black and white, stored with one pixel per byte)'
modes['L'    ] = '(8-bit pixels, black and white)'
modes['P'    ] = '(8-bit pixels, mapped to any other mode using a color palette)'
# modes['CMYK' ] = '(4x8-bit pixels, color separation)'
# modes['YCbCr'] = '(3x8-bit pixels, color video format)'
# modes['LAB'  ] = '(3x8-bit pixels, the L*a*b color space)'
# modes['HSV'  ] = '(3x8-bit pixels, Hue, Saturation, Value color space)'
# modes['I'    ] = '(32-bit signed integer pixels)'
# modes['F'    ] = '(32-bit floating point pixels)'

class ClickableLabel(QLabel):
    clicked = pyqtSignal()
    def mousePressEvent(self, mouseEvent):
        self.clicked.emit()

class MyWidget(QWidget):
    def __init__(self, fname, parent=None):
        super(MyWidget, self).__init__(parent)

        layout = QVBoxLayout()
        self.img_label = ClickableLabel()
        self.desc_label = QLabel()
        self.combo = QComboBox()
        self.combo.addItems(modes.keys())
        layout.addWidget(self.combo)
        layout.addWidget(self.desc_label)
        layout.addWidget(self.img_label)
        self.setLayout(layout)
        self.combo.currentIndexChanged['QString'].connect(self.on_combo_change)
        self.img_label.clicked.connect(self.new_file)

        self.change_image(fname)

    def on_combo_change(self, s):
        s = str(s)

        # need to persist this one, so keep it on self
        self.qi = self.im.convert(s).toqimage()
        pm = QPixmap.fromImage(self.qi)
        self.img_label.setPixmap(pm.scaled(pm.width() * self.scale, pm.height() * self.scale, Qt.IgnoreAspectRatio, Qt.FastTransformation))
        self.desc_label.setText(modes[s])

    def new_file(self):
        fname = str(QFileDialog.getOpenFileName(
            self, "Open File",
            "Tests/images", "Images (*.png *.xpm *.jpg)"))
        self.change_image(str(fname))

    def change_image(self, fname):
        self.im = Image.open(fname)
        self.scale = 1
        while max(self.im.size) * (self.scale + 1) <= 800:
            self.scale += 1

        self.on_combo_change(self.combo.currentText())

def main():
    fname = sys.argv[1]
    app = QApplication(sys.argv)
    mw = MyWidget(fname)
    mw.show()
    sys.exit(app.exec_())

if __name__ == '__main__':
    main()

@wiredfool wiredfool added this to the 3.0.0 milestone Sep 20, 2015
wiredfool added a commit that referenced this pull request Sep 20, 2015
bug fix: Qt wants data aligned to 32 bits
@wiredfool
wiredfool merged commit 68dd0ac into python-pillow:master Sep 20, 2015
@wiredfool

Copy link
Copy Markdown
Member

Thanks for the viewer. I'm not seeing the effects of this patch in the viewer, as it's working in master and this branch on 1 images. The tests, on the other hand, fail nicely in master and succeed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants