Hi @jtanx. I pushed a bunch, feel free to look at it but I know there's still more to do, so don't need a review urgently. Thanks for your early and prompt review, I fixed a bunch of things you noticed. I hope you like this feature.

Oh! Sorry, I forgot to mention—just so you know @jtanx, your comment about tests was well received. I wrote some tests today but they aren't done yet so I haven't committed them. I'll add them before re-requesting a review from you. 👍

@jtanx In order to do the Python stuff right, I have to add a brand new type, PyFFImageType. Can I please make that dependent on this PR and put it in this branch? I feel it's waste for me to have to make that separable from references, since the whole reason I'm adding it is so I can add Python tests for image reference link/unlink, and so users can e.g.:

import fontforge
import glob
f=fontforge.font()
f.createChar(ord("A"))
A=f["A"]
A.importOutlines("/tmp/test.jpg") # Now returns nothing, so I don't think I should change that as it might break scripts.
for i in A.images() # new proposed iterator
  print(repr(i)) # new proposed type <Image on glyph Y>
  for im in glob.glob('images/*'):
    linked = i.linkReference(im) # new proposed function, try to link reference
    if linked: break

PyFFImageType should have xoff, yoff, xscale, yscale able to be set, it should be possible to link and unlink it, and it should be possible to compare it (GImageSame). It might also need to be possible to hash it, so I might need to bring back GImageHash, jury's out on that though. Should users be able to access the bitmap data? I think that's pointless because Python users should already have the image in another file and can use other utilities, but await your comment. Then again, if it turns out to be not that hard to do, why not? Doing so could make it so users could export images from existing SFDs with a Python script, by using PIL to take FontForge's bitmaps and create PNGs.

PyFFImageType should have xoff, yoff, xscale, yscale able to be set, it should be possible to link and unlink it, and it should be possible to compare it (GImageSame). It might also need to be possible to hash it, so I might need to bring back GImageHash, jury's out on that though. Should users be able to access the bitmap data? I think that's pointless because Python users should already have the image in another file and can use other utilities, but await your comment. Then again, if it turns out to be not that hard to do, why not? Doing so could make it so users could export images from existing SFDs with a Python script, by using PIL to take FontForge's bitmaps and create PNGs.

I would always err on implementing the least amount necessary. It's always easier to add stuff in than take stuff out. So no, I wouldn't expose the bitmap data.

Also I'm not that familiar with this part of scripting, but:

i.linkReference(im)

Feels wrong. So correct me if my interpretation is wrong, but this is saying, for this image I already have, this file path contains that image, so make a reference to it instead of storing the data in the sfd

Why would you do that when you told it what to import the first time around?

Why would you do that when you told it what to import the first time around?

Your understanding is correct. It does two things:

1. It creates a mechanism for the migration of old files via scripts, as in my glob.glob example.
2. It creates a mechanism for moving files around on the disk, e.g. an SFD where the images are in the root can be moved to an images/ folder. Yes, images can be reimported, but if they are, the xoff/yoff/xscale/yscale is lost.

I'm working on the Python implementation of an image type, as I said. Thank you for your further comments, but I'd appreciate an answer to my most important question:

Can I please make that dependent on this PR and put it in this branch?

(You could say, that with an image Python type, script authors could just get/set image.xoff etc, but that means there would be no way to relink images in the GUI.)

It sounds like it's overcomplicating the matter to me. Just how frequently are you going to be moving files around that re-setting those xoff/yoff/scale parameters actually becomes an issue? Even the whole 'compare images' stuff sounds like overkill

Can I please make that dependent on this PR and put it in this branch?

Up to you, I don't see any deadlines on this, so whatever works.

I don't see any deadlines on this

Sure. For me there is a deadline, I require this feature for a font I'm calling TT2020. So I need to merge this by December 1, 2019. Quite far away. If I can't manage that, I'll need to step away from the PR for a month to finish my font, as I don't want to countenance releasing it after 1/1/2020.

Thanks for your approval of PyFF_Image in this branch.

You think this is over-engineered, I can see that. GImageSame is very useful for Python equality in PyFF_Image. Image references might rarely change, but the feature is useful all the same, it provides an upgrade path, and I'd like to take advantage of it.

