-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Point Observation #267
Comments
I made a simple, unoptimized test to see what would happen if Point became fully observable. This is, I think, the slowest possible way to solve this. The results were surprising:
Loading a font with observation in place only increased the load time by approximately 17%. That's not great, but it is much faster than what I recall from 13 years ago. Moving all of the points in a glyph is slower, but it's still pretty fast and that can be optimized. Note: I didn't look at memory footprint or anything else. import time
import weakref
from defcon.objects.base import BaseObject
from defcon import Font, Contour, Point
class HackedContour(Contour):
def insertPoint(self, index, point):
point._contour = weakref.ref(self)
return super(HackedContour, self).insertPoint(index, point)
class ObservablePoint(Point, BaseObject):
changeNotificationName = "Point.Changed"
_contour = None
def _get_font(self):
contour = self.contour
if contour is not None:
return contour.font
return None
font = property(_get_font)
def _get_contour(self):
if self._contour is not None:
return self._contour()
return None
contour = property(_get_contour)
# ------------------
# Property Overrides
# ------------------
def _get_segmentType(self):
return super(ObservablePoint, self)._get_segmentType()
def _set_segmentType(self, value):
super(ObservablePoint, self)._set_segmentType(value)
self.dirty = True
segmentType = property(_get_segmentType, _set_segmentType)
def _get_x(self):
return super(ObservablePoint, self)._get_x()
def _set_x(self, value):
super(ObservablePoint, self)._set_x(value)
self.dirty = True
x = property(_get_x, _set_x)
def _get_y(self):
return super(ObservablePoint, self)._get_y()
def _set_y(self, value):
super(ObservablePoint, self)._set_y(value)
self.dirty = True
y = property(_get_y, _set_y)
def _get_smooth(self):
return super(ObservablePoint, self)._get_smooth()
def _set_smooth(self, value):
super(ObservablePoint, self)._set_smooth(value)
self.dirty = True
smooth = property(_get_smooth, _set_smooth)
def _get_name(self):
return super(ObservablePoint, self)._get_name()
def _set_name(self, value):
super(ObservablePoint, self)._set_name(value)
self.dirty = True
name = property(_get_name, _set_name)
def _get_identifier(self):
return super(ObservablePoint, self)._get_identifier()
def _set_identifier(self, value):
super(ObservablePoint, self)._set_identifier(value)
self.dirty = True
identifier = property(_get_identifier, _set_identifier)
# ----
# Test
# ----
def loadFont(path, pointClass):
contourClass = None
if pointClass is not None:
contourClass = HackedContour
font = Font(
path=path,
glyphContourClass=contourClass,
glyphPointClass=pointClass
)
for layer in font.layers:
for glyph in layer:
for contour in glyph:
for point in contour:
pass
return font
def move(font):
for layer in font:
for glyph in layer:
glyph.move((10, 10))
def loadTest(path, pointClass=None):
font = loadFont(path, pointClass) # make sure module loading isn't factored in
loops = 5
start = time.time()
for i in range(loops):
loadFont(path, pointClass)
end = time.time()
elapsed = end - start
return elapsed
def moveTest(path, pointClass=None):
font = loadFont(path, pointClass)
loops = 5
start = time.time()
for i in range(loops):
move(font)
end = time.time()
elapsed = end - start
return elapsed
path = '/path/to/a.ufo'
f = Font(path)
c = sum([len(layer) for layer in f])
print("test font contains %d glyphs" % c)
t = loadTest(path)
print("load current:", t)
t = loadTest(path, ObservablePoint)
print("load observable:", t)
t = moveTest(path)
print("move current:", t)
t = moveTest(path, ObservablePoint)
print("move observable:", t) |
(Hey, @typemytype and @justvanrossum. I'm pinging you with this in case you want to follow along or contribute.) |
I don't know. I'm skeptical to enroll Point objects into the notification system, even even the slowdown seems minor. But then again, I'm skeptical about Contour objects being observable. I'll not be a great help I'm afraid. |
Almost all objects in defcon participate in the notification system (which is based on the observer pattern). Points are the main exception and it has been a problem for years. Perhaps someone could solve this.
The Problem
When you make a change to a Point, nothing is informed about the change to the font. For example:
In the current version of the Point object, the
contour
will not receive any notification thatpoint
has changed. This is fine as long as the high-level code is diligent about notifyingcontour
on it's own. In fact, if you callcontour.move
, that's what Contour does internally. However, if the high-level code does not do the notification work, things get out of sync. This sucks because defcon has a "it just works" model and this doesn't just work.Some History
When I wrote defcon, Points were just like every object in that they tied into the overall notification center. Any change made to a Point would trigger a
Point.Changed
notification through the shared contour-glyph-font notification system. As best as I can recall, this created some significant problems due to the number of points in a font:sum([len(contour) for contour in glyph])
Point.Changed
notifications to be posted and this was a serious performance drag.I removed Points from the notification system because I couldn't find a way to solve it. It has bugged (no pun intended) me since.
Possible Solutions
A lot has changed since the early days of defcon. We have better control for holding and muting notifications. Many parts of defcon have been optimized. Hardware has improved.
These are the ideas that I've had for trying to solve the problem:
I have no idea if these will work and I don't have time to take on the issue myself. If someone out there wants to give it a shot, please do. Test it by throwing some big UFOs at it and moving lots of points around. I'll stay subscribed to this issue if you want to discuss.
The text was updated successfully, but these errors were encountered: