mypaint / mypaint

MyPaint is a simple drawing and painting program that works well with Wacom-style graphics tablets.
https://mypaint.app
GNU General Public License v2.0
2.66k stars 385 forks source link

Error on save or quit after gdk pixbuf update #236

Closed jy2wong closed 9 years ago

jy2wong commented 9 years ago

I'm getting a no savev attribute error similar to the ones described in #202. I did a scons and a git submodule update right before just in case, too. Everything's been fine from January 14th up until March 1st, which seems to be the day I updated gdk-pixbuf2 to version 2.31.2. (and the day mypaint ate 20 minutes of tricky lineart fixes...)

Nothing in the changelog jumps out to me as suspicious. http://upstream.rosalinux.ru/changelogs/gdk-pixbuf/2.31.2/changelog.html

I'll be investigating more today.

Steps to reproduce:

  1. run mypaint -c /tmp/fresh
  2. press ctrl+q to quit
Mypaint version: 1.1.1-alpha.20150304+git.e5ba4c6
System information: Linux-3.18.5-1-x61t-x86_64-with-glibc2.2.5
Traceback (most recent call last):
  File "/home/jy2/code/foss/mypaint/gui/drawwindow.py", line 654, quit_cb(self=<DrawWindow object at 0x7fbf46fd00a0 (MyPaintDrawWindow at 0x2512230)>, *junk=(<Action object at 0x7fbf46343320 (GtkAction at 0x24d7470)>,))
            self.app.doc.model.sync_pending_changes()
            self.app.save_gui_config()  # FIXME: should do this periodically, not only on quit
  variables: {'self.app.save_gui_config': ('local', <bound method Application.save_gui_config of <gui.application.Application object at 0x7fbf46fbe290>>)}
  File "/home/jy2/code/foss/mypaint/gui/application.py", line 586, save_gui_config(self=<gui.application.Application object>)
            self.preferences["workspace.layout"] = workspace.get_layout()
            self.save_settings()
  variables: {'self.save_settings': ('local', <bound method Application.save_settings of <gui.application.Application object at 0x7fbf46fbe290>>)}
  File "/home/jy2/code/foss/mypaint/gui/application.py", line 400, save_settings(self=<gui.application.Application object>)
            self.brushmanager.save_brushes_for_devices()
            self.brushmanager.save_brush_history()
            self.filehandler.save_scratchpad(self.scratchpad_filename)
  variables: {'self.brushmanager.save_brush_history': ('local', <bound method BrushManager.save_brush_history of <gui.brushmanager.BrushManager object at 0x7fbf46722810>>)}
  File "/home/jy2/code/foss/mypaint/gui/brushmanager.py", line 789, save_brush_history(self=<gui.brushmanager.BrushManager object>)
            for brush in self.history:
                brush.save()
  variables: {'brush.save': ('local', <bound method ManagedBrush.save of <ManagedBrush u'history_0' p=deevad/rough>>)}
  File "/home/jy2/code/foss/mypaint/gui/brushmanager.py", line 1048, save(self=<ManagedBrush u'history_0' p=deevad/rough>)
            logger.debug("Saving brush preview to %r", preview_filename)
            lib.pixbuf.save(self.preview, preview_filename, "png")
            # Save brush settings
  variables: {'preview_filename': ('local', u'/tmp/fresh/brushes/history_0_prev.png'), 'lib.pixbuf.save': ('global', <function save at 0x7fbf484e08c0>), 'self.preview': ('local', <Pixbuf object at 0x7fbf463b0050 (GdkPixbuf at 0x2348c00)>)}
  File "/home/jy2/code/foss/mypaint/lib/pixbuf.py", line 64, save(pixbuf=<Pixbuf object at 0x7fbf463b0050 (GdkPixbuf at 0x2348c00)>, filename=u'/tmp/fresh/brushes/history_0_prev.png', type='png', **kwargs={})
        else:
            return pixbuf.savev(filename, type, kwargs.keys(), kwargs.values())
AttributeError: 'Pixbuf' object has no attribute 'savev'
achadwick commented 9 years ago

Ugh, the GdkPixbuf API varies so much from one {os, revision} to another.

Couple of things to try: does making the "backhanded" fix in https://github.com/mypaint/mypaint/blob/cf527fc2fa63d69591f2dcd27a3e68663ae0e57d/lib/pixbuf.py#L38 available for all systems fix the issue?

Also, here's the tool I use for deciding what is available in a given PyGI lib:

#!/usr/bin/python

import os.path
import sys
import gi
gi.require_version("Gdk", "3.0")

try:
    gimodname = sys.argv[1]
    substrings = sys.argv[2:]
except IndexError:
    print "usage: %s GIMODNAME [SUBSTRINGS...]" % os.path.basename(sys.argv[0])
    print
    print "Imports gi.repository.GI.MOD.NAME and searches its contents",
    print "for SUBSTRINGS."
    print "If there are no SUBSTRINGS, all contents are listed."
    sys.exit(1)

modbase = gimodname.split(".", 1)[0]
mod = __import__("gi.repository.%s" % modbase, {}, {}, [])
if substrings:
    for substring in substrings:
        print eval("[n for n in dir(gi.repository.%s) if %r in n.lower()]" %
                   (gimodname, substring.lower()))
else:
    print eval("dir(gi.repository.%s)" % gimodname)

Use it as, e.g.

$ pygi-grep GdkPixbuf.Pixbuf save
['save_to_bufferv', 'save_to_callbackv', 'save_to_stream_finish', 'savev']

to see what you have available :/

jy2wong commented 9 years ago

Maybe I missed the link in the last thread, but https://trac.macports.org/ticket/46812 seems to confirm that it is indeed 2.31.2 to blame.

* jy2 >> pygi-grep GdkPixbuf.Pixbuf save
['save', 'save_to_buffer', 'save_to_callback', 'save_to_stream_finish']

so the previous backhanded fix won't work (and will stop working eventually even for its original purpose).

...what does work, however, is renaming savev to save without changing any parameters.

achadwick commented 9 years ago

FWIW, MINGW-packages is using 2.31.1. Need to check what that supports, although .save_to_callbackv must be in there.

I guess we should try hasattr(GdkPixbuf.Pixbuf, "savev") first, because that name will always mean a vector version if it exists, and then try pixbuf.save() in the hope that it takes two vectors?

What a mess. Upstream bug is https://bugzilla.gnome.org/show_bug.cgi?id=670372#c7 - perhaps we should just wait for that to unbreak the GdkPixbuf API.

jy2wong commented 9 years ago

I was thinking

try:
    pixbuf.savev(...)
except AttributeError:
    pixbuf.save(...)

which is basically the same thing.

I'm erring towards fixing it up now. The cruft is limited to an extra three lines all in one place, and if you happen to have the wrong version of gdk-pixbuf, mypaint is...very unusable.

achadwick commented 9 years ago

GdkPixbuf save method summary:

OS Distro Version Save method names
GNU/Linux Ubuntu 12.04 2.30.7 ['save_tobufferv', 'save_tocallbackv', 'save_to_stream_finish', 'savev']
Win7 MSYS2 MINGW32 2.31.1 ['save_to_bufferv', 'save_to_callbackv', 'save_to_stream_finish', 'savev_utf8']
GNU/Linux Debian testing 2.31.1 ['save_tobufferv', 'save_tocallbackv', 'save_to_stream_finish', 'savev']
GNU/Linux Rosa? 2.31.2 ['save', 'save_tobuffer', 'save_tocallback', 'save_to_stream_finish']

I've bolded any which I know work and take vector-style arguments, italicized any that look plausible as a workaround for this issue, and struck out any which just appear to be incorrectly annotated when run. For example, for the Windows example I tried

>>> from gi.repository import GdkPixbuf
>>> GdkPixbuf.PIXBUF_VERSION
'2.31.1'
>>> p = GdkPixbuf.Pixbuf.new(GdkPixbuf.Colorspace.RGB, True, 8, 64, 64)
>>> p.savev_utf8("test1.png", "png", ["tEXt::greeting"], ["Hello, world"])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Must be string, not list
>>> p.savev_utf8("test1.png", "png", "tEXt::greeting", "Hello, world")
/mingw32/bin/python2: line 6:  3344 Segmentation fault      "$( dirname ${BASH_SOURCE[0]} )/python2.exe" "$@"

but it's supposed to work like just like the nonexistant savev(), supposedly: in the C source it's just a macro alias: https://git.gnome.org/browse/gdk-pixbuf/tree/gdk-pixbuf/gdk-pixbuf-core.h?id=2.31.1#n320


@jy2wong Can you confirm that save() works and saves that tEXt chunk on your test system? If so, the patch you suggest looks good.

(I'd be quite happy to use the backhanded save_to_callback[v] approach on all systems too, to gain some measure of control over filesystem encoding issues, which are a recurring nightmare on Windows expecially. Reducing codepaths is a good idea. Perhaps:

try:
    save_to_callbackv = pixbuf.save_to_callbackv
except AttributeError:
    save_to_callbackv = pixbuf.save_to_callback
save_to_callbackv(...)

might be the right coordinated approach?)

achadwick commented 9 years ago

Turns out my memory of "save" and no "savev" was from the PyGTK/GTK2 era, and it had sugared syntax in that old manual wrapping:

>>> from gtk import gdk
>>> [n for n in dir(gdk.Pixbuf) if "save" in n]
['save', 'save_to_callback']
>>> p = gdk.Pixbuf(gdk.COLORSPACE_RGB, True, 8, 64, 64)
>>> p.save("/tmp/test3.png", "png", {"tEXt::greeting": "Hello, world"})
>>> 

We don't have to support this any more, so testing for "save" before "savev" would be just as legit.

jy2wong commented 9 years ago
Python 2.7.9 (default, Dec 11 2014, 04:42:00) 
[GCC 4.9.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from gi.repository import GdkPixbuf
>>> GdkPixbuf.PIXBUF_VERSION
'2.31.2'
>>> p = GdkPixbuf.Pixbuf.new(GdkPixbuf.Colorspace.RGB, True, 8, 64, 64)
>>> p.save("test1.png", "png", ["tEXt::greeting"], ["Hello, world"])
True

all seems fine.