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

Text wrapping does not work properly for accented characters #135

Open
MartinZikmund opened this issue Jun 21, 2020 · 47 comments
Open

Text wrapping does not work properly for accented characters #135

MartinZikmund opened this issue Jun 21, 2020 · 47 comments
Labels
Status/1. Blocked Requires another issue or pull request to be completed before being ready. Type/Bug

Comments

@MartinZikmund
Copy link

Describe the bug

Text wrapping does not properly work for accented characters like ěščřžýáíé.

To Reproduce

  1. Open sample app for Xamarin.Forms
  2. Select Text tab
  3. Enter: ščěěščž čšřžščřž čšřžščř čšřžščř čšřžščř čšřžščř čšřžščř
  4. Resize the window. Notice the line breaks anywhere within the "words"

Expected behavior

Words with accented characters should not be broken between lines.

Environment (please complete the following information):

  • Platform: CSharpMath.SkiaSharp (in general, not just XF)
  • Package Version or Commit: latest
  • Device: Windows
@MartinZikmund MartinZikmund added Status/0. New This issue is new and is awaiting confirmation from the maintainers. Type/Bug labels Jun 21, 2020
@Happypig375
Copy link
Collaborator

@prepare Probably a bug of Typography.TextBreak.

@Happypig375 Happypig375 added Status/1. Ready This issue has been confirmed and is ready to be worked on. and removed Status/0. New This issue is new and is awaiting confirmation from the maintainers. labels Jun 21, 2020
@Happypig375
Copy link
Collaborator

Confirmed a bug of Typography.TextBreak.

@prepare
Copy link

prepare commented Jun 22, 2020

This Typography bug was fixed,

see => LayoutFarm/Typography@f350e22

include_extenedLatin

@MartinZikmund
Copy link
Author

@prepare Thanks a lot!

@Happypig375
Copy link
Collaborator

Still need to fix this for diacritics via LaTeX commands instead of Unicode, i.e. a\r aa should be one group.

@BenjaNapo
Copy link

BenjaNapo commented Jun 26, 2020

Screenshot_1593169397
That's what happen when I try to show chinese text (appear squares), is this relate to this issue?

@Happypig375
Copy link
Collaborator

You should provide your own font like in #142.

@Happypig375
Copy link
Collaborator

@BenjaNapo See #20.

@BenjaNapo
Copy link

@BenjaNapo See #20.

I wrote in the constructor of App.cs:
CSharpMath.Settings.GlobalTypefaces = new Typography.OpenFont.OpenFontReader().Read(File.Open("Cabin-Regular.ttf", FileMode.Open));

but it says that GlobalTypefaces is an onlyread attribute

@Happypig375
Copy link
Collaborator

For GlobalTypefaces, you call Add.

@BenjaNapo
Copy link

BenjaNapo commented Jun 26, 2020

For GlobalTypefaces, you call Add.

App.xaml

           <OnPlatform x:Key="Regular" x:TypeArguments="x:String">
               <On Platform="Android" Value="Cabin-Regular.ttf#Cabin" />
               <On Platform="iOS" Value="Cabin-Regular" />
           </OnPlatform>

App.cs

       public App()
       {
           InitializeComponent();
           [... various initialization ...]
           object s = new object();
           Resources.TryGetValue("Regular", out s);
           CSharpMath.Settings.GlobalTypefaces.Add(new Typography.OpenFont.OpenFontReader().Read(s as Stream));
       }

Sorry but I never imported Font from code, where did I go wrong?

@Happypig375
Copy link
Collaborator

Are you trying to override the font? The default is AddSupplement. For overrides, use AddOverride.

@BenjaNapo
Copy link

Are you trying to override the font? The default is AddSupplement. For overrides, use AddOverride.

Changed with:

            object s = new object();
            Resources.TryGetValue("Regular", out s);
            Typeface t = new Typography.OpenFont.OpenFontReader().Read(s as Stream);
            CSharpMath.Settings.GlobalTypefaces.AddOverride(t);

But 3rd line throws ArgumentNullException, I think that the problem is in reading font file in this way

@Happypig375
Copy link
Collaborator

The null means that s is probably not a Stream. Please check.

@BenjaNapo
Copy link

        public async void LoadFont() {
            using (var stream = await FileSystem.OpenAppPackageFileAsync("Helvetica-Bold.otf"))
            {
                Typeface t = new Typography.OpenFont.OpenFontReader().Read(stream as Stream);
                CSharpMath.Settings.GlobalTypefaces.AddOverride(t);
            }
        }

Now it loads the font from the stream but crash on the 4th line (Typeface t = ...) throwing this exception:
System.NotSupportedException: 'Specified method is not supported.'

@Happypig375
Copy link
Collaborator

Try following https://github.com/verybadcat/CSharpMath#sourcelink-for-ci-packages and debug into the source of Typography.

@Happypig375
Copy link
Collaborator

@BenjaNapo Or you can upload a zipped file containing your file and let me investigate what changes are needed from Typography's side.

@Happypig375
Copy link
Collaborator

This bug's fix depends on a decision on updating Typography in LayoutFarm/Typography#192.
The fix for

Still need to fix this for diacritics via LaTeX commands instead of Unicode, i.e. a\r aa should be one group.

is scheduled after #143 and #58.

