I have a little project going on and so far I'm not really having any problems. But since I haven't internalized all of Python's core features yet, I'm pretty sure my code offers pretty many subjects to optimize.

Please point out anything that could be done better. For example, I'm feeling uncomfortable with the method updateMousePosition as it seems kind of ugly to me.

import sys

from PySide import QtCore, QtGui
from PySide.QtGui import QWidget, QApplication, QPixmap, QLabel

from UI_TextureViewer import Ui_UI_TextureViewer
from ExtendedLabel import ExtendedLabel

class TextureViewer(QWidget, Ui_UI_TextureViewer):
    """ A widget that displays a single texture.

    The textures resides in a ExtendedLabel (which enables connecting to a
    mouseMovedSignal) which is put inside a QScrollArea to enable arbitrary
    The widget also shows the u and v coordinates based on the position of the

    def __init__(self, filename, parent=None):
        """ Default ctor.

        Connects all buttons, loads the texture from file and sets up a default
        zoom value of 1.0"""

        super(TextureViewer, self).__init__(parent)
        self.filename = filename


        self.u = 0
        self.v = 0

        self.zoomValue = 1.0


    def loadImage(self):
        """ Loads the image stored in self.filename and sets up the labels
        showing information about the original (umzoomed) image. """

        self.image = QPixmap()
        imgs = [str(self.image.size().width()), str(self.image.size().height())]

    def zoom(self, factor):
        """ Zooms the texture by the given factor.
        Zooming is achieved by creating a scaled copy of the original image and
        showing it by setting setPixmap of an ExtendedLabel. """

        imageLabel = ExtendedLabel()
        imageLabel.setAlignment(QtCore.Qt.AlignLeft | QtCore.Qt.AlignTop)
        ow, oh = [self.image.size().width(), self.image.size().height()]
        sw, sh = [ow * factor, oh * factor]
        zoomedImage = self.image.scaled(sw, sh)

    def updateZoomLabel(self):
        """ Updates the label that shows the current zoom value. """
        f = self.zoomValue * 100.0

    def zoomIn(self):
        """ Zooms in 25% on the image. """
        self.zoomValue += 0.25

    def zoomOut(self):
        """ Zooms out 25% of the image.
        If the zoom value is less than 25%, then the step size is decreased to
        5%. Total zoom level is clamped to 5%. """
        if self.zoomValue <= 0.25:
            v = 0.05
            v = 0.25
        if self.zoomValue - v > 0.05:
            self.zoomValue -= v

    def resetZoom(self):
        """ Resets the zoom factor to 1.0. """
        self.zoomValue = 1.0

    def updateMousePosition(self, event):
        """ Slot that is called by the mouseMovedSignal of the ExtendedLabel
        which shows the image.
        Computes the u and v coordinates of the current mouse position and
        updates the labels showing the coordinates. """
        absx, absy = [event.x(), event.y()]
        sx, sy = [self.image.width() * self.zoomValue,
             self.image.height() * self.zoomValue]
        self.u = float(absx) / float(sx)
        self.v = float(absy) / float(sy)
        if self.u > 1.0:
            self.u = 1.0
        if self.v > 1.0:
            self.v = 1.0

if __name__ == '__main__':
    app = QApplication(sys.argv)
    frame = TextureViewer("../../media/textures/DarkGrass.png")

1 Answer 1


A few things I can see that will clean up the code a tad are:

def updateMousePosition(self, event):
    """ Slot that is called by the mouseMovedSignal of the ExtendedLabel
    which shows the image.
    Computes the u and v coordinates of the current mouse position and
    updates the labels showing the coordinates. """
    sx = self.image.width() * self.zoomValue
    sy = self.image.height() * self.zoomValue
    self.u = min(float(event.x()) / float(sx), 1.0)
    self.v = min(float(event.y()) / float(sy), 1.0)

absx and absy are used once, so I don't see a point in assigning them to temp vars. I'd put sx/sy on separate lines. (As an aside, your use of square brackets is pretty spurious as "x,y = 1,2" has the same net effect as "x,y = [1,2]" without the creation of a temp list.) Your if's are basically capping self.u and self.v at 1.0. Also are you actually using the self.v and self.u anywhere else or are they just locals to get the values for the setText() calls? If so, they could just be local variables like f in updateZoomLabel().


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.