From c502e1d09538af7ea19427cceaf1705e47863bd8 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Wed, 15 Jul 2020 19:50:42 +0200 Subject: [PATCH] input_filter: prepare for refactorization This patch brings the input filter into a shape that is easier to re-mold into an event filter, reversing the client/server roles of the component. * The 'Sink &destination' is no longer passed as constructor argument to the individual filters but passed as argument to the 'generate' method. This way, the final destination does not need to exist at the construction time of the filter chain but can be created on the fly (clearing the way for using 'Event::Client::with_batch'). * A new 'Source::Filter' interface with the 'apply' method aids the cascading of filters during 'generate'. The modules now implement the 'Source::Filter::filter_event' interface instead of the 'Source::Sink::submit_event' interface. * Since the 'Sink &destination' is no longer a member of the filter modules, character-repeat events can no longer be emitted in an ad-hoc way. Instead, the character-repeat mechanism now invokes a new 'Trigger::trigger_generate' hook that prompts the execution of the regular 'generate' mechanism by the main program. This patch is supposed to leave the semantics of the input filter unchanged (validated by the input_filter.run script). Issue #3827 --- .../server/input_filter/accelerate_source.h | 21 +++--- .../input_filter/button_scroll_source.h | 27 ++++---- .../src/server/input_filter/chargen_source.h | 68 ++++++++++++------- .../os/src/server/input_filter/input_source.h | 9 ++- repos/os/src/server/input_filter/main.cc | 41 +++++++---- .../os/src/server/input_filter/merge_source.h | 10 +-- .../os/src/server/input_filter/remap_source.h | 29 ++++---- repos/os/src/server/input_filter/source.h | 40 ++++++++++- 8 files changed, 153 insertions(+), 92 deletions(-) diff --git a/repos/os/src/server/input_filter/accelerate_source.h b/repos/os/src/server/input_filter/accelerate_source.h index cc00215354..6226193539 100644 --- a/repos/os/src/server/input_filter/accelerate_source.h +++ b/repos/os/src/server/input_filter/accelerate_source.h @@ -25,7 +25,7 @@ namespace Input_filter { class Accelerate_source; } -class Input_filter::Accelerate_source : public Source, Source::Sink +class Input_filter::Accelerate_source : public Source, Source::Filter { private: @@ -33,8 +33,6 @@ class Input_filter::Accelerate_source : public Source, Source::Sink Source &_source; - Source::Sink &_destination; - /** * Look-up table used for the non-linear acceleration of motion values */ @@ -84,9 +82,9 @@ class Input_filter::Accelerate_source : public Source, Source::Sink } /** - * Sink interface + * Filter interface */ - void submit_event(Input::Event const &event) override + void filter_event(Sink &destination, Input::Event const &event) override { using namespace Input; @@ -95,26 +93,27 @@ class Input_filter::Accelerate_source : public Source, Source::Sink ev.handle_relative_motion([&] (int x, int y) { ev = Relative_motion{_apply_acceleration(x), _apply_acceleration(y)}; }); - _destination.submit_event(ev); + destination.submit_event(ev); } public: static char const *name() { return "accelerate"; } - Accelerate_source(Owner &owner, Xml_node config, Source::Sink &destination, - Source::Factory &factory) + Accelerate_source(Owner &owner, Xml_node config, Source::Factory &factory) : Source(owner), _owner(factory), - _source(factory.create_source(_owner, input_sub_node(config), *this)), - _destination(destination), + _source(factory.create_source(_owner, input_sub_node(config))), _lut (config.attribute_value("curve", 127L)), _sensitivity_percent(config.attribute_value("sensitivity_percent", 100L)), _max (config.attribute_value("max", 20L)) { } - void generate() override { _source.generate(); } + void generate(Sink &destination) override + { + Source::Filter::apply(destination, *this, _source); + } }; #endif /* _INPUT_FILTER__ACCELERATE_SOURCE_H_ */ diff --git a/repos/os/src/server/input_filter/button_scroll_source.h b/repos/os/src/server/input_filter/button_scroll_source.h index dd0f4b54da..afd30d7130 100644 --- a/repos/os/src/server/input_filter/button_scroll_source.h +++ b/repos/os/src/server/input_filter/button_scroll_source.h @@ -24,7 +24,7 @@ namespace Input_filter { class Button_scroll_source; } -class Input_filter::Button_scroll_source : public Source, Source::Sink +class Input_filter::Button_scroll_source : public Source, Source::Filter { private: @@ -147,12 +147,10 @@ class Input_filter::Button_scroll_source : public Source, Source::Sink Source &_source; - Source::Sink &_destination; - /** - * Sink interface + * Filter interface */ - void submit_event(Input::Event const &event) override + void filter_event(Sink &destination, Input::Event const &event) override { using Input::Event; @@ -169,7 +167,7 @@ class Input_filter::Button_scroll_source : public Source, Source::Sink wheel_y = _vertical_wheel .pending_motion(); if (wheel_x || wheel_y) - _destination.submit_event(Input::Wheel{wheel_x, wheel_y}); + destination.submit_event(Input::Wheel{wheel_x, wheel_y}); /* * Submit both press event and release event of magic button at @@ -187,8 +185,8 @@ class Input_filter::Button_scroll_source : public Source, Source::Sink if (_vertical_wheel .handle_deactivation(event) | _horizontal_wheel.handle_deactivation(event)) { - _destination.submit_event(Input::Press{key}); - _destination.submit_event(Input::Release{key}); + destination.submit_event(Input::Press{key}); + destination.submit_event(Input::Release{key}); } }); return; @@ -198,7 +196,7 @@ class Input_filter::Button_scroll_source : public Source, Source::Sink if (_vertical_wheel .suppressed(event)) return; if (_horizontal_wheel.suppressed(event)) return; - _destination.submit_event(event); + destination.submit_event(event); } static Xml_node _sub_node(Xml_node node, char const *type) @@ -211,18 +209,19 @@ class Input_filter::Button_scroll_source : public Source, Source::Sink static char const *name() { return "button-scroll"; } - Button_scroll_source(Owner &owner, Xml_node config, Source::Sink &destination, - Source::Factory &factory) + Button_scroll_source(Owner &owner, Xml_node config, Source::Factory &factory) : Source(owner), _vertical_wheel (_sub_node(config, "vertical")), _horizontal_wheel(_sub_node(config, "horizontal")), _owner(factory), - _source(factory.create_source(_owner, input_sub_node(config), *this)), - _destination(destination) + _source(factory.create_source(_owner, input_sub_node(config))) { } - void generate() override { _source.generate(); } + void generate(Sink &destination) override + { + Source::Filter::apply(destination, *this, _source); + } }; #endif /* _INPUT_FILTER__BUTTON_SCROLL_SOURCE_H_ */ diff --git a/repos/os/src/server/input_filter/chargen_source.h b/repos/os/src/server/input_filter/chargen_source.h index 06fc657c25..5bc665555b 100644 --- a/repos/os/src/server/input_filter/chargen_source.h +++ b/repos/os/src/server/input_filter/chargen_source.h @@ -25,7 +25,7 @@ namespace Input_filter { class Chargen_source; } -class Input_filter::Chargen_source : public Source, Source::Sink +class Input_filter::Chargen_source : public Source, Source::Filter { private: @@ -514,19 +514,19 @@ class Input_filter::Chargen_source : public Source, Source::Sink Owner _owner; - Source::Sink &_destination; - /** * Mechanism for periodically repeating the last character */ struct Char_repeater { - Source::Sink &_destination; Timer::Connection &_timer; + Source::Trigger &_trigger; Microseconds const _delay; Microseconds const _rate; + unsigned _pending_event_count = 0; + Codepoint _curr_character { Codepoint::INVALID }; enum State { IDLE, REPEAT } _state { IDLE }; @@ -534,45 +534,57 @@ class Input_filter::Chargen_source : public Source, Source::Sink void _handle_timeout(Duration) { if (_state == REPEAT) { - _destination.submit_event(Input::Press_char{Input::KEY_UNKNOWN, - _curr_character}); - _destination.submit_event(Input::Release{Input::KEY_UNKNOWN}); + _pending_event_count++; _timeout.schedule(_rate); } + + _trigger.trigger_generate(); + } + + void emit_events(Source::Sink &destination) + { + for (unsigned i = 0; i < _pending_event_count; i++) { + destination.submit_event(Input::Press_char{Input::KEY_UNKNOWN, + _curr_character}); + destination.submit_event(Input::Release{Input::KEY_UNKNOWN}); + } + + _pending_event_count = 0; } Timer::One_shot_timeout _timeout { _timer, *this, &Char_repeater::_handle_timeout }; - Char_repeater(Source::Sink &destination, Timer::Connection &timer, - Xml_node node) + Char_repeater(Timer::Connection &timer, Xml_node node, Source::Trigger &trigger) : - _destination(destination), _timer(timer), + _timer(timer), _trigger(trigger), _delay(node.attribute_value("delay_ms", (uint64_t)0)*1000), _rate (node.attribute_value("rate_ms", (uint64_t)0)*1000) { } void schedule_repeat(Codepoint character) { - _curr_character = character; - _state = REPEAT; + _curr_character = character; + _state = REPEAT; + _pending_event_count = 0; _timeout.schedule(_delay); } void cancel() { - _curr_character = Codepoint { Codepoint::INVALID }; - _state = IDLE; + _curr_character = Codepoint { Codepoint::INVALID }; + _state = IDLE; + _pending_event_count = 0; } }; Constructible _char_repeater { }; /** - * Sink interface (called from our child node) + * Filter interface */ - void submit_event(Event const &event) override + void filter_event(Sink &destination, Event const &event) override { Event ev = event; @@ -607,11 +619,13 @@ class Input_filter::Chargen_source : public Source, Source::Sink }); /* forward filtered event */ - _destination.submit_event(ev); + destination.submit_event(ev); } Source &_source; + Source::Trigger &_trigger; + void _apply_config(Xml_node const config, unsigned const max_recursion = 4) { config.for_each_sub_node([&] (Xml_node node) { @@ -669,8 +683,7 @@ class Input_filter::Chargen_source : public Source, Source::Sink * Instantiate character repeater on demand */ if (node.type() == "repeat") { - _char_repeater.construct(_destination, - _timer_accessor.timer(), node); + _char_repeater.construct(_timer_accessor.timer(), node, _trigger); return; } @@ -706,11 +719,11 @@ class Input_filter::Chargen_source : public Source, Source::Sink Chargen_source(Owner &owner, Xml_node config, - Source::Sink &destination, Source::Factory &factory, Allocator &alloc, Timer_accessor &timer_accessor, - Include_accessor &include_accessor) + Include_accessor &include_accessor, + Source::Trigger &trigger) : Source(owner), _alloc(alloc), @@ -719,8 +732,8 @@ class Input_filter::Chargen_source : public Source, Source::Sink _key_map(_alloc), _sequencer(_alloc), _owner(factory), - _destination(destination), - _source(factory.create_source(_owner, input_sub_node(config), *this)) + _source(factory.create_source(_owner, input_sub_node(config))), + _trigger(trigger) { _apply_config(config); @@ -738,7 +751,14 @@ class Input_filter::Chargen_source : public Source, Source::Sink destroy(_alloc, &mod_rom); }); } - void generate() override { _source.generate(); } + void generate(Sink &destination) override + { + if (_char_repeater.constructed()) + _char_repeater->emit_events(destination); + + Source::Filter::apply(destination, *this, _source); + + } }; #endif /* _INPUT_FILTER__CHARGEN_SOURCE_H_ */ diff --git a/repos/os/src/server/input_filter/input_source.h b/repos/os/src/server/input_filter/input_source.h index 8e99b539da..0c0f1818cc 100644 --- a/repos/os/src/server/input_filter/input_source.h +++ b/repos/os/src/server/input_filter/input_source.h @@ -26,21 +26,20 @@ class Input_filter::Input_source : public Source private: Input_connection &_connection; - Sink &_destination; public: static char const *name() { return "input"; } - Input_source(Owner &owner, Input_connection &connection, Sink &destination) + Input_source(Owner &owner, Input_connection &connection) : - Source(owner), _connection(connection), _destination(destination) + Source(owner), _connection(connection) { } - void generate() override + void generate(Sink &sink) override { _connection.for_each_event([&] (Input::Event const &event) { - _destination.submit_event(event); }); + sink.submit_event(event); }); } }; diff --git a/repos/os/src/server/input_filter/main.cc b/repos/os/src/server/input_filter/main.cc index b15122fde7..349301f791 100644 --- a/repos/os/src/server/input_filter/main.cc +++ b/repos/os/src/server/input_filter/main.cc @@ -31,7 +31,7 @@ namespace Input_filter { struct Main; } struct Input_filter::Main : Input_connection::Avail_handler, - Source::Factory + Source::Factory, Source::Trigger { Env &_env; @@ -201,7 +201,7 @@ struct Input_filter::Main : Input_connection::Avail_handler, * * \throw Source::Invalid_config */ - Source &create_source(Source::Owner &owner, Xml_node node, Source::Sink &sink) override + Source &create_source(Source::Owner &owner, Xml_node node) override { /* * Guard for the protection against too deep recursions while @@ -234,7 +234,7 @@ struct Input_filter::Main : Input_connection::Avail_handler, match = &connection; }); if (match) - return *new (_heap) Input_source(owner, *match, sink); + return *new (_heap) Input_source(owner, *match); warning("input named '", label, "' does not exist"); throw Source::Invalid_config(); @@ -242,21 +242,22 @@ struct Input_filter::Main : Input_connection::Avail_handler, /* create regular filter */ if (node.type() == Remap_source::name()) - return *new (_heap) Remap_source(owner, node, sink, *this, + return *new (_heap) Remap_source(owner, node, *this, _include_accessor); if (node.type() == Merge_source::name()) - return *new (_heap) Merge_source(owner, node, sink, *this); + return *new (_heap) Merge_source(owner, node, *this); if (node.type() == Chargen_source::name()) - return *new (_heap) Chargen_source(owner, node, sink, *this, _heap, - _timer_accessor, _include_accessor); + return *new (_heap) Chargen_source(owner, node, *this, _heap, + _timer_accessor, _include_accessor, + *this); if (node.type() == Button_scroll_source::name()) - return *new (_heap) Button_scroll_source(owner, node, sink, *this); + return *new (_heap) Button_scroll_source(owner, node, *this); if (node.type() == Accelerate_source::name()) - return *new (_heap) Accelerate_source(owner, node, sink, *this); + return *new (_heap) Accelerate_source(owner, node, *this); warning("unknown <", node.type(), "> input-source node type"); throw Source::Invalid_config(); @@ -298,13 +299,13 @@ struct Input_filter::Main : Input_connection::Avail_handler, * \throw Source::Invalid_config * \throw Genode::Out_of_memory */ - Output(Xml_node output, Source::Sink &sink, Source::Factory &factory) + Output(Xml_node output, Source::Factory &factory) : _owner(factory), - _top_level(factory.create_source(_owner, Source::input_sub_node(output), sink)) + _top_level(factory.create_source(_owner, Source::input_sub_node(output))) { } - void generate() { _top_level.generate(); } + void generate(Source::Sink &destination) { _top_level.generate(destination); } }; Constructible _output { }; @@ -344,7 +345,7 @@ struct Input_filter::Main : Input_connection::Avail_handler, pending |= connection.pending(); }); if (pending && _output.constructed()) - _output->generate(); + _output->generate(_final_sink); if (_config_update_pending && _input_connections_idle()) Signal_transmitter(_config_handler).submit(); @@ -421,8 +422,7 @@ struct Input_filter::Main : Input_connection::Avail_handler, try { if (_config.xml().has_sub_node("output")) - _output.construct(_config.xml().sub_node("output"), - _final_sink, *this); + _output.construct(_config.xml().sub_node("output"), *this); } catch (Source::Invalid_config) { warning("invalid configuration"); } @@ -438,6 +438,17 @@ struct Input_filter::Main : Input_connection::Avail_handler, Include_accessor _include_accessor { _env, _heap, _config_handler }; + /** + * Source::Trigger interface + * + * Trigger emission of character-repeat events. + */ + void trigger_generate() override + { + if (_output.constructed()) + _output->generate(_final_sink); + } + /** * Constructor */ diff --git a/repos/os/src/server/input_filter/merge_source.h b/repos/os/src/server/input_filter/merge_source.h index b570f77923..c39e2a5d5c 100644 --- a/repos/os/src/server/input_filter/merge_source.h +++ b/repos/os/src/server/input_filter/merge_source.h @@ -30,19 +30,19 @@ class Input_filter::Merge_source : public Source static char const *name() { return "merge"; } - Merge_source(Owner &owner, Xml_node config, Sink &destination, - Source::Factory &factory) + Merge_source(Owner &owner, Xml_node config, Source::Factory &factory) : Source(owner), _owner(factory) { config.for_each_sub_node([&] (Xml_node node) { if (input_node(node)) - factory.create_source(_owner, node, destination); }); + factory.create_source(_owner, node); }); } - void generate() override + void generate(Sink &destination) override { - _owner.for_each([&] (Source &source) { source.generate(); }); + _owner.for_each([&] (Source &source) { + source.generate(destination); }); } }; diff --git a/repos/os/src/server/input_filter/remap_source.h b/repos/os/src/server/input_filter/remap_source.h index b28fcbc6c5..35701e9cf6 100644 --- a/repos/os/src/server/input_filter/remap_source.h +++ b/repos/os/src/server/input_filter/remap_source.h @@ -25,7 +25,7 @@ namespace Input_filter { class Remap_source; } -class Input_filter::Remap_source : public Source, Source::Sink +class Input_filter::Remap_source : public Source, Source::Filter { private: @@ -42,18 +42,16 @@ class Input_filter::Remap_source : public Source, Source::Sink Source &_source; - Source::Sink &_destination; - /** - * Sink interface + * Filter interface */ - void submit_event(Input::Event const &event) override + void filter_event(Source::Sink &destination, Input::Event const &event) override { using Input::Event; /* forward events that are unrelated to the remapper */ if (!event.press() && !event.release()) { - _destination.submit_event(event); + destination.submit_event(event); return; } @@ -66,10 +64,10 @@ class Input_filter::Remap_source : public Source, Source::Sink auto remap = [&] (Input::Keycode key) { return _keys[key].code; }; event.handle_press([&] (Input::Keycode key, Codepoint codepoint) { - _destination.submit_event(Input::Press_char{remap(key), codepoint}); }); + destination.submit_event(Input::Press_char{remap(key), codepoint}); }); event.handle_release([&] (Input::Keycode key) { - _destination.submit_event(Input::Release{remap(key)}); }); + destination.submit_event(Input::Release{remap(key)}); }); } void _apply_config(Xml_node const config, unsigned const max_recursion = 4) @@ -126,14 +124,12 @@ class Input_filter::Remap_source : public Source, Source::Sink static char const *name() { return "remap"; } - Remap_source(Owner &owner, Xml_node config, Source::Sink &destination, - Source::Factory &factory, Include_accessor &include_accessor) + Remap_source(Owner &owner, Xml_node config, Source::Factory &factory, + Include_accessor &include_accessor) : Source(owner), - _include_accessor(include_accessor), - _owner(factory), - _source(factory.create_source(_owner, input_sub_node(config), *this)), - _destination(destination) + _include_accessor(include_accessor), _owner(factory), + _source(factory.create_source(_owner, input_sub_node(config))) { for (unsigned i = 0; i < Input::KEY_MAX; i++) _keys[i].code = Input::Keycode(i); @@ -141,7 +137,10 @@ class Input_filter::Remap_source : public Source, Source::Sink _apply_config(config); } - void generate() override { _source.generate(); } + void generate(Source::Sink &destination) override + { + Source::Filter::apply(destination, *this, _source); + } }; #endif /* _INPUT_FILTER__REMAP_SOURCE_H_ */ diff --git a/repos/os/src/server/input_filter/source.h b/repos/os/src/server/input_filter/source.h index 9078819840..828810315f 100644 --- a/repos/os/src/server/input_filter/source.h +++ b/repos/os/src/server/input_filter/source.h @@ -65,8 +65,6 @@ class Input_filter::Source throw Invalid_config { }; } - virtual void generate() = 0; - struct Owner; struct Sink : Interface @@ -74,12 +72,48 @@ class Input_filter::Source virtual void submit_event(Input::Event const &) = 0; }; + virtual void generate(Sink &) = 0; + + /** + * Interface to time-trigger the generate mechanism independently from + * incoming events. This is used for emitting character repeat events. + */ + struct Trigger : Interface + { + virtual void trigger_generate() = 0; + }; + + struct Filter : Interface + { + virtual void filter_event(Sink &, Input::Event const &) = 0; + + static void apply(Sink &destination, Filter &filter, Source &source) + { + struct Intermediate_sink : Sink + { + Sink &_destination; + Filter &_filter; + + void submit_event(Input::Event const &event) override + { + _filter.filter_event(_destination, event); + } + + Intermediate_sink(Sink &destination, Filter &filter) + : _destination(destination), _filter(filter) { } + + } intermediate_sink { destination, filter }; + + source.generate(intermediate_sink); + } + }; + struct Factory : Interface { /* * \throw Invalid_config */ - virtual Source &create_source(Owner &, Xml_node, Sink &) = 0; + virtual Source &create_source(Owner &, Xml_node) = 0; virtual void destroy_source(Source &) = 0; };