17
\$\begingroup\$

I'm currently making a manga (read: comic) viewer in Python. This project has been a code-as-you-learn project, because I have been trying to code this as I learned about Tkinter. Python I have known for some time, but not too long.

Performance-wise, I'm worried about the image loading and resizing time; it seems slow. I found out one thing by experimenting: when resizing "P" type images it is a lot slower than converting it to "L" (greyscale) and then resizing. Also the fullscreen is buggy, but this (and other minor bugs, coding format problems) is because I have thrown this together and haven't really re-coded it nicely yet (as I often do after I learn what I want it to be like, if you understand what I mean).

Format-wise, I don't think I have the best organization there is, maybe there is a better practice to hold up, or multiple files maybe (but Python can't do this?)?

Once again there are a lot of little bugs, like scrolling with the keyboard reveals extra space on the bottom, and the folder viewer doesn't always scroll to the selected folder in the dialog, and I would like to know how to fix this, but I would like more to know some good practices and optimization for image loading and resizing.

from Tkinter import *
from ttk import *
import Image, ImageTk, tkFileDialog, os


VERSION = "v0.0.3"


"""
folderDialog Class

Dialog that asks the user to select a folder. Returns folder and gets destroyed.

"""
class folderDialog(Toplevel):
    def __init__(self, parent, callback, dir="./", fileFilter=None):
        Toplevel.__init__(self, parent)
        self.transient(parent)

        self.title("Browse Folders")
        self.parent = parent
        self.dir = StringVar()
        self.callback = callback
        self.fileFilter = fileFilter

        if os.path.exists(dir) and os.path.isdir(dir):
            self.dir.set(os.path.abspath(dir))
        else:
            self.dir.set(os.path.abspath("./"))

        self.body = Frame(self)
        self.body.grid(row=0,column=0,padx=5,pady=5, sticky=(N,S,E,W))

        Label(self.body, text="Please select a folder").grid(row=0,column=0, sticky=(N,S,W), pady=3)
        Label(self.body, text="You are in folder:").grid(row=1,column=0, sticky=(N,S,W))
        Entry(self.body, textvariable=self.dir, state="readonly").grid(row=2,column=0,sticky=(N,S,E,W),columnspan=2)
        self.treeview = Treeview(self.body, columns=("dir", "imgs"), show="headings")
        self.treeview.grid(row=3,column=0,sticky=(N,S,E,W),rowspan=3,pady=5,padx=(0,5))

        self.treeview.column("imgs", width=30, anchor=E)
        self.treeview.heading("dir", text="Select a Folder:", anchor=W)
        self.treeview.heading("imgs", text="Image Count", anchor=E)
        #self.treeview.heading(0, text="Select Directory")
        #self.listbox = Listbox(self.body, activestyle="dotbox", font=("Menu", 10))
        #self.listbox.grid(row=3,column=0, sticky=(N,S,E,W),rowspan=3,pady=5,padx=(0,5))

        ok = Button(self.body, text="Use Folder")
        ok.grid(row=3,column=1,sticky=(N,E,W), pady=5)

        cancel = Button(self.body, text="Cancel")
        cancel.grid(row=4,column=1,sticky=(N,E,W), pady=5)        

        self.grab_set()

        self.protocol("WM_DELETE_WINDOW", self.cancel)
        self.bind("<Escape>", self.cancel)
        cancel.bind("<Button-1>", self.cancel)
        ok.bind("<Button-1>", self.selectFolder)
        self.treeview.bind("<Left>", self.newFolder)
        self.treeview.bind("<Right>", self.newFolder)
        self.treeview.bind("<Return>", self.selectFolder)
        self.treeview.bind("<Up>", self.onUpDown)
        self.treeview.bind("<Down>", self.onUpDown)
        self.treeview.bind("<<TreeviewSelect>>", self.onChange)

        self.geometry("%dx%d+%d+%d" % (450, 400,
            parent.winfo_rootx()+int(parent.winfo_width()/2 - 200),
            parent.winfo_rooty()+int(parent.winfo_height()/2 - 150)
        ))

        self.updateListing()
        self.treeview.focus_set()
        self.resizable(0,0)

        self.columnconfigure(0, weight=1)
        self.rowconfigure(0, weight=1)
        self.body.columnconfigure(0, weight=1)
        self.body.rowconfigure(5, weight=1)

        self.wait_window(self)

    def newFolder(self, event):
        newDir = self.dir.get()
        if event.keysym == "Left":
            #newDir = os.path.join(newDir, "..")
            self.upFolder()
            return
        else:
            selected = self.getSelected()

            if selected == ".":
                #special super cool stuff here
                self.selectFolder()
                return
            elif selected == "..":
                self.upFolder()
                return
            else:
                newDir = os.path.join(newDir, selected)

        self.dir.set(os.path.abspath(newDir))
        self.updateListing()

    def upFolder(self):
        cur = os.path.split(self.dir.get())
        newDir = cur[0]
        cur = cur[1]

        self.dir.set(os.path.abspath(newDir))
        self.updateListing()

        children = self.treeview.get_children()
        for child in children:
            if self.treeview.item(child, "text") == cur:
                self.treeview.selection_set(child)
                self.treeview.focus(child)
                #print "please see"
                self.treeview.see(child)
                return

    def onChange(self, event=None):
        #print event
        sel = self.treeview.focus()
        if sel == '':
            return #not possible, but just in case

        if self.treeview.item(sel, "values")[1] == "?":
            #print "Has ?"
            self.imgCount()

    def imgCount(self):      
        folder = os.path.join(self.dir.get(), self.getSelected())
        folder = os.path.abspath(folder)

        count = 0

        dirList = os.listdir(folder)
        for fname in dirList:
            if self.fileFilter == None:
                count = count + 1
            else:
                ext = os.path.splitext(fname)[1].lower()[1:]
                #print ext
                for fil in self.fileFilter:
                    #print fil
                    if ext == fil:
                        count = count + 1
                        break

        #print count
        sel = self.treeview.focus()  
        newV = (self.treeview.item(sel, "values")[0], str(count))
        self.treeview.item(sel, value=newV)


    def onUpDown(self, event):
        sel = self.treeview.selection()
        if len(sel) == 0:
            return
        active = self.treeview.index(sel[0])
        children = self.treeview.get_children()
        length = len(children)
        toSelect = 0
        if event.keysym == "Up" and active == 0:
            toSelect = length - 1
        elif event.keysym == "Down" and active == length-1:
            toSelect = 0
        else:
            return

        toSelect = children[toSelect]
        self.treeview.selection_set(toSelect)
        self.treeview.focus(toSelect)
        self.treeview.see(toSelect)
        return 'break'


    def updateListing(self, event=None):
        folder = self.dir.get()
        children = self.treeview.get_children()
        for child in children:
            self.treeview.delete(child)
        #self.treeview.set_children("", '')
        dirList = os.listdir(folder)

        first = self.treeview.insert("", END, text=".", values=("(.) - Current Folder", "?"))
        self.treeview.selection_set(first)
        self.treeview.focus(first)

        self.treeview.insert("", END, text="..", values=("(..)", "?"))
        #self.listbox.insert(END, "(.) - Current Folder")
        #self.listbox.insert(END, "(..)")
        for fname in dirList:
            if os.path.isdir(os.path.join(folder, fname)):
                #self.listbox.insert(END,fname+"/")
                self.treeview.insert("", END, values=(fname+"/", "?"), text=fname)

    def selectFolder(self, event=None):
        selected = os.path.join(self.dir.get(), self.getSelected())
        selected = os.path.abspath(selected)
        self.callback(selected, self)
        self.cancel()

    def getSelected(self):
        selected = self.treeview.selection()

        if len(selected) == 0:
            selected = self.treeview.identify_row(0)
        else:
            selected = selected[0]

        return self.treeview.item(selected, "text")

    def ok(self):

        #print "value is", self.e.get()

        self.top.destroy()

    def cancel(self, event=None):
        self.parent.focus_set()
        self.destroy()




