jupyter-xeus / cpp-terminal

C++ library for writing multiplatform terminal applications
https://jupyter-xeus.github.io/cpp-terminal/
Other
490 stars 53 forks source link

Term:cout clog cerr cin #271

Closed flagarde closed 1 year ago

flagarde commented 1 year ago

First step to split terminal from "iostream" on it. @TobiasWallner Maybe there is some improvements on speed in Windows. You can have a try

flagarde commented 1 year ago

@certik Hi, This would solve #260 too. Could you check #108 too ?

TobiasWallner commented 1 year ago

I will have a try. However, you may have to wait until the 19th for me to get to it.

flagarde commented 1 year ago

@TobiasWallner No problem, thanks for the time you involve on it. I do some basic test on Linux, Windows and less on Macos but but it's always nice to have more tries from people using the library. I'm still focusing on improving the library before using it on moving to it on my other projects

flagarde commented 1 year ago

@TobiasWallner @certik @MCWertGaming Hi, sorry for the big number of commits but this is a major refactoring :

1) streams are not binded to terminal class anymore 2) Mimic the iostream c++ : You have now : Term::cout (Fully buffered), Term::clog (Line buffered) Term::cerr (not buffered) Term::cin (Hacked to work in raw and cooked mode)

I did some test on Windows, Linux, Mac and didn't find major regressions.

Could you haveva try on your own and let me know ? I would like to be able to merge this before adding more feature because this is one of the biggest change in the library design

I have added more examples programs to test

TobiasWallner commented 1 year ago

Nice additions, I will try it out next week in my program.

flagarde commented 1 year ago

@TobiasWallner @certik @MCWertGaming In fact one of my goal to do this is to supress the need to have a Cooked and RawMode in the library. From the OS I would just ask it to turn rawmode and with this PR it would then be possible to simulate Cooked mode. I would like to have the possibility to have something like this :

int t;
Term:cin>>t; //Term:cin is cooked;

Term::Event event;
while(Term::cin_raw>>event)
{
  switch(event)
  {
    ...
  }
}

Only problems is the C horrible printf etc... ButI guess it should be possible to pass our own printf function I think the compiler would then pick up our and not the standard one

TobiasWallner commented 1 year ago

Would it be possible to use some kind of system call, that puts our program on the blocked queue in the OS, and puts it back into the running queue if an event happened? So that we do not have to use polling loop with a sleep?

TobiasWallner commented 1 year ago

Something that maybe then looks like this:

while(run){
  const Term::Event event = Term::event.read();
  switch(event.type()){
    ...
  }
  Therm::event.wait();
}
flagarde commented 1 year ago

Yes I have thought about this but never done a PR. I have thought to do this with a thread running on background and putting event in a std::queue

TobiasWallner commented 1 year ago

I just tested it with my application and it integrated well.

However, I noticed something new. Previously when I closed my application everything that I printed within my application would be deleted when I closed it. Now when I close my application the last screen will be still visible in the command line.

It is not bothering me, but I thought I mention it if that is not a wanted behaviour

TobiasWallner commented 1 year ago

