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

v0.9.0 unexpectedly justifies text #151

Open
notgull opened this issue Jul 7, 2023 · 4 comments
Open

v0.9.0 unexpectedly justifies text #151

notgull opened this issue Jul 7, 2023 · 4 comments

Comments

@notgull
Copy link
Contributor

notgull commented Jul 7, 2023

Here is an example program adapted from piet I wrote in my piet-cosmic-text crate. It is using cosmic-text v0.8.0

image

Here is the program after I update it to v0.9.0:

image

It appears that the text has been justified for some reason, despite there being no changes in the inputs to the code. The shaping is in Advanced mode, as this particular text sample does not render properly in Basic mode.

As far as I can tell, this is a problem on the cosmic-text side, as very little actual changes on my end were conducted between the two above samples. Here is the commit: notgull/piet-cosmic-text@d323ede

Just to see if it was my alignment system, I manually set each line's alignment to be Some(Align::Left).

diff --git a/src/lib.rs b/src/lib.rs
index caa6e42..07679cb 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -731,12 +731,7 @@ impl piet::TextLayoutBuilder for TextLayoutBuilder {
             let attrs_list = range_attributes.text_attributes(start..end, default_attrs)?;
 
             let mut line = BufferLine::new(line, attrs_list, shaping);
-            line.set_align(self.alignment.map(|a| match a {
-                TextAlignment::Start => cosmic_text::Align::Left,
-                TextAlignment::Center => cosmic_text::Align::Center,
-                TextAlignment::End => cosmic_text::Align::Right,
-                TextAlignment::Justified => cosmic_text::Align::Justified,
-            }));
+            line.set_align(Some(cosmic_text::Align::Left));
 
             buffer_lines.push(line);

I did not witness any changes in the text.

I suppose that my usage of PhysicalGlyph could be wrong, but I do not see how this could lead to justified text.

@jackpot51
Copy link
Member

I don't see any of the cosmic-text examples showing this behavior. Please provide instructions on how I can reproduce the issue.

@notgull
Copy link
Contributor Author

notgull commented Jul 7, 2023

Here is a repository demonstrating the issue, using nothing but cosmic-text as well as tiny-skia for rendering. The first commit uses cosmic-text v0.8.0. It contains an example that builds up an AttrsList, creates a buffer line, inserts it into the Buffer and then renders it using swash. The resulting image is this:

before

I then upgrade to cosmic-text v0.9.0. The upgrade in its entirety, aside from Cargo.lock updates, is contained below:

diff --git a/Cargo.toml b/Cargo.toml
index 077e591..5de006f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -6,6 +6,6 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-cosmic-text = "0.8.0"
+cosmic-text = "0.9.0"
 tiny-skia = "0.11.1"
 tracing-subscriber = "0.3.17"
diff --git a/after.png b/after.png
new file mode 100644
index 0000000..3230e43
Binary files /dev/null and b/after.png differ
diff --git a/src/main.rs b/src/main.rs
index d4f1e65..5c0cdc8 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,7 +1,7 @@
 // MIT/Apache2 License.
 //! Reproduction of cosmic text issue #151
-//! 
-//! This version uses cosmic_text v0.8.0, which works and does not justify text.
+//!
+//! This version uses cosmic_text v0.9.0, which unexpectedly justifies text.
 
 use cosmic_text::{self as ct, Buffer, BufferLine, Color, FontSystem};
 
@@ -102,7 +102,7 @@ fn main() {
     );
 
     // Create and format the text.
-    let line = BufferLine::new(TEXT, list);
+    let line = BufferLine::new(TEXT, list, ct::Shaping::Advanced);
     buffer.lines = vec![line];
     buffer.set_wrap(&mut fs, ct::Wrap::Word);
     buffer.set_size(&mut fs, 250.0, 300.0);

This is the resulting image:

after

@notgull
Copy link
Contributor Author

notgull commented Jul 7, 2023

Oddly, when I comment out the change in the font family...

diff --git a/src/main.rs b/src/main.rs
index 5c0cdc8..750eee7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -96,7 +96,7 @@ fn main() {
         240..346,
         ct::Attrs {
             color_opt: Some(RED),
-            family: ct::Family::Monospace,
+//            family: ct::Family::Monospace,
             ..default_attrs
         },
     );

...it seems to lay out the text without justifying it.

correct

Maybe there's a bug in the code used to switch fonts?

@notgull
Copy link
Contributor Author

notgull commented Jul 7, 2023

A bisection reveals that the issue started after e1e9fb5. It doesn't seem like this change would cause this? Maybe it just exposed a previously inaccessible bug.

Edit: For font reasons I suppose that it's worth mentioning that I'm on Ubuntu 22.04 Linux.

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