HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Texture viewer widget

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
widgetviewertexture

Problem

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.

```
#!/usr/bin/python
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
zooming.
The widget also shows the u and v coordinates based on the position of the
mouse."""

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.setupUi(self)
self.filename = filename

self.buttonZoomIn.clicked.connect(self.zoomIn)
self.buttonZoomOut.clicked.connect(self.zoomOut)
self.buttonResetZoom.clicked.connect(self.resetZoom)

self.u = 0
self.v = 0

self.labelFilename.setText(filename)
self.zoomValue = 1.0

self.loadImage()

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()
self.image.load(self.filename)
imgs = [str(self.image.size().width()), str(self.image.size().height())]
self.labelSize.setText("x".join(imgs))
self.zoom(self.zoomValue)

def zoom(self, factor

Solution

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)
    self.labelU.setText("{0:.4f}".format(self.u))
    self.labelV.setText("{0:.4f}".format(self.v))


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().

Code Snippets

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)
    self.labelU.setText("{0:.4f}".format(self.u))
    self.labelV.setText("{0:.4f}".format(self.v))

Context

StackExchange Code Review Q#3267, answer score: 2

Revisions (0)

No revisions yet.