when an object is self observing it is subscribed to all notifications, this makes sense in order to destroy representation factories.

But the order of calling observers puts the self observing as third option. see https://github.com/typesupply/defcon/blob/master/Lib/defcon/tools/notifications.py#L146-L151

This causes issues when a factories has no destructive notifications (aka ".Changed" notification) as this is being called afterwards.

a small example to demonstrate this issue.

I don't have any idea to push self observing notifications upfront.
The factories could add all possible notifications but that does not feels right, especially with heavy calculating factories.

from defcon import Font, Kerning, registerRepresentationFactory

count = 0

def testFactory(kerning):
    print "    *** create a new repr ***"
    global count
    count += 1
    return count

registerRepresentationFactory(Kerning, "test", testFactory, destructiveNotifications=None)


class TestKerning(Kerning):
    
    def selfNotificationCallback(self, notification):
        print "    ----> self notification", notification.name
        super(TestKerning, self).selfNotificationCallback(notification)
        print "    ----> self notification done", notification.name


class Test(object):

    def __init__(self):
        f = Font(kerningClass=TestKerning)
        f.kerning.addObserver(self, "_kerningChanged", "Kerning.Changed")
        print "testing kerning:"
        print "initiate", f.kerning.getRepresentation("test")
        f.kerning[("a", "b")] = 10
        print "done", f.kerning.getRepresentation("test")

        
    def _kerningChanged(self, notification):
        print "    kerning changed", notification.name
        print "    %s" % notification.object.getRepresentation("test"), "(should build a new representation but the factory is not destroyed yet...)"
    
Test()

testing kerning:
initiate     *** create a new repr ***
1
    ----> self notification Kerning.PairSet
    ----> self notification done Kerning.PairSet
    kerning changed Kerning.Changed
    1 (should build a new representation but the factory is not destroyed yet...)
    ----> self notification Kerning.Changed
    ----> self notification done Kerning.Changed
done     *** create a new repr ***
2

There needs to be a simple base implementation of object serialization in the objects. This needs to be easy to extend and subclass.

Is the generated API documentation (of the ufo3 branch mainly) hosted somewhere? If not, what about using gh-pages for it?

point identifiers are not stored in the main glyph.identifiers
https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/contour.py#L662

but required when removing a contour..
https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/glyph.py#L590-L592

I would propose to add those point identifiers to the main glyph identifiers

So I just implemented template glyphs through a glyph.template boolean attribute and I would like to have template glyphs not saved into the UFO.
Right now I have to copy the whole Font.save() function in my subclass to blacklist these glyphs from the glyphSet (still working with ufo2 just now, but I didn’t see any specific mechanism in the ufo3 defcon so I thought I’d open up an issue for it).

Would it be possible to update the script txt to Unicode 9.0 instead of the old 5.0 ?
see:
ftp://unicode.org/Public/9.0.0/ucd/Scripts-9.0.0d17.txt

thanks,
Jérémie

I broke the save functionality into small chunks to make it easier to subclass. For example:

def saveKerning(self, writer, saveAs=False, progressBar=None):
    """
    Save kerning. This method should not be called externally.
    Subclasses may override this method to implement custom saving behavior.
    """
    if progressBar is not None:
        progressBar.update(text="Saving kerning...", increment=0)
    if self.kerning.dirty or saveAs:
        writer.writeKerning(self.kerning)
        self.kerning.dirty = False
        self._stampKerningDataState(UFOReader(writer.path))
    if progressBar is not None:
        progressBar.update()

I just tried to use this in MetricsMachine and realized that I could make it even easier. Now it is implemented like this:

def saveKerning(self, writer):
    """
    Save kerning. This method should not be called externally.
    Subclasses may override this method to implement custom saving behavior.
    """
    writer.writeKerning(self.kerning)

This does away with the subclass needing to deal with any of the data stamping, progress bar updating or anything like that. I think it's much easier. Now I need to figure out how to make something similar for images, data and, especially, layers.