@jtanx Something missing is that you can't forcibly relink an image with a different hash. If I add that, would that change your mind? That makes the feature much more useful.

@skef: Do you mind giving comment about what the API should look like? And also if we should split importOutlines and importImages just for API sanity, even if doing so will break scripts.

I'm agnostic about splitting importOutlines() and importImages() If we want the latter the former could of course continue to work as well, possibly with a deprecation warning.

After thinking about the rest of this for (admittedly just) a few minutes, I'm worried about any "Link/Unlink reference" model at all. Or rather, I'm worried about any model like that that also retains a glyph-based importOutlines/Images. I suppose the motivation is to follow the model of outline references, but in that case the reference is to a whole glyph or at least a whole layer. What if multiple images are loaded into glyph "A"? Does a reference pick up all of them? If not, are you referring to a glyph and an image name? Or a glyph and an image "index" starting from zero?

Also, an outline reference is to something that doesn't have its own transformation matrix, but a background image does have one, or a subset of one. Does the reference inherit the referred-to transformation matrix and then modify it, or are all of them independent? If they're dependent, and the initial one changes, will the user get the result he or she expects? If they're independent will that be a pain for other reasons?

In the case of outline references a reference is visually different from a contour set. In the case of images they aren't visually different (or are they?) So how does a user know which character to pull the image from? And if the character the image is pulled from has other images, how the user pick it out? Or Can you pull references through a chain, in which case do the transformation matrices stack up? (Presumably, if they are dependent, they would stack up.)

So here's one alternative that at least initially I find very attractive:

1. I believe that images are only added to the background layer. So add an object to the glyph similar to references but that provides access to a list of "image tuples", each of which has some kind of image identifier as the first element and then transformation matrix info.
2. importOutlines/Images adds elements to that tuple for the glyph it is called on. Maybe we add a way to specify a transformation matrix via an optional third argument.
3. There are no "image references" at the UI level beyond specifying that an image should be loaded "by reference", as this pr adds. Instead, when importing the same path into multiple characters by reference, we document that the implementation will only maintain a single copy in memory. So basically, each character has its own images with separate transformation matrices, but when using the interface in a certain way the FontForge will be more efficient.

Now, to make this a bit easier at the GUI level we might want to add a list of the already loaded images into a dialog, so that instead of loading one from a file you can load one that's already been loaded. But doing so would have the same effect at the python API level.

These initial thoughts are about all I can think of at the moment.

@skef The references only dictate how images are saved/loaded, all the data is still in the GImage.

@ctrlcctrlv you asked me for UI/API advice and above you indicated you already modified importOutline and noted you had yet to add '"Link Reference" and "Unlink Reference"'. My response was one shot at going off those facts. It would probably help at this point to clarify the question: "What should the API look like" for what/to do what, specifically?

Sorry I've been gone for a bit. If we can make image relinking work, I think that it's a useful feature to have. I don't trace in FontForge very often, but I often work with linked images in other pieces of software, and I find myself using the functionality where it is present and grudgingly editing XML by hand where it is not. I think that it's perfectly fine to merge an implementation that does not expose the functionality in the GUI, particularly since the cases in which the feature is most useful are those that involve many glyphs, such that one would not use the GUI.

On the flip side, we cannot create any sort of assumption of one image per glyph.

More in a bit.

What happens now? Unfortunately I don't personally use images.

We've discussed multilayer copy/paste in the past @skef and this is part of that. What happens now is highly underwhelming, images cannot be copied from the FontView, only the CharView. Images cannot be put on a fore layer.

So if you copy the background layer do images "go with" it?

In the FontView? It's not possible to copy the background, is it?

Even using "Copy Layer to Layer" it's not possible to get an image onto the Fore.

And I guess I should ask: What is the purpose of images on layers other than the background. I'm not questioning that there is such a purpose I just want to understand it.

Re the FontView: I'm actually pretty fuzzy on the relationship between the "active layer" and the FontView. The python interface works a certain way and I don't know if the FontView pays attention to the currently selected layer.

Oh, there is no purpose that I know of. It's just that, without your script, only the fore is considered by FontView for copy/paste.

(I just tested it, and even your script totally ignores images. The only way I know to copy/paste images is to open up two CharViews—copy from one, paste into the other. Manually.)

My script can't do much about the images without an API.

But wait a minute: If there's no compelling reason to have images in other layers besides the background, should we support images in other layers? It's a lot simpler at the code level if we just restrict them to the background layer.

Under what circumstances do they copy with two CharViews? Do you have to select and copy the images explicitly or does copying when the background layer is selected in the layer palette suffice?

I can't think of a good reason to ever put images in the Fore.

Someone might want to work from two references, and maybe even put those on different layers so you can hide/show them. But those would just be additional background layers, not the Fore.

You have to select the images explicitly in the CharViews, yes.

OK, these restrictions are giving me some ideas for a interface that includes a limited python API:

1. We stipulate that images can continue to be in the background only. That means no layerimages and just a images structure, with a list of image tuples.
2. The image-representing "token" in an image tuple is a) ad-hoc and b) picked from a namespace specific to the glyph. Probably something like the filename for a by-reference image and "image-1", "image-2" etc for images loaded from the SFD.
3. You can't copy an image between glyphs just by modifying the images list. Instead you must use importOutlines(). However, we might extend importOutlines() to take a glyph and image token as an argument. (I'll have to think about this more.)
4. Therefore the images list is mostly for adjusting the transformation matrix of an image via python.

This way you can make any adjustment you need to from python, it's just cumbersome in some cases. For example I could add image support to my multilayer copy script.

Oh, hmm, additional background layers. Let's punt on that problem for now.

That solution sounds good to me @skef to get this through. We can always make a more complete API in future.

I've been looking at the code and thinking about it the past couple days. That process has yielded some preliminary design ideas for a python API but also a bunch of concerns. The overall model that is already implemented is close but I think it needs a bit more thought.

One worry is that images need some kind of namespace for identification and I don't know what it should be. Some equivalent to using the glyph name to identify an outline reference. The problem is that there doesn't seem to be any obvious informative option. IDing by (glyphname, token) is good as long as there is only one image per glyph, but once you need to care about the token it's hard to come up with something generally informative. (The filename, or a piece of the filename, works for image references but not for SFD-stored images.) (Tuple-like identifiers are also a drawback for the native scripting language, which doesn't have as handy a datastructure.)

An alternative is to give each image its own font-level identity. Unfortunately that doesn't solve the "opaque token problem" for SFD-stored images.

Then there's the question of what you should be able to do with an ID. Just reorder in the same glyph? Copy an image to a different glyph in the same font? In a different font? Presumably there should be an interface to get more data about the image referred to -- width/height, filename if you
know it, etc.)

I'm also not fond of the current ad-hoc handling of whether a reference path is relative or absolute, which seems to be "Attempt relative/fall back on absolute". Shouldn't a user be allowed to pick? Shouldn't a user be forced to specify "absolute" if "relative" is failing? Also, only having the paths be relative to the font path is too error-prone. There should be a font-level parameter (maybe set in Font Info -> General) that represents the absolute path that all relative paths are relative to, which when not set defaults to the sfd's own path. Otherwise it's far too easy to bork yourself into dozens of tiny little repairs.

Unfortunately the combination of this, the schedule, and me poses a problem. I can often implement things pretty quickly when all the questions are answered or if I'm just tracking down bugs with no design implications. But when it comes to design I tend to need to clutch my head for a while. I worry a lot about bad decisions now creating user/developer terror or misery down the road, which makes me a slow "design tinkerer".

So for now I'm going to switch back to trying to implement arcs joins in the expand stroke code. My suggestion is that we continue the foodfight discussion here in an attempt to work out a coherent (if not necessarily finished) design, which I could spend coding time on later. If that work doesn't peter out I don't see why we couldn't delay the next release a bit (if necessary) to accommodate it.

I think maybe you're thinking about this too hard @skef. To me there's an obvious ID, which I thought of while reading your question. (glyph name + - + ImageList index). Why can't this work? So the first image in a would have ID a-1. Seventh image in NameMe.65565 would be NameMe.65565-7.

You might say, well what if those two images have same underlying data? That, to the extent users even care about it, can be solved via having some method to return a hash.

