GNOME Bugzilla – Bug 612501
[patch] TypeError in do_simple_paint
Last modified: 2010-04-20 23:13:53 UTC
Original bug reported on Launchpad : https://bugs.launchpad.net/ubuntu/+source/pitivi/+bug/536624 Traceback: Traceback (most recent call last):
+ Trace 220913
cr.reset_clip() TypeError: Required argument 'cr' (pos 1) not found
This happens when I add the same clip twice on the timeline and try to overlap them. Ubuntu Lucid with PiTiVi from Git (0.13.4), PyCairo from Git. I've made some research on the reset_clip() function, and found this tutorial (http://zetcode.com/tutorials/pygtktutorial/drawingII/) containing a script using reset_clip() function, not working as well. I changed this -cr.reset_clip() +cr.reset_clip(cr, widget.window) in that script, and it finally worked. So before reporting a bug against PyCairo, I wanted to know if someone can reproduce that bug or if it has something to do with my installation.
Hmmm. AFAICT, transitions have been merged in pitivi git master a few days ago, and this may change a lot of stuff regarding overlapping clips... can you check if that still applies?
I can finally reproduce this. You have to have a timeline with a bunch of clips in it, and randomly seek to one of the various clips and try hitting the spacebar to play/pause. Once in a while, it won't work (the play button will stay in its previous state), and on Lucid this makes apport pop up everytime! Many users will stumble upon this and apport will annoy the hell out of them.
Created attachment 156627 [details] debug log
Since a one-liner patch is proposed in comment #1, I'd like someone to review it and see if it makes sense.
No, the patch doesn't directly work for pitivi; widget.window is undefined in the pitivi codebase. It must be a patch against the other tutorial code that was mentioned.
Got a traceback :
+ Trace 221136
cr.reset_clip()
oh sorry, I hadn't seen that it was already posted.
print 'reset_clip: %r %r' % (cr, cr.reset_clip) reveals the following: reset_clip: <gtk.gdk.CairoContext object at 0xa739c30> <built-in method reset_clip of gtk.gdk.CairoContext object at 0xa739c30> Then Bilboed found this: "Note that code meant to be reusable should not call reset_clip() as it will cause results unexpected by higher-level code which calls clip(). Consider using save() and restore() around clip() as a more robust means of temporarily restricting the clip region." We suspect it's a bug in pycairo, needs further investigation.
Solution for Pitivi seems to be to call cr.reset_clip(cr=cr, drawable=self.get_canvas()) Also bug seems to come from pygtk, not pycairo. See my comment on Launchpad [1]. [1] https://bugs.launchpad.net/ubuntu/+source/pitivi/+bug/537619/comments/5
Created attachment 158064 [details] [review] Patch fixing call to gtk.gdk.CairoContext.reset_clip The attached patch replaces the call to a gtk.gdk.CairoContext by adding the parameters by name. If PyGtk changes to use "self" instead of requiring a "cr" parameter, it should be transparent. I am not sure using get_canvas().window is the best way to get the GdkDrawable of the canvas. It is not very well documented, but I don't see a different solution except finding a way not to call reset_clip.
I've got a branch on my repo called simple_paint_bug_612501 which substitutes calls to cr.save()/restore() for cr.reset_clip(). I would prefer to use this since hopefully it won't break PiTiVi on earlier versions of pygtk.
Brandon, I tried that patch. Seems to work (don't get that error and timeline seems to display correctly). I'm having other side-issues which seem unrelated to that patch. It would be great if someone on lucid could test that patch to confirm. github.com/emdash/pitivi.git
I tested, and Brandon's patch fixes the issue. Merge it?
Also tested, also fixed the issue. Please merge.
commit a9795568f5a0adca5a4277bf51868a5a30ff99dd Author: Brandon Lewis <brandon_lewis@alum.berkeley.edu> Date: Tue Apr 6 10:59:58 2010 -0700 pitivi/ui/curve.py: use save/restore instead of reset_clip in simple_paint closes bug 612501, as we avoid calling the improperly-wrapped cr.restore() method