Implementing VF instance selector #300

kontur · 2022-12-13T19:14:30Z

Here's a start for #25 — I'm pretty sure there are things I missed with the overall flow of the app and with vanilla in particular, so happy to take your pointers.

I've added the instance parsing to otfFont.py for now — I'm not using the designspace workflow myself, but I assume it should expose them as well? I'm adding the family to the instance name, otherwise it becomes confusing with several files. I've opted for a default "No instance selected" and using the sliders also resets to this.

One visual thing I couldn't figure out is how to give the PopUp the same margin as the slider container; I tried with those same "margin" settings and negative widths, but that just created havoc. Now the instance Popup goes from edge to edge, when it would be nicer to have that little margin. The PopUp is placed under the axes and I'm updating the y position inside the tab based off the axis widget's calculated height — not sure if there is a more automatic way to stack (and update) items inside a Vanilla Group.

kontur · 2022-12-13T19:16:41Z

Lib/fontgoggles/mac/mainWindow.py


    @objc.python_method
    def varInstanceChanged(self, sender):
-        self.project.textSettings.varLocation = {k: v for k, v in sender.get().items() if v is not None}
+        """Instance was selected from popup"""
+        _, _, _, allInstances, _, _ = self._gatherSidebarInfo(self.project.fonts)


These calls to get the allInstances are quite clumsy, but I wanted to keep the generation of the list in one spot to avoid confusion. Maybe inside _gatherSidebarInfo the instance part could be refactored into a separate method, and in this callback in varLocationChanged that separate method would suffice.

I was wondering if all of what _gatherSidebarInfo returns shouldn't rather be e.g. an @property of Project. Then _updateSidebarItems could just access the properties as needed, and these callbacks could access the instance list as needed. The getters could cache the iteration over the font list internally, to avoid iterating the font list for each property when called without font list changes between calls to different (or same, for that matter) attributes.

Lib/fontgoggles/mac/mainWindow.py

Lib/fontgoggles/font/baseFont.py

kontur · 2022-12-13T19:18:42Z

Lib/fontgoggles/mac/mainWindow.py

@@ -303,9 +303,10 @@ def setupSidebarGroup(self):
                                                       showHiddenAxes=self.project.uiSettings.showHiddenAxes)

        self.variationsGroup.instances = LabeledView(
-            (0, 0, 0, 1),  # gets updated as axes are read
+            # Position and instances gets updated as fonts are read
+            (0, 0, 0, 20),


What does the x, y, w, h destructuring in LabledView do, if only checking for h > 0 — should it be removed?

I really don't understand the posSize ... dug down into VanillaBase but can't figure it out. I tried (10, y, -10, 0) (updated also in the _updateSidebarItems call) like the sliders, but that just results in weird overlaps and cut offs.

justvanrossum · 2022-12-13T19:39:54Z

Thanks! I'll need some time to review and advise. Please ping me here if you think it's taking too long.

Btw. what did you think of my suggestion in #25 about an alternative approach to show named instances? This bit:

An alternative may be to treat named instances like fonts in a collections (TTC). For example, we could add a contextual menu named "Expand named instances", and it would add all named instances as individual "fonts".

kontur · 2022-12-13T20:53:55Z

Sure, no problem, I might see if I can improve some of the things I commented on in the code above in the meantime.

Re. expanding the VF instances as pseudo fonts in the view: I think it is a different paradigm; the sliders and the instance selector work together, allowing the user to see what axis value an instance has (and somehow exposing what instances there are). Both are ambivalent in the sense that they affect to all fonts in the list if the font has such axes.

I could imagine dropping a VF on the UI could prompt if to add as single VF or expand to all instances, as separate feature. An alternative could be right-clicking a VF in the list and it has all instances available for switching to.

In a nutshell both approaches have the same issue with being ambivalent... a choice relevant for one font possibly affects several fonts, or it is unclear why some fonts are not affected. I don't have a clear solution to offer for that, unfortunately.

kontur · 2022-12-14T20:13:56Z

Supports parsing the instance names from designspaces now as well. There is no proper name table entries for such a constructed font, so I am using the font list label as family fallback of the instance names.

kontur · 2023-02-27T07:52:36Z

This would still be handy to have, I constantly find myself launching the app from my fork instead of the installed version, exactly because I need this feature :)

I'd really only need help with the nested label positioning for which I cannot figure out how to give it some padding:

kontur · 2024-05-27T13:17:49Z

I'd really only need help with the nested label positioning for which I cannot figure out how to give it some padding

Any pointers? ☝️

justvanrossum

I commented one idea about the margin problem in-line. I hope it helps, but I'm far from sure.

justvanrossum · 2024-05-27T13:42:41Z

Lib/fontgoggles/mac/mainWindow.py

        self.project.textSettings.varLocation = {k: v for k, v in sender.get().items() if v is not None}
        self.textEntryChangedCallback(self.textEntry, updateCharacterList=False)
+        self.variationsGroup.instances.setItems([i[0] for i in allInstances])


I'm not sure why this is needed: this callback is called when an axis slider changed, this can't cause a change in the instances list.

Left over from when I tried to select instances if matched; implemented in the latest commit.

justvanrossum · 2024-05-27T13:44:50Z

Lib/fontgoggles/mac/mainWindow.py

        variationsTab = group.feaVarTabs[1]
-        self.variationsGroup = SliderGroup(sidebarWidth - 6, {}, callback=self.varLocationChanged,
-                                           showHiddenAxes=self.project.uiSettings.showHiddenAxes)
+        self.variationsGroup = Group((0, 0, 0, 0))


Try setting the width to sidebarWidth - 6, ie:

self.variationsGroup = Group((0, 0, sidebarWidth - 6, 0))

No guarantee this is the solution to the margin problem, but since the SliderGroup is setting its width explicitly, I think you need to do it here, too. If I remember correctly, setting the width explicitly worked around some layout problems with nested groups.

I couldn't really get this to work like this because SliderGroup seemed to always place its nested items absolutely. I refactored how LabledView passes its posSize to child elements and amended this where it is used elsewhere (the top of the sidebar Orientation/Script/Language labels) — then I was able to properly offset the variationsGroup as intended.

kontur · 2024-06-05T09:07:29Z

Hmm. Has something changed with the dev flow? I can build the app, open it, but I can't open any files with it (I can select them, but nothing happens) and the main window never opens (also on master, or on v1.8).

Executing task: python App/setup.py py2app -A && ./App/dist/FontGoggles.app/Contents/MacOS/FontGoggles 

Hello .zprofile
/Users/johannes/Projects/fontgoggles/App/setup.py:2: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  import pkg_resources
running py2app
creating /Users/johannes/Projects/fontgoggles/App/build/bdist.macosx-12.2-arm64/python3.10-standalone/app
creating /Users/johannes/Projects/fontgoggles/App/build/bdist.macosx-12.2-arm64/python3.10-standalone/app/collect
creating /Users/johannes/Projects/fontgoggles/App/build/bdist.macosx-12.2-arm64/python3.10-standalone/app/temp
creating build/bdist.macosx-12.2-arm64/python3.10-standalone/app/lib-dynload
creating build/bdist.macosx-12.2-arm64/python3.10-standalone/app/Frameworks
Add paths for VENV /Users/johannes/.pyenv/versions/3.10.4 False
*** creating application bundle: FontGoggles ***
Copy '/Users/johannes/.pyenv/versions/fontgoggles/lib/python3.10/site-packages/py2app/apptemplate/prebuilt/main-arm64' -> '/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app/Contents/MacOS/FontGoggles'
sign ['/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app/Contents/MacOS/FontGoggles']
/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app/Contents/MacOS/FontGoggles: replacing existing signature
/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app/Contents/MacOS/FontGoggles: signed app bundle with Mach-O thin (arm64) [com.github.justvanrossum.FontGoggles]
/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app: replacing existing signature
/Users/johannes/Projects/fontgoggles/App/dist/FontGoggles.app: signed app bundle with Mach-O thin (arm64) [com.github.justvanrossum.FontGoggles]
Done!
/Users/johannes/Projects/fontgoggles/App/FontGoggles.py:11: DeprecationWarning: Set objc.options.verbose instead
  objc.setVerbose(True)

justvanrossum · 2024-06-05T09:14:32Z

There must be more errors once you try to open the app or try to open a window.

First thing to check: are all dependencies up to date? Perhaps try to create a new venv from scratch.

I'm currently building with Python 3.12, but it should still work on 3.10.

kontur · 2024-06-05T11:10:42Z

No errors, just no main window. I'm debugging myself along through the window instantiation with traces to see if something silently fails, but not found anything off yet.

Untitled.mov

Shouldn't make a difference, but can you try py2app without -A?

No difference.

Also: From debug traces I can see that the FGMainWindowController new constructor gets called, but the init never gets called; surely that is symptom of the root problem and explains not seeing any window, because the __init__ has all the window logic and opening calls.

justvanrossum · 2024-06-05T11:50:29Z

Hmm, can you try with pyobjc==10.2 instead of the current 10.3? That init thing may be a clue.

justvanrossum · 2024-06-05T11:54:37Z

Yup, that's the one, I'll file a pyobc bug :( We need to pin to 10.2 for now.

kontur · 2024-06-06T12:18:00Z

Lib/fontgoggles/mac/mainWindow.py

-        # Reset instance popup
-        self.variationsGroup.instances.set(0)
+
+        # Select or reset instance popup:


This is the logic for selecting a dropdown instance if the axis values match, which is a nice addition. The bit that's a tad convoluted about this is case is where there can be several fonts (even with different axes) where several of them match an instance for a specific set of axes values; for that rare case I simply opt to not select any instance. I think this is an okay trade-off for having the nicer behavior of automatically selecting an instance when matched, which is the much more likely use case.

kontur · 2024-06-06T12:19:33Z

Lib/fontgoggles/mac/sliderGroup.py

@@ -93,8 +93,7 @@ def get(self):
            slider = getattr(self, attrName)
            value = slider.get()
            if value is not None:
-                if len(self._defaultValues[tag]) != 1 or value not in self._defaultValues[tag]:
-                    state[tag] = value
+                state[tag] = value


I couldn't find any other use of sliderGroup other than the axes sliders, and there is no need to return only non-default values. The full set of current axis values is needed in the above bit to detect the currently selected instance (if there is).

kontur · 2024-06-06T12:21:01Z

Yup, that's the one, I'll file a pyobc bug :( We need to pin to 10.2 for now.

And this is indeed needed, but seemed unrelated to this PR in particular so I left that out.

justvanrossum · 2024-06-06T12:31:35Z

Yup, that's the one, I'll file a pyobc bug :( We need to pin to 10.2 for now.

And this is indeed needed, but seemed unrelated to this PR in particular so I left that out.

I just committed a pin to 10.2 to master, perhaps you want to rebase.

kontur · 2024-06-06T12:46:49Z

Rebased onto latest master 👍

kontur commented Dec 13, 2022

View reviewed changes

Lib/fontgoggles/mac/mainWindow.py Outdated Show resolved Hide resolved

kontur commented Dec 13, 2022

View reviewed changes

Lib/fontgoggles/font/baseFont.py Outdated Show resolved Hide resolved

kontur commented Dec 13, 2022

View reviewed changes

justvanrossum reviewed May 27, 2024

View reviewed changes

kontur mentioned this pull request Jun 5, 2024

Build issues (v1.7.4/v1.8/master) #415

Closed

kontur force-pushed the instance-selector branch from abae7c0 to 018c7e4 Compare June 6, 2024 12:09

kontur commented Jun 6, 2024

View reviewed changes

kontur added 6 commits June 6, 2024 15:46

Adding placeholder dropdown UI

4232d7a

Added instance parsing and dropdown activation in variationsGroup tab

e71cd98

Better instance family name and support for design space font instances

079f534

Slightly better allInstances list with unique values only

8368df7

Fixed instance dropdown positioning

0b92348

Select an instance if the slider values match

1dae7ab

kontur force-pushed the instance-selector branch from 018c7e4 to 1dae7ab Compare June 6, 2024 12:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing VF instance selector #300

Implementing VF instance selector #300

kontur commented Dec 13, 2022 •

edited

Loading

kontur Dec 13, 2022

kontur Dec 14, 2022

kontur Dec 13, 2022

kontur Dec 14, 2022

justvanrossum commented Dec 13, 2022

kontur commented Dec 13, 2022

kontur commented Dec 14, 2022

kontur commented Feb 27, 2023

kontur commented May 27, 2024

justvanrossum left a comment

justvanrossum May 27, 2024

kontur Jun 6, 2024

justvanrossum May 27, 2024

kontur Jun 6, 2024

kontur commented Jun 5, 2024 •

edited

Loading

justvanrossum commented Jun 5, 2024

kontur commented Jun 5, 2024 •

edited

Loading

justvanrossum commented Jun 5, 2024

justvanrossum commented Jun 5, 2024

kontur Jun 6, 2024

kontur Jun 6, 2024

kontur commented Jun 6, 2024

justvanrossum commented Jun 6, 2024

kontur commented Jun 6, 2024

Implementing VF instance selector #300

Are you sure you want to change the base?

Implementing VF instance selector #300

Conversation

kontur commented Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justvanrossum commented Dec 13, 2022

kontur commented Dec 13, 2022

kontur commented Dec 14, 2022

kontur commented Feb 27, 2023

kontur commented May 27, 2024

justvanrossum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kontur commented Jun 5, 2024 • edited Loading

justvanrossum commented Jun 5, 2024

kontur commented Jun 5, 2024 • edited Loading

justvanrossum commented Jun 5, 2024

justvanrossum commented Jun 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kontur commented Jun 6, 2024

justvanrossum commented Jun 6, 2024

kontur commented Jun 6, 2024

kontur commented Dec 13, 2022 •

edited

Loading

kontur commented Jun 5, 2024 •

edited

Loading

kontur commented Jun 5, 2024 •

edited

Loading