To me there's an obvious ID, which I thought of while reading your question. (glyph name + - + ImageList index). Why can't this work? So the first image in a would have ID a-1. Seventh image in NameMe.65565 would be NameMe.65565-7.

That would mean that as you're reordering the images using the API (presumably to alter which obscures which) the tokens are changing. So updating the list one-at-a-time could have a different effect than updating it all at once.

Sounds like that could just be a note in the documentation

Unfortunately a note in the documentation isn't much insurance against "user/developer terror or misery". If reordering the list borks any outstanding tokens then maybe you shouldn't be able to reorder the list. If you can reorder the list it's presumably because it can be handy to reorder it, in which case there shouldn't be strange gotchas when doing so unless absolutely necessary.

On the other hand, you can always put a counter in the font structure and increment it when needing a new token, so maybe it could be (font-reference, number). (Putting a counter on each glyph seems like overkill.) That would probably mean you need some kind of list at that level (maybe an array that can become slightly sparse over time) to provide the mapping (the counter points at the end of the list).

Given it's not possible to do anything with images right now, is not being able to reorder them without inconvenience really a problem?

By the way, I'm getting an error right now with GSUB output. Do you mind hopping on IRC? I can't figure it out.

(Just so you know, reordering doesn't seem possible in the UI without like cut/paste or re-import.)

If we adjusted that model slightly, it might be workable. In particular, if we exposed the images in a layer simply as an array, the intelligibility issues associated with that scheme seem to lessen considerably. The user could insert, remove, copy, and such.

@skef and I talked in IRC and came up with the following compromise:

• Make it so each glyph may only contain one image
• Make it so the image is always in the Background layer and never other layers

This means in Python we could just have glyph.image, either None or an Image.

I use images the most out of any of the "active contributors" and don't see either of these changes as particularly onerous. If I were to ever want multiple refs for a glyph (dubious proposition), I'd just put them in one image anyway. I can't think of a legitimate reason to be able to have multiple images per glyph in a font editor.

If you can only have one image per glyph, then having it in different background layers is really quite pointless.

It should be noted how poor the support currently is for multiple images, especially in Python. We worry though that @frank-trampe might veto this so are asking first before writing any code.

Oh, and here's a proposal for any old SFDs that might be floating out there:

• If the problem is simply that the image is in a layer other than Back, just move it to Back and emit a warning.
• If the problem is that there are multiple images in one glyph, emit a warning and at user option either:
  • only load the first; or
  • create n duplicate glyphs with the i‌th image.

@skef As there's been no reply I'd say it's safe to start.

@ctrlcctrlv, I've been exchanging thoughts a bit with @skef on the engineering side of this, and I continue to believe that it's not appreciably easier to do this the single-image way than to include decent support for multiple images. The one hang-up, on which we are both stewing, is how to tokenize the images in a way that makes it easy for the user to pick the one he wants. In the single-image case, that's not really a problem anyway. So I would propose that we just expose an array of images as they are stored internally, with appropriate insert/remove functionality like DOM children in JavaScript, and, in the simple single-image-per-glyph case, you can just push/pop that one image with no ambiguity.

This is a note regarding an IRC discussion I had with @skef on the 9^th of February about closing this issue for the following reasons:

• Although an images Python API is highly beneficial for scripts, nothing about this changeset actually implements one, and no code in this changeset is required to implement one.
• The feature itself might be pointless. What does it give us that SFDIR + PNG in SFD does not?
• The code is stale and getting staler every day.

Our resolution was basically I could voluntarily withdraw it if I wanted to, but that @skef doesn't really mind PRs remaining open for extended periods if they are complex as is this one and if there is a possibility someone will come around and close it out, maybe if not the original contributor.

I almost voluntarily withdrew this today, however as I see it reason №2, which was actually my reason, is not actually valid. What this commit buys you is not storage space saved, it's faster serialization of SFD files. So as long as SFDIR writes all glyphs on every save (which it does), and not based on whether or not the SplineChar is marked as "changed" and other heuristics on the parent SplineFont, this feature is useful in and of itself without the Python images API.

So basically, I write this to inform that I almost voluntarily withdrew this, but then decided it's actually still a good feature so I won't be withdrawing it.