eteran / edb-debugger

edb is a cross-platform AArch32/x86/x86-64 debugger.
GNU General Public License v2.0
2.66k stars 321 forks source link

A more manually coded UI, to allow more complex future customizations #817

Closed eteran closed 2 years ago

eteran commented 2 years ago

working on handling the UI in a more custom way. The idea being that we'll have more control over how the UI operates than we'd get with using the designer alone

We'll still use the designer for the really basic stuff (toolbar, menu, status bar), but the CPU/Data/Stack/etc views have long been too complex to represent in the designer meaningfully (unless we made designer plugins... but that hardly seems worth it).

eteran commented 2 years ago

@10110111 I've prototyped out an idea for the UI I'd like your opinion on. Basically I've never really loved how the logger widget works in edb currently as a dockable window. I find it usually ends up shoved tabbed with the stack widget which I usually don't make very wide. So the end result is that the log widget isn't terribly useful for me.

So my idea is to add "collapsible bottom panels" to the main UI (which is slightly complicated because of how Qt handles dock widget), but can work!

Attached is a video of what I'm mocked up (the slowness is because I'm experimenting with using WSL2 on windows with the gWSL support, so don't worry, it should be buttery smooth on native UIs).

Any thoughts?

https://user-images.githubusercontent.com/571477/171252198-4c034218-d88c-4aef-bfab-c1b0ec6293e8.mp4

10110111 commented 2 years ago

What first catches the eye is the single button at the bottom that takes up vertical space basically for nothing. Moreover, this seems to only be useful for a single widget, the logger.

Why not move this toggle-button into the toolbar? It would work similarly to the Terminal button in KDE Dolphin:

anim

eteran commented 2 years ago

Excellent feedback!

A few things.

  1. My plan long-term is to combine this with a stacked widget so that while only one widget can be shown at a time in the collapsible region, there could be multiple widgets to choose to show depending on which button you click. Similar to tabs.

I'm kinda looking to take some inspiration from things like Qt creator and to a degree, vscode.

  1. The button is actually built into the status bar, so it doesn't take up any more vertical space then we currently do for that. I didn't put in the mock-up, but it would be a long side the status message widget we already have in there.

  2. I am absolutely open to having the control in other spots such as menu items and toolbar buttons. This is just a first pass on the idea.

10110111 commented 2 years ago
  1. What benefits will it provide compared to the usual tabs? This button seems global for the window, while the tabs can control locally in a group of docked widgets.

  2. "The button is actually built into the status bar" — sounds like an abuse of status bar. The status bar was intended for, well, status: messages, progress bars etc., not for input.

I'm kinda looking to take some inspiration from things like Qt creator and to a degree, vscode.

Funny, I always considered OllyDbg-like layout the best for assembly-level debuggers. At least, for the disassembly/registers/data view; OllyDbg has also other MDI windows, which is not the best way to organize this IMHO (I just don't like MDI :D). But since EDB puts the additional views (e.g. Memory Regions) into separate windows, it doesn't have this problem.

eteran commented 2 years ago
  1. Well, the main benefit is that with a tabbed docked widget, the log window is ironically both always taking up space unnecessarily and never wide enough to be really practical since it basically always needs horizontal scrolling. So in its current for I find it 100% unusable. See the following images after a default launch.

image image

A dedicated panel makes it so the log is both out of the way when not used, and is nice and wide when wanted to be used. Strikes me as a best of both worlds.

  1. I both agree and disagree. While traditionally, I'd generally agree that a "status bar" is for status, but I also can see that UI paradigms are shifting slightly over time. Qt supports putting arbitrary widgets in the status bar presumably because at least sometimes, it's a sensible thing to do.

I believe you know what I mean when I say where I'm taking inspiration from, but just to be sure that we're talking the same language, here's a video of what I mean:

https://user-images.githubusercontent.com/571477/171299131-f6a54a84-764e-465a-9528-d4ba0c891e21.mp4

As you can see from the video, Qt has chosen to make use of some of the visual real-estate offered by what is typically a mostly empty status bar. I've seen several applications do similar with a nice effect.

Funny, I always considered OllyDbg-like layout the best for assembly-level debuggers. At least, for the disassembly/registers/data view; OllyDbg has also other MDI windows, which is not the best way to organize this IMHO (I just don't like MDI :D). But since EDB puts the additional views (e.g. Memory Regions) into separate windows, it doesn't have this problem.

Oh I agree 100%! I am looking to keep the exact same layout we've had where the registers, stack and data views are dockable windows. Literally the only thing I am looking to change is where the logger is at the moment. The rest would remain exactly as it is today. Also, we agree in not liking MDI ;-). That was a fundamental design choice when I started edb!

10110111 commented 2 years ago

Literally the only thing I am looking to change is where the logger is at the moment

Then this indeed makes my suggestion to place the button into the toolbar (and maybe also add a shortcut to toggle it) sensible. There's no need to create all this complicated machinery of QtCreator-like hidable panels to only enable toggling of a single widget.

OTOH, if you want to further develop this idea, it might be worth considering more widgets: the ones that are currently separate windows. But it might appear to be a bad decision, depending on the actual widget under consideration (the dynamics of use may require them to be undocked almost always, or to take more space etc.).

Qt has chosen to make use of some of the visual real-estate offered by what is typically a mostly empty status bar.

Actually no, there's simply no status bar there :) Moreover, their docks seem to not be based on QMainWindow. In particular, this is given away by the multi-level docked widgets, e.g. Expressions and Locals in the screenshot below. QMainWindow doesn't seem to support this.

Screenshot - 010622 - 02:43:23

eteran commented 2 years ago

Then this indeed makes my suggestion to place the button into the toolbar (and maybe also add a shortcut to toggle it) sensible. There's no need to create all this complicated machinery of QtCreator-like hidable panels to only enable toggling of a single widget.

Well, regardless of where the button is (toolbar, menu, etc), the hide/show of the bottom panel would function pretty much the same. I suppose "complicated" is subjective. That being said, I am 100% open to having the control simply be a toggleable toolbar button (likely with a companion menu item under the view menu).

OTOH, if you want to further develop this idea, it might be worth considering more widgets: the ones that are currently separate windows. But it might appear to be a bad decision, depending on the actual widget under consideration (the dynamics of use may require them to be undocked almost always, or to take more space etc.).

I'm still in the brain storming phase of things, but I do think that generally speaking, I'm planning on keeping things that are separate windows as separate windows. But I do have some ideas. For example, in the collapsible space, we could have a "command" widget would would act as a gdb style REPL widget. If we have that, it's also a good place to integrate python scripting which has been on my wishlist basically since inception.

All of that being said, for now, my thoughts are on the logging widget.

Actually no, there's simply no status bar there :)

I think we're getting into a philosophical discussion at this point, LOL. Admittedly a status bar that doesn't show any status is easily argued as "not a status bar"... but I'm not sure the difference is really worth focusing on too much since either way we're talking about a "relatively thin bar at the bottom of the main window with 'stuff' in it". We can call it a bottom toolbar just the same, LOL. At the moment, we don't give much status in the status bar, so it's bordering on a ceremonial title ;-).

Moreover, their docks seem to not be based on QMainWindow. In particular, this is given away by the multi-level docked widgets, e.g. Expressions and Locals in the screenshot below. QMainWindow doesn't seem to support this.

Agreed. Creator and vscode are most certainly going in a different direction as far as how they implement "docks" as they've opted to not really make them relocatable, but instead, just have regions which you can set to be one of a few different types of data views. To be clear, I am not looking to emulate that particular aspect of edb's UI since I rather like the way edb functions at the moment :-).

First of all, thank you for your honest opinions, this is exactly the kind of discussion I was looking to have! I think we're approaching agreement on where the button should go. I'm absolutely fine with making it a toolbar button, I honestly hadn't even considered it before you mentioned it because my mindset was based off how creator implemented the bottom panels.

The only argument I can think of for putting the button at the bottom instead of the toolbar is that it puts the cause (the button) and the effect (the panel changing) visual close to each other... but I think that as we are developing a tool for very advanced users, I don't think that it's enough of a reason to reject the toolbar button. We kinda have to assume our users are smart ;-).

Assuming that you've expressed all of your concerns/thoughts and we're generally in agreement about using a toolbar button, I'll see about making this work in this branch with the actual edb UI instead of a mockup so we can really see it in action and see if it's worth merging in. (Of course please say so if you don't think I've addressed or understood your thoughts).

eteran commented 2 years ago

@10110111 OK, so here's my current prototyping, which now uses the real edb UI for demonstration (still needs some smoothing out of edges, a proper toolbar icon, and is laggy because of gWSL stuff).

https://user-images.githubusercontent.com/571477/171314514-f6d41af2-fed8-4221-95ef-c4f814e36b12.mp4

As you can see, the dock widgets remain exactly as they are, the only thing that is changed is the logging output panel no longer being in a dock, but instead is in it's own collapsible pane that is "below" the bottom dock.

For me the big win is that the logger can now span the entire bottom when it is being viewed, making it much more usable, but takes up zero space when unused. The height of the logger is also configurable simply by using the splitter.

This PR will probably make the logger a built-in feature of edb, and no longer a plugin. though we could add an API for plugins to create these panels I suppose. Something like:

edb::v2::addPanel(tr("Logger"), logWidget);

Maybe that's the way to go? Thoughts?

10110111 commented 2 years ago

The video looks good.

Something like: edb::v2::addPanel(tr("Logger"), logWidget); Maybe that's the way to go? Thoughts?

I think until there's actual need for this, it shouldn't be done. The actual use case might require a different API, which you'd end up rewriting.

eteran commented 2 years ago

@10110111 so I've done the "primary" work on this PR. It should:

  1. Implement the built-in hidable bottom logger widget.

  2. NERF the existing logger widget. This will prevent the loading of the old one, we can likely just delete it, but eh,this'll actively block it from loading the old widget on upgraded installs that leave cruft behind.

The only negative side effect I've currently observed is that user will lose their custom layouts. They can of course re-create them, but since the widgets are created differently and with a different hierarchy, trying to restore an existing layout will not work (and would be very challenging to make work in a sensible way).

This is IMO not a show-stopper since we've almost certainly broken layouts in the past and the UI, while complex, is still simple enough that it takes a matter of a couple of minutes to move things around to one's liking.

Anyway, when you get a chance, could you take a look at the current state of this branch and let me know if you see anything that is problematic?

Thanks!

10110111 commented 2 years ago

The logger toggle button is in the toolbar, but when the toolbar is hidden (which is always so on my machines) it's not possible to toggle the logger. I think it'd be useful to also have the action under View menu, where the dock widget checkboxes are located.

eteran commented 2 years ago

Yup! I totally forgot about wanting to put it in the menu too

eteran commented 2 years ago

@10110111 Been busy, but finally got around to spending the like 5 minutes to address your comments. Please let me know if you see anything else you think should be addressed.

10110111 commented 2 years ago

Looks good to me now.