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

Decompiler view jumps when toggling a breakpoint or adding a comment #2270

Closed
1 of 3 tasks
NirmalManoj opened this issue Jun 29, 2020 · 6 comments · Fixed by #2351
Closed
1 of 3 tasks

Decompiler view jumps when toggling a breakpoint or adding a comment #2270

NirmalManoj opened this issue Jun 29, 2020 · 6 comments · Fixed by #2351
Assignees
Labels
BUG Decompiler Issues and feature requests related to the decompiler in Cutter

Comments

@NirmalManoj
Copy link
Member

NirmalManoj commented Jun 29, 2020

Environment information

  • Operating System: Ubuntu 20.04
  • Cutter version: Latest master
    • Built from source
    • Downloaded from release
    • Distribution repository

Describe the bug
Toggling a breakpoint(or adding a comment) in the decompiler widget causes the view to jump. This makes it difficult to keep track on the position of code you are looking at.

To Reproduce

  1. Open a ~2 screens high function and scroll down a bit
  2. Select a line somewhere in the middle and add a breakpoint(or comment).
  3. Observe that viewing position in the decompiler widget changes

See this GIF to see exactly what's happening
display jumps

Expected behavior

The view should not move while adding toggling a breakpoint. But the highlighting of the line should change accordingly.

Findings so far (for the problem with toggling breakpoints)
After hours of debugging, I found that this view jumping problem happens because the whole code gets decompiled again when a new breakpoint is added. Then the code tries to reset the cursor to the previous location(the line at which the breakpoint was toggled). That's how we at least get to see the line at which the breakpoint was added. But the view doesn't get back to the way it was.

@NirmalManoj NirmalManoj added BUG Decompiler Issues and feature requests related to the decompiler in Cutter labels Jun 29, 2020
@NirmalManoj NirmalManoj changed the title Decompiler view jumps when toggling a breakpoint Decompiler view jumps when toggling a breakpoint or adding a comment Jun 30, 2020
@NirmalManoj
Copy link
Member Author

It turns out to be existing for multiple options we have in the context menu of the decompiler. See the GIF below
View Jumping Common

@karliss
Copy link
Member

karliss commented Jun 30, 2020

For some of them like renaming things re-running decompiler is unavoidable, but there is room for improvements to try better preserve view position.

In case of breakpoints I don't think decompiler needs to be run.

@NirmalManoj
Copy link
Member Author

@karliss Yes, for breakpoints I think we can somehow stop the decompiler from running. In fact, that was the hack that I was thinking about. But ultimately, in order to fix this problem entirely, we have to find a way to reset the view position to exactly where it was before. In case this is not possible in the QPlainTextEdit, and since we are planning to use some other advanced text editor like KTextEditor in the future, I think it might be possible in it.

Can you explain a bit on how you think we can try to better preserve view position?

@karliss
Copy link
Member

karliss commented Jun 30, 2020

Not performing an unnecessary decompilation isn't necessary a hack. It is somewhat expensive operation and performing it when not needed makes the UI less responsive. Even with position restoration working perfectly it would be preferable to do it only when necessary.

As for scroll position preserving there two approaches I can think of two approaches that can be explored.
The first one is to do the obvious thing. To preserve the scroll position - preserve the scroll position. Save it before changing text and load it afterwards.

Other approach is to try avoiding position reset. When doing basic text modifications cursor position is more or less preserved. Maybe the code can be replaced using text editing API in a way to reduces cursor move.

@karliss
Copy link
Member

karliss commented Aug 11, 2020

@NirmalManoj Wasn't this fixed already? I guess gitthub automatically closes the issue only when PR gets merged to master. This can probably be closed.

@NirmalManoj
Copy link
Member Author

I'm closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Decompiler Issues and feature requests related to the decompiler in Cutter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants