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

Skia: wrap: no-wrap (the default) still wraps #7080

Open
ogoffart opened this issue Dec 12, 2024 · 4 comments
Open

Skia: wrap: no-wrap (the default) still wraps #7080

ogoffart opened this issue Dec 12, 2024 · 4 comments
Labels
a:renderer-skia Skia Renderer (mS) bug Something isn't working
Milestone

Comments

@ogoffart
Copy link
Member

Bug Description

Skia is different than every other renderer with respect to wrapping: it always wrap while the other renderer don't wrap by default.

In the above example, the text is being wrap with skia and only skia.

Reproducible Code (if applicable)

export component X inherits Window {
    preferred-width: 300px;
    preferred-height: 300px;
    HorizontalLayout {
        padding: 30px;
        Text {
            min-width: 10px;
            min-height: 10px;
            text: "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.\nDuis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
        }
    }
}

Environment Details

  • Backend/Renderer: Skia

Product Impact

No response

@ogoffart ogoffart added bug Something isn't working a:renderer-skia Skia Renderer (mS) need triaging Issue that the owner of the area still need to triage labels Dec 12, 2024
@ogoffart
Copy link
Member Author

Maybe related: #5318

I've tried fixing this by changing line

paragraph.layout(max_width.map_or(f32::MAX, |physical_width| physical_width.get()));

to

    paragraph.layout(
        max_width
            .filter(|_| wrap != items::TextWrap::NoWrap && overflow != items::TextOverflow::Elide)
            .map_or(f32::MAX, |physical_width| physical_width.get()),
    );

Which kind of works but then it draws outside of the bounds, and I'm afraid that would break the alignment as well.

@tronical
Copy link
Member

Yes indeed, no-wrap doesn't seem to work.

@tronical
Copy link
Member

think drawing outside of the bounds is okay - at last I think we should also do that in the other renderers.

I suspect that we may have to layout twice, like this:

diff --git a/internal/renderers/skia/textlayout.rs b/internal/renderers/skia/textlayout.rs
index ce107286f..cf5d2065a 100644
--- a/internal/renderers/skia/textlayout.rs
+++ b/internal/renderers/skia/textlayout.rs
@@ -162,6 +162,16 @@ pub fn create_layout(
     let mut paragraph = builder.build();
     paragraph.layout(max_width.map_or(f32::MAX, |physical_width| physical_width.get()));

+    // If without line breaking the text would be longer than the given max_width, lay out again
+    // without line breaks.
+    if wrap == items::TextWrap::NoWrap || overflow == items::TextOverflow::Elide {
+        if let Some(physical_width) = max_width {
+            if paragraph.max_intrinsic_width() > physical_width.get() {
+                paragraph.layout(f32::MAX);
+            }
+        }
+    }
+
     let layout_height = PhysicalLength::new(paragraph.height());

     let layout_top_y = match v_align {

@ultimaweapon
Copy link

I also observed this behavior recently. What I found is although the text is wrapped with default wrap but the layout is broken. Explicitly setting it to word-wrap fix the layout issue.

@ogoffart ogoffart removed the need triaging Issue that the owner of the area still need to triage label Jan 2, 2025
@tronical tronical added this to the 1.10 milestone Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:renderer-skia Skia Renderer (mS) bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants