Skip to content
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

Optional cross-platform outline rendering mode #451

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c5d4e18
fontgoggles.misc.plotter as abstraction above direct use of CocoaPen …
stenson Oct 13, 2024
6ab0e28
abstract out all mac-specific code from fontgoggles/font
stenson Oct 13, 2024
40e0ec2
return a RecordingPen from pathFromGlyph
stenson Oct 13, 2024
2828b01
button up Plotter class as single entry point
stenson Oct 14, 2024
ea05cf2
add a test for noCocoa mode
stenson Oct 14, 2024
5885f0a
more extensive test
stenson Oct 14, 2024
fb7a92f
pass in sys.stderr.write
stenson Oct 14, 2024
b7a40ef
delete non-test version of this file
stenson Oct 14, 2024
80050f2
move AppKit call to plotter
stenson Oct 14, 2024
49ead8c
fontgoggles.misc.plotter -> fontgoggles.misc.platform
stenson Oct 14, 2024
7341e09
move library requirements into setup.py
stenson Oct 21, 2024
efd022e
wrap build_lib.sh call with platform check so we don't run on non-mac
stenson Oct 29, 2024
fa56b6a
Merge branch 'justvanrossum:master' into master
stenson Oct 29, 2024
08a5ab8
use draw_glyph_with_pen instead of custom drawfuncs
stenson Nov 23, 2024
5b07f18
uncomment uharfbuzz line for conflict
stenson Nov 23, 2024
c4f2ff5
uncomment uharfbuzz line for conflict
stenson Nov 23, 2024
5be6bb6
Merge remote-tracking branch 'upstream/master'
stenson Nov 23, 2024
637fb2f
update setup.py version of uharfbuzz
stenson Nov 23, 2024
cbe8cd3
disable turbo for now
stenson Dec 7, 2024
6347a73
reenable Turbo/build_lib.sh in setup but wrap with a try
stenson Dec 11, 2024
343669e
PR feedback: unpin setup requires, black format new files, remove Pla…
stenson Dec 11, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Lib/fontgoggles/font/dsFont.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from ..compile.dsCompiler import getTTPaths
from ..misc.hbShape import HBShape
from ..misc.properties import cachedProperty
from ..mac.makePathFromOutline import makePathFromArrays
from ..misc.platform import Platform


class DesignSpaceSourceError(CompilerError):
Expand Down Expand Up @@ -443,7 +443,7 @@ def verticalOrigin(self):
return self.getPoints()[-2]

def getOutline(self):
return makePathFromArrays(self.getPoints(), self.tags, self.contours)
return Platform.pathFromArrays(self, self.getPoints(), self.tags, self.contours)

def draw(self, pen):
ppen = PointToSegmentPen(pen)
Expand Down
21 changes: 6 additions & 15 deletions Lib/fontgoggles/font/glyphDrawing.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from AppKit import NSGraphicsContext
from fontTools.misc.arrayTools import unionRect
from ..mac.drawing import rectFromNSRect, nsColorFromRGBA
from ..misc.platform import Platform
from ..misc.properties import cachedProperty


Expand All @@ -24,11 +23,11 @@ def __init__(self, path):
def bounds(self):
bounds = None
if self.path.elementCount():
bounds = rectFromNSRect(self.path.controlPointBounds())
bounds = Platform.convertRect(self.path.controlPointBounds())
return bounds

def draw(self, colorPalette, defaultColor):
nsColorFromRGBA(defaultColor).set()
Platform.convertColor(defaultColor).set()
self.path.fill()

def pointInside(self, pt):
Expand All @@ -46,7 +45,7 @@ def bounds(self):
for path, colorID in self.layers:
if not path.elementCount():
continue
pathBounds = rectFromNSRect(path.controlPointBounds())
pathBounds = Platform.convertRect(path.controlPointBounds())
if bounds is None:
bounds = pathBounds
else:
Expand All @@ -60,7 +59,7 @@ def draw(self, colorPalette, defaultColor):
if colorID < len(colorPalette) else
defaultColor
)
nsColorFromRGBA(color).set()
Platform.convertColor(color).set()
path.fill()

def pointInside(self, pt):
Expand All @@ -77,15 +76,7 @@ def bounds(self):
return self.colorFont.getGlyphBounds(self.glyphName)

def draw(self, colorPalette, defaultColor):
from blackrenderer.backends.coregraphics import CoreGraphicsCanvas

cgContext = NSGraphicsContext.currentContext().CGContext()
self.colorFont.drawGlyph(
self.glyphName,
CoreGraphicsCanvas(cgContext),
palette=colorPalette,
textColor=defaultColor,
)
Platform.drawCOLRv1Glyph(self.colorFont, self.glyphName, colorPalette, defaultColor)

def pointInside(self, pt):
return False # TODO: implement
11 changes: 5 additions & 6 deletions Lib/fontgoggles/font/otfFont.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,22 @@
from .baseFont import BaseFont
from .glyphDrawing import GlyphDrawing, GlyphLayersDrawing, GlyphCOLRv1Drawing
from ..compile.compilerPool import compileTTXToBytes
from ..mac.makePathFromOutline import makePathFromGlyph
from ..misc.hbShape import HBShape
from ..misc.properties import cachedProperty
from ..misc.platform import PlatformPenWrapper, Platform


class _OTFBaseFont(BaseFont):

def _getGlyphOutline(self, name):
return makePathFromGlyph(self.shaper.font, self.shaper.glyphMap[name])
return Platform.pathFromGlyph(self.shaper.font, self.shaper.glyphMap[name])