The layer set does a significant amount of work that a subclass shouldn't touch or override. But, there is a major instance of a real world subclass needing to implement custom save behavior. he easiest way to handle this may be to not let subclasses override the layer set save, but pass more info (specifically) the writer down to the layer save method. From there, more deduction could be done about how the save needs to proceed and a more easily subclassable function could be created.

This is a general issue for ideas on optimizing the overall speed of the objects.

A simple example throwing a RuntimeError: Set changed size during iteration error

this should actually work...

import defcon

f = defcon.Font()

for a in "abcd":
    f.newGlyph(a)
    
for a in f.keys(): 
    f.newGlyph("%s.sc" % a)

a quick solution is to pack the set into a list

See: unified-font-object/ufoLib#81 (comment)

One way of getting notifications on what glyphs has changed is to assign a "Glyph.Changed" notification to all the glyphs or assign it to the font dispatcher. While this works, there is a performance drawback. Imagine a glyph has changed which is used in other glyphs as a component. This causes all the other glyphs to be dirty and post notifications. If I want to perform a task on changed glyphs this task will be performed on all the changed glyphs one by one. But if the font was holding a list of changed glyphs after posting a "Font.Changed", I could only run this task once on all those glyphs which in some cases is less troubling. The "Font.Changed" notification could tell hold a dictionary of the parts which has changed. This could also prevent some recursive notifications in a case some parts are related to another. On another case if multiple fonts are posting "Glyph.Changed" notification, knowing which glyph belongs to which font requires to run getParent on each glyphs which is also an extra task. Is there a way to avoid these?

Thanks

Do this: https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/contour.py#L81

Make newGlyph return the newly created glyph.

When a reference is dead, the notification system skips it. It needs to be caught and removed from the system so that the looping is minimized. There is already a note about this in the code:

https://github.com/typesupply/defcon/blob/ufo3/Lib/defcon/tools/notifications.py#L190

Try this:

http://stackoverflow.com/questions/1165647/how-to-determine-if-a-list-of-polygon-points-are-in-clockwise-order

Test case:

  <?xml version="1.0" encoding="UTF-8"?>
  <glyph name="A" format="1">
    <advance width="500"/>
    <unicode hex="0041"/>
    <outline>
      <contour>
        <point x="77" y="108" type="line"/>
        <point x="96" y="161"/>
        <point x="154" y="209"/>
        <point x="187" y="217" type="curve"/>
        <point x="345" y="245" type="line"/>
        <point x="345" y="93" type="line"/>
        <point x="161" y="107" type="line"/>
        <point x="125" y="107"/>
        <point x="92" y="175"/>
        <point x="92" y="225" type="curve"/>
        <point x="45" y="163" type="line"/>
      </contour>
      <contour>
        <point x="81" y="-96" type="line"/>
        <point x="97" y="-64"/>
        <point x="152" y="-1"/>
        <point x="183" y="14" type="curve"/>
        <point x="391" y="18" type="line"/>
        <point x="395" y="-210" type="line"/>
        <point x="187" y="-203" type="line"/>
        <point x="145" y="-188"/>
        <point x="100" y="-105"/>
        <point x="96" y="-18" type="curve"/>
        <point x="60" y="-80" type="line"/>
      </contour>
    </outline>
    <lib>
      <dict>
        <key>com.typemytype.robofont.layerData</key>
        <dict>
        </dict>
      </dict>
    </lib>
  </glyph>

When creating a Font() object with a path passed as argument, dirty notifications are sent when layers are created and populated.

I have made it to print a stack trace when font.dirty is set:

  File "C:\Python34\lib\runpy.py", line 171, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python34\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\__main__.py", line 10, in <module>
    font = TFont(os.path.abspath(sys.argv[1]))
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 15, in __init__
    super(TFont, self).__init__(*args, **kwargs)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 140, in __init__
    layer = self._layers.newLayer(layerName, glyphSet=glyphSet)
  File "C:\Python34\lib\site-packages\defcon\objects\layerSet.py", line 196, in newLayer
    self.dirty = True
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 319, in _set_dirty
    self.postNotification(self.changeNotificationName)
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 212, in postNotification
    dispatcher.postNotification(notification=notification, observable=self, data=data)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 195, in postNotification
    callback(notificationObj)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 874, in _objectDirtyStateChange
    self.dirty = True
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 20, in _set_dirty
    traceback.print_stack()

font: True!


  File "C:\Python34\lib\runpy.py", line 171, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python34\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\__main__.py", line 10, in <module>
    font = TFont(os.path.abspath(sys.argv[1]))
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 15, in __init__
    super(TFont, self).__init__(*args, **kwargs)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 141, in __init__
    layer.dirty = False
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 319, in _set_dirty
    self.postNotification(self.changeNotificationName)
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 212, in postNotification
    dispatcher.postNotification(notification=notification, observable=self, data=data)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 195, in postNotification
    callback(notificationObj)
  File "C:\Python34\lib\site-packages\defcon\objects\layerSet.py", line 339, in _layerDirtyStateChange
    self.dirty = True
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 319, in _set_dirty
    self.postNotification(self.changeNotificationName)
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 212, in postNotification
    dispatcher.postNotification(notification=notification, observable=self, data=data)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 195, in postNotification
    callback(notificationObj)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 874, in _objectDirtyStateChange
    self.dirty = True
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 20, in _set_dirty
    traceback.print_stack()

font: True!


  File "C:\Python34\lib\runpy.py", line 171, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Python34\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\__main__.py", line 10, in <module>
    font = TFont(os.path.abspath(sys.argv[1]))
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 15, in __init__
    super(TFont, self).__init__(*args, **kwargs)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 144, in __init__
    self._layers.defaultLayer = self._layers[defaultLayerName]
  File "C:\Python34\lib\site-packages\defcon\objects\layerSet.py", line 115, in _set_defaultLayer
    self.dirty = True
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 319, in _set_dirty
    self.postNotification(self.changeNotificationName)
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 212, in postNotification
    dispatcher.postNotification(notification=notification, observable=self, data=data)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 195, in postNotification
    callback(notificationObj)
  File "C:\Python34\lib\site-packages\defcon\objects\font.py", line 874, in _objectDirtyStateChange
    self.dirty = True
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\objects\defcon.py", line 20, in _set_dirty
    traceback.print_stack()

font: True!

One solution would be to start observing changes after the if path: block: this makes sense because we don't want to start monitoring changes before we have read existing data from the font file.
@typesupply Does this looks a good solution to you? Am I missing something here?

The code block below gives me an assertion fail when it try to insert a glyph with anchors.

new_ufo = Font()
font_source = Font(fontList[0])
for glyph_name in font_source.keys():
    print glyph_name,
    glyph = font_source[glyph_name]
    new_ufo.insertGlyph(glyph)

Full stack trace is below

Traceback (most recent call last):
  File "merger.py", line 41, in <module>
    new_ufo.insertGlyph(glyph)
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/font.py", line 198, in insertGlyph
    return self._glyphSet.insertGlyph(glyph, name=name)
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/layer.py", line 222, in insertGlyph
    dest.copyDataFromGlyph(glyph)
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/glyph.py", line 1059, in copyDataFromGlyph
    self.anchors = [self.instantiateAnchor(a) for a in glyph.anchors]
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/glyph.py", line 680, in _set_anchors
    self.appendAnchor(anchor)
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/glyph.py", line 712, in appendAnchor
    self.insertAnchor(len(self._anchors), anchor)
  File "/usr/local/lib/python2.7/dist-packages/defcon/objects/glyph.py", line 725, in insertAnchor
    assert anchor.glyph != self
AssertionError

That pen was originally in robofab, then copied here when we dropped the robofab dependency.

cu2qu also has a copy of the same pen, but now that the defcon one has been slightly modified (ea38ad2), the two are now out of sync with each other.

https://github.com/googlei18n/cu2qu/blob/039e35b/Lib/cu2qu/pens.py#L230-L311

I think it would make sense to move this pen to ufoLib, so everyone can use it.

This seems like a chicken/egg problem bouncing between defcon and ufoLib

from defcon import Font
import os
import shutil
path = u"~/Desktop/test.ufo"
path = os.path.expanduser(path)

if os.path.exists(path):
    shutil.rmtree(path)
    
# create an empty font
f = Font()
# create a glyph
g = f.newGlyph("b")
# change it
g.width = 200

# create a new layer
f.newLayer("test")
# get the layer
layer = f.layers["test"]
# create a glyph
g = layer.newGlyph("a")
# change it
g.width = 100
# save the font
f.save(path)

#####################################
# reopen the font
f = Font(path)
# rename the default layer
f.layers.defaultLayer.name = u"foreground"
# save it again
f.save(path)

or changing the default layer and save it again

from defcon import Font
import os
import shutil
path = u"~/Desktop/test.ufo"
path = os.path.expanduser(path)

if os.path.exists(path):
    shutil.rmtree(path)
    
# create an empty font
f = Font()
# create a glyph
g = f.newGlyph("b")
# change it
g.width = 200

# create a new layer
f.newLayer("test")
# get the layer
layer = f.layers["test"]
# create a glyph
g = layer.newGlyph("a")
# change it
g.width = 100
# save the font
f.save(path)

#####################################
# reopen the font
f = Font(path)
# change the default layer 
f.layers.defaultLayer = f.layers["test"]
# save it again
f.save(path)

first traceback is here https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/layerSet.py#L296-L297
where renameGlyphSet takes 2 arguments https://github.com/unified-font-object/ufoLib/blob/master/Lib/ufoLib/__init__.py#L1080

This ticket is a place for me to keep track of all the things that need to be documented.

new API
new granular notifications
representations

What the title says.

now holdNotifications only rejects a notification if the last added notification is the same.
This does not work as expected, fe: setting a contour.dirty = True automatically send an Glyph.Changed notification as each glyph is subscribed to his contour changes.

This ends up with a stack duplicated notifications

see https://github.com/typesupply/defcon/blob/master/Lib/defcon/tools/notifications.py#L140

Should it not test if the notification is already being held?

like: if n not in self._holds[key]["notifications"]:

the only thing is that the order could be important... but also maybe not really...

from defcon import Font

class TestObserver(object):
    
    def action(self, notification):
        print notification.name

f = Font()
g = f.newGlyph("a")

p = g.getPen()
p.moveTo((100, 100))
p.lineTo((200, 200))
p.endPath()

observer = TestObserver()
g.addObserver(observer, "action", "Glyph.Changed")

g.holdNotifications()

g[0].dirty = True
g.dirty = True # this will be rejected
g[0].dirty = True # this will not be rejected but is the same as the previous one
#g.dirty = True # this will be rejected
g[0].dirty = True # this will not be rejected but is the same as the first one

for holds, items in g.dispatcher._holds.items():
    print len(items.get("notifications"))
    for notification in items.get("notifications", []):
        print "    ", notification[0]
    print 

# release 
g.releaseHeldNotifications()

Loading a full font in one pass is very expensive. This needs to be resolved. I'll add optimization possibilities to this ticket.

This is the profiling script that I'm using to search for optimizations:

from cStringIO import StringIO
import cProfile
import sys
from defcon import Font

path = "path to UFO, preferably a massive one"

font = Font(path)
for glyph in font:
    break

def test():
    font = Font(path)
    for glyph in font:
        pass

save = sys.stdout
temp = StringIO()
sys.stdout = temp
cProfile.run("test()")
sys.stdout = save
stats = temp.getvalue()

# generate report

print stats.strip().splitlines()[0]
print

s = []
listening = False
for line in stats.splitlines():
    line = line.strip()
    if line.startswith("ncalls"):
        listening = True
        continue
    if not listening:
        continue
    s.append(line)
stats = "\n".join(s)

def sfill(text, count):
    return " " * (count - len(text)) + text

fileNames = {}

for line in stats.splitlines():
    line = line.strip()
    if not line:
        continue
    parts = [i for i in line.split(" ") if i]
    if len(parts) > 6:
        parts = parts[:5] + [" ".join(parts[5:])]
    ncalls, tottime, percall, cumtime, percall, filename = parts
    if filename.startswith("<"):
        continue
    if filename.startswith("{"):
        func = filename
        filename = "<core python>"
    else:
        filename, func = filename.split(":")
        func = func.split("(")[1][:-1]
    if filename not in fileNames:
        fileNames[filename] = dict(time=0, data=[])
    fileNames[filename]["time"] += float(tottime)
    fileNames[filename]["data"].append((tottime, ncalls, func))

sorter = []
for filename, data in fileNames.items():
    sorter.append((data["time"], filename))

for time, filename in reversed(sorted(sorter)):
    print time, filename
    sorter = []
    for i in reversed(sorted(fileNames[filename]["data"])):
        print "   %s %s %s" % i
    print

Maybe that's working as intended, but I was just thinking that perhaps one may always want components to reference the default layer.

Right now if you add a component to a background layer it will reference the glyph of that layer.

So I was implementing a way to move a glyph to another layer. I call glyph.clearContours() after inserting the glyph into another layer.

The problem that I have is that I have representations of my glyph that are destroyed when Glyph.Changed is emitted, and I also update my view when Glyph.Changed emitted.

The thing is that apparently defcon calls my update callbacks before destroying the representations. It makes the application crash when it trys to access data that was removed:

RLS Layer.GlyphWillBeAdded <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphAdded <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04AEC430>
RLS Glyph.ContourWillBeDeleted <defconQt.objects.defcon.TGlyph object at 0x04C38CB0>
RLS Glyph.ContoursChanged <defconQt.objects.defcon.TGlyph object at 0x04C38CB0>
RLS Glyph.Changed <defconQt.objects.defcon.TGlyph object at 0x04C38CB0>
Traceback (most recent call last):
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 428, in _layerMove
    self.view.layerMove()
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 2385, in layerMove
    self._glyph.clearContours()
  File "C:\Python34\lib\site-packages\defcon\objects\glyph.py", line 523, in clearContours
    self.releaseHeldNotifications()
  File "C:\Python34\lib\site-packages\defcon\objects\base.py", line 162, in releaseHeldNotifications
    dispatcher.releaseHeldNotifications(observable=self, notification=notification)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 251, in releaseHeldNotifications
    self.postNotification(notification, observableRef(), data)
  File "C:\Python34\lib\site-packages\defcon\tools\notifications.py", line 195, in postNotification
    callback(notificationObj)
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 2010, in _glyphChanged
    self.activeGlyphChanged()
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 2032, in activeGlyphChanged
    self.updateActiveLayer()
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 2041, in updateActiveLayer
    self.addPoints()
  File "C:\Users\Admin\Downloads\trufont\Lib\defconQt\glyphView.py", line 2318, in addPoints
    self._glyph[onCurve.contourIndex],
  File "C:\Python34\lib\site-packages\defcon\objects\glyph.py", line 1023, in __getitem__
    return self._contours[index]
IndexError: list index out of range

If I return early in my widgets instead of updating I can see the calls to destroy happening:

RLS Layer.GlyphWillBeAdded <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphAdded <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.GlyphChanged <defcon.objects.layer.Layer object at 0x04C46930>
RLS Layer.Changed <defcon.objects.layer.Layer object at 0x04C46930>
RLS Glyph.ContourWillBeDeleted <defconQt.objects.defcon.TGlyph object at 0x04E4EB30>
RLS Glyph.ContoursChanged <defconQt.objects.defcon.TGlyph object at 0x04E4EB30>
RLS Glyph.Changed <defconQt.objects.defcon.TGlyph object at 0x04E4EB30>
DESTROY defconQt.QPainterPath
DESTROY defconQt.OutlineInformation
DESTROY defconQt.StartPointsInformation
DESTROY defconQt.NoComponentsQPainterPath

I could set my representations to be destroyed when ContourWillBeDeleted is emitted (is it meant to be so? ufo3 defconAppkit does not seem to care about ContourWillBeDeleted specially, but it isn't capable of editing either) but I feel like there are other cases where such a thing could happen.
@typesupply What do you think?

Please support this package and ufo2fdk with pip install!

Thank you!

in the test data there is an image tag with fileName but the file is not in the image folder (see https://github.com/typesupply/defcon/blob/ufo3/testData/TestFont.ufo/glyphs.public.background/A_.glif#L8)

I would propose that this check also test if the fileName is available...

or maybe there should be an other way to test/check if images exists...

It doesn't help. I think a reader / writer should only fail if the file / data is structurally incorrect, not semantically. Semantic validation should only happen by editors, generators, lint tools, etc. This is causing us lots of trouble, specially because fontMath's kerningMath can easily generate invalid kerning right now. I'll try to fix that, but in the mean time, I think this should also be fixed.

I actually have a better proposal: change UFO to say that if both a glyph-class and class-glyph kerning are present, then the glyph-class wins over the class-glyph one. This way, there will be no invalid kerning whatsoever.

@jamesgk

A reminder to do this, as per @typemytype's comment here: #35 (comment)

It would be good to have an overall description of this project in a top-level README.md file.
Something similar to:
https://github.com/robofab-developers/robofab

Is there any standard key for storing defcon sortDescriptor? I'll probably use com.defcon.sortDescriptor for now.

With Glyph particularly, e.g. when you paste you don't necessarily want to erase previous content.

Bad.

it should always save atomically in place when the format version is not the same.

see https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/font.py#L718-L721

Don't create a new glyph and copy the data from the old glyph if all of these conditions are met:

1. The glyph does not belong to another object.
2. The glyph is an instance of exactly the destination object's defined class. Subclasses should not match.
3. The glyph does not have a notification dispatcher.

Add some documentation about all of this to the notes in the method.

In many cases an observer needs to observe all glyphs in a font. That's only possible by subscribing to each glyph individually. That's cumbersome and becomes problematic when glyphs are added/removed. A better way to do this would be for an observer to observe all instances of a specific class. The observer would be notified for changes made to any object of that class.

This should be pretty easy to implement and shouldn't break anything. Here's an API sketch:

font.dispatcher.addObserver(
    observer=self,
    methodName="aGlyphChangedNotificationCallback",
    notification="Glyph.Changed",
    observable=font.glyphClass
)

Inside of the dispatcher there can be a loop like this:

for cls, observers in self._classObservers:
    if isinstance(objectSendingOutNotification, cls):
        for observer in observers:
            # filter based on notificaiton name and then post

I don't think this will help or harm speed, but it will definitely simplify a lot of code. That would be good.

An potential improvement to this would be to allow observations of all instances of a class that are contained within a specific object. For example, a glyph could observe the contour class rather than each individual contour. An API like this may work:

observable=(glyph, glyph.contourClass)

This shouldn't be too hard to implement inside of the dispatcher.

• Implement dispatcher changes.
• Add properties for all sub-object classes to the parent objects. Ex: font.kerningClass.
• Use this when possible within defcon to simplify internal observations.
• Backport this to master.

This method would remove the loaded glyph data but not go so far as to delete the glyph from the glyph set. This would be useful for reducing memory consumption. What should happen if a glyph has unsaved changes since loading? Those changes will be lost.

The contour splitting code doesn't work with quads. Remove it (after moving it down to the Prepolator subclass.)

In the UFO 3 branch, the glyph object loads contours shallowly to try to reduce the initial loading time. This is done here:

https://github.com/typesupply/defcon/blob/ufo3/Lib/defcon/objects/glyph.py#L410

With this:

https://github.com/typesupply/defcon/blob/ufo3/Lib/defcon/pens/glyphObjectPointPen.py#L40

When contours are needed, the contour objects are fully created:

https://github.com/typesupply/defcon/blob/ufo3/Lib/defcon/objects/glyph.py#L431

This could be made public with something like glyph.wake(). It would also be useful to make the reverse possible: compact the fully loaded contours back into shallow form with glyph.sleep().

In contour.py when constructing segments, if the points list ends with an offCurve (which means that a curve segment is split at the boundaries), there's an assert assert len(segments[0]) == 1, i.e. the code assumes that if there's an offCurve at the end, then both offCurves are (and so only the onCurve is in the part at the beginning of the contour).
Now consider a contour made of the following points:

<TPoint position: (65, -15) type: None smooth: False name: None identifier: None>
<TPoint position: (102, -15) type: curve smooth: True name: None identifier: None>
<TPoint position: (195, -15) type: None smooth: False name: None identifier: None>
<TPoint position: (264, 44) type: None smooth: False name: None identifier: None>
<TPoint position: (264, 151) type: curve smooth: True name: None identifier: None>
<TPoint position: (264, 675) type: line smooth: False name: None identifier: None>
<TPoint position: (132, 675) type: line smooth: False name: None identifier: None>
<TPoint position: (132, 168) type: line smooth: True name: None identifier: None>
<TPoint position: (132, 118) type: None smooth: False name: None identifier: None>
<TPoint position: (111, 96) type: None smooth: False name: None identifier: None>
<TPoint position: (77, 96) type: curve smooth: True name: None identifier: None>
<TPoint position: (58, 96) type: None smooth: False name: None identifier: None>
<TPoint position: (36, 102) type: None smooth: False name: None identifier: None>
<TPoint position: (10, 114) type: curve smooth: False name: None identifier: None>
<TPoint position: (-17, 13) type: line smooth: False name: None identifier: None>
<TPoint position: (25, -6) type: None smooth: False name: None identifier: None>

That triggers the above assertion, even though the contour is valid (unless having an offCurve start point is invalid but AFAIK that's not forbidden by UFO).

There is group remapping information available from the UFOReader. That needs to be either used or available somehow. Additionally, there needs to be some way to send a down conversion group mapping to UFOWriter.

Make it so that looking up a kerning value uses the same logic for handling class pairs as MM does. For example:

kerning["A", "B"] would look for the following:

A B
A (group containing B)
(group containing A) B
(group containing A) (group containing B)
If nothing is found, return 0.

Would this be too much like a scripting API? It would be useful to have this logic centralized so that others don't have to invent it. (A sample implementation is in the UFO spec, though.)

If this is implemented, the groups should have representations for side 1 and side 2 glyph to class mapping.

When a contour/component/point/guide/anchor is inserted, the identifier in the incoming object could conflict with an existing identifier. What should happen here: raise an error or silently remove the incoming identifier? I'm leaning towards silently removing the identifier. It could be changed to a new value, but that is just as problematic as removing it.

I wrote a script to rename glyphs according to https://github.com/schriftgestalt/GlyphsInfo. The renaming happens like this: glyph.name = suggestedName. Unfortunately, this breaks all components which reference the glyph and I'm left with lots of empty cells. I could go through all components and update them in an extra loop, but I think defcon should do this by itself.

There may not be a good reason to make these private.

(This was originally reported by Frederik.)

both bounds and controlBounds are wrong for components if the matrix contains some sort of rotation, scale or skew transformation

I would suggest to handle the bounds similar like the bounds in the glyph object with a bounds cache.

Until now defcon is simply transforming the bounds of the baseGlyph.

do you want me to write a patch?

When an orphan object is set dirty the representations are not cleared. There is no dispatcher so to actually does nothing.

I would propose: if the font is None create a temp dispatcher, keep it and every time a dispatcher is requested, check if there is proper parent object to get the dispatcher from... (Maybe this already to complicated)
See: https://github.com/typesupply/defcon/blob/ufo3/Lib/defcon/objects/base.py#L70

Enable Travis and AppVeyor to run tests on pull requests. Coveralls can be enabled too.

Identifiers of points that are going to be removed must be removed from the glyph.identifiers

https://github.com/typesupply/defcon/blob/master/Lib/defcon/objects/contour.py#L192

from defcon import Glyph
# create a glyph
g = Glyph()
# drawSomething
pen = g.getPen()
pen.moveTo((100, 100))
pen.lineTo((200, 100))
pen.lineTo((200, 200))
pen.closePath()
# create modifiers
for c in g:
    for p in c:
        c.generateIdentifierForPoint(p)

print g.identifiers
for c in g:
    # remove a point
    c.removePoint(c[-1])
print g.identifiers