"""
Img Class

Stores path to img and manipulates (resizes).

"""
class Img:
    def __init__(self, path):
        self.path = path
        self.size = 0,0
        self.oSize = 0,0
        self.img = None
        self.tkpi = None

        split = os.path.split(self.path)
        self.folderName = os.path.split(split[0])[1]
        self.fileName = split[1]

        self.stats()
        #print "Loaded " + path
        self.img = None

    def stats(self):
        self.img = Image.open(self.path)
        self.size = self.img.size
        self.oSize = self.img.size

    def load(self):
        self.img = Image.open(self.path)#.convert("RGB") #RGB for better resizing
        #print self.img.mode
        if self.img.mode == "P":
            self.img = self.img.convert("L") #L scales much more nicely than P

    def unload(self):
        self.img = None
        self.tkpi = None
        self.size = self.oSize

    def fit(self, size):
        #ratio = min(1.0 * size[0] / self.oSize[0], 1.0 * size[1] / self.oSize[1])
        ratio = 1.0 * size[0] / self.oSize[0]
        ratio = min(ratio, 1.0)
        #print ratio
        self.size = (int(self.oSize[0] * ratio), int(self.oSize[1] * ratio))
        #print self.size

    def resize(self, size):
        #self.fit(size)
        self.load()
        self.img = self.img.resize(self.size, Image.BICUBIC)
        #self.img = self.img.resize(self.size, Image.ANTIALIAS)
        self.tkpi = ImageTk.PhotoImage(self.img)
        return self.tkpi

    def quickResize(self, size):
        self.fit(size)
        if self.img == None:
            self.load()
        self.img = self.img.resize(self.size)
        self.tkpi = ImageTk.PhotoImage(self.img)
        return self.tkpi





"""
MangaViewer Class

The main class, runs everything.

"""
class MangaViewer:
    def __init__(self, root):
        self.root = root
        self.setTitle(VERSION)
        root.state("zoomed")

        self.frame = Frame(self.root)#, bg="#333333")#, cursor="none")
        self.canvas = Canvas(self.frame,xscrollincrement=15,yscrollincrement=15,bg="#1f1f1f", highlightthickness=0)
        scrolly = Scrollbar(self.frame, orient=VERTICAL, command=self.canvas.yview)

        self.canvas.configure(yscrollcommand=scrolly.set)

        #self.img = Image.open("C:\\Users\\Alex\\Media\\manga\\Boku wa Tomodachi ga Sukunai\\16\\02-03.png")
        #self.tkpi = ImageTk.PhotoImage(self.img)
        #self.imgId = self.canvas.create_image(0,0, image=self.tkpi, anchor="nw")
        #self.canvas.configure(scrollregion=self.canvas.bbox(ALL))
        self.files = []
        self.current = 0

        self.canvas.bind("<Configure>", self.onConfig)
        self.root.bind("<Up>", self.onScroll)
        self.root.bind("<Down>", self.onScroll)
        self.root.bind("<Left>", self.onNewImg)
        self.root.bind("<Right>", self.onNewImg)
        self.root.bind("<d>", self.getNewDirectory)
        self.root.bind("<f>", self.toggleFull)
        self.root.bind("<Motion>", self.onMouseMove)
        #Windows
        self.root.bind("<MouseWheel>", self.onMouseScroll)
        # Linux
        self.root.bind("<Button-4>", self.onMouseScroll)
        self.root.bind("<Button-5>", self.onMouseScroll)
        self.root.bind("<Escape>", lambda e: self.root.quit())

        self.frame.grid(column=0, row=0, sticky=(N,S,E,W))
        self.canvas.grid(column=0,row=0, sticky=(N,S,E,W))
        #scrolly.grid(column=1, row=0, sticky=(N,S))

        self.root.columnconfigure(0, weight=1)
        self.root.rowconfigure(0, weight=1)
        self.frame.columnconfigure(0, weight=1)
        self.frame.rowconfigure(0, weight=1)

        self.resizeTimeO = None
        self.mouseTimeO = self.root.after(1000, lambda x: x.frame.configure(cursor="none"), self)
        self.lastDir = os.path.abspath("./")
        self.imgId = None
        self.fullscreen = False

    def toggleFull(self, event=None):
        if self.fullscreen:
            root.overrideredirect(False)
        else:
            root.overrideredirect(True)

        self.fullscreen = not self.fullscreen
        self.onConfig(None)

    def setTitle(self, *titles):
        st = ""
        for title in titles:
            st = st + " " + str(title)
        self.root.title("MangaViewer  - " + st)

    def setTitleToImg(self):
        self.setTitle(self.files[self.current].folderName,"-", 
            self.files[self.current].fileName,"(",(self.current+1),"/",len(self.files),")")

    def onMouseMove(self, event):
        """hide cursor after some time"""

        #print event
        self.frame.configure(cursor="")
        if self.mouseTimeO != None:
            self.root.after_cancel(self.mouseTimeO)
        self.mouseTimeO = self.root.after(1000, lambda x: x.frame.configure(cursor="none"), self)

    def onMouseScroll(self, event):
        #mousewheel for windows, mousewheel linux, or down key
        if event.num == 4 or event.delta == 120:
            self.canvas.yview("scroll", -3, "units")
        else:
            self.canvas.yview("scroll", 3, "units")

    def onScroll(self, event):
        """called when the up or down arrow key is pressed"""

        if event.keysym == "Down":
            self.canvas.yview("scroll", 1, "units")
        else:
            self.canvas.yview("scroll", -1, "units")

    def onNewImg(self, event):
        """called when the left or right arrow key is pressed, changes the image"""

        change = 1 #right key
        if event.keysym == "Left":
            change = -1

        newImg = self.current + change
        if newImg < 0 or newImg >= len(self.files):
            self.getNewDirectory()
            return

        #self.img = self.files[newImg];
        #self.tkpi = ImageTk.PhotoImage(self.img)
        #self.canvas.delete(self.imgId)
        #self.imgId = self.canvas.create_image(0,0, image=self.tkpi, anchor="nw")
        #self.canvas.configure(scrollregion=self.canvas.bbox(ALL)) #needed?
        self.files[self.current].unload()
        self.current = newImg
        self.setTitleToImg()
        self.onConfig(None, True)

    def getNewDirectory(self, event=None):
        folderDialog(self.root, self.selNewDirectory, self.lastDir, fileFilter=["jpg", "png", "gif", "jpeg"])


    def selNewDirectory(self, dirname, fd):
        """callback given to folderDialog"""

        fd.cancel() #destroy the folderDialog

        if self.lastDir == dirname:
            return

        self.lastDir = dirname
        #print dirname
        dirList = os.listdir(dirname)
        self.files = []
        self.current = -2
        for fname in dirList:
            ext = os.path.splitext(fname)[1].lower()
            if ext == ".png" or ext == ".jpg" or ext == ".jpeg" or ext == ".gif":
                self.files.append(Img(os.path.join(dirname, fname)))
        self.current = 0
        if len(self.files) == 0:
             return
        self.setTitleToImg()
        self.onConfig(None, True)

    def resize(self, finalResize=False):
        """resizes the image"""

        canvasSize = (self.canvas.winfo_width(), self.canvas.winfo_height())
        tkpi = None
        if finalResize:
            tkpi = self.files[self.current].resize(canvasSize)
        else:
            tkpi = self.files[self.current].quickResize(canvasSize)

            if self.resizeTimeO != None: #is this the best way to do this?
                self.root.after_cancel(self.resizeTimeO)
            self.root.after(200, self.onConfig, None, True)

        if self.imgId != None:
            self.canvas.delete(self.imgId)

        self.imgId = self.canvas.create_image(0,0, image=tkpi, anchor="nw")
        #self.canvas.configure(scrollregion=self.canvas.bbox(ALL))
        bbox = self.canvas.bbox(ALL)
        #nBbox = (bbox[0], bbox[1]-60, bbox[2], bbox[3]+60)
        nBbox = bbox
        self.canvas.configure(scrollregion=nBbox)
        #print self.canvas.bbox(ALL)

    def onConfig(self, event, finalResize=False):
        """runs the resize method and centers the image"""

        if self.current < 0 or self.current >= len(self.files):
            return

        self.canvas.yview("moveto", 0.0)
        self.resize(finalResize)

        newX = (self.canvas.winfo_width() - self.files[self.current].size[0])/2
        #newY - 60 TODO change to preference padding
        newY = (self.canvas.winfo_height() - self.files[self.current].size[1])/2# - 60
        newY = max(newY, 0)

        self.canvas.coords(self.imgId, newX, newY)
        self.canvas.yview("moveto", 0.0)
        bbox = self.canvas.bbox(ALL)
        nbbox = (0,0, bbox[2], max(bbox[3], self.canvas.winfo_height()))
        self.canvas.configure(scrollregion=nbbox)

