fonttools / fonttools

A library to manipulate font files from Python.
MIT License
4.31k stars 454 forks source link

[ufoLib] No subdirectories in the images folder? #1890

Open madig opened 4 years ago

madig commented 4 years ago

I notice that ufoLib.getImageDirectoryListing purposefully skips over directories while reading the contents of the images directory:

https://github.com/fonttools/fonttools/blame/master/Lib/fontTools/ufoLib/__init__.py#L737-L740

The same code has been there since 2011: https://github.com/fonttools/fonttools/commit/e4f9b0f78c7b9719323c336484e4c6840d70413f#diff-ff5189f5afcdabc9f8cd59702c30fadfR507-R510

Is the images directory supposed to be flat or is this an oversight? The specification does not seem to imply as much.

@typesupply

typesupply commented 4 years ago

It is supposed to be flat.

anthrotype commented 4 years ago

may I ask why is that the case? It could be useful to group sets of images into distinct subfolders of the UFO images directory, and ufoLib already handles that without issues for the data subfolder (which is very similar to the images one).

typesupply commented 4 years ago

may I ask why is that the case?

(As always on the reasoning behind these design decisions, these were made many years ago and I'm working from my own memory. I think I was sitting in a hostel dining room in Iceland for a WOFF meeting when I wrote a large part of this stuff. 😄)

I don't think I thought about subdirectories of the image directory per se, but I know I gave a substantial amount of thought to requests that it be possible to reference image outside of the UFO. The logic that led to me deciding against that is also applicable to subdirectories:

The image element has to know the actual image data. I didn't want to do something gross like put the image data inside of the XML, so the images had to be located in external files. Therefore, the spec needed to define how to find those images. When I was considering referencing images located outside of the UFO, I sketched out a path location notation (this is a fuzzy bit of my memory) using something like Unix's / and .. That got gnarly because of cross-platform complexity and the spec got longer and more nuanced. I worried that 1. this was making it harder to implement and 2. that it was opening up a vector for misinterpretation. So, I opted for absolute simplicity: all files in one known subdirectory within the UFO and all images in one widely (and consistently) implemented image format.

I think a flat directory is still the best choice, even for subdirectories. Here's the logic: How would the files be referenced by the image element? Would the fileName attribute specify the relative path to the file? If so, the spec for that needs to be defined. If the fileName attribute just specifies a a file name, the implementation needs to know if the file names are required to be unique across subdirectories. If not, the spec needs to define how to resolve an image element referencing sketch.png when there are:

images
    A
        sketch.png
    B
        sketch.png
    C
        sketch.png

That's a lot of complexity to introduce to the spec and require of implementors. UFO 3 was designed to be as easy to implement consistently as possible. So, that's why I did it the way that I did.

ufoLib already handles that without issues for the data subfolder (which is very similar to the images one).

Yeah. I wrote that code too. I probably wrote it in the same hostel dining room just minutes/hours/days apart from the image specification. 😉 (Note that while ufoLib handles the data directory this way, the UFO spec specifically does not define how to find anything specific within the data directory. That was a deliberate decision.)

anthrotype commented 4 years ago

Would the fileName attribute specify the relative path to the file?

yes, a relative path with root at the UFO/images directory, just like data currently works (A/sketch.png and B/sketch.png are different images).

What I had in mind is the possibility of using the images to store PNGs for a bitmap color fonts, and grouping each "strike" of images in different subdirectories (24x, 72px, 128px, etc.).

I know I can use different filenames instead of directories but a directory is easier to browse, especially when you have thousands of files.

typesupply commented 4 years ago

yes, a relative path with root at the UFO/images directory, just like data currently works (A/sketch.png and B/sketch.png are different images).

So that would need to be specced. The thing that I always weighed was the cost of spec and implementation against the amount of added usefulness.

What I had in mind is the possibility of using the images to store PNGs for a bitmap color fonts, and grouping each "strike" of images in different subdirectories (24x, 72px, 128px, etc.).

Hm. That gets beyond the scope of what the image element was designed to do. At some point I outlined a use case for how the data directory could be used for situations when > 1 images were needed for one glyph in certain formats and authoring tools without requiring every implementation to support the structure and logic. I think I used PhotoFont as an example. That's on a mailing list somewhere. Short version from memory: Define and publish the data directory key and subdirectory structure and how that should be interpreted.