Discussion thread for UI framework rewrite

[houqp]

OK, I created this issue for discussion on the design of our new UI framework, so we don't need to hijack #128 ;P

PS: we are somehow using the issue tracker as our mail list now. :)

[houqp]

_First post from @dpavlin :_

helppage.lua need rewrite to new event system, and I don't particularly like our usage of addGroup for page keys in unireader.lua because it's dense code, hard to read and easy to miss adding shortcuts :-)

@houqp I took a quick look at all menu/select like parts of our code, and (at first) it seems to me that there is quite a lot of code duplication there. Do you think it would be doable to create single widget which would handle page buttons (on both sides :-), keyboard shortcuts (by default whole keyboard, but with ability to insert shortcuts into for pages which have custom keyboard commands like help) and fiveway move/select (which we would need to support non-keyboard kindle anyway even in help which might be only was to select zoom level without keyboard)?

I won't have much time to hack on code in next few days, but I wonder how would that turn out, and I'm hoping for some code reduction while providing consistent user experience over different select-list like interfaces in reader currently.

Could such widget call itself to provide help for commands? :-)

[houqp]

_Second post from @hwhw :_

I'm on a similar mission right now. I'm further tuning and trying how to make the presentation layer (which I named "Widget") interact with a event handler system. I think in a day I have fleshed something out and will be happily accept comments.

Re-entrance in the program flow is indeed a matter here. Thus the separation of Presentation and Layout. It's a bit difficult: I want to allow widgets to be "active". I.e. they can listen for certain events. They also should allow to change state (e.g. a button can be marked/active or unmarked/active or inactive/not selectable; an input field for text might want to be active and have a special event handling to "catch" horizontal directional movements for managing a cursor etc.).

Eventually, even the document viewer itself would go into a widget. These are big plans, and I would like to illustrate them by investing some hours to create this enhanced Widget (exists in parts) / "ActiveWidget" system, I think.

Maybe it helps to think of the whole application a bit differently. Currently, it is built from a program-flow thinking. That is fine, it makes it clear to follow the path and easily see what happens without having to understand too much abstraction layers. However, our codebase is now large enough to change this advantage. Now it is probably easier to introduce some abstractions to remove the amount of duplicate concepts - and thus duplicate code.

If you looked at the Widget code, you will see that I introduced "lightweight" objects (in fact, tables) to pass data around that belongs together. I do that for a 2 dimensional size, it is { w = , h = }. We have other cases where we have data that we pass around and which belongs together, yet we do it by duplicating something. This is something we could make shinier.

I think if we take a slow approach towards isolating redundancy of code and concepts and create a clear and minimal abstraction, we're best off.

A last thing: The most common factor of our code is an event-handling loop that waits for input. It would be nice if the result of a cleanup would be that there is exactly one single loop persists and everything else fits into this concept.

Now that was a long OT post here. For the matter of this bug, I'd like to add: fixing this now easily by adding the relevant keys is fine. In the end, we better should have an abstraction for keys that mean the same to us :-)

[houqp]

Seems that everyone, including me, is not happy with the duplication of input loop. I am totally agree with the single input loop approach.

Following is my thoughts on how to maintain a single input loop:

We can have multiple widgets running at the same time, but only one of them is active, i.e. responding to user input. The active widget will register[1] its commands to the main input loop. With the help of closure[2], the drawing task will become transparent to the input loop, all it needs to do is calling the registered closure.

For switching between multiple running widgets, we can maintain a stack. For instance, when we start kpdfview, we run filechooser and register its commands to input loop. Then we choose a pdf document in filechooser. After we hit enter, filechooser will push itself to the running stack and starts pdfreader. The newly started pdfreader will register its commands to input loop and take over the events. Now if users want to back to filechooser and pressed Back key, we exist pdfreader, pop filechooser out of the stack and have its commands registered back to input loop.

[1] No overhead, just change table reference.

[2] Actually, the first argument of the anonymous function passed to commands:add() call is redundant if we use closure.

BTW: since @hwhw is already working on the rewrite, I have assigned the issue to him. Looking forward to your hack :)

[dpavlin]

I'm looking forward to new widget system, @hwhw has done great work so far.

[hwhw]

I have it finally fleshed out. I hope it proves to be working. For sure I left some nice little bugs for you to find later :-)

What I would like is to discuss the approach I have chosen.

In order to inform you about the code, I would suggest to look at "wtest.lua" (which is an example program using the new UI features), and run it with the "kpdfviewer" executable.

There are a whole lot of new things:

• Widgets can receive Events
• There's a new Keyboard handling. It is intended to be more simple to use than what we currently have.
• There's an abstraction for keyboard-aware widgets. They can define key sequences (and can change them anytime they like). Key sequences are defined as arrays of keys OR key groups (alternative keys). Key sequences trigger Events.
• There's a UIManager, of which there is a single instance for the whole application. It has the main event loop. This code you should look at next. It is in "ui.lua".
• The UIManager also has a task scheduler. This means that you can schedule function calls to be run in the future. The main event loop in UIManager will always look at scheduled tasks and decide based on that how long to wait for input events. Pending tasks are always run as first thing in the main event loop.

The code has many comments to make it understandable what it does.

I also added in new Widgets - and there will be probably much more. Widgets are easily layered and creating new ones is done in a few lines of code.

Note that Widgets now also have an "init" method. It is important that this method works on instances, i.e. the object returned by MyWidget:new(), not MyWidget itself.

There is an abstraction for UI dialogues that have focus movement. That way, complex dialogs can be build. Look at the implementation of ConfirmBox in dialog.lua to see how it works (although there is only one row / two columns).

It took a bit longer, but I hope it was worth it. Converting our current code over won't be that easy, but I think it wouldn't be too hard either. Probably we will have to reconstruct reader.lua and unireader.lua a lot, though. As for InputBox, I'd like this to become a Widget using the new facilities for keyboard management and focus management (it can now have "OK" and "Cancel" buttons, too :-)). As for menus and chosers, I'd like to convert them to Widgets, too, with the menu items/list items being individual widgets that listen for Focus/Unfocus events. But that's for later.

Please tell me if you like it - and what parts of it you don't like. I think everything might get much cleaner soon.

Things left open / todo:

• allow to unschedule tasks, for that we need to return a unique, stable ID when we schedule a task
• probably a lot more things

[hwhw]

Oh, and it's branch new_ui_code :-)

[dpavlin]

I just did a quick glance through code and new event_map approach seems very nice :-)

Widget code is also easy to read, which I like very much, but I found a problem with scheduling: I decreased scheduleIn timeout to 3 seconds, and added seconds to os.date, but widget doesn't seem to refresh every 3 seconds if I don't generate events in-between. I will have to take a deeper look in code to figure out why.

I think we need also a widget which would trigger after timeout but in another process to show dialog message. Example of this might be pages which render longer than second.

Let me try to explain using fork: we fork widget which does sleep(1) while main process renders page. If it's still rendering after one second, forked widget would draw dialog (so that users don't think that everything blocked) and exit. If rendering finishes first, it can just kill child process and disable widget.

I'm just dumping this idea here for safe-keeping, but first I have to play around with new code a bit more :-)

[hwhw]

Yes, you are right, scheduling is currently broken because waiting for input for a certain time span is currently broken, too: it is working with a signed integer which holds the time to wait as in us units. And 2^31 us are a mere 2.1s.... I think the C code should also take two parameters, one for the seconds, one for the us. Or properly work on double precision values (which Lua uses internally anyway).

[hwhw]

As for the concurrency: It will be a bit of hard work. At the first look, it's threading, not process forking. Although we could also work with a forked process - it's indeed a bit easier to fit in... Now that I think longer about it, I think I see what you mean and it is indeed quite nice. However, it won't be easy to fit this in with the emulation layer. But maybe the other way around: Spawn a task to do the rendering... So the UI process is the main process and spawns processes for document handling. This would also keep the memory usage under some document-centric control - a bit like Chromium/Chrome does for the browser tabs... Using POSIX shared memory and a control channel via Unix sockets would allow to fit this in with a UI process that is centering around a single call to select().

[hwhw]

Task management had a bug. I think it is working now.

[houqp]

Great work, I am playing with it now :)

[dpavlin]

Great! I like idea of separate rendering process, but that also can wait until we rewrite reader to use widgets.

[hwhw]

I've just commited a first approach to a reader widget. It only has paging, panning, zooming, rotation for now. And I did only an implementation for PDF documents. I'll need to do an overview of this, I guess...

Also, I've restructured the source directory. In the end I would like a mostly clean base directory, all the glue code should also go into a "backend" directory or similar.

[houqp]

Great work @hwhw ! It looks really nice :) Cant wait to play with it...

[houqp]

I have a question, how to rewrite part of ReaderPaging for a specific format?

Now page forward/backward keys only trigger gotoPage method. however, for crengine, we don't go to pages, but offset within documents or xpointers. It seems that all format related code are moved to document modules?

[hwhw]

I would say we should add another navigator that is used in place of ReaderPaging for documents that aren't paged. This is how I planned it to do. I hope it works...

[houqp]

OK, I will wait for your work :)

[hwhw]

I did not mean to do it myself, though :-) That was just an awkward way to express that I was thinking about it that way... So go ahead if you want! I hope to spend some hours on the code, too, but there's still more than enough to do :-)

[houqp]

ha, the fact is I will have to go travel tomorrow for 4 days. So I will be away from my development laptop again. :)