root = Tk()
MangaViewer(root)
root.mainloop()
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I think you post to the wrong community. This community concentrates on "code quality" issues, rather than on a "functional issues" of the code. Try to find more appropriate place to ask this question. StackOvervflow or some forums dedicated to ImageProcessing. \$\endgroup\$
    – Moisei
    Sep 13, 2011 at 10:57
  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ Sep 19, 2017 at 17:29

2 Answers 2

5
\$\begingroup\$

As Moisei pointed out, Code Review isn't about functional issues. I guess performance can be discuted, but I don't know how to make resizing fast.

Your code looks good overall, but I don't know about Tkinter idioms. Here are a few comments:

  1. You should check if __name__ == "__main__": before running the mainloop. This will allow you or someone else to import the code you written without having it launching the GUI.

  2. st = ""
    for title in titles:
        st = st + " " + str(title)
    

    Write this as " ".join(titles)

  3. #special super cool stuff here or even #newDir = os.path.join(newDir, "..")

    Please care about comments.

\$\endgroup\$
3
\$\begingroup\$

There is a problem related to the imports:

from Tkinter import *
from ttk import *

ttk has 17 widgets, eleven of which already exist in Tkinter. Regarding the way you run the imports, you are going to use only the 11 widgets of ttk. This is because your second import overrides the first one.

What I mean is that when you code, for instance, a button widget button_name = Button(... options ... ), you are using the themed button, not the "normal" button of Tkinter. To remedy to this problem, you should code the imports statements like this:

import Tkinter as Tk
import ttk

Note that this is valid for Python2.x you are using. Otherwise, in Python3.x you ttk is not a package of its own, and you have to code from tkinter import ttk, instead.

\$\endgroup\$

Your Answer

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

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