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

[Feature Request] Support SheenBidi #138

Closed
nullgemm opened this issue Oct 7, 2021 · 12 comments
Closed

[Feature Request] Support SheenBidi #138

nullgemm opened this issue Oct 7, 2021 · 12 comments

Comments

@nullgemm
Copy link
Contributor

nullgemm commented Oct 7, 2021

Hi, I am developing a simple widget toolkit (working copy here) and plan on using libraqm to help with the text rendering, but the FriBiDi dependency worries me a little.

Anything using LGPL code must be released in a way that allows re-linking it with a modified version, and I try my best to write software everyone can use without limitations other than some form of attribution, which is possible under the Apache, MIT and BSD licenses, but not with the GPL or the LGPL.

If my understanding is correct, you guys (or at least @khaledhosny) know about SheenBiDi, which is Apache-licensed and could replace FriBiDi. Do you plan on supporting it? If yes, when will that happen? If not, would it be reasonable for me to fork libraqm?

Thank you for your work on this library :)

EDIT:
It looks like I'm not the first one to bring this up: Tehreer/SheenBidi#10

@khaledhosny
Copy link
Collaborator

I’m open to supporting SheenBiDi as alternative to FriBiDi, but I don't have time to do it myself currently, but I’ll try to review any PRs.

@nullgemm
Copy link
Contributor Author

nullgemm commented Oct 8, 2021

Ok, I'll give it a shot and see what I can help with. Quite a few boring steps will have to be taken though:

  • it would probably be better if SheenBidi supported Meson
    (it is not a very widespread package among distros so being able to rely on Meson to build it automatically would be nice)
  • we should add options to choose between FriBiDi/SheenBidi and whether or not to generate a static binary
    (which is fine when not using LGPL'd dependencies)

Does it look acceptable to you?

@khaledhosny
Copy link
Collaborator

Meson support in SheenBidi would be nice of course, and an option to switch between bidi libraries. I don’t get the point about static binaries, as we don’t statically link to any of our dependencies by default.

@nullgemm
Copy link
Contributor Author

nullgemm commented Oct 8, 2021

Meson support in SheenBidi would be nice of course, and an option to switch between bidi libraries

Ok, thanks! SheenBidi's maintainer already started working on Meson support (Tehreer/SheenBidi#12)

I don’t get the point about static binaries, as we don’t statically link to any of our dependencies by default

Actually my point was that we currently build libraqm.so but not libraqm.a. Since we can't always statically link FriBiDi in our apps (because of the LGPL) it kind of made sense to just dynamically link everything so far. If however we make libraqm use SheenBidi then apps can statically link everything and it would be helpful to also generate libraqm.a. Am I missing something?

@khaledhosny
Copy link
Collaborator

That is a standard meson feature, by default it builds only shared libraries, but you can control it with --default-library option.

@nullgemm
Copy link
Contributor Author

nullgemm commented Oct 9, 2021

Oh, right. I guess everything's clear then :)

@nullgemm
Copy link
Contributor Author

Hi, after experimenting for a while I still can't get all the tests to pass. It is probably my fault since I don't know a lot on the subject but I'd like to rule out the possibility of SheenBidi not being able to handle all libraqm test cases: is such a thing possible at all? Actually this comes to my mind because of the results of the Tehreer stack in the text rendering tests conducted by unicode. Most errors here seem to be stemming from the shaping phase but after all maybe Bidi can be faulty, too. Is it something that could cause 28 tests to fail in libraqm?

Also, I won't have as much free time to invest in this task starting from next week so I am probably going to try integrating ualyze instead to see what results I can get and draw some conclusions more easily. If it magically turns out ok, would you be interested in supporting this instead of SheenBidi?

Thanks for your patience :)

@khaledhosny
Copy link
Collaborator

I know that at least a couple if tests are wrong since they test newline characters but ram input should be a single paragraph, so send PR and we can assess the failures.

@nullgemm
Copy link
Contributor Author

I can't really open a PR now as I only have a very dirty prototype with hardcoded SheenBidi support, but I can make a clean branch if you think it's ok to already move forward. Should I just ifdef the Fribidi & SheenBidi parts or do you prefer a multi-file approach?

@nullgemm
Copy link
Contributor Author

It turns out adding utf16 support (needed by ualyze) requires making lots of other changes I am not very comfortable with so this is another dead end for me. I will stop here and clean what I have so far for SheenBidi to open a PR. Just tell me if you prefer ifdefs or one extra source file per Bidi library to separate the different versions of the functions affected by this change.

@khaledhosny
Copy link
Collaborator

#ifdef’s are fine if they are localized enough.

@khaledhosny
Copy link
Collaborator

Done in #139

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