Add playback scaling

[sabbaka]

Use css transform property

[spbnick]

Thank you, Kyrill. I tried this right now and it looks great. There is one problem, however: the terminal width seems to be constrained by less than the container width, if the browser view zoom is not 100%. Can anything be done about this? Here's a screenshot:

[sabbaka]

Well, I have one idea how this can be fixed - it's to use font-size in view point size and then transform, I'll check this. On Thu, Nov 30, 2017 at 9:58 AM, Nikolai Kondrashov < notifications@github.com> wrote:

Thank you, Kyrill. I tried this right now and looks great. There is one problem, however: the terminal width seems to be constrained by less than the container width, if the browser view zoom is not 100%. Can anything be done about this? Here's a screenshot: [image: terminal_width_constrained] https://user-images.githubusercontent.com/549246/33422143-572d6d14-d5bd-11e7-9d00-b11802ea888d.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Scribery/cockpit/pull/65#issuecomment-348123838, or mute the thread https://github.com/notifications/unsubscribe-auth/AFTQ5UeQHbbj4PJ3DM25qVdws3d67puAks5s7m4_gaJpZM4Qr3qh .

-- Kyrylo Gliebov Software Engineer Red Hat Czech

[sabbaka]

In regards for problem with extra output - so, this strange symbols are logged which were not presented upon recording are added to output and are causing problems: Those numbers 1566 and 1564 is difference between playback position and packet length ( because of this symbols ). I've tested same code on Cockpit's side and updated tlog version ( build for Fedora 27 ) with mc - and there was this problem, though if I remove "throw" it works, but with this symbol breaking output. If I use top to test resizing - playback works smoothly from end to start without problems.

[sabbaka]

As for matching size of the container, it was not a problem at all, only thing why it was appearing, that container has fluid width ( according to window size ) and I have had a static value of 630px, so now I've changed this to get actual wrapper's width, this will allow us in future to create full-screen or resizing of container features.

[sabbaka]

https://youtu.be/9EAN6Id1YxE So, here is what I have for now - controls are working, but content is not centered after zooming in/out - that's because I've to use tricky way to center it inside the above container. I've put it as a separate commit also for more simple review and also I've changed a bit work with setState - I've tried to decrease amount of calls for only one, but I don't have any metrics if it actually has any effect on performance.

[sabbaka]

Also, I've forgot to add one more detail - I've had the idea to add feature as a hand scrolling - for example - you zoomed in and you would just use drag'n'drop to move through content and not the scrollbars, but now I'm not sure if that should be there, because that will remove ability to select text and copy it! But it is possible to create some sort of switch ( button ) between regular cursor to work with text and drag'n'drop.

[spbnick]

Niice, this feels much better, and it seems the issue with non-100% zoom levels is gone. Yes, I think we need to fix the non-centered zoom-in/out as it's confusing. How about we merge the part which keeps the terminal within bounds first, and then work on scrolling/zooming/panning? That way we'll have something working merged already.

Yeah, "panning" can be done with a button, although it wouldn't be very convenient to use. Perhaps we can do the button (like http://fontawesome.io/icon/hand-paper-o/), and add a hotkey for turning it on or off, and/or a hotkey which would enable panning while pressed (i.e. hold the key and drag the screen, otherwise select).

One thing that I keep noticing is that intermediate window resizes seem to disappear and window jumps through them when skipping, unless there's I/O. Perhaps the main playback routine needs fixing. Can you check that?

Regarding the extra characters, could you please provide me with the original logs, where that reproduces? You can use the journalctl command to dump the messages for the session using the same matches Cockpit uses when playing back the session. I'll try to see where there is a problem in encoding/decoding.

[spbnick]

Oh, and another thing: the "zoom to fit" icon seems a bit confusing to me. Do you think we can draw and/or use something like a "square under a lens" icon? E.g. something like these: https://www.iconfinder.com/search/?q=zoom+fit Or it would be breaking our pattern of using Font Awesome?

[sabbaka]

Yes, for now, if you will play or skip frames zoom levels will be switched back to match container ( not preserving zoom levels basically ). This is "broken" session: mc_broken_session_dump.txt

[sabbaka]

I think we should stick with font awesome to keep overall style. I'll add hotkeys and split commits in two - for playback scaling and for controls

[sabbaka]

I've separated commits and now in this PR it's only commit for scaling without buttons.

[spbnick]

I think that there's value in a blinking cursor, and we can discuss that, but let's not lump that change with this one.

[sabbaka]

I've removed removal of blinking cursor and added ours together if/else fix from meeting.

[spbnick]

I took a look at this today and it feels much better. However, I see that sometimes window resize and repaint play back with a huge delay between them. I'm not sure where that comes from. Will investigate tomorrow.

[spbnick]

@sabbaka, I still see the old resizing code in the commit, not the one we arrived at on our meeting. Have you forgotten to commit/push something, perhaps?

[sabbaka]

I’ll need to check that.

On Tue, 12 Dec 2017 at 8:53, Nikolai Kondrashov notifications@github.com wrote:

@sabbaka https://github.com/sabbaka, I still see the old resizing code in the commit, not the one we arrived at on our meeting. Have you forgotten to commit/push something, perhaps?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/Scribery/cockpit/pull/65#issuecomment-350972153, or mute the thread https://github.com/notifications/unsubscribe-auth/AFTQ5UDHDSpIzHx5rd8-yLCG2aks9y-Mks5s_jDLgaJpZM4Qr3qh .

-- Kyrylo Gliebov Software Engineer Red Hat Czech

[sabbaka]

Code should be fine now, it seems that I've just forgot to push.

[sabbaka]

I've made changes, but I'm not sure if I understand how to align correctly.

[spbnick]

Thank you. Just make them all appear on the same column. Like this:

    cols:           80,
    rows:           25,
    title:          _("Player"),
    term:           null,
    paused:         true,
    /* Speed exponent */
    speedExp:       0,
    containerWidth: 630,
    scale:          1

[spbnick]

Thanks a lot, looks and works great! Can you do a demo on today's meeting?

Another thing: could you please leave a comment when you're done fixing up things in response to review? I just don't get any updates from GitHub when you're force-pushing over a commit, without adding a new one. And even when you add a new one, it would be great to get a comment from you saying you're done.

[sabbaka]

Yes, I will do the demo. Pardon, just missed that yesterday.