And I think some kind of setting permanently changed withing my windows console when quitting the program. Because now after I quit and am back in the comman prompt instead of up, down, left, right I get ^[[A^[[A^[[A^[[B^[[D^[[C^[[A^[[D^[[B^[[D

Could it be that before you had some kind of virtual console and now you do everything in the 'real' console?

But when quitting the optinons from Therm::set_options are still present?

TobiasWallner commented 1 year ago

I just compared on Windows

test: Term::cout std::cout
1s single inputs ~28ms ~24ms
continuous input ~7ms ~5ms

I think both are probably equally fast

I mesured it very cheaply. just printed the time using std::chrono and let the application redraw the whole screen at every keyboard input. Averaged it by eyeballing the numbers I saw.

So you see I just measured the write to cout

auto start = std::chrono::high_resolution_clock::now();

Term::cout << Term::cursor_move(0, 0) << outputString << std::flush;

auto stop = std::chrono::high_resolution_clock::now();
auto duration = duration_cast<std::chrono::microseconds>(stop - start);
this->bottomMessageBar.assign("draw time: ").append(std::to_string(duration.count())).append("us");
flagarde commented 1 year ago

And I think some kind of setting permanently changed withing my windows console when quitting the program. Because now after I quit and am back in the comman prompt instead of up, down, left, right I get ^[[A^[[A^[[A^[[B^[[D^[[C^[[A^[[D^[[B^[[D

Could it be that before you had some kind of virtual console and now you do everything in the 'real' console?

But when quitting the optinons from Therm::set_options are still present?

Oh yes something is wrong, need to do more test then..

flagarde commented 1 year ago

I just compared on Windows test: Term::cout std::cout 1s single inputs ~28ms ~24ms continuous input ~7ms ~5ms

I think both are probably equally fast

I mesured it very cheaply. just printed the time using std::chrono and let the application redraw the whole screen at every keyboard input. Averaged it by eyeballing the numbers I saw.

So you see I just measured the write to cout

auto start = std::chrono::high_resolution_clock::now();

Term::cout << Term::cursor_move(0, 0) << outputString << std::flush;

auto stop = std::chrono::high_resolution_clock::now();
auto duration = duration_cast<std::chrono::microseconds>(stop - start);
this->bottomMessageBar.assign("draw time: ").append(std::to_string(duration.count())).append("us");

Yes I think it's because you flush each time right ? I think you can save a lot of time using the buffer and just let the stream write to console by itselft. OS call are the most expensive operation I think.

flagarde commented 1 year ago

And I think some kind of setting permanently changed withing my windows console when quitting the program. Because now after I quit and am back in the comman prompt instead of up, down, left, right I get ^[[A^[[A^[[A^[[B^[[D^[[C^[[A^[[D^[[B^[[D

Could it be that before you had some kind of virtual console and now you do everything in the 'real' console?

But when quitting the optinons from Therm::set_options are still present?

I think is because the terminal destructor is not called. Need to investigate. Thx for your tests. This PR is quite huge so some corner cases should appear

TobiasWallner commented 1 year ago

Yes I think it's because you flush each time right ? I think you can save a lot of time using the buffer and just let the stream write to console by itselft. OS call are the most expensive operation I think.

Maybe but that is not viably for me. See, i render the whole screen first into a string and then pass the 8kB or something to the stream. So there is only this one and only line of code that will onle be executed once after an event happened.

Also I need the flush, because, what if the stream decides to not print everything, then the missing part will not get updated until the user invokes some kind of event, which is not viable.

Furthermore, thinking of if. You, of corse know more about it, since you just spend some time on it. I already have the whole string to print. So the string is kinda my whole frame of data, nothing more nothing less. Also I guess that a lot of cli will probably use similar approaches, like in a gui or gaming, where you calculate a frame an then just display that frame. So if you do it like that you do not need a buffered output, because I already have the whole buffer in that string.

Would it be possible to have a function, that just takes my string as a whole and flushes it and only calls the necessary system calls once? Maybe even asycronously?

Just for clarification, this is my main loop:

void Lime::run_main_loop(){
    // reserve memory in advance
    std::string outputString;   
    outputString.reserve(1024 * 10); 

    // initial render of the whole screen
    this->render(outputString);
    this->draw(outputString); // <-- call to ::cout

    // main loop
    while(this->main_loop_continue){
        auto event = Term::read_event();
        this->prozess_event(std::move(event));
        outputString.clear();
        this->render(outputString);
        this->draw(outputString); // <-- call to ::cout
    }
}
flagarde commented 1 year ago

Yes I think it's because you flush each time right ? I think you can save a lot of time using the buffer and just let the stream write to console by itselft. OS call are the most expensive operation I think.

Maybe but that is not viably for me. See, i render the whole screen first into a string and then pass the 8kB or something to the stream. So there is only this one and only line of code that will onle be executed once after an event happened.

Also I need the flush, because, what if the stream decides to not print everything, then the missing part will not get updated until the user invokes some kind of event, which is not viable.

Furthermore, thinking of if. You, of corse know more about it, since you just spend some time on it. I already have the whole string to print. So the string is kinda my whole frame of data, nothing more nothing less. Also I guess that a lot of cli will probably use similar approaches, like in a gui or gaming, where you calculate a frame an then just display that frame. So if you do it like that you do not need a buffered output, because I already have the whole buffer in that string.

Would it be possible to have a function, that just takes my string as a whole and flushes it and only calls the necessary system calls once? Maybe even asycronously?

Just for clarification, this is my main loop:

void Lime::run_main_loop(){
  // reserve memory in advance
  std::string outputString;   
  outputString.reserve(1024 * 10); 

  // initial render of the whole screen
  this->render(outputString);
  this->draw(outputString); // <-- call to ::cout

  // main loop
  while(this->main_loop_continue){
      auto event = Term::read_event();
      this->prozess_event(std::move(event));
      outputString.clear();
      this->render(outputString);
      this->draw(outputString); // <-- call to ::cout
  }
}

like std::println() one ?

flagarde commented 1 year ago

And I think some kind of setting permanently changed withing my windows console when quitting the program. Because now after I quit and am back in the comman prompt instead of up, down, left, right I get ^[[A^[[A^[[A^[[B^[[D^[[C^[[A^[[D^[[B^[[D

Could it be that before you had some kind of virtual console and now you do everything in the 'real' console?

But when quitting the optinons from Therm::set_options are still present?

@TobiasWallner I checked this and it seems it appeared before this PR so it's not a regression of this PR but it is a bug we need to fix on the future.

TobiasWallner commented 1 year ago

like std::println() one ?

I am not quite sure, if I can follow your thought prozess. If I am not mistaken then the Idea of ptrintln is to have a newline character and a flush at the end.

If your Idea of println is that the provided string will get printed directly and not written to a in between buffer before finally printing it, then yes.

That, I would say, is something of lower priority. It would be a nice to have but is not necessary.

flagarde commented 1 year ago

like std::println() one ?

I am not quite sure, if I can follow your thought prozess. If I am not mistaken then the Idea of ptrintln is to have a newline character and a flush at the end.

If your Idea of println is that the provided string will get printed directly and not written to a in between buffer before finally printing it, then yes.

That, I would say, is something of lower priority. It would be a nice to have but is not necessary.

Yes as soon as this is merged this kind of function will be easily added. I agree this could be very useful

flagarde commented 1 year ago

@TobiasWallner I found the bug, thx for your report. I will push it and you can have a try

flagarde commented 1 year ago

@TobiasWallner The bugs you mentioned should be fixed by the last commits

flagarde commented 1 year ago

@TobiasWallner Could you confirm the bugs are gone ? I made some basic tests on the 3 OS but it's always nice to have cross-checks.

@certik What do you think about this PR ?

TobiasWallner commented 1 year ago

And I think some kind of setting permanently changed withing my windows console when quitting the program. Because now after I quit and am back in the comman prompt instead of up, down, left, right I get ^[[A^[[A^[[A^[[B^[[D^[[C^[[A^[[D^[[B^[[D

Could it be that before you had some kind of virtual console and now you do everything in the 'real' console?

But when quitting the optinons from Therm::set_options are still present?

This is still present on my windows machine

I am on commit: 2d8f9e00207cb56912773ffeba6f4bb08f3a712c of branch: remotes/external-packages/file

flagarde commented 1 year ago

Strange i fixed it but today it appeared again on my laptop. Need more investigation

TobiasWallner commented 1 year ago

I will have a look tomorrow

flagarde commented 1 year ago

Should be fixed now (I hope :) )

TobiasWallner commented 1 year ago

I had a check of commit ddd41132028c9cafc9a762dbd20f14acc88e3b07 and the console closes propperly now 👍

flagarde commented 1 year ago

@TobiasWallner you didn't find any problem integrating with your software ? If not I think I should merged it to allow a polishing later. The basic is here

TobiasWallner commented 1 year ago

I only needed to change the header files from #include <cpp-terminal/io.hpp> to #include <cpp-terminal/input.hpp> and use Term::cout instead of Term::terminal. So basically plug and play

flagarde commented 1 year ago

Nice, for now we don't assure API so it's ok I think the name iostream.hpp help people to understand what they espect to have including it

flagarde commented 1 year ago

I will merge then. I really want to have this mainstream because with a bit of work it would allow to have color with old terminal in windows and all other escape code.. we need to parse the string but at least it can be done.