Skip to content

Implement tinyskia text bounds #2929

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

DKolter
Copy link
Contributor

@DKolter DKolter commented May 4, 2025

Fixes #2740

Paragraph clip bounds are currently marked todo in the tiny skia backend. This PR makes an effort to change this by:

  1. Finding the intersection between the clip_bounds argument (layer bounds) and the clip_bounds of the paragraph
  2. Adjusting the clip mask to the intersection bounds if the physical bounds extend past the clip_bounds, otherwise don't update the clip mask to avoid expensive clip mask drawing.

The original issue only mentions text_input, however text_editor and cached text have the same todo comment and are also handled by this PR.

Before

grafik

After

grafik

@lufte
Copy link
Contributor

lufte commented May 15, 2025

This looks like it also fixes #2851 <3

@lufte
Copy link
Contributor

lufte commented May 15, 2025

After more testing, though, I found a bug which is hard to explain. You can reproduce it by cloning my app from this repo https://git.sr.ht/~lufte/vimini/tree/tiny-skia-clipping (make sure you're on the tiny-skia-clipping branch), running it with tiny-skia, and scrolling down on the default page that opens up. You should notice an increasingly bigger rectangle, same color as the background, that is drawn on top of the text at the top. The tabs in the navigation bar at the top are using the new feature.

Let me know if it's hard to reproduce and I can provide assistance.

@DKolter
Copy link
Contributor Author

DKolter commented May 25, 2025

Hey @lufte, I had issues reproducing the visual bug. Do you think you could try to reproduce it based on this branch?
If not, a short video would also be a great start. Thank you for checking the PR out!

@lufte
Copy link
Contributor

lufte commented May 25, 2025

@DKolter I was able to write a short program that shows the behavior, code below. Just run it with tiny-skia and scroll down, you should see how less and less text is shown.

main.rs

#[derive(Default)]
pub struct App {
}

#[derive(Debug)]
pub enum Message {
}

impl App {
    fn update(&mut self, message: Message) -> iced::Task<Message> {
        let mut command = iced::Task::none();
        return command;
    }

    fn view(&self) -> iced::Element<Message> {
        let mut tab_bar = iced::widget::Row::new().width(iced::Length::Fill);
        tab_bar = tab_bar.push(
            iced::widget::Container::new(
                iced::widget::Text::new("Tab 1")
                .wrapping(iced::widget::text::Wrapping::None)
            )
            .width(iced::Length::Fill)
            .padding(2)
            .clip(true)
        );
        tab_bar = tab_bar.push(
            iced::widget::Container::new(
                iced::widget::Text::new("Tab 2")
                .wrapping(iced::widget::text::Wrapping::None)
            )
            .width(iced::Length::Fill)
            .padding(2)
            .clip(true)
        );
        let mut tabbed_browser = iced::widget::Column::new();
        tabbed_browser = tabbed_browser.push(tab_bar);
        let mut text_column = iced::widget::Column::new();

        for _ in [0; 100] {
            text_column = text_column.push(
                iced::widget::Text::new("A cross-platform GUI library for Rust focused on simplicity and type-safety. Inspired by Elm.")
                .size(17)
                .shaping(iced::advanced::text::Shaping::Advanced)
            );
        }
        let document_viewer = iced::widget::Container::new(
            iced::widget::scrollable::Scrollable::new(
                iced::widget::Container::new(
                    text_column
                    .max_width(800)
                    .width(iced::Length::Fill)
                    .padding(10)
                )
                .center_x(iced::Length::Fill),
            )
        )
        .height(iced::Length::Fill);
        tabbed_browser = tabbed_browser.push(document_viewer);
        return iced::widget::Container::new(tabbed_browser)
        .width(iced::Length::Fill)
        .height(iced::Length::Fill)
        .into();
    }
}

pub fn main() -> iced::Result {
    return iced::run(App::update, App::view);
}

Cargo.toml

[package]
name = "tiny-skia-bug"

[dependencies]
iced = { git = "https://github.com/DKolter/iced/", branch = "issue_2740", features=["advanced"] }

@DKolter
Copy link
Contributor Author

DKolter commented May 26, 2025

Good catch, thank you @lufte!
The issue was not applying the transformation to the clip bounds as well. The scrollable widget modified this transformation, which caused problems with the text clipping.

Shorter snippet to reproduce:

use iced::{
    Element, Length,
    widget::{column, scrollable, text},
};

#[derive(Default)]
pub struct App {}

#[derive(Debug)]
pub enum Message {}

impl App {
    fn update(&mut self, _message: ()) {}

    fn view(&self) -> Element<()> {
        scrollable(column((0..100).map(|i| text!("Item {i}").into())))
            .width(Length::Fill)
            .height(Length::Fill)
            .into()
    }
}

pub fn main() -> iced::Result {
    return iced::run(App::update, App::view);
}

@lufte
Copy link
Contributor

lufte commented May 26, 2025

Glad to be of help @DKolter. There is another bug, related with scrolling and tiny-skia but not with this patch here, for which you might be of help. If you're interested I'll try to come up with a minimal example for that too and get in touch with you.

@DKolter
Copy link
Contributor Author

DKolter commented May 26, 2025

I would be happy to take a look at that issue too.

This current branch performs much worse than master for many paragraphs, because there is a bug in the core Rectangle::is_within function. It is implemented on top of the Rectangle::contains functionality, which was changed almost 2 years ago, excluding the bottom right corner here: f5b9562

This leads to clip masks being rerendered without needing to if text bounds and clip bounds are equal sized on any axis.

Should I open a separate issue for this or fix it in this PR?

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

Successfully merging this pull request may close these issues.

[tiny-skia] does not truncate long text inside of a TextInput when the window size is changed
2 participants