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

A Label with too much text (font and font size dependent) causes wgpu panic #415

Closed
m-hugo opened this issue Oct 18, 2023 · 10 comments · Fixed by #417
Closed

A Label with too much text (font and font size dependent) causes wgpu panic #415

m-hugo opened this issue Oct 18, 2023 · 10 comments · Fixed by #417

Comments

@m-hugo
Copy link

m-hugo commented Oct 18, 2023

Displaying a big string in a scrolling view is a common use case but ScrollLabel::new(LOTS_of_TEXT) panics

error (on master but reproduces on alpha and 0.13):

thread 'main' panicked at 'Error in Surface::configure: Validation Error

Caused by:
    Not enough memory left
', /.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.17.1/src/backend/direct.rs:771:18

code

fn main() -> kas::shell::Result<()> {
    let theme = kas::theme::FlatTheme::new().with_font_size(15.0);
    kas::shell::DefaultShell::new((), theme)?
        .with(Window::new(ScrollLabel::new(LOTS_of_TEXT), "MCVE"))
        .run()
}

Scroll<Label> and just Label behave identically

for LOTS_of_TEXT you can use for example

r##"source html from https://github.com/kas-gui/kas"##

which displays at .with_font_size(1.0) but crashes at 2.0, an ebook chapter crashed at 14.0

@dhardy
Copy link
Collaborator

dhardy commented Oct 19, 2023

I tried pasting a few markdown docs into the text editor of examples/gallery.rs and didn't see this error.

Not enough memory left

This could be system-specific — one of the big issues with using hardware-accelerated rendering. I tried measuring usage locally and saw maybe 60-200MB (on top of approx. 2.4GB just from desktop apps).

Can you check your video RAM size and usage?

There should be a software-rendering fallback, but there isn't yet: #287

@m-hugo
Copy link
Author

m-hugo commented Oct 19, 2023

radeon 6700xt / linux / wayland

glxinfo | grep "video memory"
    Dedicated video memory: 12288 MB
    Currently available dedicated video memory: 11740 MB

I sure hope 12gb is enough... view-source:https://github.com/kas-gui/kas on firefox eats 20mb at most

@dhardy
Copy link
Collaborator

dhardy commented Oct 19, 2023

That's basically what I have (but only 8GB version), so probably something else...

I reproduced with that document. It turns out to only be one long (>16k) line that's responsible. Then I see this in the log:

[2023-10-19T13:29:37Z DEBUG kas_core::layout::sizer] find_constraints: min=Size(140, 107), ideal=Size(228, 10939), margins=Margins { horiz: (0, 0), vert: (0, 0) }
[2023-10-19T13:29:37Z INFO  kas_core::shell::window] new: constructed with physical size Size(342, 16409), scale factor 1.5

That's a stupidly big window, but the buffer should only be around 22MB. It might exceed the maximum texture size though.


The winit window builder:

Window builder: WindowBuilder { window: WindowAttributes { inner_size: Some(Logical(LogicalSize { width: 228.0, height: 10939.0 })), min_inner_size: Some(Logical(LogicalSize { width: 140.0, height: 107.0 })), max_inner_size: None, position: None, resizable: true, enabled_buttons: WindowButtons(CLOSE | MINIMIZE | MAXIMIZE), title: "winit window", fullscreen: None, maximized: false, visible: true, transparent: false, decorations: true, window_icon: None, preferred_theme: None, resize_increments: None, content_protected: false, window_level: Normal, parent_window: None, active: true } }

Overriding the size like this (window.rs:123) solves the issue:

builder = builder.with_inner_size(winit::dpi::LogicalSize { width: 2000, height: 1000 });

Looks like I shouldn't rely on winit to enforce a reasonable maximum window size. On the other hand, what reasonable value is there other than the display size? And given that I must supply a window size in logical pixels while the monitor sizes are only available in physical pixels and I also don't know which monitor the window will appear on, it's not obvious how to solve this.

@dhardy
Copy link
Collaborator

dhardy commented Oct 19, 2023

Note further: the crash isn't actually due to the window being constructed, but the GPU buffer. However, enforcing the window size is still probably the most sensible option.

@m-hugo
Copy link
Author

m-hugo commented Oct 19, 2023

your window creation logic is very weird but this should make sure the windows's not too big while still using your weird constraints

        // Opening a zero-size window causes a crash, so force at least 1x1:
        let ideal = solve_cache.ideal(true).max(Size(1, 1));
        let ideal = match use_logical_size {
            false => ideal.as_physical(),
            true => ideal.as_logical(),
        };
        
        let mut builder = WindowBuilder::new();

        let window = builder
            .with_maximized(true)
            .with_title(self.widget.title())
            .with_window_icon(self.widget.icon())
            .with_decorations(self.widget.decorations() == kas::Decorations::Server)
            .with_transparent(self.widget.transparent())
            .build(elwt)?;

        let (restrict_min, restrict_max) = self.widget.restrictions();
        if restrict_max {
            window.set_max_inner_size(Some(ideal));
        }
        if restrict_min {
            let min = solve_cache.min(true);
            let min = match use_logical_size {
                false => min.as_physical(),
                true => min.as_logical(),
            };
            window.set_min_inner_size(Some(min));
        }

        let scale_factor = window.scale_factor();
        shared.scale_factor = scale_factor;
        let size: Size = window.inner_size().cast();
        log::info!(
            "new: constructed with physical size {:?}, scale factor {}",
            size,
            scale_factor
        );

@m-hugo
Copy link
Author

m-hugo commented Oct 19, 2023