def _getGlyphDrawing(self, glyphName, colorLayers):
if "VarC" in self.ttFont:
from fontTools.pens.cocoaPen import CocoaPen
pen = CocoaPen(None)
penwrapper = PlatformPenWrapper(None)
location = self._currentVarLocation or {}
self.varcFont.drawGlyph(pen, glyphName, location)
return GlyphDrawing(pen.path)
self.varcFont.drawGlyph(penwrapper.pen, glyphName, location)
return GlyphDrawing(penwrapper.getOutline())
if colorLayers:
if self.colorLayers is not None:
layers = self.colorLayers.get(glyphName)
Expand Down
14 changes: 7 additions & 7 deletions Lib/fontgoggles/font/ufoFont.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from fontTools.feaLib.ast import IncludeStatement
from fontTools.feaLib.error import FeatureLibError
from fontTools.fontBuilder import FontBuilder
from fontTools.pens.cocoaPen import CocoaPen # TODO: factor out mac-specific code
from fontTools.ttLib import TTFont
from fontTools.ufoLib import UFOReader, UFOFileStructure
from fontTools.ufoLib import (FONTINFO_FILENAME, GROUPS_FILENAME, KERNING_FILENAME,
Expand All @@ -23,6 +22,7 @@
from ..compile.ufoCompiler import fetchGlyphInfo
from ..misc.hbShape import HBShape
from ..misc.properties import cachedProperty
from ..misc.platform import PlatformPenWrapper


class UFOFont(BaseFont):
Expand Down Expand Up @@ -151,9 +151,9 @@ def _getGlyph(self, glyphName, layerName=None):
return glyph

def _addOutlinePathToGlyph(self, glyph):
pen = CocoaPen(self.glyphSet)
glyph.draw(pen)
glyph.outline = pen.path
penwrapper = PlatformPenWrapper(self.glyphSet)
glyph.draw(penwrapper.pen)
glyph.outline = penwrapper.getOutline()

def _getHorizontalAdvance(self, glyphName):
glyph = self._getGlyph(glyphName)
Expand Down Expand Up @@ -258,9 +258,9 @@ def setVarLocation(self, varLocation):
pass

def getOutline(self):
pen = CocoaPen(None) # by now there are no more composites
self.draw(pen)
return pen.path
penwrapper = PlatformPenWrapper(None)
self.draw(penwrapper.pen)
return penwrapper.getOutline()


class Glyph(GLIFGlyph):
Expand Down
84 changes: 84 additions & 0 deletions Lib/fontgoggles/misc/platform.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
from fontTools.pens.recordingPen import RecordingPen

"""
An abstraction on top of CocoaPen / any Mac-specific operations
so that fontGoggles can function as a platform-agnostic library
"""


CAN_COCOA = True

try:
from fontTools.pens.cocoaPen import CocoaPen
# TODO some other feature detection?
except ImportError:
CAN_COCOA = False


class Platform():
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to get rid of this additional class. All its methods are static, so might as well be functions in this module. If you like the "[Pp]latform" prefix in the client code, you could import the module "platform" instead of its contents, but I don't mind if you'd just import the needed names, ie. from ..misc.platform import pathFromGlyph.

(Although then perhaps it's nicer to stick with the original funtion names, eg. makePathFromGlyph)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! I went with just importing the names. The idea of the Platform class was just to have an easy way to set the global variable (Platform.UseCocoa) but it’s just as easy to import as platform and then set platform.USE_COCOA (as in the updated noCocoa test)

UseCocoa = CAN_COCOA

@staticmethod
def pathFromArrays(font, points, tags, contours):
if Platform.UseCocoa:
from ..mac.makePathFromOutline import makePathFromArrays
return makePathFromArrays(points, tags, contours)
else:
rp = RecordingPen()
font.draw(rp)
return rp

@staticmethod
def pathFromGlyph(font, gid):
if Platform.UseCocoa:
from ..mac.makePathFromOutline import makePathFromGlyph
return makePathFromGlyph(font, gid)
else:
rp = RecordingPen()
font.draw_glyph_with_pen(gid, rp)
return rp

@staticmethod
def convertRect(r):
if Platform.UseCocoa:
from ..mac.drawing import rectFromNSRect
return rectFromNSRect(r)

@staticmethod
def convertColor(c):
if Platform.UseCocoa:
from ..mac.drawing import nsColorFromRGBA
return nsColorFromRGBA(c)

@staticmethod
def drawCOLRv1Glyph(colorFont, glyphName, colorPalette, defaultColor):
if Platform.UseCocoa:
from AppKit import NSGraphicsContext
from blackrenderer.backends.coregraphics import CoreGraphicsCanvas

cgContext = NSGraphicsContext.currentContext().CGContext()
colorFont.drawGlyph(
glyphName,
CoreGraphicsCanvas(cgContext),
palette=colorPalette,
textColor=defaultColor,
)
else:
raise NotImplementedError()


class PlatformPenWrapper():
def __init__(self, glyphSet, path=None):
if Platform.UseCocoa:
self.pen = CocoaPen(glyphSet, path=path)
else:
self.pen = RecordingPen()

def draw(self, pen):
self.pen.draw(pen)

def getOutline(self):
if isinstance(self.pen, CocoaPen):
return self.pen.path
else:
return self.pen
stenson marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 4 additions & 4 deletions Tests/test_makeOutline.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import sys
import pytest
from fontTools.pens.cocoaPen import CocoaPen
from fontTools.ttLib import TTFont
from fontgoggles.font.otfFont import OTFFont
from fontgoggles.misc.platform import PlatformPenWrapper
from testSupport import getFontPath


Expand All @@ -19,12 +19,12 @@ async def test_getOutlinePath():

for glyphName in ["a", "B", "O", "period", "bar", "aring"]:
p = font._getGlyphOutline(glyphName)
pen = CocoaPen(ttfGlyphSet)
ttfGlyphSet[glyphName].draw(pen)
penwrapper = PlatformPenWrapper(ttfGlyphSet)
ttfGlyphSet[glyphName].draw(penwrapper.pen)
# The paths are not identical, due to different rounding
# of the implied points, and different closepath behavior,
# so comparing is hard, so we'll settle for a bounding box.
assert p.controlPointBounds() == pen.path.controlPointBounds()
assert p.controlPointBounds() == penwrapper.pen.path.controlPointBounds()


@pytest.mark.asyncio
Expand Down
44 changes: 44 additions & 0 deletions Tests/test_noCocoa.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import pytest, sys
from asyncio import run
from fontgoggles.font import getOpener
from fontgoggles.misc.textInfo import TextInfo
from testSupport import getFontPath
from fontTools.pens.recordingPen import RecordingPen

from fontgoggles.misc.platform import Platform

font_paths = [
"MutatorSans.ttf",
"MutatorSansBoldWide.ufo",
"MutatorSans.designspace",
]


def test_cocoaAndNoCocoa():
def getDrawings(path):
fontPath = getFontPath(path)
_, opener, _ = getOpener(fontPath)
font = opener(fontPath, 0)
run(font.load(sys.stderr.write)) # to test support for non-async

textInfo = TextInfo("abc")
glyphs = font.getGlyphRunFromTextInfo(textInfo)
glyphNames = [g.name for g in glyphs]
glyphDrawings = list(font.getGlyphDrawings(glyphNames, True))

assert len(glyphs) == 3
assert len(glyphDrawings) == 3

return glyphDrawings

for font_path in font_paths:
glyphDrawings = getDrawings(font_path)
for g in glyphDrawings:
assert "NSBezierPath" in str(type(g.path))

Platform.UseCocoa = False
stenson marked this conversation as resolved.
Show resolved Hide resolved

for font_path in font_paths:
glyphDrawings = getDrawings(font_path)
for g in glyphDrawings:
assert isinstance(g.path, RecordingPen)
stenson marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 7 additions & 7 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ py2app==0.28.8
pyobjc==10.3.1
corefoundationasyncio==0.0.1
cocoa-vanilla==0.6.0
blackrenderer==0.6.0
fonttools[woff,lxml,unicode,ufo,type1]==4.53.1
### blackrenderer==0.6.0 # moved to setup.py
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should stay, as they get updated by dependabot. Also, in setup.py you should ideally not pin dependencies, although you are right they should be mentioned (but either without version, or with a minimal version).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally makes sense. I did keep the pinned == for python-bidi since there’s the issue of the interface change there, does that work?

### fonttools[woff,lxml,unicode,ufo,type1]==4.53.1 # moved to setup.py
# Temporarily add support for experimental extensions to the not-yet-official COLRv1 format:
# fonttools[woff,ufo,type1,unicode,lxml] @ git+https://github.com/justvanrossum/fonttools@colrv1-varco
uharfbuzz==0.42.0
python-bidi==0.4.2 # pin for now
### uharfbuzz==0.42.0 # moved to setup.py
### python-bidi==0.4.2 # pin for now # moved to setup.py
jundo==0.1.2
ufo2ft==3.2.8
numpy==2.1.1
unicodedata2==15.1.0
### ufo2ft==3.2.8 # moved to setup.py
### numpy==2.1.1 # moved to setup.py
### unicodedata2==15.1.0
git+https://github.com/BlackFoundryCom/rcjk-tools/
delocate==0.11.0 # pin for now
18 changes: 15 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
import re
from setuptools import setup, find_packages
import subprocess
import platform


class build(_build):
def run(self):
# Build our C library
subprocess.check_call(['./Turbo/build_lib.sh'])
_build.run(self)
if platform.system() == "Darwin":
try:
# Build our C library
subprocess.check_call(['./Turbo/build_lib.sh'])
_build.run(self)
except:
print("! Could not build turbo")


_versionRE = re.compile(r'__version__\s*=\s*\"([^\"]+)\"')
Expand All @@ -31,6 +36,13 @@ def run(self):
packages=find_packages("Lib"),
package_data={'fontgoggles.mac': ['*.dylib']},
install_requires=[
"blackrenderer==0.6.0",
"fonttools[woff,lxml,unicode,ufo,type1]==4.53.1",
"uharfbuzz==0.42.0",
"python-bidi==0.4.2",
"ufo2ft==3.2.8",
"numpy",
"unicodedata2==15.1.0",
],
extras_require={
},
Expand Down