@Happypig375 Happypig375 added Status/1. Blocked Requires another issue or pull request to be completed before being ready. and removed Status/1. Ready This issue has been confirmed and is ready to be worked on. labels Jul 16, 2020
@BenjaNapo
Copy link

https://drive.google.com/file/d/1zlvyQ2XXvb0fQJ8k-otYZ4hiR_8L3H-d/view?usp=sharing

Here you are, with this you could note all the issues that I had 'til now (text alignment, change font and height of textview)

Thanks for all the help ^^ Hope this will contribute with the evolution of the library

@Happypig375
Copy link
Collaborator

Happypig375 commented Jul 16, 2020

@BenjaNapo #144 should fix "text alignment" and "height of textview".

This leaves "change font" as to-do.

Thanks for all the help ^^ Hope this will contribute with the evolution of the library

Thank you for taking the time to report problems too!

@Happypig375
Copy link
Collaborator

@BenjaNapo I can't download the file.
image

I have requested access.

@BenjaNapo
Copy link

I have requested access.

Ow, sorry, thought that just the link will work. Does it work now?

@Happypig375
Copy link
Collaborator

Works now, thanks

@Happypig375
Copy link
Collaborator

@BenjaNapo I'm not getting the NotSupportedException.
image

@BenjaNapo
Copy link

Still not working, tried as you to put the method inside async OnStart but nothing
2020-07-23
It crash at line 53, and, as you can see, stream.Length and stream.Position are not setted, so probably the Read try to use them

@Happypig375
Copy link
Collaborator

What is the stack trace?

@BenjaNapo
Copy link

Do you mean this?
2020-07-23 (2)

@Happypig375
Copy link
Collaborator

I mean the stack trace of the exception.

@BenjaNapo
Copy link

Ehm... How can I ?

@Happypig375
Copy link
Collaborator

image

@BenjaNapo
Copy link

As you can see I don't have the "View Details" option 🙄 Probably is because I'm working on Android device and not using UWP

2020-07-24 (1)

@MartinZikmund
Copy link
Author

@Happypig375 Should this be working in the latest preview NuGet package (0.5.0-alpha4) or is it not yet released?

@Happypig375
Copy link
Collaborator

Yes, the fix should be included in 0.5.0-alpha4.

@MartinZikmund
Copy link
Author

MartinZikmund commented Sep 12, 2020

@Happypig375 It seems it does not work for me 🤔 :
image

For example on the last line of the text is the word "světovou" broken as "svě" and "tovou". Was the Typography module bumped to the updated version?

@prepare do you have some idea what could be wrong?

@Happypig375
Copy link
Collaborator

Ohhh. Yea I misrecognized this issue as the ones that were fixed. Yes this depends on updating Typography currently, but I am looking to refactor to reuse the math parser for text, so the Typography.TextBreak dependency will be gone by then. This is targeted at 0.5.0-beta.

@BenjaNapo
Copy link

In the end I resolved in this way:

            Stream stream = await FileSystem.OpenAppPackageFileAsync("GTEesti-Light.ttf");
            MemoryStream ms = new MemoryStream();
            stream.CopyTo(ms);
            ms.Position = 0;
            OpenFontReader ofr = new OpenFontReader();
            Typeface t = ofr.Read(ms);
            CSharpMath.Settings.GlobalTypefaces.AddOverride(t);

@Happypig375
Copy link
Collaborator

Oh... Maybe the stream returned by FileSystem.OpenAppPackageFileAsync is not seekable.

@BenjaNapo
Copy link

Yep, indeed the NotSupportedException was in Length/Position attributes 😅 I think that now we can close the issue (?)

@BenjaNapo
Copy link

Screenshot_1593169397
That's what happen when I try to show chinese text (appear squares), is this relate to this issue?

Anyway I'm still having this issue, on Label chinese text is showed (so it's a not problem of the font)...

@MartinZikmund
Copy link
Author

MartinZikmund commented Nov 5, 2020

Hey, I have updated to latest package (beta), but the problem seems to persist:
image

Also noticed the accented characters don't play well with bold/italics, is that a known issue?

@BenjaNapo
Copy link

Hey, I have updated to latest package (beta), but the problem seems to persist:
image

Also noticed the accented characters don't play well with bold/italics, is that a known issue?

Try to load an italic font

@charlesroddie
Copy link
Collaborator

Also noticed the accented characters don't play well with bold/italics, is that a known issue?

This is tracked here: #142

However it will be a few weeks before I can work on this.

@MartinZikmund
Copy link
Author

Hi @Happypig375, regarding the line break fix for words with accented characters, is there an estimate when a preview package with this could be released please :-) ?

@Happypig375
Copy link
Collaborator

@MartinZikmund I don't have the capacity to put in work in the near future. It's really best to just fork and submit a PR for now.

@MartinZikmund
Copy link
Author

@Happypig375 of course, understood 👍. Just to check - is there something missing on the text wrapping code-wise or is it just about releasing a new package? I thought @prepare adjusted the line breaks in Typography, which should transitively fix the problem for CSharpMath, if we update to that version.

@Happypig375
Copy link
Collaborator

See #107 (comment).

@charlesroddie
Copy link
Collaborator

@MartinZikmund I said I would do the font work but am not using TextPainter any more. I don't think I have time to progress with this by myself in the very near future but can collaborate with you on a PR. Weekends best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status/1. Blocked Requires another issue or pull request to be completed before being ready. Type/Bug
Projects
None yet
Development

No branches or pull requests

5 participants