openlp-core team mailing list archive

[Martin Thompson <mjthompson@xxxxxxx>]

Review: Approve
In render.scale_bg_image(), I have a feeling all that messing about (my wx legacy code - sorry!) keeping the aspect ratio correct is now unnecessary, just the call to image.scaled with the KeepAspectRatio param will do it if the neww and newh are just set to the size of the new screen to fit to...

In render_single_line() - the 't' variable has been replaced by self._theme - this may have a significant -ve performance impact (I should've commented this when I wrote it :) -- effectively "t" acts as a cache to the member variable, which IIRC improved the render performance.  However, given my own mantra of nor optimising too early, it's probably worth changing to the more readable version, until it's proven!

I can see I'll have to revisit bits to make the unit tests pass again :)

I think snoop_image would be better being passed a list of images?  And it's duplicated - better to have it in /lib/ ?

Line 1432 of the diff - OnMediaNewClick - the file filters should be 
u'Videos (*.avi *.mpeg);;Audio (*.mp3 *.ogg *.wma);;All files (*)')



-- 
https://code.launchpad.net/~trb143/openlp/servicing/+merge/6659
Your team openlp.org Core is subscribed to branch lp:openlp.