the crashes are fixed but there's a race condition, if you scroll the ScrollLabel and after you jiggle the mouse near/over any window border the view will snap to a different position, it doesn't happen if you don't jiggle

@m-hugo
Copy link
Author

m-hugo commented Oct 19, 2023

also, I want my view to display another string when I turn a page from my book so I replaced ScrollLabel with

ScrollRegion::new(kas::widgets::Text::new(Book::display))
//and later
.on_message(|_, book, SwitchToPage(page_number)| book.show_page(page_number));

but this view has the worst scrolling i've ever seen and the cursor scrolls instead of selecting (grab-scrolling has much less lag than wheel scrolling for some reason)

how do I make something that works like an interactive ScrollLabel?

@dhardy
Copy link
Collaborator

dhardy commented Oct 19, 2023

your window creation logic is very weird but this should make sure the windows's not too big while still using your weird constraints

Sure, things are much simpler when window size is fixed as in many systems.

Your logic "works" because you don't set the window's inner_size. I noticed that Winit does not respect the maximum (if inner_size > max_inner_size). I would prefer a different approach: limit ideal to some sensible maximum size (perhaps the size of the largest monitor). Also setting with_maximized when the ideal size exceeds or equals the monitor size sounds good. I'll do this later (already have a partial PR waiting on a new WGPU release).


the crashes are fixed but there's a race condition, if you scroll the ScrollLabel and after you jiggle the mouse near/over any window border the view will snap to a different position, it doesn't happen if you don't jiggle

I don't follow. Without --release scrolling is horribly laggy with a large text which definitely doesn't help.

Are you using drag-scrolling? There is momentum (flick) scrolling support and I have found that doesn't always stop moving correctly (I think the only issue is usually queuing extra wake-ups despite having reached the end of the scroll range).

but this view has the worst scrolling i've ever seen and the cursor scrolls instead of selecting (grab-scrolling has much less lag than wheel scrolling for some reason)

The easy option is static pages: Stack<ScrollRegion<Label>> / Stack<ScrollLabel>. Even if you have a lot of pages it should probably be okay if you use Stack::with_size_limit to avoid trying to size-check everything on start.

It looks like you are trying a harder option: re-assigning a single Text widget for each page. In that case you might need hacks to reset the scroll position when turning a page (EventCx::set_scroll(Scroll::Rect(..)) should enable this), and I have no idea what other issues you might find.

ListView is essentially an abstaction layer for this type of thing (expect for a row/column, not a stack), and re-assigning widgets to a different "task" while making the interface look sane is hard. E.g. if you support text-selection, what happens to the selection when you turn a page? ListView uses a cache of backing widgets and tries to limit re-assignments, but when a widget does get re-assigned then its selection will get dropped.

how do I make something that works like an interactive ScrollLabel?

Check the code. It's not as bad as EditField but still one of the more complex widgets. Possibly another level of abstraction could help, but it turns out that complex behaviour does require quite a bit of code...

@m-hugo
Copy link
Author

m-hugo commented Oct 19, 2023

the crashes are fixed but there's a race condition, if you scroll the ScrollLabel and after you jiggle the mouse near/over any window border the view will snap to a different position, it doesn't happen if you don't jiggle

I don't follow. Without --release scrolling is horribly laggy with a large text which definitely doesn't help.

Are you using drag-scrolling? There is momentum (flick) scrolling support and I have found that doesn't always stop moving correctly (I think the only issue is usually queuing extra wake-ups despite having reached the end of the scroll range).

I always --release, the problem for scrolling (with regular mousewheel) ScrollLabel is not scrolling or stopping, it's that once stopped, it snaps to another position if the mouse cursor has left and reentered the window between the last scrollevent and when the view stops moving

but this view has the worst scrolling i've ever seen and the cursor scrolls instead of selecting (grab-scrolling has much less lag than wheel scrolling for some reason)

The easy option is static pages: Stack<ScrollRegion<Label>> / Stack<ScrollLabel>. Even if you have a lot of pages it should probably be okay if you use Stack::with_size_limit to avoid trying to size-check everything on start.

Not an option, loading all the text of all the pages at startup (like in a Vec<String>) is too slow, I know the number of pages and the content of the first page to open at most before running the shell

It looks like you are trying a harder option: re-assigning a single Text widget for each page.

basically I use Text::new(Book::display) as a Label that recreates itself by calling Book::display on Book when Book is turned

E.g. if you support text-selection, what happens to the selection when you turn a page? ListView uses a cache of backing widgets and tries to limit re-assignments, but when a widget does get re-assigned then its selection will get dropped.

when an open page closes the selection can be dropped along with the content of the page

how do I make something that works like an interactive ScrollLabel?

Check the code. It's not as bad as EditField but still one of the more complex widgets. Possibly another level of abstraction could help, but it turns out that complex behaviour does require quite a bit of code...

kas-widgets/src/scroll_label.rs looks like black magic to me

@dhardy
Copy link
Collaborator

dhardy commented Oct 19, 2023

ScrollLabel is not scrolling or stopping, it's that once stopped, it snaps to another position if the mouse cursor has left and reentered the window between the last scrollevent and when the view stops moving

I didn't observe this — how can I reproduce?

Not an option, loading all the text of all the pages at startup (like in a Vec) is too slow, I know the number of pages and the content of the first page to open at most before running the shell

It is also possible to add new widgets to a Stack / List etc. at run-time. None of the examples do this, but it might work for your case. (Possibly also remove old pages once you hit some limit.)

kas-widgets/src/scroll_label.rs looks like black magic to me

Lol. Kind of true, if you haven't looked into how the impl_scope! and #[widget] macros work.

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 a pull request may close this issue.

2 participants