Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 331629 - [disassembly] pageUp/pageDown scrolling jumps over unknown sections unexpectedly
Summary: [disassembly] pageUp/pageDown scrolling jumps over unknown sections unexpectedly
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf (show other bugs)
Version: 7.0.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 7.0.3   Edit
Assignee: Anton Leherbauer CLA
QA Contact: Pawel Piech CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-02 02:26 EST by Kirk Beitz CLA
Modified: 2011-04-15 04:23 EDT (History)
3 users (show)

See Also:

Attachments
code to try to stay closer to previous focus when user scrolls via keypress (2.82 KB, patch)
2011-04-14 05:42 EDT, Kirk Beitz CLA 		no flags Details | Diff
Alternative fix (7.55 KB, patch)
2011-04-14 08:59 EDT, Anton Leherbauer CLA 		aleherb+eclipse: iplog-
Details | Diff
Alternative fix plus arrowUp/arrowDown cases (7.56 KB, patch)
2011-04-14 17:30 EDT, Kirk Beitz CLA 		aleherb+eclipse: iplog+
aleherb+eclipse: review+
Details | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kirk Beitz CLA 2010-12-02 02:26:26 EST 
in the disassembly view, notice that upon performing pageUp a couple of times (and this will also occur with pageDown scrolling), a section of disassembly is displayed that is not adjacent to the disassembly that was being displayed to the user prior to the scroll.

it may then take a little effort to get back to the disassembly that the user was trying to look at in the first place.

i've noticed this for a while, and it has been a bit disconcerting, but i've tried to get used to it.

however, we are getting additional reports from folks who would prefer that pageUp/pageDown would always retrieve adjacent disassembly, as opposed to jumping over the gap, and then getting the disassembly on the other side of the gap.

in other words, when scrolling up, if a "..." section is encountered, check to see what operation caused that to occur, and if it was a pageUp, attempt to acquire the memory on the bottom of the gap.
Comment 1 Kirk Beitz CLA 2011-04-14 05:42:44 EDT 
Created attachment 193234 [details]
code to try to stay closer to previous focus when user scrolls via keypress

the attached patch attempts to improve the user experience by utilizing keypress scrolling to indicate a desire for continuity on the user's part (versus scrollbar scrolling, where the user may be more interested in scrolling over/past disabled ranges).

please review the patch and try it out.

for your own testing/trials … 

i've found it was most likely  to scroll over a disabled-range boundary (using the edc x86 debugger, so probably also with the gdb x86 debugger) if i set the cursor near the top of the disassembly window and then scrolled up or near the bottom and scrolled down, as this would be more likely to set the post-scroll cursor beyond the "..." location representing a disabled range.  after applying this patch, scrolling via pageUp/pageDown feels significantly improved to me, while still allowing using the scrollbar elevator or trackpad swipe-scrolling to jump over ranges as possibly desired.

(we would also like to apply this patch to the CDT_7_0 branch if it is considered ok to do so, since our next product update coming soon would still depend on that branch.)
Comment 2 Anton Leherbauer CLA 2011-04-14 08:59:11 EDT 
Created attachment 193251 [details]
Alternative fix

I tried your patch and it is an improvement, but I think it seems a bit fragile.  Twice I got into a situation where the view would no longer update.  This seems to be caused by the gotoAddress() call inside updateVisibleArea().
Anyway, I tried a different approach by directly handling the page up/down key presses.  Please give it a try.
Comment 3 Kirk Beitz CLA 2011-04-14 17:30:50 EDT 
Created attachment 193304 [details]
Alternative fix plus arrowUp/arrowDown cases

(In reply to comment #2)
> Created attachment 193251 [details]
> Alternative fix
> 
> I tried your patch and it is an improvement, but I think it seems a bit
> fragile.  Twice I got into a situation where the view would no longer update. 
> This seems to be caused by the gotoAddress() call inside updateVisibleArea().
> Anyway, I tried a different approach by directly handling the page up/down key
> presses.  Please give it a try.

thanks for your contribution!  page scrolling is much smoother with this approach, no doubt.

while i hadn't experienced complete the "no longer update" scenario you mentioned, scrolling was much jumpier with my original patch.

one thing i had been trying to address with my patch was arrow-up/arrow-down scrolling as well (as represented by the use of generic KEY in the original patch).

i've re-factored your patch a bit in my attempt to include arrowUp/arrowDown:

• combined pageUp() & pageDown() into keyScroll(int keyCode) in DisassemblyPart
  (the 2 algorithms already had much in common)

• return boolean indicating true if keyScroll() processes the key
  (this allows it to be ignored when no viewport change occurs)

my extended thanks in advance for your review of this new patch.
Comment 4 Anton Leherbauer CLA 2011-04-15 04:16:06 EDT 
Very good!  I like it.  Committed to HEAD and cdt_7_0.