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

text in UI not reliably updating in a particular situation #908

Closed
DavidBJaffe opened this issue Jun 9, 2021 · 12 comments
Closed

text in UI not reliably updating in a particular situation #908

DavidBJaffe opened this issue Jun 9, 2021 · 12 comments

Comments

@DavidBJaffe
Copy link

DavidBJaffe commented Jun 9, 2021

The problem was exhibited on a MacBook Pro (16-inch, 2019) running macOS 10.15.7.
The Rust version was 1.52.0.

The problem is possibly specific to tapping on a trackpad, as opposed to actually clicking.

In the example, there is a button, initially labeled "Submit". When one taps on it, it SHOULD
change to "thinking" for three seconds. Sometimes it does not. This is particularly the case
when one first opens the app.

It appears to be possible to eliminate the buggy behavior by adding the single line

                    std::thread::sleep(std::time::Duration::from_millis(10));

after the line

                    self.compute_state = Thinking;

Removing all Event code also fixes the problem, even though the Event code should in principle not do anything different than the default behavior.

The example code can be run by:

git clone https://github.com/10XGenomics/enclone
cd enclone
git checkout 6082b5235ef0ef74f8f8063175ceb1821d0b4af7
cd bugs/button_text_update_issue
cargo b
target/debug/button_text_update_issue
Here is main.rs in the example.

|

use iced::{
    button, Application, Button, Clipboard, Column, Command, Container, Element, Length, Row,
    Settings, Subscription, Text,
};
use iced_native::{window, Event};

fn main() -> iced::Result {
    StrangeThing::run(Settings::default())
}

#[derive(PartialEq)]
enum ComputeState {
    WaitingForRequest,
    Thinking,
}

impl Default for ComputeState {
    fn default() -> ComputeState {
        WaitingForRequest
    }
}

use ComputeState::*;

#[derive(Default)]
struct StrangeThing {
    button: button::State,
    should_exit: bool,
    compute_state: ComputeState,
}

#[derive(Debug, Clone)]
enum Message {
    ButtonPressed,
    ComputationDone(Result<(), String>),
    EventOccurred(iced_native::Event),
}

impl Application for StrangeThing {
    type Executor = iced::executor::Default;
    type Message = Message;
    type Flags = ();

    fn new(_flags: ()) -> (StrangeThing, Command<Message>) {
        let mut x = StrangeThing::default();
        x.compute_state = WaitingForRequest;
        (x, Command::none())
    }

    fn title(&self) -> String {
        String::from("crazy")
    }

    fn update(&mut self, message: Message, _clipboard: &mut Clipboard) -> Command<Message> {
        match message {
            Message::ButtonPressed => {
                println!("pushed");
                if self.compute_state == WaitingForRequest {
                    self.compute_state = Thinking;
                    Command::perform(compute(), Message::ComputationDone)
                } else {
                    Command::none()
                }
            }
            Message::ComputationDone(_) => {
                self.compute_state = WaitingForRequest;
                Command::none()
            }
            Message::EventOccurred(ref event) => {
                if let Event::Window(window::Event::CloseRequested) = event {
                    self.should_exit = true;
                }
                Command::none()
            }
        }
    }

    fn should_exit(&self) -> bool {
        self.should_exit
    }

    fn subscription(&self) -> Subscription<Message> {
        iced_native::subscription::events().map(Message::EventOccurred)
    }

    fn view(&mut self) -> Element<Message> {
        let button = Button::new(
            &mut self.button,
            Text::new(if self.compute_state == WaitingForRequest {
                "Submit"
            } else {
                "thinking"
            }),
        )
        .on_press(Message::ButtonPressed);
        let content = Column::new().push(Row::new().spacing(10).push(button));
        Container::new(content)
            .width(Length::Fill)
            .height(Length::Fill)
            .center_x()
            .center_y()
            .into()
    }
}

async fn compute() -> Result<(), String> {
    std::thread::sleep(std::time::Duration::from_millis(3000));
    Ok(())
}

@hecrj
Copy link
Member

hecrj commented Jun 10, 2021

I cannot reproduce this on my end.

In any case, compute is blocking in an async context in your example. This violates the Future contract which could cause the GUI to block completely in some scenarios while the Command runs.

@hecrj hecrj added invalid This doesn't seem right question Further information is requested labels Jun 10, 2021
@DavidBJaffe
Copy link
Author

Thank you, do you see a way to fix the violation? All I want to do is have the button text change to "thinking" during the computation.

@13r0ck
Copy link
Member

13r0ck commented Jun 10, 2021

In your full application is the compute() command where the actual computation occurs?

@DavidBJaffe
Copy link
Author

Yes -- pretty much. It waits for a server to send the result of a computation.

@13r0ck
Copy link
Member

13r0ck commented Jun 10, 2021

ooh gotcha I thought your computation was done locally, building off what Hecrj said with the "compute async being blocking in your example", that is probably the issue.
If you are waiting on a server you want a subscription. Have the buttonpressed event change the computeState, then when the subscription is fired change it back, that way there is no blocking

@yusdacra
Copy link
Contributor

You can enable the tokio feature, add tokio as a dependency and use tokio::time::sleep instead of the std variant in compute.

@13r0ck
Copy link
Member

13r0ck commented Jun 10, 2021

That is a way better idea. I was not familiar with that. Thanks @yusdacra

@DavidBJaffe
Copy link
Author

Cool, thank you. If I change the sleep line to
let _ = tokio::time::sleep(std::time::Duration::from_millis(3000));
or
tokio::time::sleep(std::time::Duration::from_millis(3000)).await;
then the "thinking" message never shows up. In the real application, there is no three second sleep. There is some sleeping in talking to the server. So if there is a solution that does not involve a subscription (as I thought might have been suggested), that would be great. Otherwise, I'll try the subscription approach.

@DavidBJaffe
Copy link
Author

DavidBJaffe commented Jun 15, 2021

I'd like to address this but am not sure how to proceed. In my app there are now two instances:

  1. In one instance, pushing a button starts a computation, and the button text changes from "Submit" to "thinking" during the computation (which could take several seconds, or perhaps longer).
  2. The second instance is simpler. This is a button that says "Copy", and what it does is copy an image to the Mac clipboard. That's pretty much instantaneous, but SOMETHING has to change visibly, as otherwise the user might wonder if the copy had actually taken place. So I have the Copy text change to red for 0.4 seconds. Of course it could be anything.

So these two things are totally unrelated. Is the proper way to do this still to use a subscription?

It would be awesome to have an example that illustrated the flashing button thing (as in 2, above).

@13r0ck
Copy link
Member

13r0ck commented Jun 18, 2021

Ya for 2 you could also do it with a command that tokio::time::sleep's for 0.4 seconds.
I am not sure about your issue with #1, as I have never had an issue with the view not changing when the state does. Maybe it is just a MAC issue?
I know another person had a but with the Vulkan backend for WGPU on the Zulip chat, causing weird text rendering issues, maybe you could follow what he did. He switched to DX12, is DX12 on mac?

@DavidBJaffe
Copy link
Author

DavidBJaffe commented Jun 20, 2021

Thank you! DX12 appears to be a Windows thing. I think the goal here should be to eliminate the Future contract violation and see if that solves the problem.

@hecrj hecrj added compatibility rendering text and removed invalid This doesn't seem right question Further information is requested labels Jan 20, 2022
@hecrj
Copy link
Member

hecrj commented Jan 20, 2022

Since this seems to be most likely a hardware compatibility issue, I will close this for now.

If you are still struggling with the blocking in an async context issue, feel free to open a discussion and we will gladly help you! 😄

@hecrj hecrj closed this as completed Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants