nitpicker: remove periodic mode of operation

Unless nitpicker is used in 'request_framebuffer' mode, it no longer
depends on a periodic timer but merely acts as a broker between capture
clients and GUI clients. Sync signals as delivered to GUI clients are
now wired to Capture::Session::capture_at calls. So the display driver
defines the occurrence of those signals.

Note that sync signals are only delivered while a driver actively calls
'capture_at'. If a driver stops capturing, GUI clients no longer receive
any sync signal. This is a change from the previous situation where GUI
clients could depend on the periodicity of sync signals.

Issue #5347

repos/os/src/server/nitpicker/capture_session.h

@@ -32,6 +32,8 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 			 * present capture buffers.
 			 */
 			virtual void capture_buffer_size_changed() = 0;
+
+			virtual void capture_requested(Label const &) = 0;
 		};
 
 	private:
@@ -159,6 +161,8 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 
 		Affected_rects capture_at(Point pos) override
 		{
+			_handler.capture_requested(label());
+
 			if (!_buffer.constructed())
 				return Affected_rects { };
 

repos/os/src/server/nitpicker/main.cc

@@ -348,56 +348,112 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 
 	Timer::Connection _timer { _env };
 
-	Signal_handler<Main> _timer_handler = { _env.ep(), *this, &Main::_handle_period };
+	struct Ticks { uint64_t ms; };
 
-	unsigned long _timer_period_ms = 10;
+	Ticks _now() { return { .ms = _timer.curr_time().trunc_to_plain_ms().value }; }
 
-	Constructible<Framebuffer::Connection> _framebuffer { };
+	struct Input_connection : Noncopyable
-
-	struct Input_connection
 	{
-		Input::Connection connection;
+		Env  &_env;
+		Main &_main;
 
-		Attached_dataspace ev_ds;
+		Input::Connection _connection { _env };
 
-		Input_connection(Env &env)
+		Attached_dataspace _ev_ds { _env.rm(), _connection.dataspace() };
-		: connection(env), ev_ds(env.rm(), connection.dataspace()) { }
+
+		void _handle()
+		{
+			size_t const max_events = _ev_ds.size() / sizeof(Input::Event);
+
+			User_state::Input_batch const batch {
+				.events = _ev_ds.local_addr<Input::Event>(),
+				.count  = min(max_events, (size_t)_connection.flush()) };
+
+			_main.handle_input_events(batch);
+		}
+
+		Signal_handler<Input_connection> _handler {
+			_env.ep(), *this, &Input_connection::_handle };
+
+		Input_connection(Env &env, Main &main) : _env(env), _main(main)
+		{
+			_connection.sigh(_handler);
+		}
+
+		~Input_connection()
+		{
+			_connection.sigh(Signal_context_capability());
+		}
 	};
 
 	Constructible<Input_connection> _input { };
 
 	using PT = Pixel_rgb888;  /* physical pixel type */
 
-	/*
+	/**
-	 * Initialize framebuffer
+	 * Framebuffer connection used when operating in 'request_framebuffer' mode
-	 *
-	 * The framebuffer is encapsulated in a volatile object to allow its
-	 * reconstruction at runtime as a response to resolution changes.
 	 */
 	struct Framebuffer_screen
 	{
-		Framebuffer::Session &framebuffer;
+		Env  &_env;
+		Main &_main;
 
-		Framebuffer::Mode const mode = framebuffer.mode();
+		Framebuffer::Connection _fb { _env, { } };
 
-		Attached_dataspace fb_ds;
+		Framebuffer::Mode const _mode = _fb.mode();
 
-		Canvas<PT> screen = { fb_ds.local_addr<PT>(), Point(0, 0), mode.area };
+		Attached_dataspace _fb_ds { _env.rm(), _fb.dataspace() };
 
-		Area size = screen.size();
+		Canvas<PT> _screen { _fb_ds.local_addr<PT>(), Point(0, 0), _mode.area };
+
+		Area const _size = _screen.size();
 
 		using Dirty_rect = Genode::Dirty_rect<Rect, 3>;
 
-		Dirty_rect dirty_rect { };
+		Dirty_rect _dirty_rect { };
 
-		/**
+		Ticks _previous_sync { };
-		 * Constructor
+
-		 */
+		Signal_handler<Framebuffer_screen> _sync_handler {
-		Framebuffer_screen(Region_map &rm, Framebuffer::Session &fb)
+			_env.ep(), *this, &Framebuffer_screen::_handle_sync };
-		:
+
-			framebuffer(fb), fb_ds(rm, framebuffer.dataspace())
+		void _handle_sync()
 		{
-			dirty_rect.mark_as_dirty(Rect(Point(0, 0), size));
+			/* call 'Dirty_rect::flush' on a copy to preserve the state */
+			Dirty_rect dirty_rect = _dirty_rect;
+			dirty_rect.flush([&] (Rect const &rect) {
+				_main._view_stack.draw(_screen, rect); });
+
+			/* flush pixels to the framebuffer, reset dirty_rect */
+			_dirty_rect.flush([&] (Rect const &rect) {
+				_fb.refresh(rect.x1(), rect.y1(), rect.w(),  rect.h()); });
+
+			/* deliver framebuffer synchronization events */
+			for (Gui_session *s = _main._session_list.first(); s; s = s->next())
+				s->submit_sync();
+
+			_previous_sync = _main._now();
+		}
+
+		Framebuffer_screen(Env &env, Main &main) : _env(env), _main(main)
+		{
+			_fb.mode_sigh(_main._fb_screen_mode_handler);
+			_fb.sync_sigh(_sync_handler);
+			mark_as_dirty(Rect { Point { 0, 0 }, _size });
+		}
+
+		~Framebuffer_screen()
+		{
+			_fb.mode_sigh(Signal_context_capability());
+			_fb.sync_sigh(Signal_context_capability());
+		}
+
+		void mark_as_dirty(Rect rect)
+		{
+			_dirty_rect.mark_as_dirty(rect);
+
+			if (_main._now().ms - _previous_sync.ms > 40)
+				_handle_sync();
 		}
 	};
 
@@ -406,10 +462,20 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 
 	Constructible<Framebuffer_screen> _fb_screen { };
 
-	void _handle_fb_mode();
+	Signal_handler<Main> _fb_screen_mode_handler {
-	void _report_displays();
+		_env.ep(), *this, &Main::_reconstruct_fb_screen };
 
-	Signal_handler<Main> _fb_mode_handler = { _env.ep(), *this, &Main::_handle_fb_mode };
+	void _reconstruct_fb_screen()
+	{
+		_fb_screen.destruct();
+
+		if (_request_framebuffer)
+			_fb_screen.construct(_env, *this);
+
+		capture_buffer_size_changed();
+	}
+
+	void _report_displays();
 
 	/*
 	 * User-input policy
@@ -482,9 +548,8 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 	 */
 	void mark_as_damaged(Rect rect) override
 	{
-		if (_fb_screen.constructed()) {
+		if (_fb_screen.constructed())
-			_fb_screen->dirty_rect.mark_as_dirty(rect);
+			_fb_screen->mark_as_dirty(rect);
-		}
 
 		_capture_root.mark_as_damaged(rect);
 	}
@@ -492,7 +557,7 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 	void _update_input_connection()
 	{
 		bool const output_present = (_view_stack.size().count() > 0);
-		_input.conditional(_request_input && output_present, _env);
+		_input.conditional(_request_input && output_present, _env, *this);
 	}
 
 	/**
@@ -508,7 +573,7 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 		Area new_size { 0, 0 };
 
 		if (_fb_screen.constructed())
-			new_size = max_area(new_size, _fb_screen->size);
+			new_size = max_area(new_size, _fb_screen->_size);
 
 		new_size = max_area(new_size, _capture_root.bounding_box());
 
@@ -532,6 +597,16 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 		_update_input_connection();
 	}
 
+	/**
+	 * Capture_session::Handler interface
+	 */
+	void capture_requested(Capture_session::Label const &) override
+	{
+		/* deliver video-sync events */
+		for (Gui_session *s = _session_list.first(); s; s = s->next())
+			s->submit_sync();
+	}
+
 	/**
 	 * Focus_updater interface
 	 *
@@ -585,41 +660,16 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 	void _update_motion_and_focus_activity_reports();
 
 	/**
-	 * Signal handler periodically invoked for the reception of user input and redraw
+	 * Track when the user was active the last time
 	 */
-	void _handle_period();
+	Ticks _last_button_activity { },
-
+	      _last_motion_activity { };
-	Signal_handler<Main> _input_period = { _env.ep(), *this, &Main::_handle_period };
 
 	/**
-	 * Counter that is incremented periodically
+	 * Number of milliseconds since the last user interaction, after which
+	 * we regard the user as inactive
 	 */
-	unsigned _period_cnt = 0;
+	Ticks const _activity_threshold { .ms = 500 };
-
-	/**
-	 * Period counter when the user was active the last time
-	 */
-	unsigned _last_button_activity_period = 0,
-	         _last_motion_activity_period = 0;
-
-	/**
-	 * Number of periods after the last user activity when we regard the user
-	 * as becoming inactive
-	 */
-	unsigned const _activity_threshold = 50;
-
-	/**
-	 * True if the user has recently interacted with buttons or keys
-	 *
-	 * This state is reported as part of focus reports to allow the clipboard
-	 * to dynamically adjust its information-flow policy to the user activity.
-	 */
-	bool _button_activity = false;
-
-	/**
-	 * True if the user recently moved the pointer
-	 */
-	bool _motion_activity = false;
 
 	void _update_pointer_position()
 	{
@@ -636,9 +686,7 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 		_config_rom.sigh(_config_handler);
 		_handle_config();
 
-		_timer.sigh(_timer_handler);
+		_reconstruct_fb_screen();
-
-		_handle_fb_mode();
 
 		_env.parent().announce(_env.ep().manage(_gui_root));
 
@@ -648,12 +696,7 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 		if (_config_rom.xml().has_sub_node("event"))
 			_env.parent().announce(_env.ep().manage(_event_root));
 
-		/*
+		_update_motion_and_focus_activity_reports();
-		 * Detect initial motion activity such that the first hover report
-		 * contains the boot-time activity of the user in the very first
-		 * report.
-		 */
-		_handle_period();
 
 		_report_displays();
 	}
@@ -662,18 +705,13 @@ struct Nitpicker::Main : Focus_updater, Hover_updater,
 
 void Nitpicker::Main::handle_input_events(User_state::Input_batch batch)
 {
+	Ticks const now = _now();
+
 	User_state::Handle_input_result const result =
 		_user_state.handle_input_events(batch);
 
-	if (result.button_activity) {
+	if (result.button_activity) _last_button_activity = now;
-		_last_button_activity_period = _period_cnt;
+	if (result.motion_activity) _last_motion_activity = now;
-		_button_activity             = true;
-	}
-
-	if (result.motion_activity) {
-		_last_motion_activity_period = _period_cnt;
-		_motion_activity             = true;
-	}
 
 	/*
 	 * Report information about currently pressed keys whenever the key state
@@ -710,74 +748,37 @@ void Nitpicker::Main::handle_input_events(User_state::Input_batch batch)
 	/* update pointer position */
 	if (result.motion_activity)
 		_update_pointer_position();
+
+	_update_motion_and_focus_activity_reports();
 }
 
 
 void Nitpicker::Main::_update_motion_and_focus_activity_reports()
 {
-	/* flag user as inactive after activity threshold is reached */
+	Ticks const now = _now();
-	if (_period_cnt == _last_button_activity_period + _activity_threshold)
-		_button_activity = false;
 
-	if (_period_cnt == _last_motion_activity_period + _activity_threshold)
+	bool const button_activity = (now.ms - _last_button_activity.ms < _activity_threshold.ms);
-		_motion_activity = false;
+	bool const motion_activity = (now.ms - _last_motion_activity.ms < _activity_threshold.ms);
 
 	bool const hover_changed = (_reported_hover_count != _hover_count);
-	if (hover_changed || (_reported_motion_activity != _motion_activity)) {
+	if (hover_changed || (_reported_motion_activity != motion_activity)) {
-		Reporter::Xml_generator xml(_hover_reporter, [&] () {
+		Reporter::Xml_generator xml(_hover_reporter, [&] {
-			_user_state.report_hovered_view_owner(xml, _motion_activity); });
+			_user_state.report_hovered_view_owner(xml, motion_activity); });
 	}
 
 	bool const focus_changed = (_reported_focus_count != _focus_count);
-	if (focus_changed || (_reported_button_activity != _button_activity)) {
+	if (focus_changed || (_reported_button_activity != button_activity)) {
-		Reporter::Xml_generator xml(_focus_reporter, [&] () {
+		Reporter::Xml_generator xml(_focus_reporter, [&] {
-			_user_state.report_focused_view_owner(xml, _button_activity); });
+			_user_state.report_focused_view_owner(xml, button_activity); });
 	}
 
-	_reported_motion_activity = _motion_activity;
+	_reported_motion_activity = motion_activity;
-	_reported_button_activity = _button_activity;
+	_reported_button_activity = button_activity;
 	_reported_hover_count     = _hover_count;
 	_reported_focus_count     = _focus_count;
 }
 
 
-void Nitpicker::Main::_handle_period()
-{
-	_period_cnt++;
-
-	/* handle batch of pending events */
-	if (_input.constructed()) {
-
-		size_t const max_events = _input->ev_ds.size() / sizeof(Input::Event);
-
-		User_state::Input_batch const batch {
-			.events = _input->ev_ds.local_addr<Input::Event>(),
-			.count  = min(max_events, (size_t)_input->connection.flush()) };
-
-		handle_input_events(batch);
-	}
-
-	_update_motion_and_focus_activity_reports();
-
-	/* perform redraw */
-	if (_framebuffer.constructed() && _fb_screen.constructed()) {
-		/* call 'Dirty_rect::flush' on a copy to preserve the state */
-		Dirty_rect dirty_rect = _fb_screen->dirty_rect;
-		dirty_rect.flush([&] (Rect const &rect) {
-			_view_stack.draw(_fb_screen->screen, rect); });
-
-		/* flush pixels to the framebuffer, reset dirty_rect */
-		_fb_screen->dirty_rect.flush([&] (Rect const &rect) {
-			_framebuffer->refresh(rect.x1(), rect.y1(),
-			                      rect.w(),  rect.h()); });
-	}
-
-	/* deliver framebuffer synchronization events */
-	for (Gui_session *s = _session_list.first(); s; s = s->next())
-		s->submit_sync();
-}
-
-
 /**
  * Helper function for 'handle_config'
  */
@@ -880,13 +881,14 @@ void Nitpicker::Main::_handle_config()
 
 	/* update focus report since the domain colors might have changed */
 	Reporter::Xml_generator xml(_focus_reporter, [&] () {
-		_user_state.report_focused_view_owner(xml, _button_activity); });
+		bool const button_activity = (_now().ms - _last_button_activity.ms < _activity_threshold.ms);
+		_user_state.report_focused_view_owner(xml, button_activity); });
 
 	/* update framebuffer output back end */
 	bool const request_framebuffer = config.attribute_value("request_framebuffer", false);
 	if (request_framebuffer != _request_framebuffer) {
 		_request_framebuffer = request_framebuffer;
-		_handle_fb_mode();
+		_reconstruct_fb_screen();
 	}
 
 	/*
@@ -908,8 +910,8 @@ void Nitpicker::Main::_report_displays()
 	Reporter::Xml_generator xml(_displays_reporter, [&] () {
 		if (_fb_screen.constructed()) {
 			xml.node("display", [&] () {
-				xml.attribute("width",  _fb_screen->size.w);
+				xml.attribute("width",  _fb_screen->_size.w);
-				xml.attribute("height", _fb_screen->size.h);
+				xml.attribute("height", _fb_screen->_size.h);
 			});
 		}
 
@@ -918,32 +920,6 @@ void Nitpicker::Main::_report_displays()
 }
 
 
-void Nitpicker::Main::_handle_fb_mode()
-{
-	if (_request_framebuffer && !_framebuffer.constructed()) {
-		_framebuffer.construct(_env, Framebuffer::Mode{});
-		_framebuffer->mode_sigh(_fb_mode_handler);
-		_framebuffer->sync_sigh(_timer_handler);
-		_timer.trigger_periodic(0);
-	}
-
-	/* reconstruct '_fb_screen' with updated mode */
-	if (_request_framebuffer && _framebuffer.constructed())
-		_fb_screen.construct(_env.rm(), *_framebuffer);
-
-	if (!_request_framebuffer && _fb_screen.constructed())
-		_fb_screen.destruct();
-
-	if (!_request_framebuffer && _framebuffer.constructed())
-		_framebuffer.destruct();
-
-	if (!_request_framebuffer)
-		_timer.trigger_periodic(_timer_period_ms*1000);
-
-	capture_buffer_size_changed();
-}
-
-
 void Component::construct(Genode::Env &env)
 {
 	static Nitpicker::Main nitpicker(env);