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

wgpu surface unnecessasrily recreated every frame #117

Open
nebneb0703 opened this issue Jan 15, 2025 · 7 comments
Open

wgpu surface unnecessasrily recreated every frame #117

nebneb0703 opened this issue Jan 15, 2025 · 7 comments

Comments

@nebneb0703
Copy link

iced_layershell performs a wgpu surface reconfigure event at the start of every frame, even if the surface size has not changed:

IcedLayerEvent::RequestRefresh {
width,
height,
fractal_scale,
} => {
state.update_view_port(width, height, fractal_scale);
let logical_size = state.logical_size();
debug.layout_started();
user_interface = ManuallyDrop::new(
ManuallyDrop::into_inner(user_interface).relayout(logical_size, &mut renderer),
);
debug.layout_finished();
let physical_size = state.physical_size();
compositor.configure_surface(
&mut surface,
physical_size.width,
physical_size.height,
);

this often takes up a whole frame to complete (vsync?), and corresponds to unnecessary CPU time

image

i tried a naive fix to avoid the code path, but it broke the actual rendering, which randomly took up to several frames to present the surface, hence i am not submitting a PR.

image

i used samply to obtain measurements and profiling. it's possible this process may be introducing bias i am unaware of.

@Decodetalkers
Copy link
Collaborator

can you confirm if it is fixed in the new commit? I think I have fixed it

@nebneb0703
Copy link
Author

it now draws much less often, but still reconfigures the surface for every drawn frame.

additionally, on-hover events are now broken, as the iced ui only receives updates on click or from subscribers

@Decodetalkers
Copy link
Collaborator

it now draws much less often, but still reconfigures the surface for every drawn frame.

additionally, on-hover events are now broken, as the iced ui only receives updates on click or from subscribers

what do you mean by on-hover events are now broken.. seems it all fine on my example.. The redraw only will happen if there is new message or ui_state is changed. it should not be every drawn frame. or you are using backend mode?

@nebneb0703
Copy link
Author

The counter example has a subscription for all iced/window events, which run in a separate thread:

fn subscription(&self) -> iced::Subscription<Self::Message> {
event::listen().map(Message::IcedEvent)
}

This includes mouse movement events.

These are only used to debug log:

Message::IcedEvent(event) => {
println!("hello {event:?}");
Command::none()
}

Removing the subscription demonstrates the hover issue, which only appears after the latest commit

@nebneb0703
Copy link
Author

nebneb0703 commented Jan 16, 2025

This behaviour can be considered an iced bug. In the latest release of iced, button status is checked in draw not update:

https://github.com/iced-rs/iced/blob/9bfbd7cda79aceef2d115b8bb35e8f3257dcabf2/widget/src/button.rs#L356-L368

This is fixed in PR iced-rs/iced#2662, where it moves to update:

https://github.com/iced-rs/iced/blob/28ec6df8f0ebf96966bee61caf5a325695314b7a/widget/src/button.rs#L343-L355

Edit: Prior to the merging of iced-rs/iced#2662, it may be required by iced to draw every frame unconditionally, hence the fix to only draw when the ui state has changed is invalid for earlier versions of iced.

@Decodetalkers
Copy link
Collaborator

This behaviour can be considered an iced bug. In the latest release of iced, button status is checked in draw not update:

https://github.com/iced-rs/iced/blob/9bfbd7cda79aceef2d115b8bb35e8f3257dcabf2/widget/src/button.rs#L356-L368

This is fixed in PR iced-rs/iced#2662, where it moves to update:

https://github.com/iced-rs/iced/blob/28ec6df8f0ebf96966bee61caf5a325695314b7a/widget/src/button.rs#L343-L355

Edit: Prior to the merging of iced-rs/iced#2662, it may be required by iced to draw every frame unconditionally, hence the fix to only draw when the ui state has changed is invalid for earlier versions of iced.

I see.. maybe I should revert this fix.. it induced many problems

@Decodetalkers
Copy link
Collaborator

I have read the code in iced.. maybe I should wait for new iced being published, then solve this problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants