Skip to content

Optimize disassembly scroll and selection logic - #3646

Merged
thestr4ng3r merged 6 commits into
rizinorg:devfrom
PremadeS:disassembly-scroll
Jul 17, 2026
Merged

thestr4ng3r merged 6 commits into
rizinorg:devfrom
PremadeS:disassembly-scroll

Conversation

@PremadeS

@PremadeS PremadeS commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've updated the documentation with the relevant information (if needed)
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

There will be a lot of text to read so get some popcorn 🍿

This PR does the following:

1- Scroll by visual lines instead of disassembly lines (see #3604)
Previously whenever a scroll happened, cutter cleared all of the disassembled lines it had and queried rizin again, even if the scroll was only for one instruction. This causes two problems:

    1. All of the lines have to be queried again making scrolling slow.
    1. Since an instruction can have multiple metadata lines attached to it, it meant that whenever a user scrolled a single instruction all of the metadata lines will also be scrolled - making scrolling feel choppy.

To solve this we keep a buffer of "max visible lines * 5" which is filled in as user scrolls, meaning at first only "max visible lines" are queried from rizin, if user scrolls upwards the entire "max visible lines" above the current top instruction is fetched and prepended to our "lines" buffer. Since the previous lines are still saved - if the user scrolls back down we can just show those specific lines from our buffer instead of querying again.

Lines are erased from start or end based on scroll direction if the buffer exceeds the size cap of "max visible lines * 5"
The "lines" buffer is fully cleared if user seeks to some address using some external signal - like the Visual nav bar at the top (basically seeking using any method other than directly clicking on the instruction line itself)

Old:
old-scroll.webm

New:
new-scroll.webm

2- Adds infinite selection via mouse and keyboard
There was no way in cutter to select more lines than whats currently shown on screen and there was also no way to select text via keyboard. This is done via manually handling the selection instead of letting Qt handle it
selection.webm

Edit: One thing I forgot to mentions is that the selection is preserved no matter how far user scrolls up or down.

3- Fixes empty space at the bottom of disassembly widget viewport
Whenever "max visible lines" were calculated it didn't account for the fact that the last line might be partially visible meaning there is not enough space to display it fully, which created empty space at the bottom

Old: (see space at bottom - that space is preserved even when scrolling)
old-space

New:
new-space

4- Adds a visible/blinking cursor to the disassembly panel
Avoids the issue of not knowing which line we are on, if the line contains similar text that is highlighted. Also helps to know where selection will start using physical keys

Old:
old-cursor.webm

New:
new-cursor.webm

There are also some other optimizations which will be apparent from the code

From a simple test involving a lot of scrolling and seeking the outcome was:

Metric Before After Improvement
Total Renders 3589 3592
Average Render Time 21.676 ms 9.455 ms ~56.4% faster

Test plan (required)

Test every function of disassembly widget, including:

  • verifying arrows are drawn correctly
  • scrolling is fine
  • shift + scroll is good
  • copying is fine
  • left clicking + scrolling up/down for selection
  • seeking from outside the disassembly widget
  • scrollbar is updating and working correctly
    .....

Try to break it

Closing issues

closes #3604
and others (will update after one last test)

@PremadeS

PremadeS commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

Will test it one more time tomorrow on different machines
Scrolling might mess up due to the hex bytes printing stuff, because rizin is not updated yet. See rizinorg/rizin#6556

Should probably update rizin to latest dev before this

@thestr4ng3r thestr4ng3r left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Navigating by cursor keys does not work for me, but it did before. hjkl however works fine.

I also found a way to break it. Use this binary: hello-macos-arm64e-stripped.gz Seek to entry0 and scroll up. The first instruction of the function is shown twice with all its metadata:
https://github.com/user-attachments/assets/a1f9c041-68a5-491e-9f40-ca76b321c28e

Another point is the behavior when seeking to an unaligned address. When the offset is already in the visible range, the cursor just disappears. When it is outside of it, the screen shown directly after seeking is exactly what is to be expected, but after scrolling up the unaligned instruction never disappears:
https://github.com/user-attachments/assets/c622eaf1-735c-40af-8fea-a88f3ef5ecc6
However the behavior before your changed was arguably even worse, and it is hard to define what the expected behavior would be in every case, so this is not something that has to be solved right now.

As a sidenote, I think this could benefit a lot from a test suite. It was attempted to introduce one before but unfortunately never got finished: #1345

Apart from those issues I absolutely love the changes! Makes using the disassembly widget much more enjoyable.

Comment thread src/widgets/DisassemblyWidget.h
leftPanel->update();

// update buffer by erasing extra lines
constexpr int multiplier = 5;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reasoning for choosing exactly 5 here? Is it just a good tradeoff between memory usage and performance? Might be worth documenting this with the constant.

@PremadeS PremadeS Jul 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reasoning for choosing exactly 5 here? Is it just a good tradeoff between memory usage and performance? Might be worth documenting this with the constant.

No particular reason, yes it did seem like a good middle ground. From what I've seen the multiplier is usually between 3 - 10. Since the user can seek to a completely random offset (meaning the scrolling isn't always linear), keeping it on the lower side seems viable

@Rot127

Rot127 commented Jul 15, 2026

Copy link
Copy Markdown
Member

I want this in Rizin 😭

@PremadeS

Copy link
Copy Markdown
Collaborator Author

Navigating by cursor keys does not work for me, but it did before. hjkl however works fine.

Should be fixed now (tested on macos 14 - x86_64)

I also found a way to break it. Use this binary: hello-macos-arm64e-stripped.gz Seek to entry0 and scroll up. The first instruction of the function is shown twice with all its metadata: https://github.com/user-attachments/assets/a1f9c041-68a5-491e-9f40-ca76b321c28e

It seems like for some offsets rizin isn't calculating the offset before X instructions correctly, In this particular case, when rizin is queried for the offset that is exactly "max visible lines" (which was 21 in my case) before the current offset
it is for some reason returning the offset that is 20 lines/instructions before the current offset which causes an extra instruction to be disassembled and rendered.
It is fixed on the cutter side by removing lines that have offset >= currentOffset (removing duplicates in short)

it is hard to define what the expected behavior would be in every case, so this is not something that has to be solved right now.

I agree, if the seek is outside the visible range and caused by the console, we could set a flag and fetch disassembly again on scroll instead of prepending or appending, but that could cause other bugs. I think it's best to open a separate issue for it. I've left as it is for now

As a sidenote, I think this could benefit a lot from a test suite. It was attempted to introduce one before but unfortunately never got finished: #1345

Couldn't agree more with this, I think cutter itself could benefit a lot from tests, there have been too many cases where changing one thing silently breaks something else

There was also another issue, if metadata lines exceeded the number of visible lines, selecting it was causing weird behavior. That's also fixed

@thestr4ng3r thestr4ng3r left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested again and can confirm the issues are fixed. From my side, this is good to merge.

Comment thread src/common/DisassemblyHelper.cpp Outdated
@thestr4ng3r
thestr4ng3r merged commit 867c54e into rizinorg:dev Jul 17, 2026
15 checks passed
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.

Scroll by visual lines instead of disassembly lines in disassembly widget

3 participants