Exception when saving device-specific brushes: too long path?

[Greenfox62]

When using tablet pen the strokes are delayed and there is a flicker between the mouse cursor and brush but just drawing the mouse seems fine.

Bug report

> python

Mypaint version: 1.2.0+git.f62444e.dirty
System information: Windows-7-6.1.7601-SP1
Traceback (most recent call last):
  File "C:\Program Files\MyPaint\share\mypaint\gui\drawwindow.py", line 674, quit_cb(self=<DrawWindow object at 0x3cb74b8 (MyPaintDrawWindow at 0x4019160)>, *junk=(<DrawWindow object at 0x3cb74b8 (MyPaintDrawWindow at 0x4019160)>, <GdkEvent at 0x86cd0d0 type=<enum GDK_DELETE of type GdkEventType>>))
            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 0x024DC790>>)}
  File "C:\Program Files\MyPaint\share\mypaint\gui\application.py", line 619, 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 0x024DC790>>)}
  File "C:\Program Files\MyPaint\share\mypaint\gui\application.py", line 416, save_settings(self=<gui.application.Application object>)
            """Saves the current settings to persistent storage."""
            self.brushmanager.save_brushes_for_devices()
            self.brushmanager.save_brush_history()
  variables: {'self.brushmanager.save_brushes_for_devices': ('local', <bound method BrushManager.save_brushes_for_devices of <gui.brushmanager.BrushManager object at 0x03D95230>>)}
  File "C:\Program Files\MyPaint\share\mypaint\gui\brushmanager.py", line 748, save_brushes_for_devices(self=<gui.brushmanager.BrushManager object>)
            for device_name, devbrush in self.brush_by_device.iteritems():
                devbrush.save()
  variables: {'devbrush.save': ('local', <bound method ManagedBrush.save of <ManagedBrush u'devbrush_%E6%85%97%E7%91%AC%E7%81%AF%E5%90%A0%E6%89%A1%E6%95%ACt+%E7%89%90%E7%8D%A5%E7%95%B3%E6%95%B2%E5%8C%A0%E7%A5%B4%E7%95%ACs%E8%94%84%C7%A5%E7%A0%90%E7%9B%A7%E8%93%B0%C7%A5%E8%94%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4' p=classic/knife>>)}
  File "C:\Program Files\MyPaint\share\mypaint\gui\brushmanager.py", line 1077, save(self=<ManagedBrush u'devbrush_%E6%85%97%E7%91%AC%E7%8...5%EE%B5%A8%22%E2%91%8A%E6%92%B4' p=classic/knife>)
            logger.debug("Saving brush preview to %r", preview_filename)
            lib.pixbuf.save(self.preview, preview_filename, "png")
            # Save brush settings
  variables: {'lib.pixbuf.save': ('global', <function save at 0x0339D8F0>), 'preview_filename': ('local', u'C:\\Users\\Greenfox62\\AppData\\Local\\mypaint\\brushes\\devbrush_%E6%85%97%E7%91%AC%E7%81%AF%E5%90%A0%E6%89%A1%E6%95%ACt+%E7%89%90%E7%8D%A5%E7%95%B3%E6%95%B2%E5%8C%A0%E7%A5%B4%E7%95%ACs%E8%94%84%C7%A5%E7%A0%90%E7%9B%A7%E8%93%B0%C7%A5%E8%94%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4_prev.png'), 'self.preview': ('local', <Pixbuf object at 0x529ab70 (GdkPixbuf at 0x510dba8)>)}
  File "C:\Program Files\MyPaint\share\mypaint\lib\pixbuf.py", line 57, save(pixbuf=<Pixbuf object at 0x529ab70 (GdkPixbuf at 0x510dba8)>, filename=u'C:\\Users\\Greenfox62\\AppData\\Local\\mypaint...%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4_prev.png', type='png', **kwargs={})
        """
        with open(filename, 'wb') as fp:
            writer = lambda buf, size, data: fp.write(buf) or True
  variables: {'fp': (None, []), 'filename': ('local', u'C:\\Users\\Greenfox62\\AppData\\Local\\mypaint\\brushes\\devbrush_%E6%85%97%E7%91%AC%E7%81%AF%E5%90%A0%E6%89%A1%E6%95%ACt+%E7%89%90%E7%8D%A5%E7%95%B3%E6%95%B2%E5%8C%A0%E7%A5%B4%E7%95%ACs%E8%94%84%C7%A5%E7%A0%90%E7%9B%A7%E8%93%B0%C7%A5%E8%94%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4_prev.png'), 'open': ('builtin', <built-in function open>)}
IOError: [Errno 2] No such file or directory: 

> `u'C:\\Users\\Greenfox62\\AppData\\Local\\mypaint\\brushes\\devbrush_%E6%85%97%E7%91%AC%E7%81%AF%E5%90%A0%E6%89%A1%E6%95%ACt+%E7%89%90%E7%8D%A5%E7%95%B3%E6%95%B2%E5%8C%A0%E7%A5%B4%E7%95%ACs%E8%94%84%C7%A5%E7%A0%90%E7%9B%A7%E8%93%B0%C7%A5%E8%94%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4_prev.png

Tablet-Manhattan Model Number: 177412
Ram- 3GBs
Mouse- Manhattan Model:177016

[achadwick]

The stroke delay ("lag") issue is being handled elsewhere: #390. I don't know what "a flicker between the mouse cursor and the brush" means. But we can deal with that exception here 😄

@Greenfox62: It happens when you quit/exit/close MyPaint, correct? The exception tells me that it trying and failing to save the brush you last used with either your tablet or your mouse (sorry, I can't read the encoded characters!)

[achadwick]

The %XX encoding expands to a longish sequence of UTF-8 CJK characters that Google Translate autodetects as Chinese (but translates badly):

慗瑬灯吠扡敬t+牐獥畳敲匠祴畬s蔄ǥ砐盧蓰ǥ蔄ǥ"⑊撴

I'll assume that's a perfectly reasonable name for a device, although I've never seen any device name like that in Linux before.:open_mouth:

💭 I suspect the URL encoding we use to create a safe filename from these unpredictable strings might be making the path length go over a Windows OS hard limit.

The full path above is

C:\Users\Greenfox62\AppData\Local\mypaint\brushes\devbrush_%E6%85%97%E7%91%AC%E7%81%AF%E5%90%A0%E6%89%A1%E6%95%ACt+%E7%89%90%E7%8D%A5%E7%95%B3%E6%95%B2%E5%8C%A0%E7%A5%B4%E7%95%ACs%E8%94%84%C7%A5%E7%A0%90%E7%9B%A7%E8%93%B0%C7%A5%E8%94%84%C7%A5%EE%B5%A8%22%E2%91%8A%E6%92%B4_prev.png

which is 281 ASCII characters long! Does Windows have a hard limit at 256? Apparently, yes!

In the Windows API (with some exceptions discussed in the following paragraphs), the maximum length for a path is MAX_PATH, which is defined as 260 characters.

How to fix this

We should almost certainly be using hashes with standard lengths for naming the files we use when saving device brushes. A predictable UUID scheme should work: we use UUID-based temporary file names elsewhere without any issue.

UUID version 5 with a hardcoded MyPaint-specific namespace UUID looks appropriate. In python terms, that's uuid.uuid5() from the standard uuid module.

We'll need to think about backwards compatibility, and on which branch it will be appropriate to merge the fixes for this bug.

[achadwick]

At the very latest, this should be fixed properly for 1.3.0. If we're clever, a robust fix that reads new scheme names but falls back to old scheme names, and which writes both could be merged into v1.2.x before the 1.2.1 release.

[achadwick]

Retitled to refer to just the issue we can work on. Don't pack multiple problems into a single issue please! (Luckily the "extras" look like duplicates, or trivial glitches.)

This path limit is Windows-specific, but other OSes might suffer from similar problems. Let's leave it as a cross-platform one.

[Greenfox62]

Thanks alot, I hope its fixed cause MyPaint looks like a really cool program

[danpla]

Is there any reason to percent-encode a name? Passing the name as is should fix the issue.

However, for backward compatibility, this will require replacing spaces by +.

[achadwick]

@danpla
The names of devices might have file system special characters or NULs, which if left unencoded might result in files being overwritten in arbitrary locations.

In this case, a shorter hash or uuid is the right approach, I think. I'm pretty certain we don't need to do a reverse lookup from disk.

[danpla]

I can create a pull request for the issue (uses uuid5). (I did not know that you self-assigned it :( )
master...danpla:dev-uuid

BTW:

>>> '慗瑬灯吠扡敬t+牐獥畳敲匠祴畬s蔄ǥ砐盧蓰ǥ蔄ǥ"⑊撴'.encode('utf-16-le')
'Waltop Tablet\x00+\x00Pressure Stylus\x00\x04\x85\xe5\x01\x10x\xe7v\xf0\x84\xe5\x01\x04\x85\xe5\x01h\xed"\x00J$\xb4d'