Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: hyperlink support. #665

Merged
merged 4 commits into from
Jun 4, 2023
Merged

Feature: hyperlink support. #665

merged 4 commits into from
Jun 4, 2023

Conversation

ArthurSonzogni
Copy link
Owner

See the OSC 8 page. FTXUI support proposed by @aaleino in #662.

API:

auto link = text("Click here") | hyperlink("https://github.com/FTXUI")

Fixed:#662

See the [OSC 8 page](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda).
FTXUI support proposed by @aaleino in [#662](#662).

API:
```cpp
auto link = text("Click here") | hyperlink("https://github.com/FTXUI")
```

Fixed:#662
@ArthurSonzogni
Copy link
Owner Author

Hello @aaleino, Could you please take a look?

This stores the hyperlink string into every cells. Most likely with string optimization, this shouldn't translate into using too much memory when not used.

I ran a benchmark. This makes FTXUI screen rendering 50% slower. On my laptop, on a 128x128 We go from 15'000fps to 7'500fps on a 80x80 grid.

There are still plenty of room for other work. It might be fine. We might also find ways to improve it over time. We could store the string separately and store only its index.

Benchmark (after)

------------------------------------------------------------
Benchmark                  Time             CPU   Iterations
------------------------------------------------------------
BencharkBasic/0          608 ns          608 ns      1160480
BencharkBasic/16        8874 ns         8874 ns        78888
BencharkBasic/32       28301 ns        28301 ns        24764
BencharkBasic/48       50201 ns        50201 ns        14035
BencharkBasic/64       71435 ns        71432 ns         9745
BencharkBasic/80       92506 ns        92505 ns         7588
BencharkBasic/96      113480 ns       113476 ns         6145
BencharkBasic/112     135262 ns       135256 ns         5189
BencharkBasic/128     155677 ns       155674 ns         4492
BencharkBasic/144     175585 ns       175579 ns         3983
BencharkBasic/160     198503 ns       198499 ns         3560
BencharkBasic/176     225427 ns       225413 ns         3106
BencharkBasic/192     247805 ns       247793 ns         2832
BencharkBasic/208     268403 ns       268394 ns         2606
BencharkBasic/224     291348 ns       291339 ns         2407
BencharkBasic/240     312722 ns       312713 ns         2233
BencharkBasic/256     335078 ns       335071 ns         2089

Benchmark (before)

------------------------------------------------------------
Benchmark                  Time             CPU   Iterations
------------------------------------------------------------
BencharkBasic/0          598 ns          597 ns      1173144
BencharkBasic/16        5207 ns         5204 ns       134531
BencharkBasic/32        9157 ns         9151 ns        76168
BencharkBasic/48       22434 ns        22434 ns        31155
BencharkBasic/64       34216 ns        34216 ns        20463
BencharkBasic/80       46032 ns        46032 ns        15227
BencharkBasic/96       57430 ns        57428 ns        12120
BencharkBasic/112      69377 ns        69376 ns        10075
BencharkBasic/128      80807 ns        80805 ns         8685
BencharkBasic/144      93443 ns        93366 ns         7508
BencharkBasic/160     103344 ns       103342 ns         6786
BencharkBasic/176     115534 ns       115533 ns         6042
BencharkBasic/192     126967 ns       126964 ns         5523
BencharkBasic/208     138934 ns       138932 ns         5056
BencharkBasic/224     149849 ns       149844 ns         4682
BencharkBasic/240     160696 ns       160693 ns         4346
BencharkBasic/256     172293 ns       172287 ns         4066

@ArthurSonzogni
Copy link
Owner Author

In the latest patchset, I am now storing a uint8_t per Pixel/Cell. Then the Screen is registering up to 254 hyperlink. This improved performance by 2x and we are now back to the same level of performance.

@aaleino
Copy link

aaleino commented Jun 4, 2023

Thanks a lot, that was fast! It has been awhile since I used FTXUI, but i tried it by modifying the example modal_dialog.cpp:

// Definition of the modal component. The details are not important.
Component ModalComponent(std::function<void()> do_nothing,
                         std::function<void()> hide_modal) {
  auto component = Container::Vertical({
      Button("Do nothing", do_nothing, button_style),
      Button("Quit modal", hide_modal, button_style),
  });
  // Polish how the two buttons are rendered:
  component |= Renderer([&](Element inner) {
    return vbox({
               text("Modal component ") | hyperlink("http://www.google.com"),
               separator(),
               inner,
           })                               //
           | size(WIDTH, GREATER_THAN, 30)  //
           | border;                        //
  });
  return component;
}

It compiled but the hyperlink does not show? Is the test incorrect?

@ArthurSonzogni
Copy link
Owner Author

It compiled but the hyperlink does not show? Is the test incorrect?

It seems to work. Here is what I get with your example:

out-2023-06-04_17.36.57.mp4

My terminal emulator kitty only display some curly underline when the mouse is over it. You migth want to add some | color(Color::Blue) | bold to highlight the link?

@aaleino
Copy link

aaleino commented Jun 4, 2023

Ah, my mistake, I was running the example under tmux which had apparently captured the escape code. Running in windows terminal without tmux worked fine.

Then I think it works intuitively and I will definitely use this feature. I will start to incorporate this into my source code.

Thank you very much!

@ArthurSonzogni ArthurSonzogni merged commit 7b7177b into main Jun 4, 2023
@ArthurSonzogni ArthurSonzogni deleted the hyperlink-support branch June 4, 2023 19:06
ArthurSonzogni added a commit that referenced this pull request Jul 25, 2023
See the [OSC 8 page](https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda).
FTXUI support proposed by @aaleino in [#662](#662).

API:
```cpp
auto link = text("Click here") | hyperlink("https://github.com/FTXUI")
```

Fixed:#662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants