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

DrawHandler<DrawingArea>::get_context() triggers an extra draw event, causing a feedback loop #129

Open
semitangential opened this issue Jul 29, 2018 · 5 comments

Comments

@semitangential
Copy link

Description:
Calling DrawHandler<DrawingArea>::get_context() seems to trigger an extra draw event. Using it in the event handler, as in the drawing.rs example, causes a feedback loop where each draw event triggers a new one.

An example showing the issue is given below.

Setup:

  • Use nightly Rust
  • cargo init --bin relmissue
  • Replace main.rs with the code given below.
  • Add dependencies to Cargo.toml:
[dependencies]
gtk = "0.4.1"
relm = { git = "https://github.com/antoyo/relm.git", rev = "541e1dcac24101abbae39cbaa46456ea12c0b985"}
relm-attributes = { git = "https://github.com/antoyo/relm.git", rev = "541e1dcac24101abbae39cbaa46456ea12c0b985"}
relm-derive = { git = "https://github.com/antoyo/relm.git", rev = "541e1dcac24101abbae39cbaa46456ea12c0b985"}

Running the example:

  • cargo run
    Observed behavior: "Updating UpdateDrawBuffer" is printed many times per second.
    Expected behavior: "Updating UpdateDrawBuffer" should be printed only when the widget needs to be repainted, for instance when the window is resized.

Tweaking the example:

  • Change the constant TRIGGER_INFINITE_EVENTS to false, to disable the call to DrawHandler<DrawingArea>::get_context().
  • cargo run
    Observed and expected behavior: "Updating UpdateDrawBuffer" is printed only when the widget needs to be repainted, for instance when the window is resized.

Example code (main.rs):

#![feature(use_extern_macros)]

extern crate gtk;
#[macro_use]
extern crate relm;
extern crate relm_attributes;
#[macro_use]
extern crate relm_derive;

use std::time::SystemTime;
use gtk::{
    DrawingArea,
    Inhibit,
    WidgetExt,
};
use relm::{
    DrawHandler,
    Widget,
};
use relm_attributes::widget;
use self::Msg::*;

const TRIGGER_INFINITE_EVENTS : bool = true;

pub struct Model {
    draw_handler: DrawHandler<DrawingArea>,
}

#[derive(Msg, Debug)]
pub enum Msg {
    UpdateDrawBuffer,
}

#[widget]
impl Widget for Win {
    fn init_view(&mut self) {
        self.model.draw_handler.init(&self.drawing_area);
    }

    fn model() -> Model {
        Model {
            draw_handler: DrawHandler::new().unwrap(),
        }
    }

    fn update(&mut self, event: Msg) {
        println!("{:?} Updating {:?}", SystemTime::now(), event);
        if(TRIGGER_INFINITE_EVENTS){
            let _ = self.model.draw_handler.get_context();
        }
    }

    view! {
        gtk::Window {
            #[name="drawing_area"]
            gtk::DrawingArea {
                draw(_, _) => (UpdateDrawBuffer, Inhibit(false)),
            },
        }
    }
}

fn main() {
    Win::run(()).unwrap();
}
@semitangential
Copy link
Author

Supposing that this is by design, and not a bug:

My understanding based on the drawing.rs example is that the context returned by self.model.draw_handler.get_context() is necessary for drawing, and that it has to be obtained in the event handler (as opposed to obtaining the context beforehand and reusing it). Is this correct?

Is it possible to change draw(_, _) => (UpdateDrawBuffer, Inhibit(false)) into something like draw(_, context) => (UpdateDrawBuffer(context), Inhibit(false)), to pass the context to the event handler? This would avoid calling self.model.draw_handler.get_context(), but I ran into problems with incompatible lifetimes when doing that.

@antoyo
Copy link
Owner

antoyo commented Jul 30, 2018

Don't worry, the drawing code is experimental and known to be slow (perhaps for the reasons you found out, though).
So, I'd avoid using it for now.
If you're interested in fixing it, please tell me.

@semitangential
Copy link
Author

I'd be happy to give it a try. I haven't had time to look into it yet, but I will soon.

@semitangential
Copy link
Author

The reason this is happening is the following line:

self.widget.queue_draw();

What happens is that the event handler calls get_context(), which creates a DrawContext. At the end of the event handler, the DrawContext is dropped, causing the queue_draw() call.

As far as I can tell, the order things happen in when something needs to be redrawn (either after resizing a window or after explicitly calling queue_draw()) is the following:

  1. Functions registered by calling WidgetExt::connect_draw() are called.
  2. Whatever is painted at this point is shown on-screen.
  3. The event handler Update::update() is called.
    Painting in Update::update() seems to happen too late to be shown.

One way of solving this would be to replace the call to queue_draw() in the DrawContext destructor with something that is more or less equivalent, except for triggering another call to Update::update(). That does not feel like a proper solution though, since it still involves drawing to the screen twice every time something needs to be updated.

@antoyo
Copy link
Owner

antoyo commented Jul 31, 2018

That seems to be fixed in this branch.
I don't use the draw signal anymore.
I didn't wanted to merge this branch yet, as the clip rectangles do not work.

@antoyo antoyo added this to the 1.0 milestone Nov 28, 2020
@antoyo antoyo mentioned this issue Nov 28, 2020
39 tasks
@antoyo antoyo removed this from the 1.0 milestone Nov 28, 2020
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

2 participants