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

OT SVG support - patches included #629

Open
HinTak opened this issue Apr 4, 2024 · 11 comments
Open

OT SVG support - patches included #629

HinTak opened this issue Apr 4, 2024 · 11 comments

Comments

@HinTak
Copy link

HinTak commented Apr 4, 2024

Is your feature request related to a problem? Please describe.

Would you considering adding the patches to your fork?
https://github.com/HinTak/chromium-mod-CI

Describe the solution you'd like, including relevant patches or source

Patches included.

Additional Notes

See the qt web engine filing also.
You have a chance to be better than both chrome, Firefox, and Safari in terms of colour font support.

@gz83
Copy link
Collaborator

gz83 commented Apr 4, 2024

First of all, thank you very much for your work. Also, have you tried submitting these patches you wrote to Google for review?

@HinTak
Copy link
Author

HinTak commented Apr 4, 2024

The last comments in both threads are mine :
https://bugs.chromium.org/p/chromium/issues/detail?id=306078
https://issues.chromium.org/40336440 . In addition, I am in some other mailing lists where the relevant google engineers frequent too. Consider the work is mostly done by them and the patches do very little beyond flipping some options, I think their choice is more a strategy against Firefox and Webkit than technical. (As in make it work only as a competitive target to try to do a better(?) alternative).

@gz83
Copy link
Collaborator

gz83 commented Apr 5, 2024

Have you verified that the relevant support actually takes effect after applying these patches? In addition, I took a look at the patches you wrote, and there seem to be some inconsistencies in them that do not comply with Google's coding standards.

@HinTak

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

Testing and verification are done via https://github.com/HinTak/Qt6WE-OT-SVG . As for coding standard, please feel free to comment further. Yes, it is against m118, slightly older.

@gz83
Copy link
Collaborator

gz83 commented Apr 5, 2024

You only need to use the git cl format command after editing the Chromium source code to automatically reformat the code and comply with Google's coding standards. The prerequisite for running the above command is that you need to deploy Chromium's depot_tools.

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

Okay, thanks for the tips. I only work with skia in git; chromium against snapshot tar balls. I 'll think about how to do the formatting on github ci.

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

I probably should put these two links next to the patches (they are on the minimal web browser script README) - just visit them before and after applying the patches to see the difference:
https://pixelambacht.nl/chromacheck/

https://yoksel.github.io/color-fonts-demo/

(Or with a different browser - Firefox/ webkitgtk both handle OT-SVG quite well, but missing COLRv1 and others which chrome supports)

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

If you have a git repo handy, can you git cl format then post the result? If it is just white space, I 'll up date the patches.

@gz83
Copy link
Collaborator

gz83 commented Apr 5, 2024

We don't need to reformat the code yet, I just gave you a reminder. I saw in your patches that the files that need to be modified involve components such as ots, skia, and blink. Have you discussed it with the developers of ots, skia, and blink before or submitted patches to them?

By the way, due to some potential impacts (security, binary size, W3C specifications, etc.), we should modify the relevant files with caution.

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

The relevant people are all CC'ed on those two issue URLs, as well as subscribers of a few mailing lists on which I posted the work. This has been going on for 8 months. If they want to say something, they'd have done so by now.

@HinTak
Copy link
Author

HinTak commented Apr 5, 2024

The skia part of the patch has gotten into skia-python 6 months ago (and I have been invited as co-maintainer/releases since).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants