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

High chance of font family conflicts with bundled fonts #188

Open
kumattau opened this issue Dec 1, 2021 · 12 comments
Open

High chance of font family conflicts with bundled fonts #188

kumattau opened this issue Dec 1, 2021 · 12 comments

Comments

@kumattau
Copy link
Contributor

kumattau commented Dec 1, 2021

Hi,

For some fonts that qtawesome has, family remains the default.
This may break the user application or qtawesome if the user adds a different version of the font.

For example, in the following code, if the different version of "Material Design Icon" by QtGui.QFontDatabase.addApplicationFont("materialdesignicons-webfont.ttf") is added, qtawesome icon disappers.

QtGui.QFontDatabase.addApplicationFont affects application-wide, so qtawesome's internal "Materinal Design Icon" seems to be overwritten by user's one.

I think it's a good idea to add a unique string (e.g "(qtawesome)") to all families of qtawesome's internal icons to avoid the confliction.

# -*- coding: utf-8 -*-
import sys
from qtpy import QtCore, QtWidgets, QtGui
import qtawesome as qta

class AwesomeExample(QtWidgets.QDialog):

    def __init__(self):
        super().__init__()

        # Get Material Design icons by name
        mdi6_icon = qta.icon('mdi6.account-wrench')
        mdi6_button = QtWidgets.QPushButton(mdi6_icon, 'Material Design Icon # Version 6.3.95, ch=F189A')

        #
        # Add different version of 'Material Design Icon' from qtawesome's internal
        # For example, if version 6.2.95 is added, mdi6.account-wrench (ch=F1809A) disappers
        #
        QtGui.QFontDatabase.addApplicationFont("materialdesignicons-webfont.ttf")

        font = QtGui.QFont("Material Design Icon")
        fontMetrics = QtGui.QFontMetrics(font)
        print(f"font.key()                      : {font.key()}")
        print(f"fontMetrics.inFontUcs4(0xF189A) : {fontMetrics.inFontUcs4(0xF189A)}")

        grid = QtWidgets.QGridLayout()
        grid.addWidget(mdi6_button, 0, 0)
        self.setLayout(grid)
        self.setWindowTitle('Awesome')
        self.show()


def main():
    app = QtWidgets.QApplication(sys.argv)
    if hasattr(QtCore.Qt, 'AA_UseHighDpiPixmaps'):
        app.setAttribute(QtCore.Qt.AA_UseHighDpiPixmaps)
    QtCore.QTimer.singleShot(10000, app.exit)
    _ = AwesomeExample()
    sys.exit(app.exec_())


if __name__ == '__main__':
    main()
  • QtGui.QFontDatabase.addApplicationFont("materialdesignicons-webfont.ttf") is not executed
    Screenshot from 2021-12-02 00-37-48

  • QtGui.QFontDatabase.addApplicationFont("materialdesignicons-webfont.ttf") is executed
    Screenshot from 2021-12-02 00-36-12

@dalthviz dalthviz changed the title Confliction of font family High change of font family conflicts with bundled fonts Dec 1, 2021
@dalthviz dalthviz added this to the v.1.1.2 milestone Dec 1, 2021
@dalthviz
Copy link
Member

dalthviz commented Dec 1, 2021

Hi @kumattau thanks for the feedback! Makes sense to me to somehow differentiate by some sort of prefix the font names that are being bundled here to help reduce the possibility of collisions when adding custom fonts 👍

Just in case, what do you think @ccordoba12 ?

@dalthviz dalthviz changed the title High change of font family conflicts with bundled fonts High chance of font family conflicts with bundled fonts Dec 1, 2021
@ccordoba12
Copy link
Member

Yeah, I think that's a good idea. However, I don't know if simply renaming the fonts could do that, or if there's something that's necessary.

@kumattau
Copy link
Contributor Author

kumattau commented Dec 2, 2021

Thanks @ccordoba12,

Yeah, I think that's a good idea. However, I don't know if simply renaming the fonts could do that, or if there's something that's necessary.

I think changing family is enough to resolve the collision.

https://doc.qt.io/qt-5/qfont.html says the font is matched as follows:

The font matching algorithm works as follows:

  1. The specified font families (set by setFamilies()) are searched for.
  2. If not found, then if set the specified font family exists and can be used to represent the writing system in use, it will be selected.
  3. If not, a replacement font that supports the writing system is selected. The font matching algorithm will try to find the best match for all the properties set in the QFont. How this is done varies from platform to platform.
  4. If no font exists on the system that can support the text, then special "missing character" boxes will be shown in its place.

I think, by above acondition 1, changing family of the qtawesome's font, user's choice does not match qtawesome's font and qtawesome's choice does not match user's font, so the collision does not occur.

Also, changing family was used to resolve the following qtawesome internal collision and seems to be working fine.

@ccordoba12
Copy link
Member

ccordoba12 commented Dec 2, 2021

I think changing family is enough to resolve the collision.

Ok, great! Could you give us a hand with that?

@kumattau
Copy link
Contributor Author

kumattau commented Dec 2, 2021

Ok, great! Could you give us a hand with that?

Yes, I will try it soon.

@kumattau
Copy link
Contributor Author

kumattau commented Dec 6, 2021

It is better that updating font and changing family can be processed with update script, not manually.
But I feel current directory structure makes difficult to process it automatically.

For example, 5.x and 6.x material design icons are bundled in fonts.
These icons were distributed with same filename, so I renamed the filename manually.

To realize updating font and changing family, I think it it better to change the directory structures as follows.

./fonts/mdi/materialdesignicons-webfont.ttf
  keep filename as destributed
  material design icons 5.9.55
  family: qta+174c02@Material Design Icons
./fonts/mdi/materialdesignicons-webfont-charset.json
  charset of material design icons 5.9.55

./fonts/mdi6/materialdesignicons-webfont.ttf
  keep filename as destributed
  material design icons 6.3.55
  family: qta+9a2f45@Material Design Icons
./fonts/mdi6/materialdesignicons-webfont-charset.json
  charset of material design icons 5.3.55

other fonts are the same.

"qta+9a2f45@" is prefix to avoid the collision.
"9a2f45" is md5sum of original ttf file.

Additionally, the all charset is seems to be destributed in the form of css file, so I think it is better to can parse css file directly. https://pypi.org/project/cssutils may be useful for the purpose.

Above changes aren't affect public API but it is much big.
I trying to implement but Is the try worth it ?
I am worried that big change cannot be accepted for the project.

@dalthviz
Copy link
Member

dalthviz commented Dec 6, 2021

Thanks @kumattau for checking! I would say that adding the prefix when using the updating script automatically would be nice. Just in case you think the current folder structure makes it difficult because you will need to use a different font file name? I think that is done by the update script for material design 6.x when downloading the font file (materialdesignicons6-webfont.ttf), right?

Anyhow not totally sure of the need of changing the folder structure, but if you think that will make the font updates easier I guess is ok. Just in case, what do you think @ccordoba12 ?

Regarding the CSS file parsing not sure if needed with an external dependency (my guess is that the way is currently working is to avoid using an external dependency). Is that causing also conflicts somehow? If not maybe that is something to review in another issue.

Thanks again for sharing what you were thinking for the implementation!

@ccordoba12
Copy link
Member

Just in case, what do you think @ccordoba12 ?

I think it's fine as long as updating to new font versions remains easy.

@kumattau
Copy link
Contributor Author

kumattau commented Dec 6, 2021

Thank you @dalthviz , @ccordoba12.

Just in case you think the current folder structure makes it difficult because you will need to use a different font file name?

Yes, material design icon has same filename between 5.x and 6.x. so I need to rename them to avoid filename collision.

I think that is done by the update script for material design 6.x when downloading the font file (materialdesignicons6-webfont.ttf), right?

Yes. if update script knows that materialdesignicons-webfont.ttf should be saved as materialdesignicons6-webfont.ttf, the collision is avoided.
The advantage of using sub directories is that the update script does not need to know the name of the file to be saved.

Here's a way I came up with to avoid file name collisions.

  • move font and charmap to the corresponding "fonts/<prefix>"` sub directory
  • rename font and charmap to "qta+<md5>@<original filename>"

Of these, I thought the sub-directory method was easier to understand because the original file name can be kept and there is no need to define it in the script.
As for the rename method, I think it is difficult to manage the differences because the file name changes with each update.
The method of defining the file name in script or in the configuration file may be fine for managing bundled fonts.

Regarding the CSS file parsing not sure if needed with an external dependency (my guess is that the way is currently working is to avoid using an external dependency). Is that causing also conflicts somehow? If not maybe that is something to review in another issue.

Currently, regular expression is used for the conversion from css to json.
The regular expression is working fine, but if the format of upstream css is changed, it is not sure to keep to work.
I think css parser is better than the regular expression, but is no current problem because the regular expression is working now.

I'll look into how to implement it more.

@kumattau
Copy link
Contributor Author

kumattau commented Dec 8, 2021

I've tried prototyping it, but the process of updating each font (downloading, adding prefix to family, updating checksum, etc.) is duplicated, so I think I should also refactor setupbase.py.

It may take some time, but I will send a PR when I have a draft.

@kumattau
Copy link
Contributor Author

Can I use dictionary ordering support or dataclass which requires python 3.7 or later to implement font updater simply?

@dalthviz
Copy link
Member

I think it should be fine 👍

I think we should drop support for Python 3.6 in the future (probably before merging the improvements you are working on).

Thanks for asking @kumattau !

@dalthviz dalthviz modified the milestones: v1.2.0, v1.2.1 Oct 7, 2022
@dalthviz dalthviz modified the milestones: v1.2.1, v1.2.2 Oct 19, 2022
@dalthviz dalthviz modified the milestones: v1.3.0, v1.4.0 Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants